From 7e5acfbcb67f2afbc32d2d1371bccc0c96ca475f Mon Sep 17 00:00:00 2001 Message-Id: <7e5acfbcb67f2afbc32d2d1371bccc0c96ca475f@dist-git> From: Peter Krempa Date: Wed, 26 Feb 2014 14:55:22 +0100 Subject: [PATCH] snapshot: Add support for specifying snapshot disk backing type https://bugzilla.redhat.com/show_bug.cgi?id=1032370 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). Also amend the test suite to check parsing of the various new disk types that can now be specified. (cherry picked from commit 7076b4b72ce65c43a88af6b11558ed37893af692) Signed-off-by: Jiri Denemark --- docs/formatsnapshot.html.in | 15 ++++++ docs/schemas/domainsnapshot.rng | 60 ++++++++++++++++++---- src/conf/snapshot_conf.c | 43 +++++++++------- src/conf/snapshot_conf.h | 15 +++--- src/qemu/qemu_conf.c | 3 -- src/qemu/qemu_driver.c | 58 +++++++++++++-------- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 18 +++++++ .../disk_driver_name_null.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 22 +++++++- .../disk_snapshot_redefine.xml | 6 +-- 10 files changed, 178 insertions(+), 64 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 76689cb..44ed4fd 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. + + Since 1.2.2 the disk element + supports an optional attribute type if the + snapshot attribute is set to external. + This attribute specifies the snapshot target storage type and allows + to overwrite the default file type. The type + attribute along with the format of the source + sub-element is identical to the source element used in + domain disk definitions. See the + disk devices section + documentation for further information. + + Libvirt currently supports the type element in the qemu + driver and supported values are file and + block (since 1.2.2). diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 169fcfb..824a186 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -123,19 +123,57 @@ external - - - - + + + + + file + + + - - - + + + + + + + + - - - - + + + + + + block + + + + + + + + + + + + + + + + network + + + + + + + + + + + diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 3a7ed5d..3510677 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,17 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } } - def->type = -1; + if ((type = virXMLPropString(node, "type"))) { + if ((def->type = virDomainDiskTypeFromString(type)) < 0 || + def->type == VIR_DOMAIN_DISK_TYPE_VOLUME || + def->type == VIR_DOMAIN_DISK_TYPE_DIR) { + 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 +148,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 +180,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, ret = 0; cleanup: VIR_FREE(snapshot); + VIR_FREE(type); if (ret < 0) virDomainSnapshotDiskDefClear(def); return ret; @@ -495,7 +502,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; } @@ -513,8 +520,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"), @@ -577,15 +583,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, " \n", @@ -593,7 +596,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, " \n"); } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 9ed80b2..12ece0c 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 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 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 1f25675..af1e3be 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1442,9 +1442,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 1c9b190..641d118 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12529,33 +12529,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; } @@ -12591,11 +12605,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: @@ -12637,11 +12653,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/domainsnapshotxml2xmlin/disk_snapshot.xml b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml index ee6b46a..aa1522a 100644 --- a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml @@ -12,5 +12,23 @@ + + + + + + + + + + + + + + + + + + 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 @@ asdf adsf - + diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index 1a1fc02..c2e77d7 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -5,11 +5,29 @@ - + - + + + + + + + + + + + + + + + + + + + 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 @@ - + - + - + -- 1.9.0