[libvirt] [PATCH 1/5] conf: store snapshot source as pointer, for easier manipulation
Eric Blake
eblake at redhat.com
Thu May 22 04:50:44 UTC 2014
As part of the work on backing chains, I'm finding that it would
be easier to directly manipulate chains of pointers (adding a
snapshot merely adjusts pointers to form the correct list) rather
than copy data from one struct to another. This patch converts
snapshot source to be a pointer.
In this patch, the pointer is ALWAYS allocated (any code that
increases ndisks now also allocates a source pointer for each
new disk), and all other changes are just mechanical fallout of
the new type; there should be no functional change. It is
possible that we may want to leave the pointer NULL for internal
snapshots in a later patch, but as that requires a closer audit
of the source to ensure we don't fault on a null dereference, I
didn't do it here.
* src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Change
type of src.
* src/conf/snapshot_conf.c: Adjust all clients.
* src/qemu/qemu_conf.c: Likewise.
* src/qemu/qemu_driver.c: Likewise.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
src/conf/snapshot_conf.c | 56 ++++++++++++++------------
src/conf/snapshot_conf.h | 2 +-
src/qemu/qemu_conf.c | 2 +-
src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++------------------------
4 files changed, 83 insertions(+), 77 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index b0e4700..441162c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -83,7 +83,8 @@ static void
virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk)
{
VIR_FREE(disk->name);
- virStorageSourceClear(&disk->src);
+ virStorageSourceFree(disk->src);
+ disk->src = NULL;
}
void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
@@ -113,6 +114,9 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
char *type = NULL;
xmlNodePtr cur;
+ if (VIR_ALLOC(def->src) < 0)
+ goto cleanup;
+
def->name = virXMLPropString(node, "name");
if (!def->name) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -132,35 +136,35 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
}
if ((type = virXMLPropString(node, "type"))) {
- if ((def->src.type = virStorageTypeFromString(type)) <= 0 ||
- def->src.type == VIR_STORAGE_TYPE_VOLUME ||
- def->src.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->src.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->src.path &&
+ if (!def->src->path &&
xmlStrEqual(cur->name, BAD_CAST "source")) {
- if (virDomainDiskSourceParse(cur, &def->src) < 0)
+ if (virDomainDiskSourceParse(cur, def->src) < 0)
goto cleanup;
- } else if (!def->src.format &&
+ } else if (!def->src->format &&
xmlStrEqual(cur->name, BAD_CAST "driver")) {
char *driver = virXMLPropString(cur, "type");
if (driver) {
- def->src.format = virStorageFileFormatTypeFromString(driver);
- if (def->src.format < VIR_STORAGE_FILE_BACKING) {
+ def->src->format = virStorageFileFormatTypeFromString(driver);
+ if (def->src->format < VIR_STORAGE_FILE_BACKING) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- def->src.format <= 0
+ def->src->format <= 0
? _("unknown disk snapshot driver '%s'")
: _("disk format '%s' lacks backing file "
"support"),
@@ -173,7 +177,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
}
}
- if (!def->snapshot && (def->src.path || def->src.format))
+ if (!def->snapshot && (def->src->path || def->src->format))
def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
ret = 0;
@@ -506,12 +510,12 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
disk->name, tmp);
goto cleanup;
}
- if (disk->src.path &&
+ 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->src.path, disk->name);
+ disk->src->path, disk->name);
goto cleanup;
}
if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) {
@@ -534,11 +538,13 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
if (inuse)
continue;
disk = &def->disks[ndisks++];
+ if (VIR_ALLOC(disk->src) < 0)
+ goto cleanup;
if (VIR_STRDUP(disk->name, def->dom->disks[i]->dst) < 0)
goto cleanup;
disk->index = i;
disk->snapshot = def->dom->disks[i]->snapshot;
- disk->src.type = VIR_STORAGE_TYPE_FILE;
+ disk->src->type = VIR_STORAGE_TYPE_FILE;
if (!disk->snapshot)
disk->snapshot = default_snapshot;
}
@@ -551,17 +557,17 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
virDomainSnapshotDiskDefPtr disk = &def->disks[i];
if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL &&
- !disk->src.path) {
+ !disk->src->path) {
const char *original = virDomainDiskGetSource(def->dom->disks[i]);
const char *tmp;
struct stat sb;
- if (disk->src.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->src.type));
+ virStorageTypeToString(disk->src->type));
goto cleanup;
}
@@ -583,7 +589,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
tmp = strrchr(original, '.');
if (!tmp || strchr(tmp, '/')) {
- if (virAsprintf(&disk->src.path, "%s.%s", original,
+ if (virAsprintf(&disk->src->path, "%s.%s", original,
def->name) < 0)
goto cleanup;
} else {
@@ -592,7 +598,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
_("integer overflow"));
goto cleanup;
}
- if (virAsprintf(&disk->src.path, "%.*s.%s",
+ if (virAsprintf(&disk->src->path, "%.*s.%s",
(int) (tmp - original), original,
def->name) < 0)
goto cleanup;
@@ -611,7 +617,7 @@ static void
virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virDomainSnapshotDiskDefPtr disk)
{
- int type = disk->src.type;
+ int type = disk->src->type;
if (!disk->name)
return;
@@ -621,7 +627,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " snapshot='%s'",
virDomainSnapshotLocationTypeToString(disk->snapshot));
- if (!disk->src.path && disk->src.format == 0) {
+ if (!disk->src->path && disk->src->format == 0) {
virBufferAddLit(buf, "/>\n");
return;
}
@@ -629,10 +635,10 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " type='%s'>\n", virStorageTypeToString(type));
virBufferAdjustIndent(buf, 2);
- if (disk->src.format > 0)
+ if (disk->src->format > 0)
virBufferEscapeString(buf, "<driver type='%s'/>\n",
- virStorageFileFormatTypeToString(disk->src.format));
- virDomainDiskSourceFormat(buf, &disk->src, 0, 0);
+ virStorageFileFormatTypeToString(disk->src->format));
+ virDomainDiskSourceFormat(buf, disk->src, 0, 0);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</disk>\n");
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 1eb697f..21b5b62 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -52,7 +52,7 @@ struct _virDomainSnapshotDiskDef {
int index; /* index within snapshot->dom->disks that matches name */
int snapshot; /* virDomainSnapshotLocation */
- virStorageSource src; /* new wrapper file when snapshot is external */
+ virStorageSourcePtr 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 f273056..c14d700 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1427,7 +1427,7 @@ int
qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainSnapshotDiskDefPtr def)
{
- if (def->src.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 6fda50d..bf46d5d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12190,14 +12190,14 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
- if (!snapdisk->src.format)
- snapdisk->src.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->src.format),
+ virStorageFileFormatTypeToString(snapdisk->src->format),
"-o",
NULL)))
goto cleanup;
@@ -12221,10 +12221,10 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
}
/* adds cmd line args: /path/to/target/file */
- virCommandAddArg(cmd, snapdisk->src.path);
+ virCommandAddArg(cmd, snapdisk->src->path);
/* If the target does not exist, we're going to create it possibly */
- if (!virFileExists(snapdisk->src.path))
+ if (!virFileExists(snapdisk->src->path))
ignore_value(virBitmapSetBit(created, i));
if (virCommandRun(cmd, NULL) < 0)
@@ -12241,11 +12241,11 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
VIR_FREE(defdisk->src.path);
- if (VIR_STRDUP(defdisk->src.path, snapdisk->src.path) < 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->src.format;
+ defdisk->src.format = snapdisk->src->format;
}
}
@@ -12259,9 +12259,9 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
ssize_t bit = -1;
while ((bit = virBitmapNextSetBit(created, bit)) >= 0) {
snapdisk = &(snap->def->disks[bit]);
- if (unlink(snapdisk->src.path) < 0)
+ if (unlink(snapdisk->src->path) < 0)
VIR_WARN("Failed to remove snapshot image '%s'",
- snapdisk->src.path);
+ snapdisk->src->path);
}
}
virBitmapFree(created);
@@ -12422,7 +12422,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingActive(virDomainDiskDefPtr disk)
static int
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr disk)
{
- int actualType = virStorageSourceGetActualType(&disk->src);
+ int actualType = virStorageSourceGetActualType(disk->src);
switch ((virStorageType) actualType) {
case VIR_STORAGE_TYPE_BLOCK:
@@ -12430,7 +12430,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
return 0;
case VIR_STORAGE_TYPE_NETWORK:
- switch ((virStorageNetProtocol) disk->src.protocol) {
+ switch ((virStorageNetProtocol) disk->src->protocol) {
case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
return 0;
@@ -12447,7 +12447,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
virReportError(VIR_ERR_INTERNAL_ERROR,
_("external active snapshots are not supported on "
"'network' disks using '%s' protocol"),
- virStorageNetProtocolTypeToString(disk->src.protocol));
+ virStorageNetProtocolTypeToString(disk->src->protocol));
return -1;
}
@@ -12470,7 +12470,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
static int
qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr disk)
{
- int actualType = virStorageSourceGetActualType(&disk->src);
+ int actualType = virStorageSourceGetActualType(disk->src);
switch ((virStorageType) actualType) {
case VIR_STORAGE_TYPE_BLOCK:
@@ -12522,33 +12522,33 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
return -1;
}
- if (virStorageFileInit(&snapdisk->src) < 0)
+ if (virStorageFileInit(snapdisk->src) < 0)
return -1;
- if (virStorageFileStat(&snapdisk->src, &st) < 0) {
+ if (virStorageFileStat(snapdisk->src, &st) < 0) {
if (errno != ENOENT) {
virReportSystemError(errno,
_("unable to stat for disk %s: %s"),
- snapdisk->name, snapdisk->src.path);
+ snapdisk->name, snapdisk->src->path);
goto cleanup;
} else if (reuse) {
virReportSystemError(errno,
_("missing existing file for disk %s: %s"),
- snapdisk->name, snapdisk->src.path);
+ 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->src.path);
+ snapdisk->name, snapdisk->src->path);
goto cleanup;
}
ret = 0;
cleanup:
- virStorageFileDeinit(&snapdisk->src);
+ virStorageFileDeinit(snapdisk->src);
return ret;
}
@@ -12670,15 +12670,15 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
break;
case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
- 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) {
+ 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->src.format));
+ virStorageFileFormatTypeToString(disk->src->format));
goto cleanup;
}
@@ -12777,7 +12777,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
char *newsource = NULL;
virStorageNetHostDefPtr newhosts = NULL;
virStorageNetHostDefPtr persistHosts = NULL;
- int format = snap->src.format;
+ int format = snap->src->format;
const char *formatStr = NULL;
char *persistSource = NULL;
int ret = -1;
@@ -12793,27 +12793,27 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0)
goto cleanup;
- /* XXX Here, we know we are about to alter disk->src.backingStore if
+ /* XXX Here, we know we are about to alter disk->src->backingStore 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. */
virStorageSourceClearBackingStore(&disk->src);
- if (virStorageFileInit(&snap->src) < 0)
+ if (virStorageFileInit(snap->src) < 0)
goto cleanup;
- if (qemuGetDriveSourceString(&snap->src, NULL, &source) < 0)
+ if (qemuGetDriveSourceString(snap->src, NULL, &source) < 0)
goto cleanup;
- if (VIR_STRDUP(newsource, snap->src.path) < 0)
+ if (VIR_STRDUP(newsource, snap->src->path) < 0)
goto cleanup;
if (persistDisk &&
- VIR_STRDUP(persistSource, snap->src.path) < 0)
+ VIR_STRDUP(persistSource, snap->src->path) < 0)
goto cleanup;
- switch ((virStorageType)snap->src.type) {
+ switch ((virStorageType)snap->src->type) {
case VIR_STORAGE_TYPE_BLOCK:
reuse = true;
/* fallthrough */
@@ -12838,15 +12838,15 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
break;
case VIR_STORAGE_TYPE_NETWORK:
- switch (snap->src.protocol) {
+ switch (snap->src->protocol) {
case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
- if (!(newhosts = virStorageNetHostDefCopy(snap->src.nhosts,
- snap->src.hosts)))
+ if (!(newhosts = virStorageNetHostDefCopy(snap->src->nhosts,
+ snap->src->hosts)))
goto cleanup;
if (persistDisk &&
- !(persistHosts = virStorageNetHostDefCopy(snap->src.nhosts,
- snap->src.hosts)))
+ !(persistHosts = virStorageNetHostDefCopy(snap->src->nhosts,
+ snap->src->hosts)))
goto cleanup;
break;
@@ -12855,7 +12855,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("snapshots on volumes using '%s' protocol "
"are not supported"),
- virStorageNetProtocolTypeToString(snap->src.protocol));
+ virStorageNetProtocolTypeToString(snap->src->protocol));
goto cleanup;
}
break;
@@ -12866,13 +12866,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
case VIR_STORAGE_TYPE_LAST:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("snapshots are not supported on '%s' volumes"),
- virStorageTypeToString(snap->src.type));
+ virStorageTypeToString(snap->src->type));
goto cleanup;
}
/* create the actual snapshot */
- if (snap->src.format)
- formatStr = virStorageFileFormatTypeToString(snap->src.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.
@@ -12904,9 +12904,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
disk->src.path = newsource;
disk->src.format = format;
- disk->src.type = snap->src.type;
- disk->src.protocol = snap->src.protocol;
- disk->src.nhosts = snap->src.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;
@@ -12919,9 +12919,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
persistDisk->src.path = persistSource;
persistDisk->src.format = format;
- persistDisk->src.type = snap->src.type;
- persistDisk->src.protocol = snap->src.protocol;
- persistDisk->src.nhosts = snap->src.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;
@@ -12929,15 +12929,15 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
}
cleanup:
- if (need_unlink && virStorageFileUnlink(&snap->src))
+ if (need_unlink && virStorageFileUnlink(snap->src))
VIR_WARN("unable to unlink just-created %s", source);
- virStorageFileDeinit(&snap->src);
+ virStorageFileDeinit(snap->src);
VIR_FREE(device);
VIR_FREE(source);
VIR_FREE(newsource);
VIR_FREE(persistSource);
- virStorageNetHostDefFree(snap->src.nhosts, newhosts);
- virStorageNetHostDefFree(snap->src.nhosts, persistHosts);
+ virStorageNetHostDefFree(snap->src->nhosts, newhosts);
+ virStorageNetHostDefFree(snap->src->nhosts, persistHosts);
return ret;
}
--
1.9.0
More information about the libvir-list
mailing list