Re: [libvirt] [PATCH v4 0/8] bulk snapshot list/redefine (incremental backup saga)
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 :|