shaba/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-01-10 05:17:59 +03:00
Code Releases Activity

conf: store snapshot source as pointer, for easier manipulation

Browse Source 
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@redhat.com>
This commit is contained in:
Eric Blake 2014-05-21 15:21:02 -06:00
parent 61c44f589b
commit 00c6327a12
4 changed files with 87 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
src/conf/snapshot_conf.c
View File

@ -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");

4
src/conf/snapshot_conf.h
View File

@ -52,7 +52,9 @@ struct _virDomainSnapshotDiskDef {
int index; /* index within snapshot->dom->disks that matches name */
int snapshot; /* virDomainSnapshotLocation */
virStorageSource src; /* new wrapper file when snapshot is external */
/* details of wrapper external file. src is always non-NULL.
* XXX optimize this to allow NULL for internal snapshots? */
virStorageSourcePtr src;
};
/* Stores the complete snapshot metadata */

2
src/qemu/qemu_conf.c
View File

@ -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",

104
src/qemu/qemu_driver.c
View File

@ -12249,14 +12249,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;
@ -12280,10 +12280,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)
@ -12300,11 +12300,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;
}
}
@ -12318,9 +12318,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);
@ -12482,7 +12482,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:
@ -12490,7 +12490,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;
@ -12508,7 +12508,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;
}
@ -12531,7 +12531,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:
@ -12583,33 +12583,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;
}
@ -12732,15 +12732,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;
}
@ -12841,7 +12841,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;
@ -12857,27 +12857,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 */
@ -12893,24 +12893,24 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
VIR_FORCE_CLOSE(fd);
}
if (qemuDomainPrepareDiskChainElement(driver, vm, disk, &snap->src,
if (qemuDomainPrepareDiskChainElement(driver, vm, disk, snap->src,
VIR_DISK_CHAIN_READ_WRITE) < 0) {
qemuDomainPrepareDiskChainElement(driver, vm, disk, &snap->src,
qemuDomainPrepareDiskChainElement(driver, vm, disk, snap->src,
VIR_DISK_CHAIN_NO_ACCESS);
goto cleanup;
}
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;
@ -12919,7 +12919,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;
@ -12930,13 +12930,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.
@ -12968,9 +12968,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;
@ -12983,9 +12983,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;
@ -12993,15 +12993,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;
}