7:14 a.m.
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