Re: [libvirt] [PATCH v2 0/7] fix snapshot --redefine bugs (incremental backup saga)
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(a)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(-)