Re: [PATCH v2 0/7] fix snapshot --redefine bugs (incremental backup saga)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux