On 12/03/2015 09:35 AM, Matthias Gatto wrote:
> Uniformize backing store usage by calling virStorageSourceGetBackingStore
> instead of setting backing store manually.
>
> Signed-off-by: Matthias Gatto <[email protected]>
> Signed-off-by: John Ferlan <[email protected]>
> ---
> src/conf/domain_conf.c | 7 ++++---
> src/conf/storage_conf.c | 6 +++---
> src/qemu/qemu_cgroup.c | 4 ++--
> src/qemu/qemu_domain.c | 2 +-
> src/qemu/qemu_driver.c | 18 +++++++++--------
> src/qemu/qemu_monitor_json.c | 4 +++-
> src/security/security_dac.c | 2 +-
> src/security/security_selinux.c | 4 ++--
> src/security/virt-aa-helper.c | 2 +-
> src/storage/storage_backend.c | 22 ++++++++++++--------
> src/storage/storage_backend_fs.c | 38
> ++++++++++++++++++++---------------
> src/storage/storage_backend_gluster.c | 8 +++++---
> src/storage/storage_backend_logical.c | 12 +++++++----
> src/util/virstoragefile.c | 20 +++++++++---------
> tests/virstoragetest.c | 14 ++++++-------
> 15 files changed, 93 insertions(+), 70 deletions(-)
>
storage_backend_fs.c needed a slight merge with some changes I made...
There's also still a bunch of long lines. I've noted them below.
For each of them I propose adjusting the code a bit to do something like:
virStorageSourcePtr bsp;
...
bsp = virStorageSourceGetBackingStore(backingStore, 0);
then use bsp instead.
I'll add a "merge" patch to this response - it probably won't apply
cleanly since I had the merge issue to deal with first (in
storage_backend_fs.c - virStorageBackendFileSystemRefresh and call to
virStorageBackendUpdateVolTargetInfo). But I think if you update to top
of tree and then apply, a git am should work
John
I only worked on/through the first 2 patches, but can continue through
patch 5 - that'll at least get part of this series in
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e6102a0..5b413b5 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -18625,7 +18625,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
> /* We currently don't output seclabels for backing chain element */
> if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0
> ||
> virDomainDiskBackingStoreFormat(buf,
> - backingStore->backingStore,
> +
> virStorageSourceGetBackingStore(backingStore, 0),
Long line
> backingStore->backingStoreRaw,
> idx + 1) < 0)
> return -1;
> @@ -18746,7 +18746,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
> /* Don't format backingStore to inactive XMLs until the code for
> * persistent storage of backing chains is ready. */
> if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
> - virDomainDiskBackingStoreFormat(buf, def->src->backingStore,
> + virDomainDiskBackingStoreFormat(buf,
> +
> virStorageSourceGetBackingStore(def->src, 0),
Long line
> def->src->backingStoreRaw, 1) < 0)
> return -1;
>
> @@ -22714,7 +22715,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
> }
> }
>
> - for (tmp = disk->src; tmp; tmp = tmp->backingStore) {
> + for (tmp = disk->src; tmp; tmp = virStorageSourceGetBackingStore(tmp,
> 0)) {
> int actualType = virStorageSourceGetActualType(tmp);
> /* execute the callback only for local storage */
> if (actualType != VIR_STORAGE_TYPE_NETWORK &&
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 9b8abea..d048e39 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1330,7 +1330,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> if (virStorageSize(unit, capacity, &ret->target.capacity) < 0)
> goto error;
> } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) &&
> - !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
> ret->target.backingStore)) {
> + !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
> virStorageSourceGetBackingStore(&ret->target, 0))) {
Really long line
> virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity
> element"));
> goto error;
> }
> @@ -1644,9 +1644,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
> &def->target, "target") < 0)
> goto cleanup;
>
> - if (def->target.backingStore &&
> + if (virStorageSourceGetBackingStore(&def->target, 0) &&
> virStorageVolTargetDefFormat(options, &buf,
> - def->target.backingStore,
> +
> virStorageSourceGetBackingStore(&def->target, 0),
long line
> "backingStore") < 0)
> goto cleanup;
>
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index b52ce3a..6405944 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -121,7 +121,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm,
> virStorageSourcePtr next;
> bool forceReadonly = false;
>
> - for (next = disk->src; next; next = next->backingStore) {
> + for (next = disk->src; next; next =
> virStorageSourceGetBackingStore(next, 0)) {
long line
> if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) < 0)
> return -1;
>
> @@ -139,7 +139,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
> {
> virStorageSourcePtr next;
>
> - for (next = disk->src; next; next = next->backingStore) {
> + for (next = disk->src; next; next =
> virStorageSourceGetBackingStore(next, 0)) {
long line
> if (qemuSetImageCgroup(vm, next, true) < 0)
> return -1;
> }
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index ed21245..28bbe3f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3101,7 +3101,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
> if (virStorageSourceIsEmpty(disk->src))
> goto cleanup;
>
> - if (disk->src->backingStore) {
> + if (virStorageSourceGetBackingStore(disk->src, 0)) {
> if (force_probe)
> virStorageSourceBackingStoreClear(disk->src);
> else
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ae1d8e7..8ba7566 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14309,13 +14309,13 @@
> qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
>
> /* Update vm in place to match changes. */
> tmp = disk->src;
> - disk->src = tmp->backingStore;
> + disk->src = virStorageSourceGetBackingStore(tmp, 0);
> tmp->backingStore = NULL;
> virStorageSourceFree(tmp);
>
> if (persistDisk) {
> tmp = persistDisk->src;
> - persistDisk->src = tmp->backingStore;
> + persistDisk->src = virStorageSourceGetBackingStore(tmp, 0);
> tmp->backingStore = NULL;
> virStorageSourceFree(tmp);
> }
> @@ -16309,7 +16309,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
> goto endjob;
> }
>
> - if (virStorageFileGetRelativeBackingPath(disk->src->backingStore,
> + if
> (virStorageFileGetRelativeBackingPath(virStorageSourceGetBackingStore(disk->src,
> 0),
long line
> baseSource,
> &backingPath) < 0)
> goto endjob;
> @@ -16662,6 +16662,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
> virQEMUDriverConfigPtr cfg = NULL;
> const char *format = NULL;
> int desttype = virStorageSourceGetActualType(mirror);
> + virStorageSourcePtr backingStore;
>
> /* Preliminaries: find the disk we are editing, sanity checks */
> virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW |
> @@ -16705,8 +16706,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
> if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
> goto endjob;
>
> + backingStore = virStorageSourceGetBackingStore(disk->src, 0);
> /* clear the _SHALLOW flag if there is only one layer */
> - if (!disk->src->backingStore)
> + if (!backingStore)
> flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW;
>
> /* unless the user provides a pre-created file, shallow copy into a raw
> @@ -17113,7 +17115,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
> goto endjob;
> }
>
> - if (!topSource->backingStore) {
> + if (!virStorageSourceGetBackingStore(topSource, 0)) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("top '%s' in chain for '%s' has no backing file"),
> topSource->path, path);
> @@ -17121,14 +17123,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
> }
>
> if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
> - baseSource = topSource->backingStore;
> + baseSource = virStorageSourceGetBackingStore(topSource, 0);
> else if (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0
> ||
> !(baseSource = virStorageFileChainLookup(disk->src, topSource,
> base, baseIndex,
> NULL)))
> goto endjob;
>
> if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
> - baseSource != topSource->backingStore) {
> + baseSource != virStorageSourceGetBackingStore(topSource, 0)) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("base '%s' is not immediately below '%s' in chain "
> "for '%s'"),
> @@ -19406,7 +19408,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
> goto cleanup;
> visited++;
> backing_idx++;
> - src = src->backingStore;
> + src = virStorageSourceGetBackingStore(src, 0);
> }
> }
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 86b8c7b..790d7bc 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3785,7 +3785,9 @@ qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image,
> return NULL;
> if (top != target) {
> backing = virJSONValueObjectGetObject(image, "backing-image");
> - return qemuMonitorJSONDiskNameLookupOne(backing, top->backingStore,
> + virStorageSourcePtr backingStore =
> + virStorageSourceGetBackingStore(top, 0);
> + return qemuMonitorJSONDiskNameLookupOne(backing, backingStore,
> target);
> }
> if (VIR_STRDUP(ret, virJSONValueObjectGetString(image, "filename")) < 0)
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index cdde34e..7fdf40f 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -454,7 +454,7 @@ virSecurityDACSetSecurityDiskLabel(virSecurityManagerPtr
> mgr,
> {
> virStorageSourcePtr next;
>
> - for (next = disk->src; next; next = next->backingStore) {
> + for (next = disk->src; next; next =
> virStorageSourceGetBackingStore(next, 0)) {
long line
> if (virSecurityDACSetSecurityImageLabel(mgr, def, next) < 0)
> return -1;
> }
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index b8ebdcc..5d694d8 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1216,7 +1216,7 @@
> virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
> * be tracked in domain XML, at which point labelskip should be a
> * per-file attribute instead of a disk attribute. */
> if (disk_seclabel && disk_seclabel->labelskip &&
> - !src->backingStore)
> + !virStorageSourceGetBackingStore(src, 0))
> return 0;
>
> /* Don't restore labels on readonly/shared disks, because other VMs may
> @@ -1350,7 +1350,7 @@
> virSecuritySELinuxSetSecurityDiskLabel(virSecurityManagerPtr mgr,
> bool first = true;
> virStorageSourcePtr next;
>
> - for (next = disk->src; next; next = next->backingStore) {
> + for (next = disk->src; next; next =
> virStorageSourceGetBackingStore(next, 0)) {
long line
> if (virSecuritySELinuxSetSecurityImageLabelInternal(mgr, def, next,
> first) < 0)
> return -1;
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 5de56e5..dddde1c 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -967,7 +967,7 @@ get_files(vahControl * ctl)
> /* XXX - if we knew the qemu user:group here we could send it in
> * so that the open could be re-tried as that user:group.
> */
> - if (!disk->src->backingStore) {
> + if (!virStorageSourceGetBackingStore(disk->src, 0)) {
> bool probe = ctl->allowDiskFormatProbing;
> virStorageFileGetMetadata(disk->src, -1, -1, probe, false);
> }
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 194736b..08ed1dd 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -940,6 +940,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr
> conn,
> .features = vol->target.features,
> .nocow = vol->target.nocow,
> };
> + virStorageSourcePtr backingStore;
>
> virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
>
> @@ -988,12 +989,13 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr
> conn,
> }
> }
>
> - if (vol->target.backingStore) {
> + backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
> + if (backingStore) {
> int accessRetCode = -1;
> char *absolutePath = NULL;
>
> - info.backingFormat = vol->target.backingStore->format;
> - info.backingPath = vol->target.backingStore->path;
> + info.backingFormat = backingStore->format;
> + info.backingPath = backingStore->path;
>
> if (info.preallocate) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -1006,8 +1008,10 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr
> conn,
> * backing store, not really sure what use it serves though, and it
> * may cause issues with lvm. Untested essentially.
> */
> - if (inputvol && inputvol->target.backingStore &&
> - STRNEQ_NULLABLE(inputvol->target.backingStore->path,
> info.backingPath)) {
> + if (inputvol &&
> + virStorageSourceGetBackingStore(&inputvol->target, 0) &&
> +
> STRNEQ_NULLABLE(virStorageSourceGetBackingStore(&inputvol->target, 0)->path,
> + info.backingPath)) {
long line
This is also another one of those cases where using a temp variable is
going to help.
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("a different backing store cannot be
> specified."));
> return NULL;
> @@ -1197,7 +1201,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
> ATTRIBUTE_UNUSED,
> vol->target.format);
> return -1;
> }
> - if (vol->target.backingStore != NULL) {
> + if (virStorageSourceGetBackingStore(&vol->target, 0) != NULL) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("copy-on-write image not supported with "
> "qcow-create"));
> @@ -1639,14 +1643,16 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr
> vol,
> unsigned int openflags)
> {
> int ret;
> + virStorageSourcePtr backingStore;
>
> if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target,
> withBlockVolFormat,
> openflags)) < 0)
> return ret;
>
> - if (vol->target.backingStore &&
> - (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
> + backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
> + if (backingStore &&
> + (ret = virStorageBackendUpdateVolTargetInfo(backingStore,
> withBlockVolFormat,
>
> VIR_STORAGE_VOL_OPEN_DEFAULT |
>
> VIR_STORAGE_VOL_OPEN_NOERROR) < 0))
> diff --git a/src/storage/storage_backend_fs.c
> b/src/storage/storage_backend_fs.c
> index 99ea394..7b05d4d 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -70,6 +70,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
> int ret = -1;
> int rc;
> virStorageSourcePtr meta = NULL;
> + virStorageSourcePtr backingStore;
> struct stat sb;
>
> if (encryption)
> @@ -99,37 +100,39 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
> if (!(target->backingStore = virStorageSourceNewFromBacking(meta)))
> goto cleanup;
>
> - target->backingStore->format = backingStoreFormat;
> + backingStore = virStorageSourceGetBackingStore(target, 0);
> + backingStore->format = backingStoreFormat;
>
> /* XXX: Remote storage doesn't play nicely with volumes backed by
> * remote storage. To avoid trouble, just fake the backing store is
> RAW
> * and put the string from the metadata as the path of the target. */
> - if (!virStorageSourceIsLocalStorage(target->backingStore)) {
> - virStorageSourceFree(target->backingStore);
> + if (!virStorageSourceIsLocalStorage(backingStore)) {
> + virStorageSourceFree(backingStore);
Although you cleaned up the reference Peter noted before - there's
double free problem here if the VIR_ALLOC fails. Currently backingStore
is just the local copy of target->backingStore. If we free it here,
then target->backingStore still contains the address/pointer. If we jump
to cleanup, then target->backingStore could be free'd again
Perhaps one way around this is to do the 'set' code first, but I think
at least the following would be OK after the SourceFree
target->backingStore = NULL;
>
> - if (VIR_ALLOC(target->backingStore) < 0)
> + if (VIR_ALLOC(backingStore) < 0)
> goto cleanup;
>
> - target->backingStore->type = VIR_STORAGE_TYPE_NETWORK;
> - target->backingStore->path = meta->backingStoreRaw;
> + target->backingStore = backingStore;
> + backingStore->type = VIR_STORAGE_TYPE_NETWORK;
> + backingStore->path = meta->backingStoreRaw;
> meta->backingStoreRaw = NULL;
> - target->backingStore->format = VIR_STORAGE_FILE_RAW;
> + backingStore->format = VIR_STORAGE_FILE_RAW;
> }
>
> - if (target->backingStore->format == VIR_STORAGE_FILE_AUTO) {
> - if ((rc = virStorageFileProbeFormat(target->backingStore->path,
> + if (backingStore->format == VIR_STORAGE_FILE_AUTO) {
> + if ((rc = virStorageFileProbeFormat(backingStore->path,
> -1, -1)) < 0) {
> /* If the backing file is currently unavailable or is
> * accessed via remote protocol only log an error, fake the
> * format as RAW and continue. Returning -1 here would
> * disable the whole storage pool, making it unavailable for
> * even maintenance. */
> - target->backingStore->format = VIR_STORAGE_FILE_RAW;
> + backingStore->format = VIR_STORAGE_FILE_RAW;
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("cannot probe backing volume format: %s"),
> - target->backingStore->path);
> + backingStore->path);
> } else {
> - target->backingStore->format = rc;
> + backingStore->format = rc;
> }
> }
> }
> @@ -860,6 +863,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
> ATTRIBUTE_UNUSED,
> struct stat statbuf;
> virStorageVolDefPtr vol = NULL;
> virStorageSourcePtr target = NULL;
> + virStorageSourcePtr backingStore;
> int direrr;
> int fd = -1, ret = -1;
>
> @@ -918,10 +922,12 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
> ATTRIBUTE_UNUSED,
> if (vol->target.format == VIR_STORAGE_FILE_DIR)
> vol->type = VIR_STORAGE_VOL_DIR;
>
> - if (vol->target.backingStore) {
> -
> ignore_value(virStorageBackendUpdateVolTargetInfo(vol->target.backingStore,
> - false,
> -
> VIR_STORAGE_VOL_OPEN_DEFAULT));
> + backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
> + if (backingStore) {
> + ignore_value(virStorageBackendUpdateVolTargetInfo(
> + backingStore,
> + false,
> + VIR_STORAGE_VOL_OPEN_DEFAULT));
> /* If this failed, the backing file is currently unavailable,
> * the capacity, allocation, owner, group and mode are unknown.
> * An error message was raised, but we just continue. */
> diff --git a/src/storage/storage_backend_gluster.c
> b/src/storage/storage_backend_gluster.c
> index d2e79bc..9bddb3b 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
> @@ -247,6 +247,7 @@
> virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
> char *header = NULL;
> ssize_t len = VIR_STORAGE_MAX_HEADER;
> int backingFormat;
> + virStorageSourcePtr backingStore;
>
> *volptr = NULL;
>
> @@ -302,13 +303,14 @@
> virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
> if (meta->backingStoreRaw) {
> if (VIR_ALLOC(vol->target.backingStore) < 0)
> goto cleanup;
> + backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
>
> - vol->target.backingStore->path = meta->backingStoreRaw;
> + backingStore->path = meta->backingStoreRaw;
>
> if (backingFormat < 0)
> - vol->target.backingStore->format = VIR_STORAGE_FILE_RAW;
> + backingStore->format = VIR_STORAGE_FILE_RAW;
> else
> - vol->target.backingStore->format = backingFormat;
> + backingStore->format = backingFormat;
> meta->backingStoreRaw = NULL;
> }
>
> diff --git a/src/storage/storage_backend_logical.c
> b/src/storage/storage_backend_logical.c
> index 536e617..e2a287d 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -88,6 +88,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
> size_t i;
> int err, nextents, nvars, ret = -1;
> const char *attrs = groups[9];
> + virStorageSourcePtr backingStore;
>
> /* Skip inactive volume */
> if (attrs[4] != 'a')
> @@ -151,11 +152,12 @@ virStorageBackendLogicalMakeVol(char **const groups,
> if (VIR_ALLOC(vol->target.backingStore) < 0)
> goto cleanup;
>
> - if (virAsprintf(&vol->target.backingStore->path, "%s/%s",
> + backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
I see you fixed up the references here, but we still have a problem if
backingStore == NULL (now *technically* I know that cannot happen yet,
but at some point in the future it could.
So what should be done if backingStore == NULL here? Something like:
backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
if (backingStore) {
if (virAsprintf(&backingStore->path, "%s/%s",
pool->def->target.path, groups[1]) < 0)
goto cleanup;
backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
}
> + if (virAsprintf(&backingStore->path, "%s/%s",
> pool->def->target.path, groups[1]) < 0)
> goto cleanup;
>
> - vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
> + backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
> }
>
> if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
> @@ -732,6 +734,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> virErrorPtr err;
> struct stat sb;
> bool created = false;
> + virStorageSourcePtr backingStore;
>
> if (vol->target.encryption != NULL) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -764,8 +767,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> }
> virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity,
> 1024));
> - if (vol->target.backingStore)
> - virCommandAddArgList(cmd, "-s", vol->target.backingStore->path,
> NULL);
> + backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
> + if (backingStore)
> + virCommandAddArgList(cmd, "-s", backingStore->path, NULL);
> else
> virCommandAddArg(cmd, pool->def->source.name);
>
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 016beaa..af17d82 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1076,10 +1076,10 @@ virStorageFileChainGetBroken(virStorageSourcePtr
> chain,
> if (!chain)
> return 0;
>
> - for (tmp = chain; tmp; tmp = tmp->backingStore) {
> + for (tmp = chain; tmp; tmp = virStorageSourceGetBackingStore(tmp, 0)) {
> /* Break when we hit end of chain; report error if we detected
> * a missing backing file, infinite loop, or other error */
> - if (!tmp->backingStore && tmp->backingStoreRaw) {
> + if (!virStorageSourceGetBackingStore(tmp, 0) &&
> tmp->backingStoreRaw) {
> if (VIR_STRDUP(*brokenFile, tmp->backingStoreRaw) < 0)
> return -1;
>
> @@ -1334,8 +1334,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
> *parent = NULL;
>
> if (startFrom) {
> - while (chain && chain != startFrom->backingStore) {
> - chain = chain->backingStore;
> + while (chain && chain != virStorageSourceGetBackingStore(startFrom,
> 0)) {
> + chain = virStorageSourceGetBackingStore(chain, 0);
> i++;
> }
>
> @@ -1352,7 +1352,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
>
> while (chain) {
> if (!name && !idx) {
> - if (!chain->backingStore)
> + if (!virStorageSourceGetBackingStore(chain, 0))
> break;
> } else if (idx) {
> VIR_DEBUG("%zu: %s", i, chain->path);
> @@ -1387,7 +1387,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
> }
> }
> *parent = chain;
> - chain = chain->backingStore;
> + chain = virStorageSourceGetBackingStore(chain, 0);
> i++;
> }
>
> @@ -1893,8 +1893,8 @@ virStorageSourceCopy(const virStorageSource *src,
> !(ret->auth = virStorageAuthDefCopy(src->auth)))
> goto error;
>
> - if (backingChain && src->backingStore) {
> - if (!(ret->backingStore = virStorageSourceCopy(src->backingStore,
> + if (backingChain && virStorageSourceGetBackingStore(src, 0)) {
> + if (!(ret->backingStore =
> virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
> true)))
long line
> goto error;
> }
> @@ -2035,7 +2035,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr
> def)
> VIR_FREE(def->backingStoreRaw);
>
> /* recursively free backing chain */
> - virStorageSourceFree(def->backingStore);
> + virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
> def->backingStore = NULL;
> }
>
> @@ -2898,7 +2898,7 @@
> virStorageFileGetRelativeBackingPath(virStorageSourcePtr top,
>
> *relpath = NULL;
>
> - for (next = top; next; next = next->backingStore) {
> + for (next = top; next; next = virStorageSourceGetBackingStore(next, 0)) {
> if (!next->relPath) {
> ret = 1;
> goto cleanup;
> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
> index 38ce09e..5bd4637 100644
> --- a/tests/virstoragetest.c
> +++ b/tests/virstoragetest.c
> @@ -403,7 +403,7 @@ testStorageChain(const void *args)
> }
> VIR_FREE(expect);
> VIR_FREE(actual);
> - elt = elt->backingStore;
> + elt = virStorageSourceGetBackingStore(elt, 0);
> i++;
> }
> if (i != data->nfiles) {
> @@ -1044,8 +1044,8 @@ mymain(void)
> ret = -1;
> goto cleanup;
> }
> - chain2 = chain->backingStore;
> - chain3 = chain2->backingStore;
> + chain2 = virStorageSourceGetBackingStore(chain, 0);
> + chain3 = virStorageSourceGetBackingStore(chain2, 0);
>
> #define TEST_LOOKUP_TARGET(id, target, from, name, index, result, \
> meta, parent) \
> @@ -1110,8 +1110,8 @@ mymain(void)
> ret = -1;
> goto cleanup;
> }
> - chain2 = chain->backingStore;
> - chain3 = chain2->backingStore;
> + chain2 = virStorageSourceGetBackingStore(chain, 0);
> + chain3 = virStorageSourceGetBackingStore(chain2, 0);
>
> TEST_LOOKUP(28, NULL, "bogus", NULL, NULL, NULL);
> TEST_LOOKUP(29, chain, "bogus", NULL, NULL, NULL);
> @@ -1157,8 +1157,8 @@ mymain(void)
> ret = -1;
> goto cleanup;
> }
> - chain2 = chain->backingStore;
> - chain3 = chain2->backingStore;
> + chain2 = virStorageSourceGetBackingStore(chain, 0);
> + chain3 = virStorageSourceGetBackingStore(chain2, 0);
>
> TEST_LOOKUP(56, NULL, "bogus", NULL, NULL, NULL);
> TEST_LOOKUP(57, NULL, "sub/link2", chain->path, chain, NULL);
>
>From 408c85dfd2aee9da7917d9a1b137e51827846a9f Mon Sep 17 00:00:00 2001
From: John Ferlan <[email protected]>
Date: Mon, 14 Dec 2015 18:26:18 -0500
Subject: [PATCH 4/8] merge
Signed-off-by: John Ferlan <[email protected]>
---
src/conf/domain_conf.c | 20 ++++++++++++--------
src/conf/storage_conf.c | 15 +++++++++------
src/qemu/qemu_cgroup.c | 6 ++++--
src/qemu/qemu_driver.c | 6 ++++--
src/security/security_dac.c | 3 ++-
src/security/security_selinux.c | 3 ++-
src/storage/storage_backend.c | 17 ++++++++++-------
src/storage/storage_backend_fs.c | 1 +
src/storage/storage_backend_logical.c | 10 ++++++----
src/util/virstoragefile.c | 9 +++++----
10 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 18f9628..88804d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18730,6 +18730,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
{
const char *type;
const char *format;
+ virStorageSourcePtr bsp;
if (!backingStore) {
if (!backingStoreRaw)
@@ -18759,9 +18760,11 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<format type='%s'/>\n", format);
/* We currently don't output seclabels for backing chain element */
- if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 ||
- virDomainDiskBackingStoreFormat(buf,
- virStorageSourceGetBackingStore(backingStore, 0),
+ if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0)
+ return -1;
+
+ bsp = virStorageSourceGetBackingStore(backingStore, 0);
+ if (virDomainDiskBackingStoreFormat(buf, bsp,
backingStore->backingStoreRaw,
idx + 1) < 0)
return -1;
@@ -18881,11 +18884,12 @@ virDomainDiskDefFormat(virBufferPtr buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
- if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
- virDomainDiskBackingStoreFormat(buf,
- virStorageSourceGetBackingStore(def->src, 0),
- def->src->backingStoreRaw, 1) < 0)
- return -1;
+ if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
+ virStorageSourcePtr bsp = virStorageSourceGetBackingStore(def->src, 0);
+ if (virDomainDiskBackingStoreFormat(buf, bsp,
+ def->src->backingStoreRaw, 1) < 0)
+ return -1;
+ }
virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name);
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index d048e39..d02db8f 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1330,7 +1330,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if (virStorageSize(unit, capacity, &ret->target.capacity) < 0)
goto error;
} else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) &&
- !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && virStorageSourceGetBackingStore(&ret->target, 0))) {
+ !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
+ virStorageSourceGetBackingStore(&ret->target, 0))) {
virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element"));
goto error;
}
@@ -1644,11 +1645,13 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
&def->target, "target") < 0)
goto cleanup;
- if (virStorageSourceGetBackingStore(&def->target, 0) &&
- virStorageVolTargetDefFormat(options, &buf,
- virStorageSourceGetBackingStore(&def->target, 0),
- "backingStore") < 0)
- goto cleanup;
+ if (virStorageSourceGetBackingStore(&def->target, 0)) {
+ virStorageSourcePtr bsp =
+ virStorageSourceGetBackingStore(&def->target, 0);
+ if (virStorageVolTargetDefFormat(options, &buf, bsp,
+ "backingStore") < 0)
+ goto cleanup;
+ }
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</volume>\n");
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 90bd4ee..bcf4448 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -121,7 +121,8 @@ qemuSetupDiskCgroup(virDomainObjPtr vm,
virStorageSourcePtr next;
bool forceReadonly = false;
- for (next = disk->src; next; next = virStorageSourceGetBackingStore(next, 0)) {
+ for (next = disk->src; next;
+ next = virStorageSourceGetBackingStore(next, 0)) {
if (qemuSetImageCgroupInternal(vm, next, false, forceReadonly) < 0)
return -1;
@@ -139,7 +140,8 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
{
virStorageSourcePtr next;
- for (next = disk->src; next; next = virStorageSourceGetBackingStore(next, 0)) {
+ for (next = disk->src; next;
+ next = virStorageSourceGetBackingStore(next, 0)) {
if (qemuSetImageCgroup(vm, next, true) < 0)
return -1;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6773c6e..4a05ceb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16324,6 +16324,8 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
if (baseSource) {
if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
+ virStorageSourcePtr bsp;
+
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("this QEMU binary doesn't support relative "
@@ -16331,8 +16333,8 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
goto endjob;
}
- if (virStorageFileGetRelativeBackingPath(virStorageSourceGetBackingStore(disk->src, 0),
- baseSource,
+ bsp = virStorageSourceGetBackingStore(disk->src, 0);
+ if (virStorageFileGetRelativeBackingPath(bsp, baseSource,
&backingPath) < 0)
goto endjob;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 95144b5..aa2ba42 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -454,7 +454,8 @@ virSecurityDACSetSecurityDiskLabel(virSecurityManagerPtr mgr,
{
virStorageSourcePtr next;
- for (next = disk->src; next; next = virStorageSourceGetBackingStore(next, 0)) {
+ for (next = disk->src; next;
+ next = virStorageSourceGetBackingStore(next, 0)) {
if (virSecurityDACSetSecurityImageLabel(mgr, def, next) < 0)
return -1;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 0a6b2f7..770122c 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1350,7 +1350,8 @@ virSecuritySELinuxSetSecurityDiskLabel(virSecurityManagerPtr mgr,
bool first = true;
virStorageSourcePtr next;
- for (next = disk->src; next; next = virStorageSourceGetBackingStore(next, 0)) {
+ for (next = disk->src; next;
+ next = virStorageSourceGetBackingStore(next, 0)) {
if (virSecuritySELinuxSetSecurityImageLabelInternal(mgr, def, next,
first) < 0)
return -1;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 016d6ef..9ed3290 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1008,13 +1008,16 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
* backing store, not really sure what use it serves though, and it
* may cause issues with lvm. Untested essentially.
*/
- if (inputvol &&
- virStorageSourceGetBackingStore(&inputvol->target, 0) &&
- STRNEQ_NULLABLE(virStorageSourceGetBackingStore(&inputvol->target, 0)->path,
- info.backingPath)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("a different backing store cannot be specified."));
- return NULL;
+ if (inputvol) {
+ virStorageSourcePtr bsp =
+ virStorageSourceGetBackingStore(&inputvol->target, 0);
+
+ if (bsp && STRNEQ_NULLABLE(bsp->path, info.backingPath)) {
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("a different backing store cannot be specified."));
+ return NULL;
+ }
}
if (!(backingType = virStorageFileFormatTypeToString(info.backingFormat))) {
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 725d712..fcd52ed 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -108,6 +108,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
* and put the string from the metadata as the path of the target. */
if (!virStorageSourceIsLocalStorage(backingStore)) {
virStorageSourceFree(backingStore);
+ target->backingStore = NULL;
if (VIR_ALLOC(backingStore) < 0)
goto cleanup;
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 830076a..0bd4a65 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -153,11 +153,13 @@ virStorageBackendLogicalMakeVol(char **const groups,
goto cleanup;
backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
- if (virAsprintf(&backingStore->path, "%s/%s",
- pool->def->target.path, groups[1]) < 0)
- goto cleanup;
+ if (backingStore) {
+ if (virAsprintf(&backingStore->path, "%s/%s",
+ pool->def->target.path, groups[1]) < 0)
+ goto cleanup;
- backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
+ backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
+ }
}
if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 091b945..a4384e7 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1334,7 +1334,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
*parent = NULL;
if (startFrom) {
- while (chain && chain != virStorageSourceGetBackingStore(startFrom, 0)) {
+ while (chain &&
+ chain != virStorageSourceGetBackingStore(startFrom, 0)) {
chain = virStorageSourceGetBackingStore(chain, 0);
i++;
}
@@ -1893,9 +1894,9 @@ virStorageSourceCopy(const virStorageSource *src,
!(ret->auth = virStorageAuthDefCopy(src->auth)))
goto error;
- if (backingChain && virStorageSourceGetBackingStore(src, 0)) {
- if (!(ret->backingStore = virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
- true)))
+ if (backingChain) {
+ virStorageSourcePtr bsp = virStorageSourceGetBackingStore(src, 0);
+ if (!(ret->backingStore = virStorageSourceCopy(bsp, true)))
goto error;
}
--
2.5.0
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
