<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:02 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 3:29 PM, Nir Soffer wrote:<br> <br> >>> snapshots =<br> >> srcdom.ListAllSnapshots(libvirt.VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL)<br> >>> for snapshot in snapshts:<br> >>> xml = snapshot.XMLDesc()<br> >>> dstdom.SnapshotCreateXML(xml)<br> >>><br> >>> I don't really see a reason why this is so hard that it justifies<br> >>> adding these bulk snapshot list/define APIs.<br> >><br> >> Nir, do you want to chime in here?<br> >><br> > <br> > We don't have a need to list or define snapshots since we managed 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></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Why do we need <domain> in a checkpoint?</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)">You can see here what we store in a checkpoint (based on your patches v2 or v3):</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><a href="https://ovirt.org/develop/release-management/features/storage/incremental-backup.html#engine-database-changes">https://ovirt.org/develop/release-management/features/storage/incremental-backup.html#engine-database-changes</a></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)">Which is:</div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"><br></div>vm_checkpoints<span class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> table</span><br><br><span class="gmail_default" style="font-size:small;color:rgb(0,0,0)">- </span>checkpoint_id: UUID<br><span class="gmail_default" style="font-size:small;color:rgb(0,0,0)">- </span>parent: UUID<br><span class="gmail_default" style="font-size:small;color:rgb(0,0,0)">- </span>vm_id: UUID<br><span class="gmail_default" style="font-size:small;color:rgb(0,0,0)">- </span>creation_date: TIMESTAMP</div><div class="gmail_quote"><br></div><div class="gmail_quote"><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">vm_checkpoint_disk_map table</div><div class="gmail_default" style="color:rgb(0,0,0)">- disk_id: UUID</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">- checkpoint_id: UUID</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></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Using the parent UUID we can create the correct chain in the right order,</div></div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">and this is what we send to vdsm. Based on this, vdsm will generate the XML</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">for libvirt.</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)">There is no domain info related to these checkpoints, and I hope we are not</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">adding such requirements at this stage.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > 3. When libvirt starts a backup, libvirt add the checkpoint oVirt created<br> > 4. Steps 2 and 3 are repeated while the VM is running...<br> > 5. Stop VM - at this point we undefine the VM, so libivrt lost all info<br> > about checkopints<br> > 6. Start VM on some host<br> > 7. oVirt use the bulk checkpoint API to redefine all checkpoints in oVirt<br> > database<br> > 8. If there is unknown bitmap on a disk, or a bitmap is missing, it means<br> > that someone<br> > modified the disk behind oVirt back, and oVirt will delete the affected<br> > checkpoints<br> > and use libvirt checkpoints API to delete the checkpoints and bitmaps.<br> > This forces<br> > full backup for the next backup of the affected disks.<br> > <br> > We would like that libvirt will fail to redefine the checkpoints if the<br> > info in the checkpoints<br> > does not match the the bitmap on the disks mentioned in the checkpoints.<br> <br> 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></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">Checking for unknown bitmap on disk is not possible unless you have the complete</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">list of checkpoints. Unless you add something like:</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)"> virDomainCheckpointValidate()</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)">Which will fail if the checkpoints known to libvirt do not match the bitmaps in the </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">underlying qcow2 images.</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 even if we add this, the user will have to do:</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)"> try:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> for checkpoint in orderdered_checkpoints:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> generate xml...</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> redefine checkpoint...</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> except ...:</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)"> remove all 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)"> validate 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)">Unless the xml for single checkpoint is big, I don't see why all libvirt users</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">need to redefine checkopoints one by one and handle all the possible errors</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">(e.g. some checkpoints redefined, some not).</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 that vdsm may be killed in the middle of the redefine loop, and in this case</div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">we leave livbirt with partial info about checkpoints, and we need to redefine </div><div class="gmail_default" style="font-size:small;color:rgb(0,0,0)">the checkpoints again handling this partial sate.</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 means all libvirt clients that need to managed more than one host.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > To<br> > do this,<br> > libvirt must have the complete checkpoint state. It cannot if we define<br> > checkpoints one<br> > by one.<br> <br> As long as you redefine checkpoints in topological order, I don't see<br> how defining one at a time or bulk changes whether libvirt can validate<br> that the checkpoints are correct. And even then, redefining in<br> topological order is only necessary so that libvirt can ensure that any<br> <parent/> checkpoint exists before attempting to define the metadata for<br> a child checkpoint.<br> <br> > <br> > If libivirt cannot fail, we need to a way to list all checkpoints and<br> > bitmaps, so we can<br> > sync oVirt database with reality.<br> > <br> > If we don't validate bitmaps with the expected bitmaps, the backup may fail<br> > when starting<br> > a backup (good), or succeed, including wrong data in the backup.<br> > <br> > It also does not make sense to redefine checkpoint state in incremental<br> > way, this like<br> > building a VM XML with many devices with one call per device.<br> <br> The argument here is that it will be the same amount of XML whether it<br> is one call per checkpoint or one call for all checkpoints (note - there<br> are multiple devices in one checkpoint, so it is NOT one call per<br> 'device * checkpoint')<br> <br> > <br> > The same checkpoint management will have to be duplicated in any libvirt<br> > client that<br> > manage more then one host (e.g. KubeVirt, OpenStack), since libvirt does<br> > not handle<br> > more than one host.<br> <br> But again, if you are moving checkpoint metadata from one host to<br> another, doing so one-at-a-time in topological order shouldn't be much<br> harder than doing so in one single call, but without the drawbacks of<br> exceeding XML size over RPC call limits.<br> <br> > <br> > For more info see<br> > -<br> > <a href="https://ovirt.org/develop/release-management/features/storage/incremental-backup.html" rel="noreferrer" target="_blank">https://ovirt.org/develop/release-management/features/storage/incremental-backup.html</a><br> > - <a href="https://github.com/oVirt/vdsm/blob/master/lib/vdsm/api/vdsm-api.yml#L11525" rel="noreferrer" target="_blank">https://github.com/oVirt/vdsm/blob/master/lib/vdsm/api/vdsm-api.yml#L11525</a><br> > -<br> > <a href="https://github.com/oVirt/vdsm/blob/0874b89a19af2d7726ed2f0b9d674cf0bb48a67e/lib/vdsm/api/vdsm-api.yml#L8129" rel="noreferrer" target="_blank">https://github.com/oVirt/vdsm/blob/0874b89a19af2d7726ed2f0b9d674cf0bb48a67e/lib/vdsm/api/vdsm-api.yml#L8129</a><br> > <br> <br> >> Unless Nir speaks up with strong reasons in favor of new bulk APIs<br> >> (rather than living with the new topological sort flags), I'm okay<br> >> dropping this series.<br> > <br> > <br> > I'm ok with dropping bulk snapshots, but I think libvirt should provide<br> > bulk APIs for checkpoints.<br> > <br> > Nir<br> ><br> <br> Given that the same amount of XML is involved across RPC, I'm<br> hard-pressed to include bulk operations on one object without providing<br> it on both.<br> <br> -- <br> Eric Blake, Principal Software Engineer<br> Red Hat, Inc. +1-919-301-3226<br> Virtualization: <a href="http://qemu.org" rel="noreferrer" target="_blank">qemu.org</a> | <a href="http://libvirt.org" rel="noreferrer" target="_blank">libvirt.org</a><br> <br> </blockquote></div></div></div></div>