Create virStorageSourceGetBackingStore function in preparation for quorum: Actually, if we want to get a backing store inside a virStorageSource we have to do it manually(src->backingStore = backingStore). The problem is that with a quorum, a virStorageSource can contain multiple backing stores, and src->backingStore can be treated as a virStorageSourcePtr or a virStorageSourcePtrPtr. Due to these reason, we need to simplify the manipulation of virStorageSource, and create a function to get the asked backingStore in a virStorageSource For now, this function only return the backingStore field Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 8 ++++++++ src/util/virstoragefile.h | 3 +++ 3 files changed, 12 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2eec83..3f4d02c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2023,6 +2023,7 @@ virStorageSourceClear; virStorageSourceCopy; virStorageSourceFree; virStorageSourceGetActualType; +virStorageSourceGetBackingStore; virStorageSourceGetSecurityLabelDef; virStorageSourceInitChainElement; virStorageSourceIsEmpty; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7a4f9a0..dc6cf8f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1798,6 +1798,14 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +virStorageSourcePtr +virStorageSourceGetBackingStore(const virStorageSource *src, + size_t pos ATTRIBUTE_UNUSED) +{ + return src->backingStore; +} + + /** * virStorageSourcePtr: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index b4c3808..c37ddcf 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -289,6 +289,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, + size_t pos); + int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); int virStorageFileProbeFormatFromBuf(const char *path, char *buf, -- 1.8.3.1

Uniformize backing store usage by calling virStorageSourceGetBackingStore instead of setting backing store manually. Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/conf/domain_conf.c | 7 ++++--- src/conf/storage_conf.c | 18 ++++++++++-------- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 30 ++++++++++++++++-------------- src/storage/storage_backend_fs.c | 31 +++++++++++++++++-------------- src/storage/storage_backend_gluster.c | 8 +++++--- src/storage/storage_backend_logical.c | 6 +++--- src/util/virstoragefile.c | 20 ++++++++++---------- tests/virstoragetest.c | 14 +++++++------- 14 files changed, 85 insertions(+), 75 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8792f5e..0668a5b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16704,7 +16704,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), backingStore->backingStoreRaw, idx + 1) < 0) return -1; @@ -16826,7 +16826,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), def->src->backingStoreRaw, 1) < 0) return -1; @@ -20868,7 +20869,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 e9aaa8a..f4f7e24 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1339,20 +1339,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { + virStorageSourcePtr backingStorePtr; if (VIR_ALLOC(ret->target.backingStore) < 0) goto error; - ret->target.backingStore->path = backingStore; + backingStorePtr = virStorageSourceGetBackingStore(&ret->target, 0); + backingStorePtr->path = backingStore; backingStore = NULL; if (options->formatFromString) { char *format = virXPathString("string(./backingStore/format/@type)", ctxt); if (format == NULL) - ret->target.backingStore->format = options->defaultFormat; + backingStorePtr->format = options->defaultFormat; else - ret->target.backingStore->format = (options->formatFromString)(format); + backingStorePtr->format = (options->formatFromString)(format); - if (ret->target.backingStore->format < 0) { + if (backingStorePtr->format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); VIR_FREE(format); @@ -1361,9 +1363,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(format); } - if (VIR_ALLOC(ret->target.backingStore->perms) < 0) + if (VIR_ALLOC(backingStorePtr->perms) < 0) goto error; - if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms, + if (virStorageDefParsePerms(ctxt, backingStorePtr->perms, "./backingStore/permissions", DEFAULT_VOL_PERM_MODE) < 0) goto error; @@ -1641,9 +1643,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), "backingStore") < 0) goto cleanup; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d71ffbc..82b5f2f 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)) { 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)) { if (qemuSetImageCgroup(vm, next, true) < 0) return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..68a2a95 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2725,7 +2725,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 5994558..547d2b5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13499,13 +13499,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); } @@ -15624,7 +15624,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto endjob; } - if (virStorageFileGetRelativeBackingPath(disk->src->backingStore, + if (virStorageFileGetRelativeBackingPath(virStorageSourceGetBackingStore(disk->src, 0), baseSource, &backingPath) < 0) goto endjob; @@ -16336,7 +16336,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); @@ -16344,14 +16344,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'"), diff --git a/src/security/security_dac.c b/src/security/security_dac.c index deb6980..0c2a226 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -379,7 +379,7 @@ virSecurityDACSetSecurityDiskLabel(virSecurityManagerPtr mgr, { virStorageSourcePtr next; - for (next = disk->src; next; next = next->backingStore) { + 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 6e67a86..a1cab9f 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1146,7 +1146,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 @@ -1276,7 +1276,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)) { 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 e53779e..7a67071 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -942,7 +942,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 b990a82..4a94d3f 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -822,6 +822,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, char *opts = NULL; bool convert = false; bool backing = false; + virStorageSourcePtr backingStore; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); @@ -873,11 +874,12 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, } - if (vol->target.backingStore) { + backingStore = virStorageSourceGetBackingStore(&vol->target, 0); + if (backingStore) { int accessRetCode = -1; char *absolutePath = NULL; - backingType = virStorageFileFormatTypeToString(vol->target.backingStore->format); + backingType = virStorageFileFormatTypeToString(backingStore->format); if (preallocate) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -890,9 +892,9 @@ virStorageBackendCreateQemuImgCmd(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, - vol->target.backingStore->path)) { + if (inputvol && virStorageSourceGetBackingStore(&(inputvol->target), 0) && + STRNEQ_NULLABLE(virStorageSourceGetBackingStore(&(inputvol->target), 0)->path, + backingStore->path)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("a different backing store cannot be specified.")); return NULL; @@ -901,24 +903,24 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, if (backingType == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol backing store type %d"), - vol->target.backingStore->format); + backingStore->format); return NULL; } /* Convert relative backing store paths to absolute paths for access * validation. */ - if ('/' != *(vol->target.backingStore->path) && + if ('/' != *(backingStore->path) && virAsprintf(&absolutePath, "%s/%s", pool->def->target.path, - vol->target.backingStore->path) < 0) + backingStore->path) < 0) return NULL; accessRetCode = access(absolutePath ? absolutePath - : vol->target.backingStore->path, R_OK); + : backingStore->path, R_OK); VIR_FREE(absolutePath); if (accessRetCode != 0) { virReportSystemError(errno, _("inaccessible backing store volume %s"), - vol->target.backingStore->path); + backingStore->path); return NULL; } } @@ -959,7 +961,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, cmd = virCommandNew(create_tool); convert = !!inputvol; - backing = !inputvol && vol->target.backingStore; + backing = !inputvol && virStorageSourceGetBackingStore(&(vol->target), 0); if (convert) virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, NULL); @@ -1086,7 +1088,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")); @@ -1491,8 +1493,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, openflags)) < 0) return ret; - if (vol->target.backingStore && - (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore, + if (virStorageSourceGetBackingStore(&(vol->target), 0) && + (ret = virStorageBackendUpdateVolTargetInfo(virStorageSourceGetBackingStore(&(vol->target), 0), updateCapacity, withBlockVolFormat, VIR_STORAGE_VOL_OPEN_DEFAULT | diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 34f2153..56cfc56 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) @@ -96,27 +97,29 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup; if (meta->backingStoreRaw) { - if (!(target->backingStore = virStorageSourceNewFromBacking(meta))) + backingStore = virStorageSourceGetBackingStore(target, 0); + if (!(backingStore = virStorageSourceNewFromBacking(meta))) goto cleanup; - target->backingStore->format = backingStoreFormat; + 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); - 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 (backingStore->format == VIR_STORAGE_FILE_AUTO) { if ((rc = virStorageFileProbeFormat(target->backingStore->path, -1, -1)) < 0) { /* If the backing file is currently unavailable or is @@ -124,12 +127,12 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, * 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; } } } @@ -900,8 +903,8 @@ 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, + if (virStorageSourceGetBackingStore(&(vol->target), 0)) { + ignore_value(virStorageBackendUpdateVolTargetInfo(virStorageSourceGetBackingStore(&(vol->target), 0), true, false, VIR_STORAGE_VOL_OPEN_DEFAULT)); /* If this failed, the backing file is currently unavailable, diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 53e4632..fc48f0f 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 8aa68a6..fcec31f 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -151,11 +151,11 @@ virStorageBackendLogicalMakeVol(char **const groups, if (VIR_ALLOC(vol->target.backingStore) < 0) goto cleanup; - if (virAsprintf(&vol->target.backingStore->path, "%s/%s", + if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s", pool->def->target.path, groups[1]) < 0) goto cleanup; - vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2; + virStorageSourceGetBackingStore(&vol->target, 0)->format = VIR_STORAGE_POOL_LOGICAL_LVM2; } if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0) @@ -764,7 +764,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, } virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity, 1024)); - if (vol->target.backingStore) + if (virStorageSourceGetBackingStore(&(vol->target), 0)) virCommandAddArgList(cmd, "-s", vol->target.backingStore->path, NULL); else virCommandAddArg(cmd, pool->def->source.name); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dc6cf8f..052debf 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++; } *parent = startFrom; @@ -1343,7 +1343,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); @@ -1378,7 +1378,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, } } *parent = chain; - chain = chain->backingStore; + chain = virStorageSourceGetBackingStore(chain, 0); i++; } @@ -1880,8 +1880,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))) goto error; } @@ -2018,7 +2018,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(def->backingStore); + virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); def->backingStore = NULL; } @@ -2832,7 +2832,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 2601edc..12670da 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) { @@ -1029,8 +1029,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) \ @@ -1095,8 +1095,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); @@ -1142,8 +1142,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); -- 1.8.3.1

As explained for virStorageSourceGetBackingStore, quorum allows multiple backing store, this make the operation to set bs complex because we have to check if the backingStore is used as an array or a pointer, and set it differently in both case. In order to help the manipulation of backing store, I've added a function virStorageSourceSetBackingStore. For now virStorageSourceSetBackingStore don't handle the case where we have more than one backing store in virStorageSource. Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 10 ++++++++++ src/util/virstoragefile.h | 3 +++ 3 files changed, 14 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3f4d02c..980235b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2033,6 +2033,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 052debf..84eefa3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1806,6 +1806,16 @@ virStorageSourceGetBackingStore(const virStorageSource *src, } +virStorageSourcePtr +virStorageSourceSetBackingStore(virStorageSourcePtr src, + virStorageSourcePtr backingStore, + size_t pos ATTRIBUTE_UNUSED) +{ + src->backingStore = backingStore; + return src->backingStore; +} + + /** * virStorageSourcePtr: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c37ddcf..d5cf7e6 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -289,6 +289,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, + virStorageSourcePtr backingStore, + size_t pos); virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos); -- 1.8.3.1

Replace the parts of the code where a backing store is set manually with virStorageSourceSetBackingStore Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 4 ++-- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_backend_fs.c | 8 ++++---- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 8 +++++--- tests/virstoragetest.c | 4 ++-- 7 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0668a5b..9d6b888 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5659,7 +5659,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) goto cleanup; - src->backingStore = backingStore; + virStorageSourceSetBackingStore(src, backingStore, 0); ret = 0; cleanup: diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index f4f7e24..fac85fa 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1340,10 +1340,10 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { virStorageSourcePtr backingStorePtr; - if (VIR_ALLOC(ret->target.backingStore) < 0) + if (VIR_ALLOC(backingStorePtr) < 0) goto error; - backingStorePtr = virStorageSourceGetBackingStore(&ret->target, 0); + backingStorePtr = virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0); backingStorePtr->path = backingStore; backingStore = NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 547d2b5..b3afccd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13500,13 +13500,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ tmp = disk->src; disk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + virStorageSourceSetBackingStore(tmp, NULL, 0); virStorageSourceFree(tmp); if (persistDisk) { tmp = persistDisk->src; persistDisk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + virStorageSourceSetBackingStore(tmp, NULL, 0); virStorageSourceFree(tmp); } } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 56cfc56..a3b6688 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -97,8 +97,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup; if (meta->backingStoreRaw) { - backingStore = virStorageSourceGetBackingStore(target, 0); - if (!(backingStore = virStorageSourceNewFromBacking(meta))) + if (!(backingStore = virStorageSourceSetBackingStore(target, + virStorageSourceNewFromBacking(meta), + 0))) goto cleanup; backingStore->format = backingStoreFormat; @@ -111,8 +112,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (VIR_ALLOC(backingStore) < 0) goto cleanup; - - target->backingStore = backingStore; + virStorageSourceSetBackingStore(target, backingStore, 0); backingStore->type = VIR_STORAGE_TYPE_NETWORK; backingStore->path = meta->backingStoreRaw; meta->backingStoreRaw = NULL; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 66dc994..eeb2a4c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2899,7 +2899,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; } - src->backingStore = backingStore; + virStorageSourceSetBackingStore(src, backingStore, 0); backingStore = NULL; ret = 0; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 84eefa3..ba38827 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1891,8 +1891,10 @@ virStorageSourceCopy(const virStorageSource *src, goto error; if (backingChain && virStorageSourceGetBackingStore(src, 0)) { - if (!(ret->backingStore = virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0), - true))) + if (!virStorageSourceSetBackingStore(ret, + virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0), + true), + 0)) goto error; } @@ -2029,7 +2031,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) /* recursively free backing chain */ virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); - def->backingStore = NULL; + virStorageSourceSetBackingStore(def, NULL, 0); } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 12670da..b7820d4 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -583,9 +583,9 @@ testPathRelativePrepare(void) for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) { if (i < ARRAY_CARDINALITY(backingchain) - 1) - backingchain[i].backingStore = &backingchain[i + 1]; + virStorageSourceSetBackingStore(&backingchain[i], &backingchain[i + 1], 0); else - backingchain[i].backingStore = NULL; + virStorageSourceSetBackingStore(&backingchain[i], NULL, 0); backingchain[i].relPath = NULL; } -- 1.8.3.1

As explain in the former patchs, backingStore can be treat an array or a pointer. If we have only one backingStore we have to use it as a normal ptr but if there is more backing store, we use it as a pointer's array. Because it would be complicated to expend backingStore manually, and do the convertion from a pointer to an array, I've created the virStorageSourcePushBackingStore function to help. This function allocate an array of pointer only if there is more than 1 bs. Because we can not remove a child from a quorum, i didn't create the function virStorageSourcePopBackingStore. I've changed virStorageSourceBackingStoreClear, virStorageSourceSetBackingStore and virStorageSourceGetBackingStore to handle the case where backingStore is an array. Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/conf/storage_conf.c | 7 ++- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 2 + src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 89 ++++++++++++++++++++++++++++++++--- src/util/virstoragefile.h | 2 + 6 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index fac85fa..41887eb 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1343,7 +1343,12 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (VIR_ALLOC(backingStorePtr) < 0) goto error; - backingStorePtr = virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0); + if (!virStorageSourcePushBackingStore(&ret->target)) + goto error; + + backingStorePtr = virStorageSourceSetBackingStore(&ret->target, + backingStorePtr, + ret->target.nBackingStores - 1); backingStorePtr->path = backingStore; backingStore = NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 980235b..5752907 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2033,6 +2033,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourcePushBackingStore; virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index a3b6688..0d8c5ad 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -112,6 +112,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (VIR_ALLOC(backingStore) < 0) goto cleanup; + if (virStorageSourcePushBackingStore(target) == false) + goto cleanup; virStorageSourceSetBackingStore(target, backingStore, 0); backingStore->type = VIR_STORAGE_TYPE_NETWORK; backingStore->path = meta->backingStoreRaw; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index fcec31f..cd8de85 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -148,7 +148,7 @@ virStorageBackendLogicalMakeVol(char **const groups, * lv is created with "--virtualsize"). */ if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (virStorageSourcePushBackingStore(&vol->target) == false) goto cleanup; if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s", diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ba38827..554aa8b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1798,21 +1798,88 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +/** + * virStorageSourceGetBackingStore: + * @src: virStorageSourcePtr containing the backing stores + * @pos: position of the backing store to get + * + * return the backingStore at the position of @pos + */ virStorageSourcePtr -virStorageSourceGetBackingStore(const virStorageSource *src, - size_t pos ATTRIBUTE_UNUSED) +virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos) +{ + if (!src->backingStore || (pos > 1 && pos >= src->nBackingStores)) + return NULL; + if (src->nBackingStores < 2) + return src->backingStore; + return ((virStorageSourcePtr *)src->backingStore)[pos]; +} + + +/** + * virStorageSourcePushBackingStore: + * @src: virStorageSourcePtr to allocate the new backing store + * + * Allocate size for a new backingStorePtr in src->backingStore + * and update src->nBackingStores + * If we have less than 2 backing stores, we treat src->backingStore + * as a pointer otherwise we treat it as an array of virStorageSourcePtr + */ +bool +virStorageSourcePushBackingStore(virStorageSourcePtr src) { - return src->backingStore; + virStorageSourcePtr tmp; + virStorageSourcePtr *tmp2; + + if (!src) + return false; + if (src->nBackingStores == 1) { + /* If we need more than one backing store we need an array + * Because we don't want to lose our data from the old Backing Store + * we copy the pointer from src->backingStore to src->backingStore[0] */ + tmp = src->backingStore; + if (VIR_ALLOC_N(tmp2, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + src->nBackingStores += 1; + virStorageSourceSetBackingStore(src, tmp, 0); + } else if (src->nBackingStores > 1) { + tmp2 = ((virStorageSourcePtr *)src->backingStore); + if (VIR_EXPAND_N(tmp2, src->nBackingStores, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + } else { + /* Most of the time we use only one backingStore + * So we don't need to allocate an array */ + src->nBackingStores += 1; + } + return true; } +/** + * virStorageSourceSetBackingStore: + * @src: virStorageSourcePtr to hold @backingStore + * @backingStore: backingStore to store + * @pos: position of the backing store to store + * + * Set @backingStore at @pos in src->backingStore + */ virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, virStorageSourcePtr backingStore, - size_t pos ATTRIBUTE_UNUSED) + size_t pos) { - src->backingStore = backingStore; - return src->backingStore; + if (!src || (pos > 1 && pos >= src->nBackingStores)) + goto error; + if (src->nBackingStores > 1) { + ((virStorageSourcePtr *)src->backingStore)[pos] = backingStore; + } else { + src->backingStore = backingStore; + } + return backingStore; + error: + return NULL; } @@ -2023,6 +2090,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) void virStorageSourceBackingStoreClear(virStorageSourcePtr def) { + size_t i; + if (!def) return; @@ -2030,7 +2099,13 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); + for (i = 0; i < def->nBackingStores; ++i) + virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); + if (def->nBackingStores > 1) { + /* in this case def->backingStores is treat as an array so we have to free it*/ + VIR_FREE(def->backingStore); + } + def->nBackingStores = 0; virStorageSourceSetBackingStore(def, NULL, 0); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d5cf7e6..74c363b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -271,6 +271,7 @@ struct _virStorageSource { /* backing chain of the storage source */ virStorageSourcePtr backingStore; + size_t nBackingStores; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; @@ -294,6 +295,7 @@ virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, size_t pos); virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos); +bool virStorageSourcePushBackingStore(virStorageSourcePtr src); int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); int virStorageFileProbeFormatFromBuf(const char *path, -- 1.8.3.1

On Wed, Jan 28, 2015 at 11:10 AM, Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: I was trying to follow this: http://www.redhat.com/archives/libvir-list/2014-May/msg00546.html , and I think I've misunderstand this: "PtrPtr doesn't make sense. Just keep it as a single pointer, but add an nBackingStores field and treat it as an array (all existing callers are now an array of 1, quorum is a new array of N)" But if I can use a 'Ptr *' I wil, change my code. Thanks you for the review. Matthias

Add the capabiltty to libvirt to parse and format the quorum syntax as described here: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/conf/domain_conf.c | 165 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 120 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 849e63a..64fe06b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5605,20 +5605,58 @@ virDomainDiskSourceParse(xmlNodePtr node, } +static bool +virDomainDiskThresholdParse(virStorageSourcePtr src, + xmlNodePtr node) +{ + char *threshold = virXMLPropString(node, "threshold"); + int ret; + + if (!threshold) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing threshold in quorum")); + return false; + } + ret = virStrToLong_ul(threshold, NULL, 10, &src->threshold); + if (ret < 0 || src->threshold < 2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unexpected threshold %s"), + "threshold must be a decimal number superior to 2 " + "and inferior to the number of children"); + VIR_FREE(threshold); + return false; + } + VIR_FREE(threshold); + return true; +} + + static int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, - virStorageSourcePtr src) + virStorageSourcePtr src, + xmlNodePtr node, + size_t pos) { virStorageSourcePtr backingStore = NULL; xmlNodePtr save_ctxt = ctxt->node; - xmlNodePtr source; + xmlNodePtr source = NULL; + xmlNodePtr cur = NULL; char *type = NULL; char *format = NULL; int ret = -1; + size_t i = 0; - if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { - ret = 0; - goto cleanup; + if (src->type == VIR_STORAGE_TYPE_QUORUM) { + if (!node) { + ret = 0; + goto cleanup; + } + ctxt->node = node; + } else { + if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { + ret = 0; + goto cleanup; + } } if (VIR_ALLOC(backingStore) < 0) @@ -5650,6 +5688,22 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + if (!virDomainDiskThresholdParse(backingStore, node)) + goto cleanup; + + for (cur = node->children, i = 0; cur != NULL; cur = cur->next) { + if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if (virStorageSourcePushBackingStore(backingStore) == false) + goto cleanup; + if ((virDomainDiskBackingStoreParse(ctxt, backingStore, cur, i) < 0)) + goto cleanup; + ++i; + } + } + goto exit; + } + if (!(source = virXPathNode("./source", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store source")); @@ -5657,10 +5711,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, } if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) goto cleanup; - virStorageSourceSetBackingStore(src, backingStore, 0); + exit: + virStorageSourceSetBackingStore(src, backingStore, pos); ret = 0; cleanup: @@ -5672,7 +5727,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, return ret; } - #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -6166,6 +6220,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ + } else if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if (virStorageSourcePushBackingStore(def->src) == false) + goto error; + if (virDomainDiskBackingStoreParse(ctxt, def->src, cur, + def->src->nBackingStores - 1) < 0) + goto error; } } cur = cur->next; @@ -6189,12 +6249,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def->device = VIR_DOMAIN_DISK_DEVICE_DISK; } + if (def->src->type == VIR_STORAGE_TYPE_QUORUM && + !virDomainDiskThresholdParse(def->src, node)) + goto error; + + snapshot = virXMLPropString(node, "snapshot"); + /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ if (source == NULL && def->src->hosts == NULL && !def->src->srcpool && (def->device == VIR_DOMAIN_DISK_DEVICE_DISK || - (flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) { + (flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) && + def->src->type != VIR_STORAGE_TYPE_QUORUM) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); goto error; @@ -6538,9 +6605,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, def) < 0) goto error; - - if (virDomainDiskBackingStoreParse(ctxt, def->src) < 0) - goto error; } cleanup: @@ -16669,12 +16733,14 @@ virDomainDiskSourceFormat(virBufferPtr buf, static int virDomainDiskBackingStoreFormat(virBufferPtr buf, - virStorageSourcePtr backingStore, - const char *backingStoreRaw, + virStorageSourcePtr src, unsigned int idx) { + const char *backingStoreRaw = src->backingStoreRaw; + virStorageSourcePtr backingStore = virStorageSourceGetBackingStore(src, 0); const char *type; const char *format; + size_t i = 0; if (!backingStore) { if (!backingStoreRaw) @@ -16682,37 +16748,43 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, return 0; } - if (!backingStore->type || - !(type = virStorageTypeToString(backingStore->type))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected disk backing store type %d"), - backingStore->type); - return -1; - } + do { + backingStore = virStorageSourceGetBackingStore(src, i); + if (!backingStore->type || + !(type = virStorageTypeToString(backingStore->type))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected disk backing store type %d"), + backingStore->type); + return -1; + } - if (backingStore->format <= 0 || - !(format = virStorageFileFormatTypeToString(backingStore->format))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected disk backing store format %d"), - backingStore->format); - return -1; - } + if (backingStore->format <= 0 || + !(format = virStorageFileFormatTypeToString(backingStore->format))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected disk backing store format %d"), + backingStore->format); + return -1; + } - virBufferAsprintf(buf, "<backingStore type='%s' index='%u'>\n", - type, idx); - virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<backingStore type='%s' index='%u'", + type, idx); + if (backingStore->threshold) + virBufferAsprintf(buf, " threshold='%lu'", backingStore->threshold); + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); - 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), - backingStore->backingStoreRaw, - idx + 1) < 0) - return -1; + 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, + backingStore, + idx + 1) < 0) + return -1; - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</backingStore>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</backingStore>\n"); + ++i; + } while (i < src->nBackingStores); return 0; } @@ -16782,6 +16854,10 @@ virDomainDiskDefFormat(virBufferPtr buf, def->src->readonly)) virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(def->snapshot)); + if (def->src->threshold) { + virBufferAsprintf(buf, " threshold='%lu'", + def->src->threshold); + } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); @@ -16827,10 +16903,9 @@ 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) + if ((!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) || + def->src->type == VIR_STORAGE_TYPE_QUORUM) && + virDomainDiskBackingStoreFormat(buf, def->src, 1) < 0) return -1; virDomainDiskGeometryDefFormat(buf, def); -- 1.8.3.1

Add nodename inside virstoragefile During xml backingStore parsing, look for a nodename attribute in the disk declaration if this one is a quorum, if a nodename is found, add it to the virStorageSource otherwise create a new one with a random name. Take inspiration from this patch to create the nodename: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html Durring xml backingStore formating, look for a nodename attribute inside the virStorageSource struct, and add it to the disk element. Use the nodename to create the quorum in qemuBuildQuorumStr. Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- docs/formatdomain.html.in | 7 +++++++ src/conf/domain_conf.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_command.c | 3 +++ src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 1 + 5 files changed, 40 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7ab8cbb..1e1baf8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2135,6 +2135,13 @@ <code>vda[2]</code> refers to the backing store with <code>index='2'</code> of the disk with <code>vda</code> target. </dd> + <dt><code>nodename</code> attribute + <span class="since">since 1.2.13</span></dt> + <dd> + When the backing store is a quorum child, we can use this attribute + to define the node-name of a child. If this atribute is undefine, + a random nodename is generate. + </dd> <dt><code>format</code> sub-element</dt> <dd> The <code>format</code> element contains <code>type</code> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 64fe06b..e674960 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -56,6 +56,7 @@ #include "network_conf.h" #include "virtpm.h" #include "virstring.h" +#include "virrandom.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -5631,6 +5632,8 @@ virDomainDiskThresholdParse(virStorageSourcePtr src, } +#define GEN_NODE_NAME_PREFIX "libvirt" +#define GEN_NODE_NAME_MAX_LEN (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8) static int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virStorageSourcePtr src, @@ -5642,6 +5645,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, xmlNodePtr source = NULL; xmlNodePtr cur = NULL; char *type = NULL; + char *nodeName = NULL; char *format = NULL; int ret = -1; size_t i = 0; @@ -5675,6 +5679,23 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (src->type == VIR_STORAGE_TYPE_QUORUM) { + if ((nodeName = virXMLPropString(ctxt->node, "nodename"))) { + backingStore->nodeName = nodeName; + } else { + int len; + if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN) < 0) + goto cleanup; + snprintf(nodeName, GEN_NODE_NAME_MAX_LEN, + "%s%08x", GEN_NODE_NAME_PREFIX, (int)pos); + len = strlen(nodeName); + while (len < GEN_NODE_NAME_MAX_LEN - 1) + nodeName[len++] = virRandomInt('Z' - 'A') + 'A'; + nodeName[GEN_NODE_NAME_MAX_LEN - 1] = '\0'; + backingStore->nodeName = nodeName; + } + } + if (!(format = virXPathString("string(./format/@type)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store format")); @@ -5726,6 +5747,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, ctxt->node = save_ctxt; return ret; } +#undef GEN_NODE_NAME_PREFIX +#undef GEN_NODE_NAME_MAX_LEN #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -16770,6 +16793,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, type, idx); if (backingStore->threshold) virBufferAsprintf(buf, " threshold='%lu'", backingStore->threshold); + if (backingStore->nodeName) + virBufferAsprintf(buf, " nodename='%s'", backingStore->nodeName); virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index be7f66c..5a5e9cd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3273,6 +3273,9 @@ qemuBuildQuorumStr(virConnectPtr conn, toAppend, i, virStorageFileFormatTypeToString(backingStore->format)); + virBufferAsprintf(opt, ",%schildren.%lu.node-name=%s", + toAppend, i, backingStore->nodeName); + if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == false) goto error; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d0aa404..1623618 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1961,6 +1961,9 @@ virStorageSourceCopy(const virStorageSource *src, !(ret->auth = virStorageAuthDefCopy(src->auth))) goto error; + if (src->nodeName && VIR_STRDUP(ret->nodeName, src->nodeName) < 0) + goto error; + for (i = 0; i < src->nBackingStores; ++i) { if (backingChain && virStorageSourceGetBackingStore(src, i)) { if (!virStorageSourceSetBackingStore(ret, @@ -2106,6 +2109,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->relPath); VIR_FREE(def->backingStoreRaw); + VIR_FREE(def->nodeName); /* recursively free backing chain */ for (i = 0; i < def->nBackingStores; ++i) diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 3b43a6d..1c8c8cb 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -275,6 +275,7 @@ struct _virStorageSource { virStorageSourcePtr backingStore; size_t nBackingStores; size_t threshold; + char *nodeName; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; -- 1.8.3.1