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