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

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 <danielhb...@gmail.com>


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

Reply via email to