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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list