From 238fef920f05bc4505320702018c9b1211ff3dcc Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 9 Mar 2022 15:52:45 +0100
Subject: [PATCH] conf: snapshot: Use proper types for snapshot location
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Refactor the code to use proper types for the memory and disk snapshot
location and fix the parsing code to be compatible with an unsigned
type.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/snapshot_conf.c | 39 +++++++++++++++++----------------------
 src/conf/snapshot_conf.h |  4 ++--
 2 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 594492ccd2..e2442441d0 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -130,7 +130,6 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
                                  unsigned int flags,
                                  virDomainXMLOption *xmlopt)
 {
-    g_autofree char *snapshot = NULL;
     g_autofree char *driver = NULL;
     g_autofree char *name = NULL;
     g_autoptr(virStorageSource) src = virStorageSourceNew();
@@ -145,16 +144,12 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
         return -1;
     }
 
-    snapshot = virXMLPropString(node, "snapshot");
-    if (snapshot) {
-        def->snapshot = virDomainSnapshotLocationTypeFromString(snapshot);
-        if (def->snapshot <= 0) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("unknown disk snapshot setting '%s'"),
-                           snapshot);
-            return -1;
-        }
-    }
+    if (virXMLPropEnumDefault(node, "snapshot",
+                              virDomainSnapshotLocationTypeFromString,
+                              VIR_XML_PROP_NONZERO,
+                              &def->snapshot,
+                              VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) < 0)
+        return -1;
 
     if (virXMLPropEnumDefault(node, "type",
                               virStorageTypeFromString,
@@ -196,7 +191,8 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
         return -1;
     }
 
-    if (!def->snapshot && (src->path || src->format))
+    if (def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
+        (src->path || src->format))
         def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 
     def->name = g_steal_pointer(&name);
@@ -220,7 +216,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
     g_autofree xmlNodePtr *diskNodes = NULL;
     size_t i;
     int n;
-    g_autofree char *memorySnapshot = NULL;
+    xmlNodePtr memoryNode = NULL;
     bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
     virSaveCookieCallbacks *saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt);
     int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -307,16 +303,15 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
         return NULL;
     }
 
-    memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt);
-    def->memorysnapshotfile = virXPathString("string(./memory/@file)", ctxt);
-    if (memorySnapshot) {
-        def->memory = virDomainSnapshotLocationTypeFromString(memorySnapshot);
-        if (def->memory <= 0) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("unknown memory snapshot setting '%s'"),
-                           memorySnapshot);
+    if ((memoryNode = virXPathNode("./memory", ctxt))) {
+        def->memorysnapshotfile = virXMLPropString(memoryNode, "file");
+
+        if (virXMLPropEnumDefault(memoryNode, "snapshot",
+                                  virDomainSnapshotLocationTypeFromString,
+                                  VIR_XML_PROP_NONZERO,
+                                  &def->memory,
+                                  VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) < 0)
             return NULL;
-        }
     }
 
     if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 8823af1ac1..1f787f1a94 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -51,7 +51,7 @@ G_STATIC_ASSERT((int)VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT == VIR_DOMAIN_LAST);
 typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
 struct _virDomainSnapshotDiskDef {
     char *name;     /* name matching the <target dev='...' of the domain */
-    int snapshot;   /* virDomainSnapshotLocation */
+    virDomainSnapshotLocation snapshot;
 
     /* details of wrapper external file. src is always non-NULL.
      * XXX optimize this to allow NULL for internal snapshots? */
@@ -70,7 +70,7 @@ struct _virDomainSnapshotDef {
     /* Additional public XML.  */
     int state; /* virDomainSnapshotState */
 
-    int memory; /* virDomainMemorySnapshot */
+    virDomainSnapshotLocation memory;
     char *memorysnapshotfile; /* memory state file when snapshot is external */
 
     size_t ndisks; /* should not exceed dom->ndisks */