Refactor the function to accept a virStorageSourcePtr instead of just the path, add a check to run it only on local storage and fix callers (possibly by using a newly introduced wrapper that wraps a path in the virStorageSource struct for legacy code) --- src/qemu/qemu_driver.c | 77 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fca1a91..ac0de1e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12013,22 +12013,26 @@ static int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - const char *file, + virStorageSourcePtr elem, qemuDomainDiskChainMode mode) { /* The easiest way to label a single file with the same * permissions it would have as if part of the disk chain is to * temporarily modify the disk in place. */ - char *origsrc = disk->src.path; - int origformat = disk->src.format; - virStorageSourcePtr origchain = disk->src.backingStore; + virStorageSource origdisk; bool origreadonly = disk->readonly; + virQEMUDriverConfigPtr cfg = NULL; int ret = -1; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - disk->src.path = (char *) file; /* casting away const is safe here */ - disk->src.format = VIR_STORAGE_FILE_RAW; - disk->src.backingStore = NULL; + /* XXX Labelling of non-local files isn't currently supported */ + if (virStorageSourceGetActualType(&disk->src) == VIR_STORAGE_TYPE_NETWORK) + return 0; + + cfg = virQEMUDriverGetConfig(driver); + + /* XXX This will be easier when disk->src will be a pointer */ + memcpy(&origdisk, &disk->src, sizeof(origdisk)); + memcpy(&disk->src, elem, sizeof(*elem)); disk->readonly = mode == VIR_DISK_CHAIN_READ_ONLY; if (mode == VIR_DISK_CHAIN_NO_ACCESS) { @@ -12051,9 +12055,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, ret = 0; cleanup: - disk->src.path = origsrc; - disk->src.format = origformat; - disk->src.backingStore = origchain; + memcpy(&disk->src, &origdisk, sizeof(origdisk)); disk->readonly = origreadonly; virObjectUnref(cfg); return ret; @@ -12061,6 +12063,25 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, static int +qemuDomainPrepareDiskChainElementPath(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + const char *file, + qemuDomainDiskChainMode mode) +{ + virStorageSource src; + + memset(&src, 0, sizeof(src)); + + src.type = VIR_STORAGE_TYPE_FILE; + src.format = VIR_STORAGE_FILE_RAW; + src.path = (char *) file; /* casting away const is safe here */ + + return qemuDomainPrepareDiskChainElement(driver, vm, disk, &src, mode); +} + + +static int qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -12765,9 +12786,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, VIR_FORCE_CLOSE(fd); } - if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source, + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, &snap->src, VIR_DISK_CHAIN_READ_WRITE) < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, disk, source, + qemuDomainPrepareDiskChainElement(driver, vm, disk, &snap->src, VIR_DISK_CHAIN_NO_ACCESS); goto cleanup; } @@ -12898,7 +12919,7 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src.path, + qemuDomainPrepareDiskChainElement(driver, vm, disk, &disk->src, VIR_DISK_CHAIN_NO_ACCESS); if (need_unlink && virStorageFileStat(&disk->src, &st) == 0 && S_ISREG(st.st_mode) && @@ -15208,10 +15229,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm, if (VIR_STRDUP(mirror, dest) < 0) goto endjob; - if (qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, - VIR_DISK_CHAIN_READ_WRITE) < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, - VIR_DISK_CHAIN_NO_ACCESS); + if (qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest, + VIR_DISK_CHAIN_READ_WRITE) < 0) { + qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest, + VIR_DISK_CHAIN_NO_ACCESS); goto endjob; } @@ -15222,8 +15243,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm, virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, - VIR_DISK_CHAIN_NO_ACCESS); + qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest, + VIR_DISK_CHAIN_NO_ACCESS); goto endjob; } @@ -15400,12 +15421,12 @@ qemuDomainBlockCommit(virDomainPtr dom, * operation succeeds, but doing that requires tracking the * operation in XML across libvirtd restarts. */ clean_access = true; - if (qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource->path, + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource, VIR_DISK_CHAIN_READ_WRITE) < 0 || (top_parent && top_parent != disk->src.path && - qemuDomainPrepareDiskChainElement(driver, vm, disk, - top_parent, - VIR_DISK_CHAIN_READ_WRITE) < 0)) + qemuDomainPrepareDiskChainElementPath(driver, vm, disk, + top_parent, + VIR_DISK_CHAIN_READ_WRITE) < 0)) goto endjob; /* Start the commit operation. Pass the user's original spelling, @@ -15423,12 +15444,12 @@ qemuDomainBlockCommit(virDomainPtr dom, endjob: if (ret < 0 && clean_access) { /* Revert access to read-only, if possible. */ - qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource->path, + qemuDomainPrepareDiskChainElement(driver, vm, disk, baseSource, VIR_DISK_CHAIN_READ_ONLY); if (top_parent && top_parent != disk->src.path) - qemuDomainPrepareDiskChainElement(driver, vm, disk, - top_parent, - VIR_DISK_CHAIN_READ_ONLY); + qemuDomainPrepareDiskChainElementPath(driver, vm, disk, + top_parent, + VIR_DISK_CHAIN_READ_ONLY); } if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.2

The gluster volume name was previously stored as part of the source path string. This isn't a good idea when we want to start to deal with resolving of relative paths on gluster as a storage volume can't actually be changed by ../-ing it. Parse and store the volume name separately for gluster storage volumes and use the newly stored variable appropriately. --- src/conf/domain_conf.c | 33 ++++++++++++++++++++++++++++++++- src/qemu/qemu_command.c | 19 ++++++++++++++----- src/storage/storage_backend_gluster.c | 27 ++++++++------------------- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 5 files changed, 56 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c3bdad..3cb6693 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5002,6 +5002,27 @@ virDomainDiskSourceParse(xmlNodePtr node, goto cleanup; } + /* for historical reasons the volume name for gluster volume is stored + * as a part of the path. This is hard to work with when dealing with + * relative names. Split out the volume into a separate variable */ + if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { + char *tmp; + if (!(tmp = strchr(src->path, '/')) || + tmp == src->path) { + virReportError(VIR_ERR_XML_ERROR, + _("missing volume name or file name in " + "gluster source path '%s'"), src->path); + goto cleanup; + } + + src->volume = src->path; + + if (VIR_STRDUP(src->path, tmp) < 0) + goto cleanup; + + tmp[0] = '\0'; + } + child = node->children; while (child != NULL) { if (child->type == XML_ELEMENT_NODE && @@ -14841,6 +14862,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, unsigned int flags) { size_t n; + char *path = NULL; const char *startupPolicy = NULL; if (policy) @@ -14876,7 +14898,16 @@ virDomainDiskSourceFormat(virBufferPtr buf, case VIR_STORAGE_TYPE_NETWORK: virBufferAsprintf(buf, "<source protocol='%s'", virStorageNetProtocolTypeToString(src->protocol)); - virBufferEscapeString(buf, " name='%s'", src->path); + + + if (src->volume) { + if (virAsprintf(&path, "%s%s", src->volume, src->path) < 0) + return -1; + } + + virBufferEscapeString(buf, " name='%s'", path ? path : src->path); + + VIR_FREE(path); if (src->nhosts == 0) { virBufferAddLit(buf, "/>\n"); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c1e17d..c0f2fb6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3594,6 +3594,7 @@ qemuNetworkDriveGetPort(int protocol, static char * qemuBuildNetworkDriveURI(int protocol, const char *src, + const char *volume, size_t nhosts, virStorageNetHostDefPtr hosts, const char *username, @@ -3693,11 +3694,18 @@ qemuBuildNetworkDriveURI(int protocol, if ((uri->port = qemuNetworkDriveGetPort(protocol, hosts->port)) < 0) goto cleanup; - if (src && - virAsprintf(&uri->path, "%s%s", - src[0] == '/' ? "" : "/", - src) < 0) - goto cleanup; + if (src) { + if (volume) { + if (virAsprintf(&uri->path, "/%s%s", + volume, src) < 0) + goto cleanup; + } else { + if (virAsprintf(&uri->path, "%s%s", + src[0] == '/' ? "" : "/", + src) < 0) + goto cleanup; + } + } if (hosts->socket && virAsprintf(&uri->query, "socket=%s", hosts->socket) < 0) @@ -3859,6 +3867,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, case VIR_STORAGE_TYPE_NETWORK: if (!(*source = qemuBuildNetworkDriveURI(src->protocol, src->path, + src->volume, src->nhosts, src->hosts, username, diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 2f0592f..8679d96 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -533,8 +533,6 @@ typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr; struct _virStorageFileBackendGlusterPriv { glfs_t *vol; - char *volname; - char *path; }; @@ -547,7 +545,6 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src) if (priv->vol) glfs_fini(priv->vol); - VIR_FREE(priv->volname); VIR_FREE(priv); src->drv->priv = NULL; @@ -564,21 +561,14 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) VIR_DEBUG("initializing gluster storage file %p(%s/%s)", src, hostname, src->path); - if (VIR_ALLOC(priv) < 0) - return -1; - - if (VIR_STRDUP(priv->volname, src->path) < 0) - goto error; - - if (!(priv->path = strchr(priv->volname, '/'))) { + if (!src->volume) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid path of gluster volume: '%s'"), - src->path); - goto error; + _("missing gluster volume name for path '%s'"), src->path); + return -1; } - *priv->path = '\0'; - priv->path++; + if (VIR_ALLOC(priv) < 0) + return -1; if (host->port && virStrToLong_i(host->port, NULL, 10, &port) < 0) { @@ -591,7 +581,7 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) hostname = host->socket; - if (!(priv->vol = glfs_new(priv->volname))) { + if (!(priv->vol = glfs_new(src->volume))) { virReportOOMError(); goto error; } @@ -617,7 +607,6 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) return 0; error: - VIR_FREE(priv->volname); if (priv->vol) glfs_fini(priv->vol); VIR_FREE(priv); @@ -632,7 +621,7 @@ virStorageFileBackendGlusterUnlink(virStorageSourcePtr src) virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; int ret; - ret = glfs_unlink(priv->vol, priv->path); + ret = glfs_unlink(priv->vol, src->path); /* preserve errno */ VIR_DEBUG("removing storage file %p(%s/%s): ret=%d, errno=%d", @@ -648,7 +637,7 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src, virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; int ret; - ret = glfs_stat(priv->vol, priv->path, st); + ret = glfs_stat(priv->vol, src->path, st); /* preserve errno */ VIR_DEBUG("stat of storage file %p(%s/%s): ret=%d, errno=%d", diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 96af27b..cf270f6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1755,6 +1755,7 @@ virStorageSourceClear(virStorageSourcePtr def) return; VIR_FREE(def->path); + VIR_FREE(def->volume); virStorageSourcePoolDefFree(def->srcpool); VIR_FREE(def->driverName); virBitmapFree(def->features); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 6072409..3777d66 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -210,6 +210,7 @@ typedef virStorageSource *virStorageSourcePtr; struct _virStorageSource { int type; /* enum virStorageType */ char *path; + char *volume; /* volume name for remote storage */ int protocol; /* enum virStorageNetProtocol */ size_t nhosts; virStorageNetHostDefPtr hosts; -- 1.9.2

Print the debug statements of individual file access functions from the main API functions instead of the individual backend functions. Also enhance initialization debug messages on a per-backend basis. --- src/storage/storage_backend_fs.c | 43 ++++++++++++++++++++++------------- src/storage/storage_backend_gluster.c | 30 +++++++++--------------- src/storage/storage_driver.c | 27 +++++++++++++++++++--- 3 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index de27890..3d088ba 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1339,18 +1339,31 @@ virStorageBackend virStorageBackendNetFileSystem = { }; +static void +virStorageFileBackendFileDeinit(virStorageSourcePtr src) +{ + VIR_DEBUG("deinitializing FS storage file %p (%s:%s)", src, + virStorageTypeToString(virStorageSourceGetActualType(src)), + src->path); + +} + + static int -virStorageFileBackendFileUnlink(virStorageSourcePtr src) +virStorageFileBackendFileInit(virStorageSourcePtr src) { - int ret; + VIR_DEBUG("initializing FS storage file %p (%s:%s)", src, + virStorageTypeToString(virStorageSourceGetActualType(src)), + src->path); - ret = unlink(src->path); - /* preserve errno */ + return 0; +} - VIR_DEBUG("removing storage file %p(%s): ret=%d, errno=%d", - src, src->path, ret, errno); - return ret; +static int +virStorageFileBackendFileUnlink(virStorageSourcePtr src) +{ + return unlink(src->path); } @@ -1358,21 +1371,16 @@ static int virStorageFileBackendFileStat(virStorageSourcePtr src, struct stat *st) { - int ret; - - ret = stat(src->path, st); - /* preserve errno */ - - VIR_DEBUG("stat of storage file %p(%s): ret=%d, errno=%d", - src, src->path, ret, errno); - - return ret; + return stat(src->path, st); } virStorageFileBackend virStorageFileBackendFile = { .type = VIR_STORAGE_TYPE_FILE, + .backendInit = virStorageFileBackendFileInit, + .backendDeinit = virStorageFileBackendFileDeinit, + .storageFileUnlink = virStorageFileBackendFileUnlink, .storageFileStat = virStorageFileBackendFileStat, }; @@ -1381,6 +1389,9 @@ virStorageFileBackend virStorageFileBackendFile = { virStorageFileBackend virStorageFileBackendBlock = { .type = VIR_STORAGE_TYPE_BLOCK, + .backendInit = virStorageFileBackendFileInit, + .backendDeinit = virStorageFileBackendFileDeinit, + .storageFileStat = virStorageFileBackendFileStat, }; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 8679d96..e578e73 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -539,10 +539,12 @@ struct _virStorageFileBackendGlusterPriv { static void virStorageFileBackendGlusterDeinit(virStorageSourcePtr src) { - VIR_DEBUG("deinitializing gluster storage file %p(%s/%s)", - src, src->hosts[0].name, src->path); virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%s/%s%s)", + src, src->hosts->name, src->hosts->port ? src->hosts->port : "0", + src->volume, src->path); + if (priv->vol) glfs_fini(priv->vol); @@ -558,12 +560,14 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) const char *hostname = host->name; int port = 0; - VIR_DEBUG("initializing gluster storage file %p(%s/%s)", - src, hostname, src->path); + VIR_DEBUG("initializing gluster storage file %p (gluster://%s:%s/%s%s)", + src, hostname, host->port ? host->port : "0", + NULLSTR(src->volume), src->path); if (!src->volume) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing gluster volume name for path '%s'"), src->path); + _("missing gluster volume name for path '%s'"), + src->path); return -1; } @@ -619,14 +623,8 @@ static int virStorageFileBackendGlusterUnlink(virStorageSourcePtr src) { virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - int ret; - - ret = glfs_unlink(priv->vol, src->path); - /* preserve errno */ - VIR_DEBUG("removing storage file %p(%s/%s): ret=%d, errno=%d", - src, src->hosts[0].name, src->path, ret, errno); - return ret; + return glfs_unlink(priv->vol, src->path); } @@ -635,14 +633,8 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src, struct stat *st) { virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - int ret; - ret = glfs_stat(priv->vol, src->path, st); - /* preserve errno */ - - VIR_DEBUG("stat of storage file %p(%s/%s): ret=%d, errno=%d", - src, src->hosts[0].name, src->path, ret, errno); - return ret; + return glfs_stat(priv->vol, src->path, st); } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 2cb8347..db327f4 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2808,13 +2808,20 @@ virStorageFileInit(virStorageSourcePtr src) int virStorageFileCreate(virStorageSourcePtr src) { + int ret; + if (!virStorageFileIsInitialized(src) || !src->drv->backend->storageFileCreate) { errno = ENOSYS; return -2; } - return src->drv->backend->storageFileCreate(src); + ret = src->drv->backend->storageFileCreate(src); + + VIR_DEBUG("created storage file %p: ret=%d, errno=%d", + src, ret, errno); + + return ret; } @@ -2831,13 +2838,20 @@ virStorageFileCreate(virStorageSourcePtr src) int virStorageFileUnlink(virStorageSourcePtr src) { + int ret; + if (!virStorageFileIsInitialized(src) || !src->drv->backend->storageFileUnlink) { errno = ENOSYS; return -2; } - return src->drv->backend->storageFileUnlink(src); + ret = src->drv->backend->storageFileUnlink(src); + + VIR_DEBUG("unlinked storage file %p: ret=%d, errno=%d", + src, ret, errno); + + return ret; } @@ -2854,11 +2868,18 @@ int virStorageFileStat(virStorageSourcePtr src, struct stat *st) { + int ret; + if (!virStorageFileIsInitialized(src) || !src->drv->backend->storageFileStat) { errno = ENOSYS; return -2; } - return src->drv->backend->storageFileStat(src, st); + ret = src->drv->backend->storageFileStat(src, st); + + VIR_DEBUG("stat of storage file %p: ret=%d, errno=%d", + src, ret, errno); + + return ret; } -- 1.9.2

Skip networked storage but continue iteration through backing chain to iterate through all the local paths in the backing chain. --- src/conf/domain_conf.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3cb6693..25758d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18686,34 +18686,37 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, int ret = -1; size_t depth = 0; virStorageSourcePtr tmp; - const char *path = virDomainDiskGetSource(disk); - int type = virDomainDiskGetType(disk); + char *brokenRaw = NULL; - if (!path || type == VIR_STORAGE_TYPE_NETWORK || - (type == VIR_STORAGE_TYPE_VOLUME && - disk->src.srcpool && - disk->src.srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT)) - return 0; - - if (iter(disk, path, 0, opaque) < 0) - goto cleanup; + if (!ignoreOpenFailure) { + if (virStorageFileChainGetBroken(&disk->src, &brokenRaw) < 0) + goto cleanup; - tmp = disk->src.backingStore; - while (tmp && virStorageIsFile(tmp->path)) { - if (!ignoreOpenFailure && tmp->backingStoreRaw && !tmp->backingStore) { + if (brokenRaw) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to visit backing chain file %s"), - tmp->backingStoreRaw); + brokenRaw); goto cleanup; } - if (iter(disk, tmp->path, ++depth, opaque) < 0) - goto cleanup; - tmp = tmp->backingStore; + } + + for (tmp = &disk->src; tmp; tmp = tmp->backingStore) { + int actualType = virStorageSourceGetActualType(tmp); + /* execute the callback only for local storage */ + if (actualType != VIR_STORAGE_TYPE_NETWORK && + actualType != VIR_STORAGE_TYPE_VOLUME && + tmp->path) { + if (iter(disk, tmp->path, depth, opaque) < 0) + goto cleanup; + } + + depth++; } ret = 0; cleanup: + VIR_FREE(brokenRaw); return ret; } -- 1.9.2

Currently the protocol type with index 0 was NBD which made it hard to distinguish wether the protocol type was actually assigned. Add a new protocol type with index 0 to distinguish it explicitly. --- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_driver.c | 3 +++ src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 1 + 4 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c0f2fb6..48eeae0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3582,6 +3582,7 @@ qemuNetworkDriveGetPort(int protocol, case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_LAST: + case VIR_STORAGE_NET_PROTOCOL_NONE: /* not aplicable */ return -1; } @@ -3798,6 +3799,7 @@ qemuBuildNetworkDriveURI(int protocol, case VIR_STORAGE_NET_PROTOCOL_LAST: + case VIR_STORAGE_NET_PROTOCOL_NONE: goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac0de1e..b0a79e4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12338,6 +12338,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) case VIR_STORAGE_TYPE_NETWORK: switch ((virStorageNetProtocol) disk->src.protocol) { + case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -12403,6 +12404,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d case VIR_STORAGE_NET_PROTOCOL_GLUSTER: return 0; + case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -12545,6 +12547,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, case VIR_STORAGE_TYPE_NETWORK: switch ((virStorageNetProtocol) disk->src.protocol) { + case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cf270f6..73cef3c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -72,6 +72,7 @@ VIR_ENUM_IMPL(virStorageFileFeature, ) VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST, + "none", "nbd", "rbd", "sheepdog", diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 3777d66..95f2e45 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -118,6 +118,7 @@ struct _virStorageTimestamps { /* Information related to network storage */ typedef enum { + VIR_STORAGE_NET_PROTOCOL_NONE, VIR_STORAGE_NET_PROTOCOL_NBD, VIR_STORAGE_NET_PROTOCOL_RBD, VIR_STORAGE_NET_PROTOCOL_SHEEPDOG, -- 1.9.2

To allow using the storage driver APIs to access files on various storage sources in an universal fashion possibly on storage such as nfs with root squash we'll need to store the desired uid/gid in the metadata. Add new initialisation API that will store the desired uid/gid and a wrapper for the current use. Additionally add docs for the two APIs. --- src/storage/storage_backend.h | 3 +++ src/storage/storage_driver.c | 39 ++++++++++++++++++++++++++++++++++++++- src/storage/storage_driver.h | 5 +++-- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 456b9d7..fcbb6da 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -169,6 +169,9 @@ typedef virStorageFileBackend *virStorageFileBackendPtr; struct _virStorageDriverData { virStorageFileBackendPtr backend; void *priv; + + uid_t uid; + gid_t gid; }; typedef int diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index db327f4..4c283fa 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2774,13 +2774,37 @@ virStorageFileDeinit(virStorageSourcePtr src) } +/** + * virStorageFileInitAs: + * + * @src: storage source definition + * @uid: uid to access the file as, -1 for current uid + * @gid: gid to access the file as, -1 for current gid + * + * Initialize a storage source to be used with storage driver. Use the provided + * uid and gid if possible for the operations. + * + * Returns 0 if the storage file was successfully initialized, -1 if the + * initialization failed. Libvirt error is reported. + */ int -virStorageFileInit(virStorageSourcePtr src) +virStorageFileInitAs(virStorageSourcePtr src, + uid_t uid, gid_t gid) { int actualType = virStorageSourceGetActualType(src); if (VIR_ALLOC(src->drv) < 0) return -1; + if (uid == (uid_t) -1) + src->drv->uid = geteuid(); + else + src->drv->uid = uid; + + if (gid == (gid_t) -1) + src->drv->gid = getegid(); + else + src->drv->gid = gid; + if (!(src->drv->backend = virStorageFileBackendForType(actualType, src->protocol))) goto error; @@ -2798,6 +2822,19 @@ virStorageFileInit(virStorageSourcePtr src) /** + * virStorageFileInit: + * + * See virStorageFileInitAs. The file is initialized to be accessed by the + * current user. + */ +int +virStorageFileInit(virStorageSourcePtr src) +{ + return virStorageFileInitAs(src, (uid_t) -1, (gid_t) -1); +} + + +/** * virStorageFileCreate: Creates an empty storage file via storage driver * * @src: file structure pointing to the file diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index fb03870..49be999 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -29,8 +29,9 @@ # include "storage_conf.h" # include "virstoragefile.h" -int -virStorageFileInit(virStorageSourcePtr src); +int virStorageFileInit(virStorageSourcePtr src); +int virStorageFileInitAs(virStorageSourcePtr src, + uid_t uid, gid_t gid); void virStorageFileDeinit(virStorageSourcePtr src); int virStorageFileCreate(virStorageSourcePtr src); -- 1.9.2

Different protocols have different means to uniquely identify a storage file. This patch implements a storage driver API to retrieve a unique string describing a volume. The current implementation works for local storage only and returns the canonical path of the volume. To add caching support the local filesystem driver now has a private structure holding the cached string, which is created only when it's initially accessed. This patch provides the implementation for local files only for start. --- src/storage/storage_backend.h | 3 +++ src/storage/storage_backend_fs.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/storage/storage_driver.c | 30 ++++++++++++++++++++++++ src/storage/storage_driver.h | 1 + 4 files changed, 83 insertions(+) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 6be5ca7..5d71cde 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -195,6 +195,8 @@ typedef ssize_t ssize_t max_len, char **buf); +typedef const char * +(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src); virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol); @@ -211,6 +213,7 @@ struct _virStorageFileBackend { virStorageFileBackendInit backendInit; virStorageFileBackendDeinit backendDeinit; virStorageFileBackendReadHeader storageFileReadHeader; + virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier; /* The following group of callbacks is expected to set errno * and return -1 on error. No libvirt error shall be reported */ diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 33551e7..1f436fa 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1339,6 +1339,14 @@ virStorageBackend virStorageBackendNetFileSystem = { }; +typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv; +typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr; + +struct _virStorageFileBackendFsPriv { + char *uid; /* unique file identifier (canonical path) */ +}; + + static void virStorageFileBackendFileDeinit(virStorageSourcePtr src) { @@ -1346,16 +1354,27 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src) virStorageTypeToString(virStorageSourceGetActualType(src)), src->path); + virStorageFileBackendFsPrivPtr priv = src->drv->priv; + + VIR_FREE(priv->uid); + VIR_FREE(priv); } static int virStorageFileBackendFileInit(virStorageSourcePtr src) { + virStorageFileBackendFsPrivPtr priv = NULL; + VIR_DEBUG("initializing FS storage file %p (%s:%s)", src, virStorageTypeToString(virStorageSourceGetActualType(src)), src->path); + if (VIR_ALLOC(priv) < 0) + return -1; + + src->drv->priv = priv; + return 0; } @@ -1403,6 +1422,23 @@ virStorageFileBackendFileReadHeader(virStorageSourcePtr src, } +static const char * +virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src) +{ + virStorageFileBackendFsPrivPtr priv = src->drv->priv; + + if (!priv->uid) { + if (!(priv->uid = canonicalize_file_name(src->path))) { + virReportSystemError(errno, _("can't canonicalize path '%s'"), + src->path); + return NULL; + } + } + + return priv->uid; +} + + virStorageFileBackend virStorageFileBackendFile = { .type = VIR_STORAGE_TYPE_FILE, @@ -1412,6 +1448,8 @@ virStorageFileBackend virStorageFileBackendFile = { .storageFileUnlink = virStorageFileBackendFileUnlink, .storageFileStat = virStorageFileBackendFileStat, .storageFileReadHeader = virStorageFileBackendFileReadHeader, + + .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; @@ -1423,7 +1461,18 @@ virStorageFileBackend virStorageFileBackendBlock = { .storageFileStat = virStorageFileBackendFileStat, .storageFileReadHeader = virStorageFileBackendFileReadHeader, + + .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; +virStorageFileBackend virStorageFileBackendDir = { + .type = VIR_STORAGE_TYPE_DIR, + + .backendInit = virStorageFileBackendFileInit, + .backendDeinit = virStorageFileBackendFileDeinit, + + .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, +}; + #endif /* WITH_STORAGE_FS */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8da8386..2fb70f2 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2960,3 +2960,33 @@ virStorageFileReadHeader(virStorageSourcePtr src, return ret; } + + +/* + * virStorageFileGetUniqueIdentifier: Get a unique string describing the volume + * + * @src: file structure pointing to the file + * + * Returns a string uniquely describing a single volume (canonical path). + * The string shall not be freed. Returns NULL on error and sets a libvirt + * error code */ +const char * +virStorageFileGetUniqueIdentifier(virStorageSourcePtr src) +{ + if (!virStorageFileIsInitialized(src)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("storage file backend not initialized")); + return NULL; + } + + if (!src->drv->backend->storageFileGetUniqueIdentifier) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unique storage file identifier not implemented for " + "storage type %s (protocol: %s)'"), + virStorageTypeToString(src->type), + virStorageNetProtocolTypeToString(src->protocol)); + return NULL; + } + + return src->drv->backend->storageFileGetUniqueIdentifier(src); +} diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index d67d74b..9280ef0 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -41,6 +41,7 @@ int virStorageFileStat(virStorageSourcePtr src, ssize_t virStorageFileReadHeader(virStorageSourcePtr src, ssize_t max_len, char **buf); +const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); int storageRegister(void); -- 1.9.2

My future work will modify the metadata crawler function to use the storage driver file APIs to access the files instead of accessing them directly so that we will be able to request the metadata for remote files too. To avoid linking the storage driver to every helper file using the utils code, the backing chain traversal function needs to be moved to the storage driver source. Additionally the virt-aa-helper and virstoragetest programs need to be linked with the storage driver as a result of this change. --- cfg.mk | 2 +- src/Makefile.am | 2 + src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 2 + src/security/virt-aa-helper.c | 2 + src/storage/storage_driver.c | 233 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_driver.h | 5 + src/util/virstoragefile.c | 233 +----------------------------------------- src/util/virstoragefile.h | 7 +- tests/Makefile.am | 7 +- tests/virstoragetest.c | 2 + 11 files changed, 258 insertions(+), 239 deletions(-) diff --git a/cfg.mk b/cfg.mk index 3f4bba0..cfe5fcc 100644 --- a/cfg.mk +++ b/cfg.mk @@ -774,7 +774,7 @@ sc_prohibit_cross_inclusion: access/ | conf/) safe="($$dir|conf|util)";; \ locking/) safe="($$dir|util|conf|rpc)";; \ cpu/| network/| node_device/| rpc/| security/| storage/) \ - safe="($$dir|util|conf)";; \ + safe="($$dir|util|conf|storage)";; \ xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \ *) safe="($$dir|$(mid_dirs)|util)";; \ esac; \ diff --git a/src/Makefile.am b/src/Makefile.am index e9dc9e0..072e761 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2564,8 +2564,10 @@ virt_aa_helper_LDFLAGS = \ $(PIE_LDFLAGS) \ $(NULL) virt_aa_helper_LDADD = \ + libvirt.la \ libvirt_conf.la \ libvirt_util.la \ + libvirt_driver_storage_impl.la \ ../gnulib/lib/libgnu.la if WITH_DTRACE_PROBES virt_aa_helper_LDADD += libvirt_probes.lo diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index afda71f..78221aa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1847,9 +1847,9 @@ virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; virStorageFileGetLVMKey; -virStorageFileGetMetadata; virStorageFileGetMetadataFromBuf; virStorageFileGetMetadataFromFD; +virStorageFileGetMetadataFromFDInternal; virStorageFileGetSCSIKey; virStorageFileIsClusterFS; virStorageFileParseChainIndex; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1301f64..7ea9032 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -39,6 +39,8 @@ #include "virstoragefile.h" #include "virstring.h" +#include "storage/storage_driver.h" + #include <sys/time.h> #include <fcntl.h> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 32fc04a..bf540b4 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -55,6 +55,8 @@ #include "virrandom.h" #include "virstring.h" +#include "storage/storage_driver.h" + #define VIR_FROM_THIS VIR_FROM_SECURITY static char *progname; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index dd58400..25772e8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -49,6 +49,7 @@ #include "configmake.h" #include "virstring.h" #include "viraccessapicheck.h" +#include "dirname.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -3014,3 +3015,235 @@ virStorageFileAccess(virStorageSourcePtr src, return src->drv->backend->storageFileAccess(src, mode); } + + +/** + * Given a starting point START (a directory containing the original + * file, if the original file was opened via a relative path; ignored + * if NAME is absolute), determine the location of the backing file + * NAME (possibly relative), and compute the relative DIRECTORY + * (optional) and CANONICAL (mandatory) location of the backing file. + * Return 0 on success, negative on error. + */ +static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) +virFindBackingFile(const char *start, const char *path, + char **directory, char **canonical) +{ + /* FIXME - when we eventually allow non-raw network devices, we + * must ensure that we handle backing files the same way as qemu. + * For a qcow2 top file of gluster://server/vol/img, qemu treats + * the relative backing file 'rel' as meaning + * 'gluster://server/vol/rel', while the backing file '/abs' is + * used as a local file. But we cannot canonicalize network + * devices via canonicalize_file_name(), because they are not part + * of the local file system. */ + char *combined = NULL; + int ret = -1; + + if (*path == '/') { + /* Safe to cast away const */ + combined = (char *)path; + } else if (virAsprintf(&combined, "%s/%s", start, path) < 0) { + goto cleanup; + } + + if (directory && !(*directory = mdir_name(combined))) { + virReportOOMError(); + goto cleanup; + } + + if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid()) < 0) { + virReportSystemError(errno, + _("Cannot access backing file '%s'"), + combined); + goto cleanup; + } + + if (!(*canonical = canonicalize_file_name(combined))) { + virReportSystemError(errno, + _("Can't canonicalize path '%s'"), path); + goto cleanup; + } + + ret = 0; + + cleanup: + if (combined != path) + VIR_FREE(combined); + return ret; +} + + +/* Recursive workhorse for virStorageFileGetMetadata. */ +static int +virStorageFileGetMetadataRecurse(virStorageSourcePtr src, + const char *canonPath, + uid_t uid, gid_t gid, + bool allow_probe, + virHashTablePtr cycle) +{ + int fd; + int ret = -1; + virStorageSourcePtr backingStore = NULL; + int backingFormat; + + VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d", + src->path, canonPath, NULLSTR(src->relDir), src->format, + (int)uid, (int)gid, allow_probe); + + if (virHashLookup(cycle, canonPath)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("backing store for %s is self-referential"), + src->path); + return -1; + } + + if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) + return -1; + + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { + if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) { + virReportSystemError(-fd, _("Failed to open file '%s'"), + src->path); + return -1; + } + + if (virStorageFileGetMetadataFromFDInternal(src, fd, + &backingFormat) < 0) { + VIR_FORCE_CLOSE(fd); + return -1; + } + + if (VIR_CLOSE(fd) < 0) + VIR_WARN("could not close file %s", src->path); + } else { + /* TODO: currently we call this only for local storage */ + return 0; + } + + /* check whether we need to go deeper */ + if (!src->backingStoreRaw) + return 0; + + if (VIR_ALLOC(backingStore) < 0) + return -1; + + if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0) + goto cleanup; + + if (virStorageIsFile(src->backingStoreRaw)) { + backingStore->type = VIR_STORAGE_TYPE_FILE; + + if (virFindBackingFile(src->relDir, + src->backingStoreRaw, + &backingStore->relDir, + &backingStore->path) < 0) { + /* the backing file is (currently) unavailable, treat this + * file as standalone: + * backingStoreRaw is kept to mark broken image chains */ + VIR_WARN("Backing file '%s' of image '%s' is missing.", + src->backingStoreRaw, src->path); + ret = 0; + goto cleanup; + } + } else { + /* TODO: To satisfy the test case, copy the network URI as path. This + * will be removed later. */ + backingStore->type = VIR_STORAGE_TYPE_NETWORK; + + if (VIR_STRDUP(backingStore->path, src->backingStoreRaw) < 0) + goto cleanup; + } + + if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) + backingStore->format = VIR_STORAGE_FILE_RAW; + else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) + backingStore->format = VIR_STORAGE_FILE_AUTO; + else + backingStore->format = backingFormat; + + if (virStorageFileGetMetadataRecurse(backingStore, + backingStore->path, + uid, gid, allow_probe, + cycle) < 0) { + /* if we fail somewhere midway, just accept and return a + * broken chain */ + ret = 0; + goto cleanup; + } + + src->backingStore = backingStore; + backingStore = NULL; + ret = 0; + + cleanup: + virStorageSourceFree(backingStore); + return ret; +} + + +/** + * virStorageFileGetMetadata: + * + * Extract metadata about the storage volume with the specified + * image format. If image format is VIR_STORAGE_FILE_AUTO, it + * will probe to automatically identify the format. Recurses through + * the entire chain. + * + * Open files using UID and GID (or pass -1 for the current user/group). + * Treat any backing files without explicit type as raw, unless ALLOW_PROBE. + * + * Callers are advised never to use VIR_STORAGE_FILE_AUTO as a + * format, since a malicious guest can turn a raw file into any + * other non-raw format at will. + * + * Caller MUST free result after use via virStorageSourceFree. + */ +int +virStorageFileGetMetadata(virStorageSourcePtr src, + uid_t uid, gid_t gid, + bool allow_probe) +{ + VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d", + src->path, src->format, (int)uid, (int)gid, allow_probe); + + virHashTablePtr cycle = NULL; + char *canonPath = NULL; + int ret = -1; + + if (!(cycle = virHashCreate(5, NULL))) + return -1; + + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { + if (!(canonPath = canonicalize_file_name(src->path))) { + virReportSystemError(errno, _("unable to resolve '%s'"), + src->path); + goto cleanup; + } + + if (!src->relPath && + VIR_STRDUP(src->relPath, src->path) < 0) + goto cleanup; + + if (!src->relDir && + !(src->relDir = mdir_name(src->path))) { + virReportOOMError(); + goto cleanup; + } + } else { + /* TODO: currently unimplemented for non-local storage */ + ret = 0; + goto cleanup; + } + + if (src->format <= VIR_STORAGE_FILE_NONE) + src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; + + ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid, + allow_probe, cycle); + + cleanup: + VIR_FREE(canonPath); + virHashFree(cycle); + return ret; +} diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index 5452df2..bd9e9ed 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -44,6 +44,11 @@ ssize_t virStorageFileReadHeader(virStorageSourcePtr src, const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); int virStorageFileAccess(virStorageSourcePtr src, int mode); +int virStorageFileGetMetadata(virStorageSourcePtr src, + uid_t uid, gid_t gid, + bool allow_probe) + ATTRIBUTE_NONNULL(1); + int storageRegister(void); #endif /* __VIR_STORAGE_DRIVER_H__ */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 73cef3c..347c2c3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -28,7 +28,6 @@ #include <unistd.h> #include <fcntl.h> #include <stdlib.h> -#include "dirname.h" #include "viralloc.h" #include "virerror.h" #include "virlog.h" @@ -563,62 +562,6 @@ qedGetBackingStore(char **res, return BACKING_STORE_OK; } -/** - * Given a starting point START (a directory containing the original - * file, if the original file was opened via a relative path; ignored - * if NAME is absolute), determine the location of the backing file - * NAME (possibly relative), and compute the relative DIRECTORY - * (optional) and CANONICAL (mandatory) location of the backing file. - * Return 0 on success, negative on error. - */ -static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) -virFindBackingFile(const char *start, const char *path, - char **directory, char **canonical) -{ - /* FIXME - when we eventually allow non-raw network devices, we - * must ensure that we handle backing files the same way as qemu. - * For a qcow2 top file of gluster://server/vol/img, qemu treats - * the relative backing file 'rel' as meaning - * 'gluster://server/vol/rel', while the backing file '/abs' is - * used as a local file. But we cannot canonicalize network - * devices via canonicalize_file_name(), because they are not part - * of the local file system. */ - char *combined = NULL; - int ret = -1; - - if (*path == '/') { - /* Safe to cast away const */ - combined = (char *)path; - } else if (virAsprintf(&combined, "%s/%s", start, path) < 0) { - goto cleanup; - } - - if (directory && !(*directory = mdir_name(combined))) { - virReportOOMError(); - goto cleanup; - } - - if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid()) < 0) { - virReportSystemError(errno, - _("Cannot access backing file '%s'"), - combined); - goto cleanup; - } - - if (!(*canonical = canonicalize_file_name(combined))) { - virReportSystemError(errno, - _("Can't canonicalize path '%s'"), path); - goto cleanup; - } - - ret = 0; - - cleanup: - if (combined != path) - VIR_FREE(combined); - return ret; -} - static bool virStorageFileMatchesMagic(int format, @@ -1017,7 +960,7 @@ virStorageFileGetMetadataFromBuf(const char *path, /* Internal version that also supports a containing directory name. */ -static int +int virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta, int fd, int *backingFormat) @@ -1106,180 +1049,6 @@ virStorageFileGetMetadataFromFD(const char *path, } -/* Recursive workhorse for virStorageFileGetMetadata. */ -static int -virStorageFileGetMetadataRecurse(virStorageSourcePtr src, - const char *canonPath, - uid_t uid, gid_t gid, - bool allow_probe, - virHashTablePtr cycle) -{ - int fd; - int ret = -1; - virStorageSourcePtr backingStore = NULL; - int backingFormat; - - VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d", - src->path, canonPath, NULLSTR(src->relDir), src->format, - (int)uid, (int)gid, allow_probe); - - if (virHashLookup(cycle, canonPath)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("backing store for %s is self-referential"), - src->path); - return -1; - } - - if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) - return -1; - - if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { - if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) { - virReportSystemError(-fd, _("Failed to open file '%s'"), - src->path); - return -1; - } - - if (virStorageFileGetMetadataFromFDInternal(src, fd, - &backingFormat) < 0) { - VIR_FORCE_CLOSE(fd); - return -1; - } - - if (VIR_CLOSE(fd) < 0) - VIR_WARN("could not close file %s", src->path); - } else { - /* TODO: currently we call this only for local storage */ - return 0; - } - - /* check whether we need to go deeper */ - if (!src->backingStoreRaw) - return 0; - - if (VIR_ALLOC(backingStore) < 0) - return -1; - - if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0) - goto cleanup; - - if (virStorageIsFile(src->backingStoreRaw)) { - backingStore->type = VIR_STORAGE_TYPE_FILE; - - if (virFindBackingFile(src->relDir, - src->backingStoreRaw, - &backingStore->relDir, - &backingStore->path) < 0) { - /* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ - VIR_WARN("Backing file '%s' of image '%s' is missing.", - src->backingStoreRaw, src->path); - ret = 0; - goto cleanup; - } - } else { - /* TODO: To satisfy the test case, copy the network URI as path. This - * will be removed later. */ - backingStore->type = VIR_STORAGE_TYPE_NETWORK; - - if (VIR_STRDUP(backingStore->path, src->backingStoreRaw) < 0) - goto cleanup; - } - - if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) - backingStore->format = VIR_STORAGE_FILE_RAW; - else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) - backingStore->format = VIR_STORAGE_FILE_AUTO; - else - backingStore->format = backingFormat; - - if (virStorageFileGetMetadataRecurse(backingStore, - backingStore->path, - uid, gid, allow_probe, - cycle) < 0) { - /* if we fail somewhere midway, just accept and return a - * broken chain */ - ret = 0; - goto cleanup; - } - - src->backingStore = backingStore; - backingStore = NULL; - ret = 0; - - cleanup: - virStorageSourceFree(backingStore); - return ret; -} - - -/** - * virStorageFileGetMetadata: - * - * Extract metadata about the storage volume with the specified - * image format. If image format is VIR_STORAGE_FILE_AUTO, it - * will probe to automatically identify the format. Recurses through - * the entire chain. - * - * Open files using UID and GID (or pass -1 for the current user/group). - * Treat any backing files without explicit type as raw, unless ALLOW_PROBE. - * - * Callers are advised never to use VIR_STORAGE_FILE_AUTO as a - * format, since a malicious guest can turn a raw file into any - * other non-raw format at will. - * - * Caller MUST free result after use via virStorageSourceFree. - */ -int -virStorageFileGetMetadata(virStorageSourcePtr src, - uid_t uid, gid_t gid, - bool allow_probe) -{ - VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d", - src->path, src->format, (int)uid, (int)gid, allow_probe); - - virHashTablePtr cycle = NULL; - char *canonPath = NULL; - int ret = -1; - - if (!(cycle = virHashCreate(5, NULL))) - return -1; - - if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { - if (!(canonPath = canonicalize_file_name(src->path))) { - virReportSystemError(errno, _("unable to resolve '%s'"), - src->path); - goto cleanup; - } - - if (!src->relPath && - VIR_STRDUP(src->relPath, src->path) < 0) - goto cleanup; - - if (!src->relDir && - !(src->relDir = mdir_name(src->path))) { - virReportOOMError(); - goto cleanup; - } - } else { - /* TODO: currently unimplemented for non-local storage */ - ret = 0; - goto cleanup; - } - - if (src->format <= VIR_STORAGE_FILE_NONE) - src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; - - ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid, - allow_probe, cycle); - - cleanup: - VIR_FREE(canonPath); - virHashFree(cycle); - return ret; -} - /** * virStorageFileChainCheckBroken * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 95f2e45..ae27936 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -263,10 +263,9 @@ struct _virStorageSource { int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); -int virStorageFileGetMetadata(virStorageSourcePtr src, - uid_t uid, gid_t gid, - bool allow_probe) - ATTRIBUTE_NONNULL(1); +int virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta, + int fd, + int *backingFormat); virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, int fd, int format); diff --git a/tests/Makefile.am b/tests/Makefile.am index 5ef8940..4d24822 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -869,7 +869,12 @@ virstringtest_LDADD = $(LDADDS) virstoragetest_SOURCES = \ virstoragetest.c testutils.h testutils.c -virstoragetest_LDADD = $(LDADDS) +virstoragetest_LDADD = $(LDADDS) \ + ../src/libvirt.la \ + ../src/libvirt_conf.la \ + ../src/libvirt_util.la \ + ../src/libvirt_driver_storage_impl.la \ + ../gnulib/lib/libgnu.la viridentitytest_SOURCES = \ viridentitytest.c testutils.h testutils.c diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index d49098e..fb96c71 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -31,6 +31,8 @@ #include "virstring.h" #include "dirname.h" +#include "storage/storage_driver.h" + #define VIR_FROM_THIS VIR_FROM_NONE VIR_LOG_INIT("tests.storagetest"); -- 1.9.2

Stat the path of the storage file being tested to set the correct type into the virStorageSource. This will avoid breaking the test suite when inquiring metadata of directory paths in the next patches. --- tests/virstoragetest.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index fb96c71..746bf63 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -98,6 +98,7 @@ testStorageFileGetMetadata(const char *path, uid_t uid, gid_t gid, bool allow_probe) { + struct stat st; virStorageSourcePtr ret = NULL; if (VIR_ALLOC(ret) < 0) @@ -106,6 +107,15 @@ testStorageFileGetMetadata(const char *path, ret->type = VIR_STORAGE_TYPE_FILE; ret->format = format; + if (stat(path, &st) == 0) { + if (S_ISDIR(st.st_mode)) { + ret->type = VIR_STORAGE_TYPE_DIR; + ret->format = VIR_STORAGE_FILE_DIR; + } else if (S_ISBLK(st.st_mode)) { + ret->type = VIR_STORAGE_TYPE_BLOCK; + } + } + if (VIR_STRDUP(ret->relPath, path) < 0) goto error; -- 1.9.2

Use the virStorageFileGetUniqueIdentifier() function to get a unique identifier regardless of the target storage type instead of relying on canonicalize_path(). A new function that checks wether we support a given image is introduced to avoid errors for unimplemented backends. --- src/storage/storage_driver.c | 77 +++++++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 23 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 163b402..cdf01e7 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2761,6 +2761,30 @@ virStorageFileIsInitialized(virStorageSourcePtr src) return !!src->drv; } + +static bool +virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) +{ + int actualType = virStorageSourceGetActualType(src); + virStorageFileBackendPtr backend; + + if (!src) + return false; + + if (src->drv) { + backend = src->drv->backend; + } else { + if (!(backend = virStorageFileBackendForTypeInternal(actualType, + src->protocol, + false))) + return false; + } + + return backend->storageFileGetUniqueIdentifier && + backend->storageFileReadHeader && + backend->storageFileAccess; +} + void virStorageFileDeinit(virStorageSourcePtr src) { @@ -3077,7 +3101,6 @@ virFindBackingFile(const char *start, const char *path, /* Recursive workhorse for virStorageFileGetMetadata. */ static int virStorageFileGetMetadataRecurse(virStorageSourcePtr src, - const char *canonPath, uid_t uid, gid_t gid, bool allow_probe, virHashTablePtr cycle) @@ -3085,49 +3108,63 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, int fd; int ret = -1; struct stat st; + const char *uniqueName; virStorageSourcePtr backingStore = NULL; int backingFormat; - VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d", - src->path, canonPath, NULLSTR(src->relDir), src->format, + VIR_DEBUG("path=%s dir=%s format=%d uid=%d gid=%d probe=%d", + src->path, NULLSTR(src->relDir), src->format, (int)uid, (int)gid, allow_probe); - if (virHashLookup(cycle, canonPath)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("backing store for %s is self-referential"), - src->path); + /* exit if we can't load information about the current image */ + if (!virStorageFileSupportsBackingChainTraversal(src)) + return 0; + + if (virStorageFileInitAs(src, uid, gid) < 0) return -1; + + if (!(uniqueName = virStorageFileGetUniqueIdentifier(src))) + goto cleanup; + + if (virHashLookup(cycle, uniqueName)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("backing store for %s (%s) is self-referential"), + src->path, uniqueName); + goto cleanup; } - if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) - return -1; + if (virHashAddEntry(cycle, uniqueName, (void *)1) < 0) + goto cleanup; if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) { virReportSystemError(-fd, _("Failed to open file '%s'"), src->path); - return -1; + goto cleanup; } if (virStorageFileGetMetadataFromFDInternal(src, fd, &backingFormat) < 0) { VIR_FORCE_CLOSE(fd); - return -1; + goto cleanup; } if (VIR_CLOSE(fd) < 0) VIR_WARN("could not close file %s", src->path); } else { /* TODO: currently we call this only for local storage */ - return 0; + ret = 0; + goto cleanup; } /* check whether we need to go deeper */ - if (!src->backingStoreRaw) - return 0; + if (!src->backingStoreRaw) { + ret = 0; + goto cleanup; + } if (VIR_ALLOC(backingStore) < 0) - return -1; + goto cleanup; if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0) goto cleanup; @@ -3174,7 +3211,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, backingStore->format = backingFormat; if (virStorageFileGetMetadataRecurse(backingStore, - backingStore->path, uid, gid, allow_probe, cycle) < 0) { /* if we fail somewhere midway, just accept and return a @@ -3188,6 +3224,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, ret = 0; cleanup: + virStorageFileDeinit(src); virStorageSourceFree(backingStore); return ret; } @@ -3226,12 +3263,6 @@ virStorageFileGetMetadata(virStorageSourcePtr src, return -1; if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) { - if (!(canonPath = canonicalize_file_name(src->path))) { - virReportSystemError(errno, _("unable to resolve '%s'"), - src->path); - goto cleanup; - } - if (!src->relPath && VIR_STRDUP(src->relPath, src->path) < 0) goto cleanup; @@ -3250,7 +3281,7 @@ virStorageFileGetMetadata(virStorageSourcePtr src, if (src->format <= VIR_STORAGE_FILE_NONE) src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; - ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid, + ret = virStorageFileGetMetadataRecurse(src, uid, gid, allow_probe, cycle); cleanup: -- 1.9.2

Use virStorageFileAccess() to to check wether the file is accessible in the main part of the metadata crawler. --- src/storage/storage_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f2c548d..cf969c8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -3076,13 +3076,6 @@ virFindBackingFile(const char *start, const char *path, goto cleanup; } - if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid()) < 0) { - virReportSystemError(errno, - _("Cannot access backing file '%s'"), - combined); - goto cleanup; - } - if (!(*canonical = canonicalize_file_name(combined))) { virReportSystemError(errno, _("Can't canonicalize path '%s'"), path); @@ -3124,6 +3117,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if (virStorageFileInitAs(src, uid, gid) < 0) return -1; + if (virStorageFileAccess(src, F_OK) < 0) { + virReportSystemError(errno, + _("Cannot access backing file %s"), + src->path); + goto cleanup; + } + if (!(uniqueName = virStorageFileGetUniqueIdentifier(src))) goto cleanup; -- 1.9.2

Use the new backing store parser in the backing chain crawler. This change needs one test change where information about the NBD image are now parsed differently. --- src/storage/storage_driver.c | 90 +------------------------------------------- tests/virstoragetest.c | 14 ++++--- 2 files changed, 9 insertions(+), 95 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index cf969c8..ac49553 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -3041,56 +3041,6 @@ virStorageFileAccess(virStorageSourcePtr src, } -/** - * Given a starting point START (a directory containing the original - * file, if the original file was opened via a relative path; ignored - * if NAME is absolute), determine the location of the backing file - * NAME (possibly relative), and compute the relative DIRECTORY - * (optional) and CANONICAL (mandatory) location of the backing file. - * Return 0 on success, negative on error. - */ -static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) -virFindBackingFile(const char *start, const char *path, - char **directory, char **canonical) -{ - /* FIXME - when we eventually allow non-raw network devices, we - * must ensure that we handle backing files the same way as qemu. - * For a qcow2 top file of gluster://server/vol/img, qemu treats - * the relative backing file 'rel' as meaning - * 'gluster://server/vol/rel', while the backing file '/abs' is - * used as a local file. But we cannot canonicalize network - * devices via canonicalize_file_name(), because they are not part - * of the local file system. */ - char *combined = NULL; - int ret = -1; - - if (*path == '/') { - /* Safe to cast away const */ - combined = (char *)path; - } else if (virAsprintf(&combined, "%s/%s", start, path) < 0) { - goto cleanup; - } - - if (directory && !(*directory = mdir_name(combined))) { - virReportOOMError(); - goto cleanup; - } - - if (!(*canonical = canonicalize_file_name(combined))) { - virReportSystemError(errno, - _("Can't canonicalize path '%s'"), path); - goto cleanup; - } - - ret = 0; - - cleanup: - if (combined != path) - VIR_FREE(combined); - return ret; -} - - /* Recursive workhorse for virStorageFileGetMetadata. */ static int virStorageFileGetMetadataRecurse(virStorageSourcePtr src, @@ -3099,7 +3049,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, virHashTablePtr cycle) { int ret = -1; - struct stat st; const char *uniqueName; char *buf = NULL; ssize_t headerLen; @@ -3151,46 +3100,9 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; } - if (VIR_ALLOC(backingStore) < 0) + if (!(backingStore = virStorageSourceNewFromBacking(src))) goto cleanup; - if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0) - goto cleanup; - - if (virStorageIsFile(src->backingStoreRaw)) { - backingStore->type = VIR_STORAGE_TYPE_FILE; - - if (virFindBackingFile(src->relDir, - src->backingStoreRaw, - &backingStore->relDir, - &backingStore->path) < 0) { - /* the backing file is (currently) unavailable, treat this - * file as standalone: - * backingStoreRaw is kept to mark broken image chains */ - VIR_WARN("Backing file '%s' of image '%s' is missing.", - src->backingStoreRaw, src->path); - ret = 0; - goto cleanup; - } - - /* update the type for local storage */ - if (stat(backingStore->path, &st) == 0) { - if (S_ISDIR(st.st_mode)) { - backingStore->type = VIR_STORAGE_TYPE_DIR; - backingStore->format = VIR_STORAGE_FILE_DIR; - } else if (S_ISBLK(st.st_mode)) { - backingStore->type = VIR_STORAGE_TYPE_BLOCK; - } - } - } else { - /* TODO: To satisfy the test case, copy the network URI as path. This - * will be removed later. */ - backingStore->type = VIR_STORAGE_TYPE_NETWORK; - - if (VIR_STRDUP(backingStore->path, src->backingStoreRaw) < 0) - goto cleanup; - } - if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) backingStore->format = VIR_STORAGE_FILE_RAW; else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 746bf63..29297ef 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -730,20 +730,22 @@ mymain(void) /* Rewrite qcow2 to use an nbd: protocol as backend */ virCommandFree(cmd); cmd = virCommandNewArgList(qemuimg, "rebase", "-u", "-f", "qcow2", - "-F", "raw", "-b", "nbd:example.org:6000", + "-F", "raw", "-b", "nbd:example.org:6000:exportname=blah", "qcow2", NULL); if (virCommandRun(cmd, NULL) < 0) ret = -1; - qcow2.expBackingStore = "nbd:example.org:6000"; - qcow2.expBackingStoreRaw = "nbd:example.org:6000"; + qcow2.expBackingStore = "blah"; + qcow2.expBackingStoreRaw = "nbd:example.org:6000:exportname=blah"; /* Qcow2 file with backing protocol instead of file */ testFileData nbd = { - .pathRel = "nbd:example.org:6000", - .pathAbs = "nbd:example.org:6000", - .path = "nbd:example.org:6000", + .pathRel = "nbd:example.org:6000:exportname=blah", + .pathAbs = "nbd:example.org:6000:exportname=blah", + .path = "blah", .type = VIR_STORAGE_TYPE_NETWORK, .format = VIR_STORAGE_FILE_RAW, + .relDirRel = ".", + .relDirAbs = ".", }; TEST_CHAIN(11, "qcow2", absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2, &nbd), EXP_PASS, -- 1.9.2

Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b0a79e4..e9cf5d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14901,6 +14901,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, virDomainDiskDefPtr disk; virStorageSourcePtr baseSource = NULL; unsigned int baseIndex = 0; + char *basePath = NULL; if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -14969,9 +14970,16 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, base, baseIndex, NULL)))) goto endjob; + if (baseIndex) { + if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) + goto endjob; + } else { + if (VIR_STRDUP(basePath, base) < 0) + goto endjob; + } + qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorBlockJob(priv->mon, device, - baseIndex ? baseSource->path : base, + ret = qemuMonitorBlockJob(priv->mon, device, basePath, bandwidth, info, mode, async); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) @@ -15042,6 +15050,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } cleanup: + VIR_FREE(basePath); VIR_FREE(device); if (vm) virObjectUnlock(vm); -- 1.9.2

Add code to fake gluster storage path canonicalization while I'm working on the code. --- src/storage/storage_backend_gluster.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 1a844c9..411851b 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -533,6 +533,7 @@ typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr; struct _virStorageFileBackendGlusterPriv { glfs_t *vol; + char *uid; }; @@ -547,6 +548,7 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src) if (priv->vol) glfs_fini(priv->vol); + VIR_FREE(priv->uid); VIR_FREE(priv); src->drv->priv = NULL; @@ -606,6 +608,10 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src) goto error; } + if (virAsprintf(&priv->uid, "gluster://%s:%d/%s/%s", hostname, port, + src->volume, src->path) < 0) + goto error; + src->drv->priv = priv; return 0; @@ -713,6 +719,15 @@ virStorageFileBackendGlusterAccess(virStorageSourcePtr src, } +static const char * +virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + return priv->uid; +} + + virStorageFileBackend virStorageFileBackendGluster = { .type = VIR_STORAGE_TYPE_NETWORK, .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER, @@ -724,4 +739,8 @@ virStorageFileBackend virStorageFileBackendGluster = { .storageFileStat = virStorageFileBackendGlusterStat, .storageFileReadHeader = virStorageFileBackendGlusterReadHeader, .storageFileAccess = virStorageFileBackendGlusterAccess, + + .storageFileGetUniqueIdentifier = virStorageFileBackendGlusterGetUniqueIdentifier, + + }; -- 1.9.2