On 9/9/19 5:52 PM, Eric Blake wrote:
Commit f10562799 introduced a regression: if reverting to a snapshot fails early (such as when we refuse to revert to an external snapshot), we lose track of the domain's current snapshot. See: https://bugzilla.redhat.com/1738747 Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> ---
I don't understand why f10562799 broke this code like this - tried looking the changes made in the commit and the "if (snap)" was there since a long time, no changes were made in the 'snap' variable as well - but this change didn't break anything else, so: Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx> ps: I think adding a test case for this failure (in tests/virsh-snapshot ?) is worth considering, especially considering that we changes from from Maxiwell (adding an inactive XML to the snapshot) that can add up more complexity in the snapshot mechanics. Thanks, DHB
src/qemu/qemu_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b28a26c3d6..093b15f500 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16941,8 +16941,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainSnapshotSetCurrent(vm->snapshots, NULL); ret = -1; } - } else if (snap) { - virDomainSnapshotSetCurrent(vm->snapshots, NULL); } if (ret == 0 && config && vm->persistent && !(ret = virDomainSaveConfig(cfg->configDir, driver->caps,
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list