<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><span style="color:rgb(34,34,34)">On Tue, Mar 12, 2019 at 11:52 PM Eric Blake <<a href="mailto:eblake@redhat.com">eblake@redhat.com</a>> wrote:</span><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 3/12/19 4:35 PM, Nir Soffer wrote:<br>
<br>
>>> We don't have a need to list or define snapshots since we managed<br>
>> snapshots<br>
>>> on oVirt side.<br>
>>> We want an API to list and redefine checkpoints.<br>
>><br>
>> But the proposed <domaincheckpoint> also has a <domain> subelement, so<br>
>> it has the same problem (the XML for a bulk operation can become<br>
>> prohibitively large).<br>
>><br>
> <br>
> Why do we need <domain> in a checkpoint?<br>
<br>
The initial design for snapshots did not have <domain>, and it bit us<br>
hard; you cannot safely revert to a snapshot if you don't know the state<br>
of the domain at that time. The initial design for checkpoints has thus<br>
mandated that <domain> be present (my v5 series fails to redefine a<br>
snapshot if you omit <domain>, even though it has a NO_DOMAIN flag<br>
during dumpxml for reduced output). If we are convinced that defining a<br>
snapshot without <domain> is safe enough, then I can relax the<br>
checkpoint code to allow redefined metadata without <domain> the way<br>
snapshots already have to do it, even though I was hoping that<br>
checkpoints could start life with fewer back-compats that snapshot has<br>
had to carry along. But I'd rather start strict and relax later<br>
(require <domain> and then remove it when proven safe), and not start<br>
loose (leave <domain> optional, and then wish we had made it mandatory).<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">You keep mentioning snapshots, but we are not going to redefine snapshots,</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">only checkpoints.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">We need to do this only because qcow2 does not store a chain of bitmaps, but</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">un-ordered bitmaps without any metadata, so we need to remind libvirt which</div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">bitmaps we have in every disk, and what is the order of the bitmaps. We will be</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">happy if we could just specify the bitmaps and disks in the backup API, and avoid</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">rebuilding the checkopints history, just like we avoid rebuilding the snapshot </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">history.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> You can see here what we store in a checkpoint (based on your patches v2 or<br>
> v3):<br>
> <a href="https://ovirt.org/develop/release-management/features/storage/incremental-backup.html#engine-database-changes" rel="noreferrer" target="_blank">https://ovirt.org/develop/release-management/features/storage/incremental-backup.html#engine-database-changes</a><br>
> <br>
> Which is:<br>
> <br>
> vm_checkpoints table<br>
> <br>
> - checkpoint_id: UUID<br>
> - parent: UUID<br>
> - vm_id: UUID<br>
> - creation_date: TIMESTAMP<br>
> <br>
> vm_checkpoint_disk_map table<br>
> - disk_id: UUID<br>
> - checkpoint_id: UUID<br>
> <br>
<br>
And no state of the <domain> at the time the checkpoint was created?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">No, but the configuration of the VM is stored in oVirt, so the backup application</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">can get this configuration from oVirt at the time of the backup.</div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>><br>
>>> Our use case is:<br>
>>><br>
>>> 1. Start VM on some host<br>
>>> 2. When oVirt starts a backup, it adds a checkpoint in oVirt database<br>
>><br>
>> And does this database preserve topological ordering? If not, why not?<br>
>><br>
> <br>
> Using the parent UUID we can create the correct chain in the right order,<br>
> and this is what we send to vdsm. Based on this, vdsm will generate the XML<br>
> for libvirt.<br>
> <br>
> There is no domain info related to these checkpoints, and I hope we are not<br>
> adding such requirements at this stage.<br>
<br>
Rather, that requirement has been there in all of my drafts, and it<br>
sounds like you are trying to get me to relax that requirement to not<br>
need a <domain> during checkpoint metadata redefine.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">We never understood this requirement it in the html documentation built from your patches.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Looking again in v3 - I see:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><div class="gmail_default"> domain</div><div class="gmail_default"> The inactive domain configuration at the time the checkpoint was created. Readonly.</div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">And we have an example:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><div class="gmail_default"><domaincheckpoint></div><div class="gmail_default"> <description>Completion of updates after OS install</description></div><div class="gmail_default"> <disks></div><div class="gmail_default"> <disk name='vda' checkpoint='bitmap'/></div><div class="gmail_default"> <disk name='vdb' checkpoint='no'/></div><div class="gmail_default"> </disks></div><div class="gmail_default"></domaincheckpoint></div><div class="gmail_default"><br></div><div class="gmail_default">Which does not have a domain, since it is read-only.</div><div class="gmail_default"><br></div><div class="gmail_default">There is an example of the xml we get from libvirt:</div><div class="gmail_default"><br></div><div class="gmail_default"><div class="gmail_default"><domaincheckpoint></div><div class="gmail_default"> <name>1525889631</name></div><div class="gmail_default"> <description>Completion of updates after OS install</description></div><div class="gmail_default"> <creationTime>1525889631</creationTime></div><div class="gmail_default"> <parent></div><div class="gmail_default"> <name>1525111885</name></div><div class="gmail_default"> </parent></div><div class="gmail_default"> <disks></div><div class="gmail_default"> <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/></div><div class="gmail_default"> <disk name='vdb' checkpoint='no'/></div><div class="gmail_default"> </disks></div><div class="gmail_default"> <domain type='qemu'></div><div class="gmail_default"> <name>fedora</name></div><div class="gmail_default"> <uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid></div><div class="gmail_default"> <memory>1048576</memory></div><div class="gmail_default"> ...</div><div class="gmail_default"> <devices></div><div class="gmail_default"> <disk type='file' device='disk'></div><div class="gmail_default"> <driver name='qemu' type='qcow2'/></div><div class="gmail_default"> <source file='/path/to/file1'/></div><div class="gmail_default"> <target dev='vda' bus='virtio'/></div><div class="gmail_default"> </disk></div><div class="gmail_default"> <disk type='file' device='disk' snapshot='external'></div><div class="gmail_default"> <driver name='qemu' type='raw'/></div><div class="gmail_default"> <source file='/path/to/file2'/></div><div class="gmail_default"> <target dev='vdb' bus='virtio'/></div><div class="gmail_default"> </disk></div><div class="gmail_default"> ...</div><div class="gmail_default"> </devices></div><div class="gmail_default"> </domain></div><div class="gmail_default"></domaincheckpoint></div></div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">But as the domain is read-only, we assumed that we don't need to pass it in the</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">domaincheckpoint xml. We also don't need it since we keep the the disks uuids</div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">included in every checkpoint, and from this you can find the information needed</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">to generate the tiny checkpoint xml above.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Can you explain why historic domain xml is needed for performing a backup?</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">The caller of the API is responsible to give you the list of bitmaps that should be</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">included for every disk. Libvirt needs to create a new active bitmap and start the </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">NBD server exposing the bitmaps specified by the caller. How having the xml for</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">every checkpoint/bitmap is needed for perform these steps?</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Note also that libvirt does not have snapshot history on oVirt host, since we manage</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">snapshots externally. When libvirt starts a VM, the only thing it knows about snapshots</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">is the chain of disks. Even if we restore the domain xml for every checkpoint, libvirt </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">cannot validate it against the non-existing snapshots.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">While we use persistent VMs since 4.2, our VMs are actually transient. We started to use</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">persistent VMs because they are more tested, and we can use the VM <metadata> to persist</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">stuff, instead of maintaining our own temporary metadata persistence solution. </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>> That's a different feature request - you're asking for<br>
>> virDomainCheckpointCreateXML(REDEFINE) to have some additional flag<br>
>> where the metadata re-creation fails if the underlying bitmap is<br>
>> corrupt/missing. Which may still be a good flag to add, but is something<br>
>> you can do with one-at-a-time checkpoint redefinitions, and doesn't, in<br>
>> itself, require bulk redefinition.<br>
>><br>
> <br>
> Checking for unknown bitmap on disk is not possible unless you have the<br>
> complete<br>
> list of checkpoints. Unless you add something like:<br>
> <br>
> virDomainCheckpointValidate()<br>
> <br>
> Which will fail if the checkpoints known to libvirt do not match the<br>
> bitmaps in the<br>
> underlying qcow2 images.<br>
<br>
Yes, an API like that may be better than trying to validate on a<br>
per-redefine basis whether libvirt metadata matches qcow2 metadata.<br>
<br>
> <br>
> But even if we add this, the user will have to do:<br>
> <br>
> try:<br>
> for checkpoint in orderdered_checkpoints:<br>
> generate xml...<br>
> redefine checkpoint...<br>
> except ...:<br>
> remove all checkpoints...<br>
<span class="gmail_default" style="font-size:small;color:rgb(0,0,0)"></span>> <br>
> validate checkpoints<br>
<br>
Yes, but that's not hard. And it's no different whether the 'try/except'<br>
code is a single libvirt API (with bulk redefine) or multiple libvirt<br>
APIs (no bulk redefine, but easy enough to one-at-a-time redefine in<br>
topological order). Why you have to generate the XML instead of reusing<br>
the XML that libvirt produces is something I'm not sure of (that is,<br>
what's wrong with using libvirt's dumpxml output as your input, rather<br>
than trying to reconstruct it on the fly?).<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">The xml libvirt produces may not be unstable. For example the disk uuid</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">"22e5a7d6-5409-482d-bda4-f5939b27aad4" may be called now "sda", but</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">when the vm was running on another host 2 weeks ago maybe it was called</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">"sdb", since the user added or removed some devices.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">When we create the xml using the disk uuuid, we will always refer to the current</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">name of the disk, regardless of the history of the vm.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Unless the xml for single checkpoint is big, I don't see why all libvirt<br>
> users<br>
> need to redefine checkopoints one by one and handle all the possible errors<br>
> (e.g. some checkpoints redefined, some not).<br>
<br>
As currently proposed, <domaincheckpoint> includes a mandatory <domain><br>
sub-element on redefinition, which means that the XML _is_ big for a<br>
series of checkpoints. It also means that you're better off storing the<br>
XML produced by libvirt than trying to regenerate it by hand, when it<br>
comes to metadata redefinition.<br>
<br>
> <br>
> Note that vdsm may be killed in the middle of the redefine loop, and in<br>
> this case<br>
> we leave livbirt with partial info about checkpoints, and we need to<br>
> redefine<br>
> the checkpoints again handling this partial sate.<br>
<br>
But that's relatively easy - if you don't know whether libvirt might<br>
have partial data, then wipe the data and start the redefine loop from<br>
scratch.<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">But this easy boilerplate will be duplicated in all libvirt clients, instead of </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">having single API to restore the state.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Of course if we must keep historic domain configuration for every checkpoint</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">bulk API does not make sense, but I don't understand this requirement.</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Nir</div></div></div></div></div></div></div>