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