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