Now that we have a common struct, it's time to start using it! Since external snapshots make a longer backing chain, it is only natural to use the same struct for the file created by the snapshot as what we use for <domain> disks. * src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Use common struct instead of open-coded duplicate fields. * src/conf/snapshot_conf.c (virDomainSnapshotDiskDefClear) (virDomainSnapshotDiskDefParseXML, virDomainSnapshotAlignDisks) (virDomainSnapshotDiskDefFormat) (virDomainSnapshotDiskGetActualType): Adjust clients. * src/qemu/qemu_conf.c (qemuTranslateSnapshotDiskSourcePool): Likewise. * src/qemu/qemu_driver.c (qemuDomainSnapshotDiskGetSourceString) (qemuDomainSnapshotCreateInactiveExternal) (qemuDomainSnapshotPrepareDiskExternalOverlayActive) (qemuDomainSnapshotPrepareDiskExternal) (qemuDomainSnapshotPrepare) (qemuDomainSnapshotCreateSingleDiskActive): Likewise. * src/storage/storage_driver.c (virStorageFileInitFromSnapshotDef): Likewise. Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> --- src/conf/snapshot_conf.c | 68 +++++++++++++++++------------------ src/conf/snapshot_conf.h | 8 ++--- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_driver.c | 86 ++++++++++++++++++++++---------------------- src/storage/storage_driver.c | 8 ++--- 5 files changed, 85 insertions(+), 87 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index ebb3bb4..3bd4732 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -83,9 +83,7 @@ static void virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) { VIR_FREE(disk->name); - VIR_FREE(disk->file); - virStorageNetHostDefFree(disk->nhosts, disk->hosts); - disk->nhosts = 0; + virStorageSourceClear(&disk->src); } void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) @@ -134,39 +132,39 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } if ((type = virXMLPropString(node, "type"))) { - if ((def->type = virStorageTypeFromString(type)) < 0 || - def->type == VIR_STORAGE_TYPE_VOLUME || - def->type == VIR_STORAGE_TYPE_DIR) { + if ((def->src.type = virStorageTypeFromString(type)) < 0 || + def->src.type == VIR_STORAGE_TYPE_VOLUME || + def->src.type == VIR_STORAGE_TYPE_DIR) { virReportError(VIR_ERR_XML_ERROR, _("unknown disk snapshot type '%s'"), type); goto cleanup; } } else { - def->type = VIR_STORAGE_TYPE_FILE; + def->src.type = VIR_STORAGE_TYPE_FILE; } for (cur = node->children; cur; cur = cur->next) { if (cur->type != XML_ELEMENT_NODE) continue; - if (!def->file && + if (!def->src.path && xmlStrEqual(cur->name, BAD_CAST "source")) { if (virDomainDiskSourceDefParse(cur, - def->type, - &def->file, - &def->protocol, - &def->nhosts, - &def->hosts, + def->src.type, + &def->src.path, + &def->src.protocol, + &def->src.nhosts, + &def->src.hosts, NULL) < 0) goto cleanup; - } else if (!def->format && + } else if (!def->src.format && xmlStrEqual(cur->name, BAD_CAST "driver")) { char *driver = virXMLPropString(cur, "type"); if (driver) { - def->format = virStorageFileFormatTypeFromString(driver); - if (def->format <= 0) { + def->src.format = virStorageFileFormatTypeFromString(driver); + if (def->src.format <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown disk snapshot driver '%s'"), driver); @@ -178,7 +176,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } } - if (!def->snapshot && (def->file || def->format)) + if (!def->snapshot && (def->src.path || def->src.format)) def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; ret = 0; @@ -511,12 +509,12 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, disk->name, tmp); goto cleanup; } - if (disk->file && + if (disk->src.path && disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("file '%s' for disk '%s' requires " "use of external snapshot mode"), - disk->file, disk->name); + disk->src.path, disk->name); goto cleanup; } if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) { @@ -543,7 +541,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; disk->index = i; disk->snapshot = def->dom->disks[i]->snapshot; - disk->type = VIR_STORAGE_TYPE_FILE; + disk->src.type = VIR_STORAGE_TYPE_FILE; if (!disk->snapshot) disk->snapshot = default_snapshot; } @@ -556,16 +554,17 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, virDomainSnapshotDiskDefPtr disk = &def->disks[i]; if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL && - !disk->file) { + !disk->src.path) { const char *original = virDomainDiskGetSource(def->dom->disks[i]); const char *tmp; struct stat sb; - if (disk->type != VIR_STORAGE_TYPE_FILE) { + if (disk->src.type != VIR_STORAGE_TYPE_FILE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name " "for disk '%s' on a '%s' device"), - disk->name, virStorageTypeToString(disk->type)); + disk->name, + virStorageTypeToString(disk->src.type)); goto cleanup; } @@ -587,7 +586,8 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, tmp = strrchr(original, '.'); if (!tmp || strchr(tmp, '/')) { - if (virAsprintf(&disk->file, "%s.%s", original, def->name) < 0) + if (virAsprintf(&disk->src.path, "%s.%s", original, + def->name) < 0) goto cleanup; } else { if ((tmp - original) > INT_MAX) { @@ -595,7 +595,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, _("integer overflow")); goto cleanup; } - if (virAsprintf(&disk->file, "%.*s.%s", + if (virAsprintf(&disk->src.path, "%.*s.%s", (int) (tmp - original), original, def->name) < 0) goto cleanup; @@ -614,7 +614,7 @@ static void virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainSnapshotDiskDefPtr disk) { - int type = disk->type; + int type = disk->src.type; if (!disk->name) return; @@ -624,7 +624,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(disk->snapshot)); - if (!disk->file && disk->format == 0) { + if (!disk->src.path && disk->src.format == 0) { virBufferAddLit(buf, "/>\n"); return; } @@ -632,16 +632,16 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " type='%s'>\n", virStorageTypeToString(type)); virBufferAdjustIndent(buf, 2); - if (disk->format > 0) + if (disk->src.format > 0) virBufferEscapeString(buf, "<driver type='%s'/>\n", - virStorageFileFormatTypeToString(disk->format)); + virStorageFileFormatTypeToString(disk->src.format)); virDomainDiskSourceDefFormatInternal(buf, type, - disk->file, + disk->src.path, 0, - disk->protocol, - disk->nhosts, - disk->hosts, + disk->src.protocol, + disk->src.nhosts, + disk->src.hosts, 0, NULL, NULL, 0); virBufferAdjustIndent(buf, -2); @@ -1308,5 +1308,5 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, int virDomainSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def) { - return def->type; + return def->src.type; } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index f6fec88..1444d77 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -51,12 +51,8 @@ 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 virStorageType */ - 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 */ - virStorageNetHostDefPtr hosts; /* network source hosts */ + + virStorageSource src; /* new wrapper file when snapshot is external */ }; /* Stores the complete snapshot metadata */ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 209558d..198ee2f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1422,7 +1422,7 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainSnapshotDiskDefPtr def) { - if (def->type != VIR_STORAGE_TYPE_VOLUME) + if (def->src.type != VIR_STORAGE_TYPE_VOLUME) return 0; virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9d48a46..9a2de12 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12028,10 +12028,10 @@ qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk, *source = NULL; return qemuGetDriveSourceString(virDomainSnapshotDiskGetActualType(disk), - disk->file, - disk->protocol, - disk->nhosts, - disk->hosts, + disk->src.path, + disk->src.protocol, + disk->src.nhosts, + disk->src.hosts, NULL, NULL, source); @@ -12178,14 +12178,14 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) continue; - if (!snapdisk->format) - snapdisk->format = VIR_STORAGE_FILE_QCOW2; + if (!snapdisk->src.format) + snapdisk->src.format = VIR_STORAGE_FILE_QCOW2; /* creates cmd line args: qemu-img create -f qcow2 -o */ if (!(cmd = virCommandNewArgList(qemuImgPath, "create", "-f", - virStorageFileFormatTypeToString(snapdisk->format), + virStorageFileFormatTypeToString(snapdisk->src.format), "-o", NULL))) goto cleanup; @@ -12209,10 +12209,10 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, } /* adds cmd line args: /path/to/target/file */ - virCommandAddArg(cmd, snapdisk->file); + virCommandAddArg(cmd, snapdisk->src.path); /* If the target does not exist, we're going to create it possibly */ - if (!virFileExists(snapdisk->file)) + if (!virFileExists(snapdisk->src.path)) ignore_value(virBitmapSetBit(created, i)); if (virCommandRun(cmd, NULL) < 0) @@ -12229,11 +12229,11 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { VIR_FREE(defdisk->src.path); - if (VIR_STRDUP(defdisk->src.path, snapdisk->file) < 0) { + if (VIR_STRDUP(defdisk->src.path, snapdisk->src.path) < 0) { /* we cannot rollback here in a sane way */ goto cleanup; } - defdisk->src.format = snapdisk->format; + defdisk->src.format = snapdisk->src.format; } } @@ -12247,9 +12247,9 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, ssize_t bit = -1; while ((bit = virBitmapNextSetBit(created, bit)) >= 0) { snapdisk = &(snap->def->disks[bit]); - if (unlink(snapdisk->file) < 0) + if (unlink(snapdisk->src.path) < 0) VIR_WARN("Failed to remove snapshot image '%s'", - snapdisk->file); + snapdisk->src.path); } } virBitmapFree(created); @@ -12417,7 +12417,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d return 0; case VIR_STORAGE_TYPE_NETWORK: - switch ((enum virStorageNetProtocol) disk->protocol) { + switch ((enum virStorageNetProtocol) disk->src.protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: return 0; @@ -12434,7 +12434,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d virReportError(VIR_ERR_INTERNAL_ERROR, _("external active snapshots are not supported on " "'network' disks using '%s' protocol"), - virStorageNetProtocolTypeToString(disk->protocol)); + virStorageNetProtocolTypeToString(disk->src.protocol)); return -1; } @@ -12515,19 +12515,19 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, if (errno != ENOENT) { virReportSystemError(errno, _("unable to stat for disk %s: %s"), - snapdisk->name, snapdisk->file); + snapdisk->name, snapdisk->src.path); goto cleanup; } else if (reuse) { virReportSystemError(errno, _("missing existing file for disk %s: %s"), - snapdisk->name, snapdisk->file); + snapdisk->name, snapdisk->src.path); goto cleanup; } } else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("external snapshot file for disk %s already " "exists and is not a block device: %s"), - snapdisk->name, snapdisk->file); + snapdisk->name, snapdisk->src.path); goto cleanup; } @@ -12654,15 +12654,15 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, break; case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: - if (!disk->format) { - disk->format = VIR_STORAGE_FILE_QCOW2; - } else if (disk->format != VIR_STORAGE_FILE_QCOW2 && - disk->format != VIR_STORAGE_FILE_QED) { + if (!disk->src.format) { + disk->src.format = VIR_STORAGE_FILE_QCOW2; + } else if (disk->src.format != VIR_STORAGE_FILE_QCOW2 && + disk->src.format != VIR_STORAGE_FILE_QED) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("external snapshot format for disk %s " "is unsupported: %s"), disk->name, - virStorageFileFormatTypeToString(disk->format)); + virStorageFileFormatTypeToString(disk->src.format)); goto cleanup; } @@ -12750,7 +12750,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, char *newsource = NULL; virStorageNetHostDefPtr newhosts = NULL; virStorageNetHostDefPtr persistHosts = NULL; - int format = snap->format; + int format = snap->src.format; const char *formatStr = NULL; char *persistSource = NULL; int ret = -1; @@ -12781,14 +12781,14 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, if (qemuDomainSnapshotDiskGetSourceString(snap, &source) < 0) goto cleanup; - if (VIR_STRDUP(newsource, snap->file) < 0) + if (VIR_STRDUP(newsource, snap->src.path) < 0) goto cleanup; if (persistDisk && - VIR_STRDUP(persistSource, snap->file) < 0) + VIR_STRDUP(persistSource, snap->src.path) < 0) goto cleanup; - switch (snap->type) { + switch (snap->src.type) { case VIR_STORAGE_TYPE_BLOCK: reuse = true; /* fallthrough */ @@ -12813,13 +12813,15 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, break; case VIR_STORAGE_TYPE_NETWORK: - switch (snap->protocol) { + switch (snap->src.protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - if (!(newhosts = virStorageNetHostDefCopy(snap->nhosts, snap->hosts))) + if (!(newhosts = virStorageNetHostDefCopy(snap->src.nhosts, + snap->src.hosts))) goto cleanup; if (persistDisk && - !(persistHosts = virStorageNetHostDefCopy(snap->nhosts, snap->hosts))) + !(persistHosts = virStorageNetHostDefCopy(snap->src.nhosts, + snap->src.hosts))) goto cleanup; break; @@ -12828,7 +12830,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("snapshots on volumes using '%s' protocol " "are not supported"), - virStorageNetProtocolTypeToString(snap->protocol)); + virStorageNetProtocolTypeToString(snap->src.protocol)); goto cleanup; } break; @@ -12836,13 +12838,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("snapshots are not supported on '%s' volumes"), - virStorageTypeToString(snap->type)); + virStorageTypeToString(snap->src.type)); goto cleanup; } /* create the actual snapshot */ - if (snap->format) - formatStr = virStorageFileFormatTypeToString(snap->format); + if (snap->src.format) + formatStr = virStorageFileFormatTypeToString(snap->src.format); /* The monitor is only accessed if qemu doesn't support transactions. * Otherwise the following monitor command only constructs the command. @@ -12874,9 +12876,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, disk->src.path = newsource; disk->src.format = format; - disk->src.type = snap->type; - disk->src.protocol = snap->protocol; - disk->src.nhosts = snap->nhosts; + disk->src.type = snap->src.type; + disk->src.protocol = snap->src.protocol; + disk->src.nhosts = snap->src.nhosts; disk->src.hosts = newhosts; newsource = NULL; @@ -12889,9 +12891,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, persistDisk->src.path = persistSource; persistDisk->src.format = format; - persistDisk->src.type = snap->type; - persistDisk->src.protocol = snap->protocol; - persistDisk->src.nhosts = snap->nhosts; + persistDisk->src.type = snap->src.type; + persistDisk->src.protocol = snap->src.protocol; + persistDisk->src.nhosts = snap->src.nhosts; persistDisk->src.hosts = persistHosts; persistSource = NULL; @@ -12906,8 +12908,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, VIR_FREE(source); VIR_FREE(newsource); VIR_FREE(persistSource); - virStorageNetHostDefFree(snap->nhosts, newhosts); - virStorageNetHostDefFree(snap->nhosts, persistHosts); + virStorageNetHostDefFree(snap->src.nhosts, newhosts); + virStorageNetHostDefFree(snap->src.nhosts, persistHosts); return ret; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index cdb8536..6fca3d2 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2802,10 +2802,10 @@ virStorageFilePtr virStorageFileInitFromSnapshotDef(virDomainSnapshotDiskDefPtr disk) { return virStorageFileInitInternal(virDomainSnapshotDiskGetActualType(disk), - disk->file, - disk->protocol, - disk->nhosts, - disk->hosts); + disk->src.path, + disk->src.protocol, + disk->src.nhosts, + disk->src.hosts); } -- 1.9.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list