+++ This bug was initially created as a clone of Bug #1901830 +++ Description of problem: Currently, when having a checkpoint that needs to be defined for a VM with a backup, the checkpoint should be defined by giving the <domaincheckpoint> XML and the entire VM <domain> XML. Supplying the VM domain XML is redundant. In RHV, it forces us to persists the XML that was created when the backup was taken in the RHV engine database and use it when the checkpoint should be redefined. Moreover, RHV may encounter some errors during the backup (communication issue etc..) and might lose the returned checkpoint and domain XML so we added a recovery mechanism to fetch the entire XML. If Libvirt will provide a way to redefine the checkpoint without the domain XML, it will facilitate the process and reduce many error-prone stages. The call for defining a checkpoint when backup is started using Libvirt python SDK is currently - dom.backupBegin(backup_xml, checkpoint_xml, flags=flags) The call for defining a checkpoint after a backup was taken using Libvirt python SDK is currently - dom.checkpointCreateXML(checkpoint_xml, flags) The solution might be to provide a flag for backupBegin and checkpointCreateXML that will indicate whether the domain XML is needed or not (similar to virDomainCheckpointGetXMLDesc with VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN flag) Version-Release number of selected component (if applicable): 6.6.0-6.module+el8.3.0+8125+aefcf088.x86_64 How reproducible: 100% Steps to Reproduce: 1. 2. 3. Actual results: Expected results: Additional info: - --- Additional comment from Peter Krempa on 2020-12-04 16:31:43 CET --- Upstream: commit abf12f071b17d8be51ce5fe368175f2f5612eafe Author: Peter Krempa <pkrempa> Date: Wed Dec 2 14:18:40 2020 +0100 conf: checkpoint: Don't require <domain> when redefining checkpoints The domain definition stored with a checkpoint isn't used currently apart from matching disks when creating a new checkpoints. As some users of the incremental backup API want to provide backups in offline mode under their control (obviously while compying with our documentation on how the on-disk state should be handled) and then want to define the checkpoint for live use, supplying a <domain> sub-element is overly complex and not actually needed by the code. Relax the restriction when re-defining a checkpoint so that <domain> is not necessary and add (alibistic) documentation saying that future actions may not work if it's missing. Signed-off-by: Peter Krempa <pkrempa> Reviewed-by: Daniel Henrique Barboza <danielhb413> commit 392eacfeb1c924a7532955d8866677d905fa74fa Author: Peter Krempa <pkrempa> Date: Wed Dec 2 14:13:17 2020 +0100 conf: checkpoint: Prepare internals for missing domain definition Conditionalize code which assumes that the domain definition stored in the checkpoint is present. Signed-off-by: Peter Krempa <pkrempa> Reviewed-by: Daniel Henrique Barboza <danielhb413> commit 9fd8ba3b2d681e5d0c58fc68f99224dbf0a5f810 Author: Peter Krempa <pkrempa> Date: Wed Dec 2 13:35:29 2020 +0100 virDomainCheckpointRedefineCommit: Don't check ABI of definition in checkpoint Checking the definition ABI when redefining checkpoints doesn't make much sense for the following reasons: * the domain definition in the checkpoint is mostly unused (a relic adopted from the snapshot code) * can be very easily overridden by deleting the checkpoint metadata before redefinition Rather than complicating the logic when we'll be taking into account that the domain definition may be missing, let's just remove the check. Signed-off-by: Peter Krempa <pkrempa> Reviewed-by: Daniel Henrique Barboza <danielhb413> commit 9a58f1a53c8d94cf867fd3a76515d222750d5d77 Author: Peter Krempa <pkrempa> Date: Wed Dec 2 14:33:21 2020 +0100 virDomainCheckpointDefParse: Use 'unsigned int' for flags Fix the type for a variable holding flags to the usual 'unsigned int' and change the name to be more appropriate to its use. Signed-off-by: Peter Krempa <pkrempa> Reviewed-by: Daniel Henrique Barboza <danielhb413> commit d1fd4a37555123fbdf9ebb77f3a004320d75b1a3 Author: Peter Krempa <pkrempa> Date: Wed Dec 2 14:29:30 2020 +0100 virDomainCheckpointDefParse: Don't extract unused domain type We can extract './domain' directly and let the parser deal with the type. Signed-off-by: Peter Krempa <pkrempa> Reviewed-by: Daniel Henrique Barboza <danielhb413>
PreVerified with upstream build: libvirt-6.10.0-1.fc34.x86_64 ➜ ~ virsh checkpoint-create-as pc ck1 Domain checkpoint ck1 created ➜ ~ virsh checkpoint-list pc Name Creation Time ----------------------------------- ck1 2021-01-07 08:57:47 +0000 ➜ ~ virsh checkpoint-dumpxml pc ck1 --no-domain > ck1.xml ➜ ~ virsh checkpoint-delete pc ck1 --metadata Domain checkpoint ck1 deleted ➜ ~ virsh checkpoint-create pc ck1.xml --redefine Domain checkpoint ck1 created from 'ck1.xml' ➜ ~ virsh checkpoint-list pc Name Creation Time ----------------------------------- ck1 2021-01-07 08:57:47 +0000
test result PASSED on libvirt-7.0.0-1.module+el8.4.0+9464+3e71831a.x86_64
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (virt:av bug fix and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2021:2098