On Mon, Mar 11, 2019 at 09:38:31PM -0500, Eric Blake wrote: > While looking at my work on incremental backups, Nir raised a good > point: if we want to recreate a set of known checkpoints on one > machine that will be taking over a domain from another machine, > my initial proposal required making multiple API calls to list the > XML for each checkpoint on the source, and then more API calls to > redefine each checkpoint on the destination; it also had the drawback > that the list has to be presented in topological order (the API won't > let you define a child checkpoint if the parent is not defined first). > He asked if I could instead add bulk APIs, for getting the XML for > all checkpoints at once, and then for redefining all checkpoints at > once. To me the key blocking problem here is the topological sorting one as it makes it incredibly painful to run individual APIs. Your other series on list addresses this nicely by adding the topological flag. Using that flag, in psuedo-python like code the process of exporting and importing snapshots from one host to another should look approx like this snapshots = srcdom.ListAllSnapshots(libvirt.VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL) for snapshot in snapshts: xml = snapshot.XMLDesc() dstdom.SnapshotCreateXML(xml) I don't really see a reason why this is so hard that it justifies adding these bulk snapshot list/define APIs. At the same time there is a very real downside to providing these bulk snapshot APIs in terms of scalability. The <domain> XML for a guest can get very large. The <domainsnapshot> XML contains a copy of the <domain> XML from the point in time of the snapsht. Therefore a <snapshots> XML document is going to contain *many* <domain> XML documents concatenated. For a domain with many devices and many snapshots, the bulk list APIs have non-negligible risk of hitting the RPC protocol limit on string size. We've seen this before with the bulk domain stats APIs we added. We none the less still added the bulk domain stats APIs because it was a compelling performance benefit for something that is run very frequently. For apps to be scalable though, they have to limit how many guests they request bulk stats for at any time. This in fact makes the code using the bulk stats APIs more complex than if it just called the individual APIs. The performance benefit is still a win though in this case. In the bulk snapshot import/export case though, I'm not seeing a compelling performance reason for their usage. Making the APIs scalable would require to limit how many snapshots are returned in any single call. There would then need to be repeated calls to fetch the rest. This makes it more complicated than just fetching/defining each individual snapshot IOW, overall I'm not seeing a big enough justification to add these new APIs, that would outweigh the downsides they bring in terms of scalability. The topological sorting flag solves the really big pain point well enough. > I'm also toying with the idea of followup patches that store all > snapshots alongside the persistent <domain> XML in a single file, > rather than in one snapshot per <domainsnapshot> file (we'd still > support reading from the old method at libvirtd startup, but would > not need to output in that manner any more). What would be the benefit to having it all in one file ? During initial libvirtd startup we could read the info from one files, but we still need code to read the info from many files so we're not saving code in that respect. Most of the snapshot APIs that save XML to disk only care about a single <domainsnapshot> document so wouldn't benefit from having all snapshots in one place. Re-writing the entire <domain> XML means that a host failure during a snapshot save can put the entire domain definition at risk of loss. Though we do try to mitigate that by writing to a temp file and doing an atomic rename, that doesn't eliminate the risk entirely. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list