snapshot: Rework parse logic during libvirt startup

Rework the logic in qemuDomainSnapshotLoad() to set
vm->current_snapshot only once at the end of the loop, rather than
repeatedly querying it during the loop, to make it easier for the next
patch to use accessor functions rather than direct manipulation of
vm->current_snapshot.  When encountering multiple snapshots claiming
to be current (based on the presence of an <active>1</active> element
in the XML, which libvirt only outputs for internal use and not for
any public API), this changes behavior from warning only once and
running with no current snapshot, to instead warning on each duplicate
and selecting the last one encountered (which is arbitrary based on
readdir() ordering, but actually stands a fair chance of being the
most-recently created snapshot whether by timestamp or by the
propensity of humans to name things in ascending order).

Note that the code in question is only run by libvirtd when it first
starts, reading state from disk from the previous run into memory for
this run. Since the data resides somewhere that only libvirt should be
touching (typically /var/lib/libvirt/qemu/snapshot/*), it should be
clean.  So in the common case, the code touched here is unreachable.
But if someone is actually messing with files behind libvirt's back,
they deserve the change in behavior.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 13a34ef8601fdb8421229ba5ffbc6074fb035c34..e49924ce1a9594e7e4a8273a8c864396d8eb5107 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1,7 +1,7 @@
 /*
  * qemu_driver.c: core driver methods for managing qemu guests
  *
- * Copyright (C) 2006-2016 Red Hat, Inc.
+ * Copyright (C) 2006-2019 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -482,9 +482,11 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
         if (snap == NULL) {
             virDomainSnapshotDefFree(def);
         } else if (cur) {
+            if (current)
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Too many snapshots claiming to be current for domain %s"),
+                               vm->def->name);
             current = snap;
-            if (!vm->current_snapshot)
-                vm->current_snapshot = snap;
         }
 
         VIR_FREE(fullpath);
@@ -495,13 +497,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
                        _("Failed to fully read directory %s"),
                        snapDir);
 
-    if (vm->current_snapshot != current) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Too many snapshots claiming to be current for domain %s"),
-                       vm->def->name);
-        vm->current_snapshot = NULL;
-    }
-
+    vm->current_snapshot = current;
     if (virDomainSnapshotUpdateRelations(vm->snapshots) < 0)
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Snapshots have inconsistent relations for domain %s"),