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

[Daniel P. Berrangé]

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 :|