[PATCH v2 1/6] conf: Add snapshotName attribute for internal disk snapshot

[Or Ozeri]

Add a new attribute to snapshot disks XML element,
which allows specifying the name that will be used to refer
to internal snapshots.
If unspecified, the parent snapshot name will be used.
For now, make this attribute unsupported for qemu domains.

Signed-off-by: Or Ozeri <oro at il.ibm.com>
---
 docs/formatsnapshot.rst             |  5 +++++
 src/conf/schemas/domainsnapshot.rng |  4 ++++
 src/conf/snapshot_conf.c            | 27 +++++++++++++++++++++++++++
 src/conf/snapshot_conf.h            |  2 ++
 src/qemu/qemu_snapshot.c            |  9 +++++++++
 5 files changed, 47 insertions(+)

diff --git a/docs/formatsnapshot.rst b/docs/formatsnapshot.rst
index 085c712053..d98066dd8c 100644
--- a/docs/formatsnapshot.rst
+++ b/docs/formatsnapshot.rst
@@ -145,6 +145,11 @@ The top-level ``domainsnapshot`` element may contain the following elements:
       driver and supported values are ``file``, ``block`` and ``network``
       :since:`(since 1.2.2)`.
 
+      :since:`Since 9.1.0` the ``disk`` element supports an optional attribute
+            ``snapshotName`` if the ``snapshot`` attribute is set to ``internal``. This
+            attribute specifies the name that will be used to refer to the
+            internal disk snapshot.
+
       ``source``
 
          If the snapshot mode is external (whether specified or inherited),
diff --git a/src/conf/schemas/domainsnapshot.rng b/src/conf/schemas/domainsnapshot.rng
index 4048266f1d..19f097d2b3 100644
--- a/src/conf/schemas/domainsnapshot.rng
+++ b/src/conf/schemas/domainsnapshot.rng
@@ -209,6 +209,10 @@
           <value>manual</value>
         </attribute>
       </choice>
+      <optional>
+        <attribute name="snapshotName">
+        </attribute>
+      </optional>
     </element>
   </define>
 
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 9bf3c78353..58a6afa26d 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -150,6 +150,8 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
                               VIR_STORAGE_TYPE_FILE) < 0)
         return -1;
 
+    def->snapshot_name = virXMLPropString(node, "snapshotName");
+
     if (src->type == VIR_STORAGE_TYPE_VOLUME ||
         src->type == VIR_STORAGE_TYPE_DIR) {
         virReportError(VIR_ERR_XML_ERROR,
@@ -192,6 +194,10 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
         (src->path || src->format))
         def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 
+    if (def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
+        def->snapshot_name)
+        def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
+
     def->name = g_steal_pointer(&name);
     def->src = g_steal_pointer(&src);
 
@@ -670,6 +676,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
             return -1;
         }
 
+        if (snapdisk->snapshot_name &&
+            snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("snapshotName for disk '%s' requires "
+                             "use of internal snapshot mode"),
+                           snapdisk->name);
+            return -1;
+        }
+
         if (snapdisk->src->path &&
             snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -714,6 +729,16 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
     if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
         return -1;
 
+    /* set default snapshot name for internal snapshots */
+    for (i = 0; i < snapdef->ndisks; i++) {
+        virDomainSnapshotDiskDef * disk = &snapdef->disks[i];
+
+        if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL &&
+            !disk->snapshot_name) {
+            disk->snapshot_name = g_strdup(snapdef->parent.name);
+        }
+    }
+
     return 0;
 }
 
@@ -748,6 +773,8 @@ virDomainSnapshotDiskDefFormat(virBuffer *buf,
     if (disk->snapshot > 0)
         virBufferAsprintf(&attrBuf, " snapshot='%s'",
                           virDomainSnapshotLocationTypeToString(disk->snapshot));
+    if (disk->snapshot_name)
+        virBufferAsprintf(&attrBuf, " snapshotName='%s'", disk->snapshot_name);
 
     if (disk->snapshotDeleteInProgress)
         virBufferAddLit(&childBuf, "<snapshotDeleteInProgress/>\n");
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 96c77ef42b..c133c105c7 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -57,6 +57,8 @@ struct _virDomainSnapshotDiskDef {
     /* details of wrapper external file. src is always non-NULL.
      * XXX optimize this to allow NULL for internal snapshots? */
     virStorageSource *src;
+
+    char *snapshot_name; /* snapshot name for internal snapshots */
 };
 
 void
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index b8416808b3..1aa2f05300 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -660,6 +660,15 @@ qemuSnapshotPrepare(virDomainObj *vm,
                                virStorageFileFormatTypeToString(dom_disk->src->format));
                 return -1;
             }
+
+            if (disk->snapshot_name) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("snapshot name setting for disk %s unsupported "
+                                 "for storage type %s"),
+                               disk->name,
+                               virStorageFileFormatTypeToString(dom_disk->src->format));
+                return -1;
+            }
             break;
 
         case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
-- 
2.25.1