[libvirt] [PATCH v10 00/19] Checkpoint APIs (incremental backup saga)

[Daniel P. Berrangé]

On Wed, Jul 24, 2019 at 12:55:50AM -0500, Eric Blake wrote:
> This is a subset of v10 of incremental backup, fixing Peter's review
> comments from v9 (and even some from v8 that I had missed earlier).
> There's still a lot more rebasing churn to resolve in the backup
> portion of the series before I can post a full backup-v10 label.
> 
> There's still a decision to make: do we want this series in 5.6
> (possibly with the addition of just patch 2/10 at [1] to introduce the
> backup API to make it possible for downstream to backport features
> without bumping .so)?  This series has checkpoint support for test and
> qemu drivers, and I think is probably clean enough to finally satisfy
> everything Peter has been pointing out;

In general my preference is to incrementally merge code as it is
ready. For large development tasks, I think it is counterproductive
to try to wait until everything is absolutely perfect & able to merge
in a big bang. Longer patch series tend to scare off reviewers, making
them take even longer to turn around & merge, and increase the burden
on the developer to continually rebase wich also delays things.

IOW, assuming positive review on this v10 series, I see no compelling
reason to artificially delay merge of these checkpoint patches waiting
for the followup backup patch series.


>                                           but while the backup API
> itself seems reasonable, the qemu implementation will likely miss 5.6
> (as that half of the v9 posting was further behind, and still has a
> lot of rebase churn to resolve).  Or do we delay the checkpoint API to
> 5.7, to only go in with backup API?  There's also still an outstanding
> question of whether the backup API needs a tweak to use 'const char *'
> instead of 'int' for the job identifier, given that Peter has a
> proposal for overhauling the representation of libvirt jobs.

I definitely think we do *not* want to use an 'int'. The only place
we've used an int for a unique identifier was the virDomainPtr and
even that's largely a historical mistake from early days of our Xen
support.

A 'const char*' is reasonable, though I have raised the question in
Peter proposal whether we should go all the way and pick a UUID as
the most unique identifier, so that we don't acceidentally confuse
jobs between domains.


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