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@redhat.com> 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(-)