Re: [libvirt] [PATCH v10 00/19] Checkpoint APIs (incremental backup saga)
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 :|