On 2/27/19 3:04 PM, Eric Blake wrote: > John pointed out that patch 2 of my original bug fix did too > much in one patch. Now that we are in freeze for 5.1, I've > proposed two variants of the same fix: patch 1 is the bare > minimum to fix the bug and nothing else, while patches 3-7 > are more in line with my v1 patch at doing other refactoring > work along the way (but now split into multiple logical steps) > that will prove useful for my other pending snapshot improvements > (https://www.redhat.com/archives/libvir-list/2019-February/msg01350.html, > but now 5.2 material). > > I'm proposing both variants for comparison, although I already > suspect the answer will be 'use patch 1 for 5.1, then rebase > what remains of patches 3-7 into the other snapshot cleanups > for 5.2'. > > variant2 is cleaner than my v1 patch 2/2 in that I no longer have > to use a default: label to hack around gcc's enum sanity checking > within a switch statement, but it required introducing a large > mechanical rename of all use of snapshot state values. > I was hoping someone else would take a bite before me, but I guess not... So my choices are let's do some refactoring, add/modify some enums, and make a cleaner fix, but have a longer series of changes or do a quick short term hack for 5.1.0 and then as soon as 5.2.0 opens, revert that, and do things properly. More less - a fair assessment? There's also the has anyone outside of you noticed up to this point. I vote for doing it right, but if there's strong opinions to hack and replace, then I won't complain either. You have my - Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> John > Eric Blake (7): > qemu: Fix snapshot redefine vs. domain state bug > Revert "qemu: Fix snapshot redefine vs. domain state bug" > qemu: Refactor check for _LIVE vs. _REDEFINE > qemu: Factor out qemuDomainSnapshotValidate() helper > snapshot: Rework virDomainSnapshotState enum > qemu: Use virDomainSnapshotState for switch statements > qemu: Fix snapshot redefine vs. domain state bug > > src/conf/snapshot_conf.h | 21 ++++- > src/conf/snapshot_conf.c | 28 +++---- > src/qemu/qemu_driver.c | 160 +++++++++++++++++++++++---------------- > src/test/test_driver.c | 20 ++--- > src/vbox/vbox_common.c | 4 +- > 5 files changed, 137 insertions(+), 96 deletions(-) > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
