[PATCHv2 4/9] snapshot: Add support for specifying snapshot disk backing type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Add support for specifying various types when doing snapshots. This will
later allow to do snapshots on network backed volumes. Disks of type
'volume' are not supported by snapshots (yet).
---

Notes:
    Version 2:
    - always format disk type and fix fallout

 docs/formatsnapshot.html.in                        | 15 +++++
 docs/schemas/domainsnapshot.rng                    | 76 ++++++++++++++++++----
 src/conf/snapshot_conf.c                           | 42 +++++++-----
 src/conf/snapshot_conf.h                           | 15 +++--
 src/qemu/qemu_conf.c                               |  3 -
 src/qemu/qemu_driver.c                             | 58 +++++++++++------
 .../disk_driver_name_null.xml                      |  2 +-
 tests/domainsnapshotxml2xmlout/disk_snapshot.xml   |  4 +-
 .../disk_snapshot_redefine.xml                     |  6 +-
 9 files changed, 157 insertions(+), 64 deletions(-)

diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 76689cb..c2cd18c 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -170,6 +170,21 @@
             snapshots, the original file name becomes the read-only
             snapshot, and the new file name contains the read-write
             delta of all disk changes since the snapshot.
+
+            <span class="since">Since 1.2.2</span> the <code>disk</code> element
+            supports an optional attribute <code>type</code> if the
+            <code>snapshot</code> attribute is set to <code>external</code>.
+            This attribute specifies the snapshot target storage type and allows
+            to overwrite the default <code>file</code>type. The <code>type</code>
+            attribute along with the format of the <code>source</code>
+            sub-element is identical to the <code>source</code> element used in
+            domain disk definitions. See the
+            <a href="formatdomain.html#elementsDisks">disk devices</a> section
+            documentation for further information.
+
+            Libvirt currently supports the <code>type</code> element in the qemu
+            driver and supported values are <code>file</code> and
+            <code>block</code> <span class="since">(since 1.2.2)</span>.
           </dd>
         </dl>
       </dd>
diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 169fcfb..de9e788 100644
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -123,19 +123,73 @@
               <value>external</value>
             </attribute>
           </optional>
-          <interleave>
-            <ref name='disksnapshotdriver'/>
-            <optional>
-              <element name='source'>
+          <choice>
+            <group>
+              <optional>
+                <attribute name='type'>
+                  <value>file</value>
+                </attribute>
+              </optional>
+              <interleave>
                 <optional>
-                  <attribute name='file'>
-                    <ref name='absFilePath'/>
-                  </attribute>
+                  <element name='source'>
+                    <optional>
+                      <attribute name='file'>
+                        <ref name='absFilePath'/>
+                      </attribute>
+                    </optional>
+                    <empty/>
+                  </element>
                 </optional>
-                <empty/>
-              </element>
-            </optional>
-          </interleave>
+                <ref name='disksnapshotdriver'/>
+              </interleave>
+            </group>
+            <group>
+              <attribute name='type'>
+                <value>block</value>
+              </attribute>
+              <interleave>
+                <optional>
+                  <element name="source">
+                    <attribute name="dev">
+                      <ref name="absFilePath"/>
+                    </attribute>
+                    <empty/>
+                  </element>
+                </optional>
+                <ref name='disksnapshotdriver'/>
+              </interleave>
+            </group>
+            <group>
+              <attribute name="type">
+                <value>dir</value>
+              </attribute>
+              <interleave>
+                <optional>
+                  <element name="source">
+                    <attribute name="dir">
+                      <ref name="absFilePath"/>
+                    </attribute>
+                    <empty/>
+                  </element>
+                </optional>
+                <ref name='disksnapshotdriver'/>
+              </interleave>
+            </group>
+            <group>
+              <attribute name="type">
+                <value>network</value>
+              </attribute>
+              <interleave>
+                <optional>
+                  <element name="source">
+                    <ref name='diskSourceNetwork'/>
+                  </element>
+                </optional>
+                <ref name='disksnapshotdriver'/>
+              </interleave>
+            </group>
+          </choice>
         </group>
       </choice>
     </element>
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index fb0b4cc..b5b522c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -108,6 +108,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 {
     int ret = -1;
     char *snapshot = NULL;
+    char *type = NULL;
     xmlNodePtr cur;

     def->name = virXMLPropString(node, "name");
@@ -128,7 +129,16 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
         }
     }

-    def->type = -1;
+    if ((type = virXMLPropString(node, "type"))) {
+        if ((def->type = virDomainDiskTypeFromString(type)) < 0 ||
+            def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("unknown disk snapshot type '%s'"), type);
+            goto cleanup;
+        }
+    } else {
+        def->type = VIR_DOMAIN_DISK_TYPE_FILE;
+    }

     for (cur = node->children; cur; cur = cur->next) {
         if (cur->type != XML_ELEMENT_NODE)
@@ -137,17 +147,12 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
         if (!def->file &&
             xmlStrEqual(cur->name, BAD_CAST "source")) {

-            int backingtype = def->type;
-
-            if (backingtype < 0)
-                backingtype = VIR_DOMAIN_DISK_TYPE_FILE;
-
             if (virDomainDiskSourceDefParse(cur,
-                                            backingtype,
+                                            def->type,
                                             &def->file,
-                                            NULL,
-                                            NULL,
-                                            NULL,
+                                            &def->protocol,
+                                            &def->nhosts,
+                                            &def->hosts,
                                             NULL) < 0)
                 goto cleanup;

@@ -174,6 +179,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
     ret = 0;
 cleanup:
     VIR_FREE(snapshot);
+    VIR_FREE(type);
     if (ret < 0)
         virDomainSnapshotDiskDefClear(def);
     return ret;
@@ -532,7 +538,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
             goto cleanup;
         disk->index = i;
         disk->snapshot = def->dom->disks[i]->snapshot;
-        disk->type = -1;
+        disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
         if (!disk->snapshot)
             disk->snapshot = default_snapshot;
     }
@@ -550,8 +556,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
             const char *tmp;
             struct stat sb;

-            if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE &&
-                disk->type != -1) {
+            if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("cannot generate external snapshot name "
                                  "for disk '%s' on a '%s' device"),
@@ -614,15 +619,12 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
         virBufferAsprintf(buf, " snapshot='%s'",
                           virDomainSnapshotLocationTypeToString(disk->snapshot));

-    if (type < 0)
-        type = VIR_DOMAIN_DISK_TYPE_FILE;
-
     if (!disk->file && disk->format == 0) {
         virBufferAddLit(buf, "/>\n");
         return;
     }

-    virBufferAddLit(buf, ">\n");
+    virBufferAsprintf(buf, " type='%s'>\n", virDomainDiskTypeToString(type));

     if (disk->format > 0)
         virBufferEscapeString(buf, "      <driver type='%s'/>\n",
@@ -630,7 +632,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
     virDomainDiskSourceDefFormatInternal(buf,
                                          type,
                                          disk->file,
-                                         0, 0, 0, NULL, 0, NULL, NULL, 0);
+                                         0,
+                                         disk->protocol,
+                                         disk->nhosts,
+                                         disk->hosts,
+                                         0, NULL, NULL, 0);

     virBufferAddLit(buf, "    </disk>\n");
 }
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 241d63c..bcd92dc 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -48,12 +48,15 @@ enum virDomainSnapshotState {
 typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
 typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr;
 struct _virDomainSnapshotDiskDef {
-    char *name; /* name matching the <target dev='...' of the domain */
-    int index; /* index within snapshot->dom->disks that matches name */
-    int snapshot; /* enum virDomainSnapshotLocation */
-    int type; /* enum virDomainDiskType */
-    char *file; /* new source file when snapshot is external */
-    int format; /* enum virStorageFileFormat */
+    char *name;     /* name matching the <target dev='...' of the domain */
+    int index;      /* index within snapshot->dom->disks that matches name */
+    int snapshot;   /* enum virDomainSnapshotLocation */
+    int type;       /* enum virDomainDiskType */
+    char *file;     /* new source file when snapshot is external */
+    int format;     /* enum virStorageFileFormat */
+    int protocol;   /* network source protocol */
+    size_t nhosts;  /* network source hosts count */
+    virDomainDiskHostDefPtr hosts; /* network source hosts */
 };

 /* Stores the complete snapshot metadata */
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 4378791..c102ddc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1473,9 +1473,6 @@ cleanup:
 int
 qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def)
 {
-    if (def->type == -1)
-        return VIR_DOMAIN_DISK_TYPE_FILE;
-
     return def->type;
 }

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ebb77dc..7f01014 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12207,33 +12207,47 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     }

     if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 ||
-        VIR_STRDUP(source, snap->file) < 0 ||
         (persistDisk && VIR_STRDUP(persistSource, source) < 0))
         goto cleanup;

-    /* create the stub file and set selinux labels; manipulate disk in
-     * place, in a way that can be reverted on failure. */
-    if (!reuse) {
-        fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
-                          &need_unlink, NULL);
-        if (fd < 0)
-            goto cleanup;
-        VIR_FORCE_CLOSE(fd);
-    }
-
     /* XXX Here, we know we are about to alter disk->backingChain if
-     * successful, so we nuke the existing chain so that future
-     * commands will recompute it.  Better would be storing the chain
-     * ourselves rather than reprobing, but this requires modifying
-     * domain_conf and our XML to fully track the chain across
-     * libvirtd restarts.  */
+     * successful, so we nuke the existing chain so that future commands will
+     * recompute it.  Better would be storing the chain ourselves rather than
+     * reprobing, but this requires modifying domain_conf and our XML to fully
+     * track the chain across libvirtd restarts.  */
     virStorageFileFreeMetadata(disk->backingChain);
     disk->backingChain = NULL;

-    if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
-                                          VIR_DISK_CHAIN_READ_WRITE) < 0) {
-        qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
-                                          VIR_DISK_CHAIN_NO_ACCESS);
+    switch (snap->type) {
+    case VIR_DOMAIN_DISK_TYPE_BLOCK:
+        reuse = true;
+        /* fallthrough */
+    case VIR_DOMAIN_DISK_TYPE_FILE:
+        if (VIR_STRDUP(source, snap->file) < 0)
+            goto cleanup;
+
+        /* create the stub file and set selinux labels; manipulate disk in
+         * place, in a way that can be reverted on failure. */
+        if (!reuse) {
+            fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
+                              &need_unlink, NULL);
+            if (fd < 0)
+                goto cleanup;
+            VIR_FORCE_CLOSE(fd);
+        }
+
+        if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+                                              VIR_DISK_CHAIN_READ_WRITE) < 0) {
+            qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+                                              VIR_DISK_CHAIN_NO_ACCESS);
+            goto cleanup;
+        }
+        break;
+
+    default:
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("snapshots are not supported on '%s' volumes"),
+                       virDomainDiskTypeToString(snap->type));
         goto cleanup;
     }

@@ -12252,11 +12266,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
     disk->src = source;
     source = NULL;
     disk->format = format;
+    disk->type = snap->type;
     if (persistDisk) {
         VIR_FREE(persistDisk->src);
         persistDisk->src = persistSource;
         persistSource = NULL;
         persistDisk->format = format;
+        persistDisk->type = snap->type;
     }

 cleanup:
@@ -12298,11 +12314,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
     disk->src = source;
     source = NULL;
     disk->format = origdisk->format;
+    disk->type = origdisk->type;
     if (persistDisk) {
         VIR_FREE(persistDisk->src);
         persistDisk->src = persistSource;
         persistSource = NULL;
         persistDisk->format = origdisk->format;
+        persistDisk->type = origdisk->type;
     }

 cleanup:
diff --git a/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml b/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml
index 41961f1..ddd350d 100644
--- a/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml
+++ b/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml
@@ -2,7 +2,7 @@
   <name>asdf</name>
   <description>adsf</description>
   <disks>
-    <disk name='vda' snapshot='external'>
+    <disk name='vda' snapshot='external' type='file'>
       <source file='/tmp/foo'/>
     </disk>
   </disks>
diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml
index 1a1fc02..0ea7129 100644
--- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml
+++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml
@@ -5,10 +5,10 @@
     <disk name='/dev/HostVG/QEMUGuest1'/>
     <disk name='hdb' snapshot='no'/>
     <disk name='hdc' snapshot='internal'/>
-    <disk name='hdd' snapshot='external'>
+    <disk name='hdd' snapshot='external' type='file'>
       <driver type='qed'/>
     </disk>
-    <disk name='hde' snapshot='external'>
+    <disk name='hde' snapshot='external' type='file'>
       <source file='/path/to/new'/>
     </disk>
   </disks>
diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml
index 5f42bf5..c267db5 100644
--- a/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml
+++ b/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml
@@ -11,15 +11,15 @@
     <disk name='hda' snapshot='no'/>
     <disk name='hdb' snapshot='no'/>
     <disk name='hdc' snapshot='internal'/>
-    <disk name='hdd' snapshot='external'>
+    <disk name='hdd' snapshot='external' type='file'>
       <driver type='qed'/>
       <source file='/path/to/generated4'/>
     </disk>
-    <disk name='hde' snapshot='external'>
+    <disk name='hde' snapshot='external' type='file'>
       <driver type='qcow2'/>
       <source file='/path/to/new'/>
     </disk>
-    <disk name='hdf' snapshot='external'>
+    <disk name='hdf' snapshot='external' type='file'>
       <driver type='qcow2'/>
       <source file='/path/to/generated5'/>
     </disk>
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]