[libvirt] [PATCH v4 0/8] bulk snapshot list/redefine (incremental backup saga)

[Eric Blake]

On 3/12/19 4:35 PM, Nir Soffer wrote:

>>> We don't have a need to list or define snapshots since we managed
>> snapshots
>>> on oVirt side.
>>> We want an API to list and redefine checkpoints.
>>
>> But the proposed <domaincheckpoint> also has a <domain> subelement, so
>> it has the same problem (the XML for a bulk operation can become
>> prohibitively large).
>>
> 
> Why do we need <domain> in a checkpoint?

The initial design for snapshots did not have <domain>, and it bit us
hard; you cannot safely revert to a snapshot if you don't know the state
of the domain at that time. The initial design for checkpoints has thus
mandated that <domain> be present (my v5 series fails to redefine a
snapshot if you omit <domain>, even though it has a NO_DOMAIN flag
during dumpxml for reduced output).  If we are convinced that defining a
snapshot without <domain> is safe enough, then I can relax the
checkpoint code to allow redefined metadata without <domain> the way
snapshots already have to do it, even though I was hoping that
checkpoints could start life with fewer back-compats that snapshot has
had to carry along.  But I'd rather start strict and relax later
(require <domain> and then remove it when proven safe), and not start
loose (leave <domain> optional, and then wish we had made it mandatory).

> 
> You can see here what we store in a checkpoint (based on your patches v2 or
> v3):
> https://ovirt.org/develop/release-management/features/storage/incremental-backup.html#engine-database-changes
> 
> Which is:
> 
> vm_checkpoints table
> 
> - checkpoint_id: UUID
> - parent: UUID
> - vm_id: UUID
> - creation_date: TIMESTAMP
> 
> vm_checkpoint_disk_map table
> - disk_id: UUID
> - checkpoint_id: UUID
> 

And no state of the <domain> at the time the checkpoint was created?

>>
>>> Our use case is:
>>>
>>> 1. Start VM on some host
>>> 2. When oVirt starts a backup, it adds a checkpoint in oVirt database
>>
>> And does this database preserve topological ordering? If not, why not?
>>
> 
> Using the parent UUID we can create the correct chain in the right order,
> and this is what we send to vdsm. Based on this, vdsm will generate the XML
> for libvirt.
> 
> There is no domain info related to these checkpoints, and I hope we are not
> adding such requirements at this stage.

Rather, that requirement has been there in all of my drafts, and it
sounds like you are trying to get me to relax that requirement to not
need a <domain> during checkpoint metadata redefine.


>> That's a different feature request - you're asking for
>> virDomainCheckpointCreateXML(REDEFINE) to have some additional flag
>> where the metadata re-creation fails if the underlying bitmap is
>> corrupt/missing. Which may still be a good flag to add, but is something
>> you can do with one-at-a-time checkpoint redefinitions, and doesn't, in
>> itself, require bulk redefinition.
>>
> 
> Checking for unknown bitmap on disk is not possible unless you have the
> complete
> list of checkpoints. Unless you add something like:
> 
>     virDomainCheckpointValidate()
> 
> Which will fail if the checkpoints known to libvirt do not match the
> bitmaps in the
> underlying qcow2 images.

Yes, an API like that may be better than trying to validate on a
per-redefine basis whether libvirt metadata matches qcow2 metadata.

> 
> But even if we add this, the user will have to do:
> 
>     try:
>         for checkpoint in orderdered_checkpoints:
>            generate xml...
>            redefine checkpoint...
>     except ...:
>         remove all checkpoints...
> 
>     validate checkpoints

Yes, but that's not hard. And it's no different whether the 'try/except'
code is a single libvirt API (with bulk redefine) or multiple libvirt
APIs (no bulk redefine, but easy enough to one-at-a-time redefine in
topological order).  Why you have to generate the XML instead of reusing
the XML that libvirt produces is something I'm not sure of (that is,
what's wrong with using libvirt's dumpxml output as your input, rather
than trying to reconstruct it on the fly?).

> 
> Unless the xml for single checkpoint is big, I don't see why all libvirt
> users
> need to redefine checkopoints one by one and handle all the possible errors
> (e.g. some checkpoints redefined, some not).

As currently proposed, <domaincheckpoint> includes a mandatory <domain>
sub-element on redefinition, which means that the XML _is_ big for a
series of checkpoints.  It also means that you're better off storing the
XML produced by libvirt than trying to regenerate it by hand, when it
comes to metadata redefinition.

> 
> Note that vdsm may be killed in the middle of the redefine loop, and in
> this case
> we leave livbirt with partial info about checkpoints, and we need to
> redefine
> the checkpoints again handling this partial sate.

But that's relatively easy - if you don't know whether libvirt might
have partial data, then wipe the data and start the redefine loop from
scratch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190312/9f7af5b1/attachment-0001.sig>