Bug 1904486 - [RFE][CBT][incremental backup] Allow redefine VM backup checkpoint without the domain XML
Summary: [RFE][CBT][incremental backup] Allow redefine VM backup checkpoint without th...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux Advanced Virtualization
Classification: Red Hat
Component: libvirt
Version: 8.3
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: rc
: 8.4
Assignee: Peter Krempa
QA Contact: yisun
URL:
Whiteboard:
Depends On:
Blocks: 1891470 1901835
TreeView+ depends on / blocked
 
Reported: 2020-12-04 15:39 UTC by Peter Krempa
Modified: 2021-05-25 06:46 UTC (History)
11 users (show)

Fixed In Version: libvirt-7.0.0-1.el8
Doc Type: Enhancement
Doc Text:
Clone Of: 1901830
Environment:
Last Closed: 2021-05-25 06:45:10 UTC
Type: Feature Request
Target Upstream Version: 7.0.0
Embargoed:


Attachments (Terms of Use)

Description Peter Krempa 2020-12-04 15:39:37 UTC
+++ 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>

Comment 1 yisun 2021-01-07 09:02:25 UTC
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

Comment 5 yisun 2021-01-19 07:05:13 UTC
test result PASSED on libvirt-7.0.0-1.module+el8.4.0+9464+3e71831a.x86_64

Comment 8 errata-xmlrpc 2021-05-25 06:45:10 UTC
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


Note You need to log in before you can comment on or make changes to this bug.