[libvirt] [PATCH v5 0/9] qemu: Add quorum support to libvirt

The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child. 5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one) V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. V4: -Rebase on master V5: -Rebase on master -patch 1-4/9: use patchs from John Ferlan -patch 4/9: check return of virStorageSourceSetBackingStore -patch 5/9: report type of error on virStorageSourceSetBackingStore Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name docs/formatdomain.html.in | 30 +++++- docs/schemas/domaincommon.rng | 26 ++++- docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c | 195 ++++++++++++++++++++++++++-------- src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++ src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 39 ++++--- src/qemu/qemu_migration.c | 1 + 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 | 11 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 3 +- src/util/virstoragefile.c | 122 ++++++++++++++++++--- src/util/virstoragefile.h | 13 ++- tests/virstoragetest.c | 18 ++-- 24 files changed, 551 insertions(+), 141 deletions(-) -- 2.3.5

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@outscale.com> --- 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 8c50ea2..6893054 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2114,6 +2114,7 @@ virStorageSourceClear; virStorageSourceCopy; virStorageSourceFree; virStorageSourceGetActualType; +virStorageSourceGetBackingStore; virStorageSourceGetSecurityLabelDef; virStorageSourceInitChainElement; virStorageSourceIsEmpty; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9507ca1..e6ed573 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1809,6 +1809,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 aa17a00..4262b36 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, -- 2.3.5

Uniformize backing store usage by calling virStorageSourceGetBackingStore instead of setting backing store manually. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- 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 | 20 ++++++++++++-------- src/storage/storage_backend_fs.c | 31 +++++++++++++++++-------------- src/storage/storage_backend_gluster.c | 8 +++++--- src/storage/storage_backend_logical.c | 10 ++++++---- src/util/virstoragefile.c | 20 ++++++++++---------- tests/virstoragetest.c | 14 +++++++------- 15 files changed, 84 insertions(+), 68 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9aad782..2a05291 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17751,7 +17751,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; @@ -17873,7 +17873,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; @@ -22044,7 +22045,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 4852dfb..5781374 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1326,7 +1326,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))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element")); goto error; } @@ -1635,9 +1635,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 e83342d..44363c9 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 1368386..5415f04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2807,7 +2807,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 82f34ec..26be9d6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14322,13 +14322,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); } @@ -16338,7 +16338,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, goto endjob; } - if (virStorageFileGetRelativeBackingPath(disk->src->backingStore, + if (virStorageFileGetRelativeBackingPath(virStorageSourceGetBackingStore(disk->src, 0), baseSource, &backingPath) < 0) goto endjob; @@ -16702,6 +16702,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 | @@ -16745,9 +16746,10 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) goto endjob; + backingStore = virStorageSourceGetBackingStore(disk->src, 0); if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) && mirror->format == VIR_STORAGE_FILE_RAW && - disk->src->backingStore->path) { + backingStore->path) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk '%s' has backing file, so raw shallow copy " "is not possible"), @@ -17147,7 +17149,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); @@ -17155,14 +17157,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'"), @@ -19422,7 +19424,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 3af319c..ac3f359 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3894,7 +3894,9 @@ qemuMonitorJSONDiskNameLookupOne(virJSONValuePtr image, return NULL; if (top != target) { backing = virJSONValueObjectGet(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 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 35423b5..5670140 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -946,7 +946,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 e0311e1..269a93b 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -885,6 +885,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, .features = vol->target.features, .nocow = vol->target.nocow, }; + virStorageSourcePtr backingStore; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); @@ -933,12 +934,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", @@ -951,8 +953,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)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("a different backing store cannot be specified.")); return NULL; @@ -1142,7 +1146,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")); @@ -1577,8 +1581,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), 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 521dc70..151af47 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); - 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; } } } @@ -905,8 +908,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), 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 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 11c5683..27fdc64 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) @@ -731,6 +731,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virCommandPtr cmd = NULL; virErrorPtr err; struct stat sb; + virStorageSourcePtr backingStore; if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -763,8 +764,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 e6ed573..620f490 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++; } @@ -1891,8 +1891,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; } @@ -2033,7 +2033,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(def->backingStore); + virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); def->backingStore = NULL; } @@ -2855,7 +2855,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); -- 2.3.5

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@outscale.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- 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 6893054..ee0db4f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2124,6 +2124,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 620f490..d69f49d 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1817,6 +1817,16 @@ virStorageSourceGetBackingStore(const virStorageSource *src, } +bool +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 4262b36..5f76324 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -289,6 +289,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +bool virStorageSourceSetBackingStore(virStorageSourcePtr src, + virStorageSourcePtr backingStore, + size_t pos); virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos); -- 2.3.5

Replace the parts of the code where a backing store is set manually with virStorageSourceSetBackingStore Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/storage_conf.c | 17 ++++++++++------- src/qemu/qemu_driver.c | 17 +++++++++++------ src/storage/storage_backend_fs.c | 7 +++++-- src/storage/storage_backend_gluster.c | 5 +++-- src/storage/storage_backend_logical.c | 9 ++++++--- src/storage/storage_driver.c | 3 ++- src/util/virstoragefile.c | 8 +++++--- tests/virstoragetest.c | 4 ++-- 9 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a05291..09f0bca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6006,7 +6006,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) goto cleanup; - src->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + goto cleanup; ret = 0; cleanup: diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5781374..ca3a6d5 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1254,6 +1254,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *capacity = NULL; char *unit = NULL; char *backingStore = NULL; + virStorageSourcePtr backingStorePtr; xmlNodePtr node; xmlNodePtr *nodes = NULL; size_t i; @@ -1290,20 +1291,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(ret->target.backingStore) < 0) + if (VIR_ALLOC(backingStorePtr) < 0) goto error; - ret->target.backingStore->path = backingStore; + if (!virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0)) + goto error; + 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); @@ -1312,9 +1315,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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26be9d6..1a98601 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14274,13 +14274,18 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ need_unlink = false; - newDiskSrc->backingStore = disk->src; - disk->src = newDiskSrc; + if (!virStorageSourceSetBackingStore(newDiskSrc, disk->src, 0)) { + ret = -1; + goto cleanup; + } newDiskSrc = NULL; if (persistDisk) { - persistDiskSrc->backingStore = persistDisk->src; - persistDisk->src = persistDiskSrc; + if (!virStorageSourceSetBackingStore(persistDiskSrc, + persistDisk->src, 0)) { + ret = -1; + goto cleanup; + } persistDiskSrc = NULL; } @@ -14323,13 +14328,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ tmp = disk->src; disk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp); if (persistDisk) { tmp = persistDisk->src; persistDisk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp); } } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 151af47..bab2569 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -97,7 +97,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup; if (meta->backingStoreRaw) { - if (!(target->backingStore = virStorageSourceNewFromBacking(meta))) + if (!virStorageSourceSetBackingStore(target, + virStorageSourceNewFromBacking(meta), + 0)) goto cleanup; backingStore = virStorageSourceGetBackingStore(target, 0); @@ -112,7 +114,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (VIR_ALLOC(backingStore) < 0) goto cleanup; - target->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(target, backingStore, 0)) + goto cleanup; backingStore->type = VIR_STORAGE_TYPE_NETWORK; backingStore->path = meta->backingStoreRaw; meta->backingStoreRaw = NULL; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 9bddb3b..f0a3b9b 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -301,9 +301,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup; if (meta->backingStoreRaw) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (VIR_ALLOC(backingStore) < 0) goto cleanup; - backingStore = virStorageSourceGetBackingStore(&vol->target, 0); + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0)) + goto error; backingStore->path = meta->backingStoreRaw; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 27fdc64..16d508e 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') @@ -148,14 +149,16 @@ 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 (VIR_ALLOC(backingStore) < 0) goto cleanup; - if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s", + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0)) + goto cleanup; + if (virAsprintf(&backingStore->path, "%s/%s", pool->def->target.path, groups[1]) < 0) goto cleanup; - virStorageSourceGetBackingStore(&vol->target, 0)->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/storage/storage_driver.c b/src/storage/storage_driver.c index 306d98e..cae7484 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -3044,7 +3044,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; } - src->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + goto cleanup; backingStore = NULL; ret = 0; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d69f49d..234a72b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1902,8 +1902,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; } @@ -2044,7 +2046,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 5bd4637..58f505d 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; } -- 2.3.5

On 04/23/2015 08:41 AM, Matthias Gatto wrote:
Replace the parts of the code where a backing store is set manually with virStorageSourceSetBackingStore
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/storage_conf.c | 17 ++++++++++------- src/qemu/qemu_driver.c | 17 +++++++++++------ src/storage/storage_backend_fs.c | 7 +++++-- src/storage/storage_backend_gluster.c | 5 +++-- src/storage/storage_backend_logical.c | 9 ++++++--- src/storage/storage_driver.c | 3 ++- src/util/virstoragefile.c | 8 +++++--- tests/virstoragetest.c | 4 ++-- 9 files changed, 46 insertions(+), 27 deletions(-)
Other than a minor goto error issue in storage_backend_gluster.c - up through here things seem fine to me. Doesn't seem to be any new readers or setters of "->backingStore" in recent changes (since patches 5-9 compile too). The only references are now ->backingStoreRaw fetches and saves. However, for patches 5-9 as I understand it have some concerns from Peter which hopefully he can elaborate on. So that progress can be made and readers/writers of backingStore go through the virStorageSource{Get|Set}BackingStore API's until the rest of the concerns are understood - I'm of the opinion the patches 1-4 can be pushed, but I'll wait until Peter chimes in before doing so... John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a05291..09f0bca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6006,7 +6006,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) goto cleanup;
- src->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + goto cleanup; ret = 0;
cleanup: diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5781374..ca3a6d5 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1254,6 +1254,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *capacity = NULL; char *unit = NULL; char *backingStore = NULL; + virStorageSourcePtr backingStorePtr; xmlNodePtr node; xmlNodePtr *nodes = NULL; size_t i; @@ -1290,20 +1291,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, }
if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(ret->target.backingStore) < 0) + if (VIR_ALLOC(backingStorePtr) < 0) goto error;
- ret->target.backingStore->path = backingStore; + if (!virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0)) + goto error; + 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); @@ -1312,9 +1315,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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26be9d6..1a98601 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14274,13 +14274,18 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ need_unlink = false;
- newDiskSrc->backingStore = disk->src; - disk->src = newDiskSrc; + if (!virStorageSourceSetBackingStore(newDiskSrc, disk->src, 0)) { + ret = -1; + goto cleanup; + } newDiskSrc = NULL;
if (persistDisk) { - persistDiskSrc->backingStore = persistDisk->src; - persistDisk->src = persistDiskSrc; + if (!virStorageSourceSetBackingStore(persistDiskSrc, + persistDisk->src, 0)) { + ret = -1; + goto cleanup; + } persistDiskSrc = NULL; }
@@ -14323,13 +14328,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ tmp = disk->src; disk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp);
if (persistDisk) { tmp = persistDisk->src; persistDisk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp); } } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 151af47..bab2569 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -97,7 +97,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup;
if (meta->backingStoreRaw) { - if (!(target->backingStore = virStorageSourceNewFromBacking(meta))) + if (!virStorageSourceSetBackingStore(target, + virStorageSourceNewFromBacking(meta), + 0)) goto cleanup;
backingStore = virStorageSourceGetBackingStore(target, 0); @@ -112,7 +114,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (VIR_ALLOC(backingStore) < 0) goto cleanup;
- target->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(target, backingStore, 0)) + goto cleanup; backingStore->type = VIR_STORAGE_TYPE_NETWORK; backingStore->path = meta->backingStoreRaw; meta->backingStoreRaw = NULL; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 9bddb3b..f0a3b9b 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -301,9 +301,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup;
if (meta->backingStoreRaw) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (VIR_ALLOC(backingStore) < 0) goto cleanup; - backingStore = virStorageSourceGetBackingStore(&vol->target, 0); + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0)) + goto error;
Should be goto cleanup - build failure otherwise
backingStore->path = meta->backingStoreRaw;
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 27fdc64..16d508e 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') @@ -148,14 +149,16 @@ 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 (VIR_ALLOC(backingStore) < 0) goto cleanup;
- if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s", + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0)) + goto cleanup; + if (virAsprintf(&backingStore->path, "%s/%s", pool->def->target.path, groups[1]) < 0) goto cleanup;
- virStorageSourceGetBackingStore(&vol->target, 0)->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/storage/storage_driver.c b/src/storage/storage_driver.c index 306d98e..cae7484 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -3044,7 +3044,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; }
- src->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + goto cleanup; backingStore = NULL; ret = 0;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d69f49d..234a72b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1902,8 +1902,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; }
@@ -2044,7 +2046,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 5bd4637..58f505d 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; }

On Mon, May 4, 2015 at 7:21 PM, John Ferlan <jferlan@redhat.com> wrote:
On 04/23/2015 08:41 AM, Matthias Gatto wrote:
Replace the parts of the code where a backing store is set manually with virStorageSourceSetBackingStore
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/storage_conf.c | 17 ++++++++++------- src/qemu/qemu_driver.c | 17 +++++++++++------ src/storage/storage_backend_fs.c | 7 +++++-- src/storage/storage_backend_gluster.c | 5 +++-- src/storage/storage_backend_logical.c | 9 ++++++--- src/storage/storage_driver.c | 3 ++- src/util/virstoragefile.c | 8 +++++--- tests/virstoragetest.c | 4 ++-- 9 files changed, 46 insertions(+), 27 deletions(-)
Other than a minor goto error issue in storage_backend_gluster.c - up through here things seem fine to me. Doesn't seem to be any new readers or setters of "->backingStore" in recent changes (since patches 5-9 compile too). The only references are now ->backingStoreRaw fetches and saves.
However, for patches 5-9 as I understand it have some concerns from Peter which hopefully he can elaborate on.
So that progress can be made and readers/writers of backingStore go through the virStorageSource{Get|Set}BackingStore API's until the rest of the concerns are understood - I'm of the opinion the patches 1-4 can be pushed, but I'll wait until Peter chimes in before doing so...
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a05291..09f0bca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6006,7 +6006,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) goto cleanup;
- src->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + goto cleanup; ret = 0;
cleanup: diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5781374..ca3a6d5 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1254,6 +1254,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *capacity = NULL; char *unit = NULL; char *backingStore = NULL; + virStorageSourcePtr backingStorePtr; xmlNodePtr node; xmlNodePtr *nodes = NULL; size_t i; @@ -1290,20 +1291,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, }
if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(ret->target.backingStore) < 0) + if (VIR_ALLOC(backingStorePtr) < 0) goto error;
- ret->target.backingStore->path = backingStore; + if (!virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0)) + goto error; + 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); @@ -1312,9 +1315,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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 26be9d6..1a98601 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14274,13 +14274,18 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ need_unlink = false;
- newDiskSrc->backingStore = disk->src; - disk->src = newDiskSrc; + if (!virStorageSourceSetBackingStore(newDiskSrc, disk->src, 0)) { + ret = -1; + goto cleanup; + } newDiskSrc = NULL;
if (persistDisk) { - persistDiskSrc->backingStore = persistDisk->src; - persistDisk->src = persistDiskSrc; + if (!virStorageSourceSetBackingStore(persistDiskSrc, + persistDisk->src, 0)) { + ret = -1; + goto cleanup; + } persistDiskSrc = NULL; }
@@ -14323,13 +14328,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ tmp = disk->src; disk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp);
if (persistDisk) { tmp = persistDisk->src; persistDisk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0)); virStorageSourceFree(tmp); } } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 151af47..bab2569 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -97,7 +97,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup;
if (meta->backingStoreRaw) { - if (!(target->backingStore = virStorageSourceNewFromBacking(meta))) + if (!virStorageSourceSetBackingStore(target, + virStorageSourceNewFromBacking(meta), + 0)) goto cleanup;
backingStore = virStorageSourceGetBackingStore(target, 0); @@ -112,7 +114,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (VIR_ALLOC(backingStore) < 0) goto cleanup;
- target->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(target, backingStore, 0)) + goto cleanup; backingStore->type = VIR_STORAGE_TYPE_NETWORK; backingStore->path = meta->backingStoreRaw; meta->backingStoreRaw = NULL; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 9bddb3b..f0a3b9b 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -301,9 +301,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup;
if (meta->backingStoreRaw) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (VIR_ALLOC(backingStore) < 0) goto cleanup; - backingStore = virStorageSourceGetBackingStore(&vol->target, 0); + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0)) + goto error;
Should be goto cleanup - build failure otherwise
backingStore->path = meta->backingStoreRaw;
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 27fdc64..16d508e 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') @@ -148,14 +149,16 @@ 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 (VIR_ALLOC(backingStore) < 0) goto cleanup;
- if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s", + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0)) + goto cleanup; + if (virAsprintf(&backingStore->path, "%s/%s", pool->def->target.path, groups[1]) < 0) goto cleanup;
- virStorageSourceGetBackingStore(&vol->target, 0)->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/storage/storage_driver.c b/src/storage/storage_driver.c index 306d98e..cae7484 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -3044,7 +3044,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; }
- src->backingStore = backingStore; + if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + goto cleanup; backingStore = NULL; ret = 0;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d69f49d..234a72b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1902,8 +1902,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; }
@@ -2044,7 +2046,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 5bd4637..58f505d 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; }
ok sorry for the goto mistake. If I can help you to understand my I'll be happy to help.

The backingStore field was a virStorageSourcePtr. because a quorum can contain more that one backingStore at the same level it's now a 'virStorageSourcePtr *'. This patch rename src->backingStore to src->backingStores, add a static function virStorageSourceExpandBackingStore (virStorageSourcePushBackingStore in the V2) and made the necessary modification to virStorageSourceSetBackingStore and virStorageSourceGetBackingStore. virStorageSourceSetBackingStore can now expand size of src->backingStores by calling virStorageSourceExpandBackingStore if necessary. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/storage/storage_backend.c | 2 +- src/storage/storage_backend_fs.c | 2 +- src/util/virstoragefile.c | 75 +++++++++++++++++++++++++++++++++++----- src/util/virstoragefile.h | 3 +- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 269a93b..b0eb976 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -487,7 +487,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - if (vol->target.backingStore) { + if (vol->target.backingStores) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("backing storage not supported for raw volumes")); goto cleanup; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bab2569..dd9ccb5 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1048,7 +1048,7 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - if (vol->target.backingStore) { + if (vol->target.backingStores) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("backing storage not supported for directories volumes")); return -1; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 234a72b..f0450b5 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1809,21 +1809,72 @@ 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) { - return src->backingStore; + if (!src || !src->backingStores || pos >= src->nBackingStores) + return NULL; + return src->backingStores[pos]; } +/** + * virStorageSourcePushBackingStore: + * + * Expand src->backingStores and update src->nBackingStores + */ +static bool +virStorageSourceExpandBackingStore(virStorageSourcePtr src, size_t nbr) +{ + if (!src) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("src is NULL")); + return false; + } + if (src->nBackingStores > 0) { + if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0) + goto allocation_failed; + } else { + if (VIR_ALLOC_N(src->backingStores, nbr) < 0) + goto allocation_failed; + src->nBackingStores += nbr; + } + return true; + allocation_failed: + virReportOOMError(); + return false; +} + + +/** + * virStorageSourceSetBackingStore: + * @src: virStorageSourcePtr to hold @backingStore + * @backingStore: backingStore to store + * @pos: position of the backing store to store + * + * Set @backingStore at @pos in src->backingStores. + * If src->backingStores don't have the space to contain + * @backingStore, we expand src->backingStores + */ bool virStorageSourceSetBackingStore(virStorageSourcePtr src, virStorageSourcePtr backingStore, - size_t pos ATTRIBUTE_UNUSED) + size_t pos) { - src->backingStore = backingStore; - return !!src->backingStore; + if (!src) + return false; + if (pos >= src->nBackingStores && + !virStorageSourceExpandBackingStore(src, pos - src->nBackingStores + 1)) + return false; + src->backingStores[pos] = backingStore; + return true; } @@ -2038,6 +2089,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) void virStorageSourceBackingStoreClear(virStorageSourcePtr def) { + size_t i; + if (!def) return; @@ -2045,8 +2098,14 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); - virStorageSourceSetBackingStore(def, NULL, 0); + for (i = 0; i < def->nBackingStores; ++i) + virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); + if (def->nBackingStores > 0) { + /* in this case def->backingStores is treat as an array so we have to free it*/ + VIR_FREE(def->backingStores); + } + def->nBackingStores = 0; + def->backingStores = NULL; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 5f76324..34ea6fb 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -270,7 +270,8 @@ struct _virStorageSource { bool shared; /* backing chain of the storage source */ - virStorageSourcePtr backingStore; + virStorageSourcePtr *backingStores; + size_t nBackingStores; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; -- 2.3.5

On Thu, Apr 23, 2015 at 14:41:17 +0200, Matthias Gatto wrote:
The backingStore field was a virStorageSourcePtr. because a quorum can contain more that one backingStore at the same level it's now a 'virStorageSourcePtr *'.
This patch rename src->backingStore to src->backingStores, add a static function virStorageSourceExpandBackingStore (virStorageSourcePushBackingStore in the V2) and made the necessary modification to virStorageSourceSetBackingStore and virStorageSourceGetBackingStore. virStorageSourceSetBackingStore can now expand size of src->backingStores by calling virStorageSourceExpandBackingStore if necessary.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/storage/storage_backend.c | 2 +- src/storage/storage_backend_fs.c | 2 +- src/util/virstoragefile.c | 75 +++++++++++++++++++++++++++++++++++----- src/util/virstoragefile.h | 3 +- 4 files changed, 71 insertions(+), 11 deletions(-)
...
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 234a72b..f0450b5 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1809,21 +1809,72 @@ 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) { - return src->backingStore; + if (!src || !src->backingStores || pos >= src->nBackingStores) + return NULL; + return src->backingStores[pos]; }
+/** + * virStorageSourcePushBackingStore: + * + * Expand src->backingStores and update src->nBackingStores + */ +static bool +virStorageSourceExpandBackingStore(virStorageSourcePtr src, size_t nbr) +{ + if (!src) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("src is NULL")); + return false; + } + if (src->nBackingStores > 0) { + if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0)
This internally reallocates the memory even if the original pointer is NULL, so there's no need for the if statement)
+ goto allocation_failed; + } else { + if (VIR_ALLOC_N(src->backingStores, nbr) < 0) + goto allocation_failed; + src->nBackingStores += nbr;
+ } + return true; + allocation_failed: + virReportOOMError();
Almost every libvirt memory allocation function reports the OOM error internally, so there's no need to do it here.
+ return false; +} + + +/** + * virStorageSourceSetBackingStore: + * @src: virStorageSourcePtr to hold @backingStore + * @backingStore: backingStore to store + * @pos: position of the backing store to store + * + * Set @backingStore at @pos in src->backingStores. + * If src->backingStores don't have the space to contain + * @backingStore, we expand src->backingStores + */ bool virStorageSourceSetBackingStore(virStorageSourcePtr src, virStorageSourcePtr backingStore, - size_t pos ATTRIBUTE_UNUSED) + size_t pos) { - src->backingStore = backingStore; - return !!src->backingStore; + if (!src) + return false; + if (pos >= src->nBackingStores && + !virStorageSourceExpandBackingStore(src, pos - src->nBackingStores + 1))
+ return false; + src->backingStores[pos] = backingStore; + return true;
In general virStorageSourceExpandBackingStore is almost useless. It could be folded in this function.
}
@@ -2038,6 +2089,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) void virStorageSourceBackingStoreClear(virStorageSourcePtr def) { + size_t i; + if (!def) return;
@@ -2045,8 +2098,14 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw);
/* recursively free backing chain */ - virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); - virStorageSourceSetBackingStore(def, NULL, 0); + for (i = 0; i < def->nBackingStores; ++i) + virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); + if (def->nBackingStores > 0) { + /* in this case def->backingStores is treat as an array so we have to free it*/
s/treat/treated/ ... or drop the comment
+ VIR_FREE(def->backingStores); + } + def->nBackingStores = 0; + def->backingStores = NULL;
VIR_FREE sets the pointer to NULL already.
}
In general, the code can be done simpler. I'll have to look at other patches to see how this will be used and if it makes sense. Also the commit message wording is a bit awkward. Peter

Add VIR_STORAGE_TYPE_QUORUM in virStorageType. Add VIR_STORAGE_FILE_QUORUM in virStorageFileFormat. Add threshold value in _virStorageSource Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- docs/formatdomain.html.in | 23 ++++++++++++++++++++--- docs/schemas/domaincommon.rng | 21 ++++++++++++++++++++- docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c | 2 ++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_driver.c | 4 ++++ src/qemu/qemu_migration.c | 1 + src/util/virstoragefile.c | 25 +++++++++++++++++-------- src/util/virstoragefile.h | 3 +++ 10 files changed, 70 insertions(+), 12 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e921749..7d058ec 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1878,8 +1878,9 @@ <dd> Valid values are "file", "block", "dir" (<span class="since">since 0.7.5</span>), - "network" (<span class="since">since 0.8.7</span>), or - "volume" (<span class="since">since 1.0.5</span>) + "network" (<span class="since">since 0.8.7</span>), + "volume" (<span class="since">since 1.0.5</span>), or + "quorum" (<span class="since">since 1.2.13</span>) and refer to the underlying source for the disk. </dd> <dt><code>device</code> attribute @@ -1940,6 +1941,14 @@ <code>snapshot='yes'</code> with a transient disk generally does not make sense. </dd> + <dt><code>threshold</code> attribute + <span class="since">since 1.2.13</span></dt> + <dd> + Only use with a quorum disk. + Indicate the minimum of positive vote a quorum must have to validate + a data to be write. The minimum value is "2". The number of backingStores + contain by the quorum must be superior to the threshold. + </dd> </dl> </dd> <dt><code>source</code></dt> @@ -2010,6 +2019,11 @@ 'file=/dev/disk/by-path/ip-example.com:3260-iscsi-iqn.2013-07.com.example:iscsi-pool-lun-1'). </p> </dd> + <dt><code>type='quorum'</code> + <span class="since">since 1.2.13</span></dt> + <dd> + A quorum contain no source element, but a serie of backingStores instead. + </dd> </dl> With "file", "block", and "volume", one or more optional sub-elements <code>seclabel</code>, <a href="#seclabel">described @@ -2149,7 +2163,10 @@ property, but using existing external files for snapshot or block copy operations requires the end user to pre-create the file correctly). The following attributes and sub-elements are - supported in <code>backingStore</code>: + supported in. + <span class="since">Since 1.2.15</span>. This elements is used to + describe a quorum child. + <code>backingStore</code>: <dl> <dt><code>type</code> attribute</dt> <dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 19461f5..6cd834e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1217,9 +1217,15 @@ </interleave> </define> + <define name="diskBackingStoreArray"> + <zeroOrMore> + <ref name="diskBackingStore"/> + </zeroOrMore> + </define> + <define name="diskBackingChain"> <choice> - <ref name="diskBackingStore"/> + <ref name="diskBackingStoreArray"/> <ref name="diskBackingStoreLast"/> </choice> </define> @@ -1261,9 +1267,22 @@ <ref name="diskSourceDir"/> <ref name="diskSourceNetwork"/> <ref name="diskSourceVolume"/> + <ref name="diskSourceQuorum"/> </choice> </define> + <define name="diskSourceQuorum"> + <optional> + <attribute name="type"> + <value>quorum</value> + </attribute> + <attribute name="threshold"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </define> + + <define name="diskSourceFile"> <optional> <attribute name="type"> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index 5f71b10..ba9f485 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -76,6 +76,7 @@ <value>fat</value> <value>vhd</value> <value>ploop</value> + <value>quorum</value> <ref name='storageFormatBacking'/> </choice> </define> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 7450547..a718576 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -20,6 +20,7 @@ <value>dir</value> <value>network</value> <value>netdir</value> + <value>quorum</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 09f0bca..a3a6c13 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5928,6 +5928,7 @@ virDomainDiskSourceParse(xmlNodePtr node, if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0) goto cleanup; break; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -17691,6 +17692,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, skipSeclabels); break; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 29b876e..6c40d3e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3398,6 +3398,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, goto cleanup; break; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1a98601..44f36ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13804,6 +13804,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) } break; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: @@ -13867,6 +13868,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d } break; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: @@ -13891,6 +13893,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr case VIR_STORAGE_TYPE_FILE: return 0; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: @@ -14010,6 +14013,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, } break; + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1da687c..59bfab3 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1517,6 +1517,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_QUORUM: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index f0450b5..92344f2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -54,7 +54,8 @@ VIR_ENUM_IMPL(virStorage, VIR_STORAGE_TYPE_LAST, "block", "dir", "network", - "volume") + "volume", + "quorum") VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, @@ -63,7 +64,7 @@ VIR_ENUM_IMPL(virStorageFileFormat, "cloop", "dmg", "iso", "vpc", "vdi", /* Not direct file formats, but used for various drivers */ - "fat", "vhd", "ploop", + "fat", "vhd", "ploop", "quorum", /* Formats with backing file below here */ "cow", "qcow", "qcow2", "qed", "vmdk") @@ -1892,6 +1893,7 @@ virStorageSourceCopy(const virStorageSource *src, bool backingChain) { virStorageSourcePtr ret = NULL; + size_t i; if (VIR_ALLOC(ret) < 0) return NULL; @@ -1904,6 +1906,8 @@ virStorageSourceCopy(const virStorageSource *src, ret->physical = src->physical; ret->readonly = src->readonly; ret->shared = src->shared; + ret->nBackingStores = src->nBackingStores; + ret->threshold = src->threshold; /* storage driver metadata are not copied */ ret->drv = NULL; @@ -1952,12 +1956,14 @@ virStorageSourceCopy(const virStorageSource *src, !(ret->auth = virStorageAuthDefCopy(src->auth))) goto error; - if (backingChain && virStorageSourceGetBackingStore(src, 0)) { - if (!virStorageSourceSetBackingStore(ret, - virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0), - true), - 0)) - goto error; + for (i = 0; i < src->nBackingStores; ++i) { + if (backingChain && virStorageSourceGetBackingStore(src, i)) { + if (!virStorageSourceSetBackingStore(ret, + virStorageSourceCopy(virStorageSourceGetBackingStore(src, i), + true), + 0)) + goto error; + } } return ret; @@ -2041,6 +2047,9 @@ virStorageSourceIsLocalStorage(virStorageSourcePtr src) case VIR_STORAGE_TYPE_FILE: case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: + /* A quorum is actually both, but we consider it as local + * because it keep us to add a lot of exeption in the code*/ + case VIR_STORAGE_TYPE_QUORUM: return true; case VIR_STORAGE_TYPE_NETWORK: diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 34ea6fb..61ff969 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -49,6 +49,7 @@ typedef enum { VIR_STORAGE_TYPE_DIR, VIR_STORAGE_TYPE_NETWORK, VIR_STORAGE_TYPE_VOLUME, + VIR_STORAGE_TYPE_QUORUM, VIR_STORAGE_TYPE_LAST } virStorageType; @@ -73,6 +74,7 @@ typedef enum { VIR_STORAGE_FILE_FAT, VIR_STORAGE_FILE_VHD, VIR_STORAGE_FILE_PLOOP, + VIR_STORAGE_FILE_QUORUM, /* Not a format, but a marker: all formats below this point have * libvirt support for following a backing chain */ @@ -272,6 +274,7 @@ struct _virStorageSource { /* backing chain of the storage source */ virStorageSourcePtr *backingStores; size_t nBackingStores; + size_t threshold; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; -- 2.3.5

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@outscale.com> --- src/conf/domain_conf.c | 164 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 119 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3a6c13..ec93b6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5952,20 +5952,56 @@ 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; char *type = NULL; char *format = NULL; int ret = -1; - 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) @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + xmlNodePtr cur = NULL; + + if (!virDomainDiskThresholdParse(backingStore, node)) + goto cleanup; + + for (cur = node->children; cur != NULL; cur = cur->next) { + if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if ((virDomainDiskBackingStoreParse(ctxt, + backingStore, + cur, + backingStore->nBackingStores) < 0)) { + goto cleanup; + } + } + } + goto exit; + } + if (!(source = virXPathNode("./source", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store source")); @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, } if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) goto cleanup; - if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + exit: + if (!virStorageSourceSetBackingStore(src, backingStore, pos)) goto cleanup; ret = 0; @@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, return ret; } - #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -6518,6 +6573,10 @@ 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 (virDomainDiskBackingStoreParse(ctxt, def->src, cur, + def->src->nBackingStores) < 0) + goto error; } } cur = cur->next; @@ -6541,12 +6600,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 (virStorageSourceIsEmpty(def->src) && (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; @@ -6892,9 +6958,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: @@ -17717,12 +17780,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) @@ -17730,37 +17795,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; } @@ -17830,6 +17901,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); @@ -17875,10 +17950,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; virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name); -- 2.3.5

On Thu, Apr 23, 2015 at 14:41:19 +0200, Matthias Gatto wrote:
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@outscale.com> --- src/conf/domain_conf.c | 164 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 119 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3a6c13..ec93b6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5952,20 +5952,56 @@ 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; char *type = NULL; char *format = NULL; int ret = -1;
- 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) @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; }
+ if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + xmlNodePtr cur = NULL; + + if (!virDomainDiskThresholdParse(backingStore, node)) + goto cleanup; + + for (cur = node->children; cur != NULL; cur = cur->next) { + if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if ((virDomainDiskBackingStoreParse(ctxt, + backingStore, + cur, + backingStore->nBackingStores) < 0)) { + goto cleanup; + } + } + } + goto exit; + } + if (!(source = virXPathNode("./source", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store source")); @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, }
if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) goto cleanup;
- if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + exit: + if (!virStorageSourceSetBackingStore(src, backingStore, pos)) goto cleanup; ret = 0;
@@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, return ret; }
- #define VENDOR_LEN 8 #define PRODUCT_LEN 16
@@ -6518,6 +6573,10 @@ 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 (virDomainDiskBackingStoreParse(ctxt, def->src, cur, + def->src->nBackingStores) < 0) + goto error; } } cur = cur->next; @@ -6541,12 +6600,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 (virStorageSourceIsEmpty(def->src) && (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; @@ -6892,9 +6958,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: @@ -17717,12 +17780,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) @@ -17730,37 +17795,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);
So now this change is wrong. Every single backing store element of the driver will have the same index, so you will not be able to reference them uniquely later. The indexes were designed in such way that they can be used to address backing chain elements (as node names in qemu) so you cannot make them pointless. Note that there's a lot of functions that use the information especially in the block job code that will need to be fixed. Additionally you'll need to fix the block job code in such way that it will work correctly with the quorums. Also note that a lot of places in the code don't properly implement transformations of the backing chain after block operations finish and resort to re-detection of the backing chain. That obviously won't work with quorums since they are not exposed in the backing chain (afaik). This means that basically for a proper implementation of this you'll need either to fix all code that alters the backing chains so that it works properly and then implement quorums on top of that or block basically every operation that transforms the backing chain so that the internal state does not get corrupted. Additionally this will probably also require modifying the storage handling code in libvirt in such way that it will handle the backing store information internally and not try to reload the state from the metadata in the image. Peter

On Tue, May 12, 2015 at 5:04 PM, Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Apr 23, 2015 at 14:41:19 +0200, Matthias Gatto wrote:
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@outscale.com> --- src/conf/domain_conf.c | 164 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 119 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3a6c13..ec93b6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5952,20 +5952,56 @@ 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; char *type = NULL; char *format = NULL; int ret = -1;
- 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) @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; }
+ if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + xmlNodePtr cur = NULL; + + if (!virDomainDiskThresholdParse(backingStore, node)) + goto cleanup; + + for (cur = node->children; cur != NULL; cur = cur->next) { + if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if ((virDomainDiskBackingStoreParse(ctxt, + backingStore, + cur, + backingStore->nBackingStores) < 0)) { + goto cleanup; + } + } + } + goto exit; + } + if (!(source = virXPathNode("./source", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store source")); @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, }
if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) goto cleanup;
- if (!virStorageSourceSetBackingStore(src, backingStore, 0)) + exit: + if (!virStorageSourceSetBackingStore(src, backingStore, pos)) goto cleanup; ret = 0;
@@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, return ret; }
- #define VENDOR_LEN 8 #define PRODUCT_LEN 16
@@ -6518,6 +6573,10 @@ 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 (virDomainDiskBackingStoreParse(ctxt, def->src, cur, + def->src->nBackingStores) < 0) + goto error; } } cur = cur->next; @@ -6541,12 +6600,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 (virStorageSourceIsEmpty(def->src) && (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; @@ -6892,9 +6958,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: @@ -17717,12 +17780,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) @@ -17730,37 +17795,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);
So now this change is wrong. Every single backing store element of the driver will have the same index, so you will not be able to reference them uniquely later. The indexes were designed in such way that they can be used to address backing chain elements (as node names in qemu) so you cannot make them pointless.
Note that there's a lot of functions that use the information especially in the block job code that will need to be fixed.
Additionally you'll need to fix the block job code in such way that it will work correctly with the quorums.
Also note that a lot of places in the code don't properly implement transformations of the backing chain after block operations finish and resort to re-detection of the backing chain. That obviously won't work with quorums since they are not exposed in the backing chain (afaik).
This means that basically for a proper implementation of this you'll need either to fix all code that alters the backing chains so that it works properly and then implement quorums on top of that or block basically every operation that transforms the backing chain so that the internal state does not get corrupted. Additionally this will probably also require modifying the storage handling code in libvirt in such way that it will handle the backing store information internally and not try to reload the state from the metadata in the image.
Peter
Ok, I'm working on fixing the backing chains.

Allow to libvirt to build the quorum string used by quemu. Add 2 static functions: qemuBuildQuorumStr and qemuBuildAndAppendDriveStrToVirBuffer. qemuBuildQuorumStr is made because a quorum can have another quorum as a child, so we may need to call qemuBuildQuorumStr recursively. qemuBuildQuorumFileSourceStr was basically made to share the code use to build the source between qemuBuildQuorumStr and qemuBuildDriveStr, but there is some difference betwin the syntax use by libvirt to declare a disk and the one qemu need to build a quorum: a quorum need a syntaxe like: "domaine_name.children.X.file.filename=filename" where libvirt don't use "file.filename=" but directly "file=". Therfore I use this function only for quorum. But as explained in the cover letter and here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html We miss some informations in _virStorageSource to have a complet quorum support in libvirt. Ideally I'd like to refactore virDomainDiskDefFormat to allow qemuBuildQuorumStr to call this function in a loop. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/qemu/qemu_command.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c40d3e..80cbb7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) return -1; } +static bool +qemuBuildQuorumFileSourceStr(virConnectPtr conn, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ + char *source = NULL; + int actualType = virStorageSourceGetActualType(src); + + if (qemuGetDriveSourceString(src, conn, &source) < 0) + goto error; + + if (source) { + + virBufferAsprintf(opt, ",%sfilename=", toAppend); + + switch (actualType) { + case VIR_STORAGE_TYPE_DIR: + /* QEMU only supports magic FAT format for now */ + if (src->format > 0 && + src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(src->format)); + goto error; + } + + if (!src->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto error; + } + + virBufferAddLit(opt, "fat:"); + + break; + + default: + break; + } + virBufferAsprintf(opt, "%s", source); + } + + return true; + error: + return false; +} + + +static bool +qemuBuildQuorumStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ + char *tmp = NULL; + int ret; + virStorageSourcePtr backingStore; + size_t i; + + if (!src->threshold) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("threshold missing in the quorum configuration")); + return false; + } + if (src->nBackingStores < 2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("a quorum must have at last 2 children")); + return false; + } + if (src->threshold > src->nBackingStores) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("threshold must not exceed the number of childrens")); + return false; + } + virBufferAsprintf(opt, ",%svote-threshold=%lu", + toAppend, src->threshold); + for (i = 0; i < src->nBackingStores; ++i) { + backingStore = virStorageSourceGetBackingStore(src, i); + ret = virAsprintf(&tmp, "%schildren.%lu.file.", toAppend, i); + if (ret < 0) + return false; + + virBufferAsprintf(opt, ",%schildren.%lu.driver=%s", + toAppend, i, + virStorageFileFormatTypeToString(backingStore->format)); + + if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == false) + goto error; + + /* This operation avoid me to made another copy */ + tmp[ret - sizeof("file")] = '\0'; + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp)) + goto error; + } + VIR_FREE(tmp); + } + return true; + error: + VIR_FREE(tmp); + return false; +} + /* Qemu 1.2 and later have a binary flag -enable-fips that must be * used for VNC auth to obey FIPS settings; but the flag only @@ -3952,6 +4057,11 @@ qemuBuildDriveStr(virConnectPtr conn, disk->blkdeviotune.size_iops_sec); } + if (actualType == VIR_STORAGE_TYPE_QUORUM) { + if (!qemuBuildQuorumStr(conn, disk, disk->src, &opt, "")) + goto error; + } + if (virBufferCheckError(&opt) < 0) goto error; -- 2.3.5

On Thu, Apr 23, 2015 at 14:41:20 +0200, Matthias Gatto wrote:
Allow to libvirt to build the quorum string used by quemu.
Add 2 static functions: qemuBuildQuorumStr and qemuBuildAndAppendDriveStrToVirBuffer. qemuBuildQuorumStr is made because a quorum can have another quorum as a child, so we may need to call qemuBuildQuorumStr recursively.
qemuBuildQuorumFileSourceStr was basically made to share the code use to build the source between qemuBuildQuorumStr and qemuBuildDriveStr, but there is some difference betwin the syntax use by libvirt to declare a disk and the one qemu need to build a quorum: a quorum need a syntaxe like: "domaine_name.children.X.file.filename=filename" where libvirt don't use "file.filename=" but directly "file=". Therfore I use this function only for quorum.
But as explained in the cover letter and here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html We miss some informations in _virStorageSource to have a complet quorum support in libvirt. Ideally I'd like to refactore virDomainDiskDefFormat to allow qemuBuildQuorumStr to call this function in a loop.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/qemu/qemu_command.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)
...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c40d3e..80cbb7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) return -1; }
+static bool +qemuBuildQuorumFileSourceStr(virConnectPtr conn, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ + char *source = NULL; + int actualType = virStorageSourceGetActualType(src); + + if (qemuGetDriveSourceString(src, conn, &source) < 0) + goto error; + + if (source) { + + virBufferAsprintf(opt, ",%sfilename=", toAppend);
virBufferStrcat
+ + switch (actualType) { + case VIR_STORAGE_TYPE_DIR:
I'd forbid the DIR type altogether with quorums.
+ /* QEMU only supports magic FAT format for now */ + if (src->format > 0 && + src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(src->format)); + goto error; + } + + if (!src->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto error; + } + + virBufferAddLit(opt, "fat:"); + + break; + + default: + break; + } + virBufferAsprintf(opt, "%s", source);
virBufferAdd
+ } + + return true; + error: + return false;
Error can be returned right away since there is nothing to clean up.
+} + + +static bool +qemuBuildQuorumStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ + char *tmp = NULL; + int ret; + virStorageSourcePtr backingStore; + size_t i; + + if (!src->threshold) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("threshold missing in the quorum configuration")); + return false; + } + if (src->nBackingStores < 2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("a quorum must have at last 2 children")); + return false; + } + if (src->threshold > src->nBackingStores) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("threshold must not exceed the number of childrens"));
'children' is the proper plural
+ return false; + } + virBufferAsprintf(opt, ",%svote-threshold=%lu", + toAppend, src->threshold); + for (i = 0; i < src->nBackingStores; ++i) { + backingStore = virStorageSourceGetBackingStore(src, i); + ret = virAsprintf(&tmp, "%schildren.%lu.file.", toAppend, i); + if (ret < 0) + return false; + + virBufferAsprintf(opt, ",%schildren.%lu.driver=%s", + toAppend, i, + virStorageFileFormatTypeToString(backingStore->format)); + + if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == false) + goto error; + + /* This operation avoid me to made another copy */ + tmp[ret - sizeof("file")] = '\0'; + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp)) + goto error;
This won't work if the quorum would be after an intermediate layer. That's why this needs the internal backing chain tracking.
+ } + VIR_FREE(tmp); + } + return true; + error: + VIR_FREE(tmp); + return false; +} +
/* Qemu 1.2 and later have a binary flag -enable-fips that must be * used for VNC auth to obey FIPS settings; but the flag only @@ -3952,6 +4057,11 @@ qemuBuildDriveStr(virConnectPtr conn, disk->blkdeviotune.size_iops_sec); }
+ if (actualType == VIR_STORAGE_TYPE_QUORUM) { + if (!qemuBuildQuorumStr(conn, disk, disk->src, &opt, "")) + goto error; + }
This is rather ugly. Since qemuBuildDriveStr skips the quorum storage sources rather accidentally as they appear "empty" to the virStorageSourceIsEmpty function. If you want/need to have a different function you should make it explicit how the code is formatting the source. Additionally once the quorum disk won't be on top, sice you for example created a snapshot on top of the quorum unified device, then this code will terribly break since qemuBuildQuorumStr will never be called. As I've explained in a previous patch, quorums will need libvirt to do the houskeeping of the whole backing chain internally since it cannot be recreated that easily. Peter

On Tue, May 12, 2015 at 5:38 PM, Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Apr 23, 2015 at 14:41:20 +0200, Matthias Gatto wrote:
Allow to libvirt to build the quorum string used by quemu.
Add 2 static functions: qemuBuildQuorumStr and qemuBuildAndAppendDriveStrToVirBuffer. qemuBuildQuorumStr is made because a quorum can have another quorum as a child, so we may need to call qemuBuildQuorumStr recursively.
qemuBuildQuorumFileSourceStr was basically made to share the code use to build the source between qemuBuildQuorumStr and qemuBuildDriveStr, but there is some difference betwin the syntax use by libvirt to declare a disk and the one qemu need to build a quorum: a quorum need a syntaxe like: "domaine_name.children.X.file.filename=filename" where libvirt don't use "file.filename=" but directly "file=". Therfore I use this function only for quorum.
But as explained in the cover letter and here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html We miss some informations in _virStorageSource to have a complet quorum support in libvirt. Ideally I'd like to refactore virDomainDiskDefFormat to allow qemuBuildQuorumStr to call this function in a loop.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/qemu/qemu_command.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)
...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c40d3e..80cbb7d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) return -1; }
+static bool +qemuBuildQuorumFileSourceStr(virConnectPtr conn, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ + char *source = NULL; + int actualType = virStorageSourceGetActualType(src); + + if (qemuGetDriveSourceString(src, conn, &source) < 0) + goto error; + + if (source) { + + virBufferAsprintf(opt, ",%sfilename=", toAppend);
virBufferStrcat
+ + switch (actualType) { + case VIR_STORAGE_TYPE_DIR:
I'd forbid the DIR type altogether with quorums.
+ /* QEMU only supports magic FAT format for now */ + if (src->format > 0 && + src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(src->format)); + goto error; + } + + if (!src->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto error; + } + + virBufferAddLit(opt, "fat:"); + + break; + + default: + break; + } + virBufferAsprintf(opt, "%s", source);
virBufferAdd
+ } + + return true; + error: + return false;
Error can be returned right away since there is nothing to clean up.
+} + + +static bool +qemuBuildQuorumStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virBuffer *opt, + const char *toAppend) +{ + char *tmp = NULL; + int ret; + virStorageSourcePtr backingStore; + size_t i; + + if (!src->threshold) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("threshold missing in the quorum configuration")); + return false; + } + if (src->nBackingStores < 2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("a quorum must have at last 2 children")); + return false; + } + if (src->threshold > src->nBackingStores) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("threshold must not exceed the number of childrens"));
'children' is the proper plural
+ return false; + } + virBufferAsprintf(opt, ",%svote-threshold=%lu", + toAppend, src->threshold); + for (i = 0; i < src->nBackingStores; ++i) { + backingStore = virStorageSourceGetBackingStore(src, i); + ret = virAsprintf(&tmp, "%schildren.%lu.file.", toAppend, i); + if (ret < 0) + return false; + + virBufferAsprintf(opt, ",%schildren.%lu.driver=%s", + toAppend, i, + virStorageFileFormatTypeToString(backingStore->format)); + + if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == false) + goto error; + + /* This operation avoid me to made another copy */ + tmp[ret - sizeof("file")] = '\0'; + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp)) + goto error;
This won't work if the quorum would be after an intermediate layer. That's why this needs the internal backing chain tracking.
+ } + VIR_FREE(tmp); + } + return true; + error: + VIR_FREE(tmp); + return false; +} +
/* Qemu 1.2 and later have a binary flag -enable-fips that must be * used for VNC auth to obey FIPS settings; but the flag only @@ -3952,6 +4057,11 @@ qemuBuildDriveStr(virConnectPtr conn, disk->blkdeviotune.size_iops_sec); }
+ if (actualType == VIR_STORAGE_TYPE_QUORUM) { + if (!qemuBuildQuorumStr(conn, disk, disk->src, &opt, "")) + goto error; + }
This is rather ugly. Since qemuBuildDriveStr skips the quorum storage sources rather accidentally as they appear "empty" to the virStorageSourceIsEmpty function. If you want/need to have a different function you should make it explicit how the code is formatting the source.
Additionally once the quorum disk won't be on top, sice you for example created a snapshot on top of the quorum unified device, then this code will terribly break since qemuBuildQuorumStr will never be called.
As I've explained in a previous patch, quorums will need libvirt to do the houskeeping of the whole backing chain internally since it cannot be recreated that easily.
Peter
I don't really understand what you mean by: "quorums will need libvirt to do the houskeeping of the whole backing chain internally" Libvirt, already keep information about the backing chain, do you think that we need function, which simplify the manipulation of the backing chain, like function allowing to get/set/modify a backingStore taking in parameter an index and the virDomainDiskDefPtr of the quorum ? Do you think that we miss information for the backingStore ? Matthias

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@outscale.com> --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_command.c | 3 +++ src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 1 + 6 files changed, 47 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7d058ec..d9afe36 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2183,6 +2183,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/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6cd834e..30694d9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1236,6 +1236,11 @@ <ref name="positiveInteger"/> </attribute> <interleave> + <optional> + <attribute name="nodename"> + <text/> + </attribute> + </optional> <ref name="diskSource"/> <ref name="diskBackingChain"/> <ref name="diskFormat"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ec93b6f..46eeae2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -54,6 +54,7 @@ #include "network_conf.h" #include "virtpm.h" #include "virstring.h" +#include "virrandom.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -5978,6 +5979,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, @@ -6020,6 +6023,26 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (src->type == VIR_STORAGE_TYPE_QUORUM) { + char *nodeName = NULL; + + if ((nodeName = virXMLPropString(ctxt->node, "nodename"))) { + backingStore->nodeName = nodeName; + } else { + size_t len; + + if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN) < 0) + goto cleanup; + if (snprintf(nodeName, GEN_NODE_NAME_MAX_LEN, + "%s%08x", GEN_NODE_NAME_PREFIX, (int)pos) < 0) + goto cleanup; + for (len = strlen(nodeName); len < GEN_NODE_NAME_MAX_LEN - 1; ++len) + 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")); @@ -6075,6 +6098,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 @@ -17817,6 +17842,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 80cbb7d..ac09750 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3567,6 +3567,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 92344f2..3536513 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1956,6 +1956,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, @@ -2105,6 +2108,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 61ff969..f8f64c3 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -275,6 +275,7 @@ struct _virStorageSource { virStorageSourcePtr *backingStores; size_t nBackingStores; size_t threshold; + char *nodeName; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; -- 2.3.5

On Thu, Apr 23, 2015 at 14:41:21 +0200, Matthias Gatto wrote:
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@outscale.com> ---
Once we decide that we want to deal with node names (which we definitely should do soon we will need to take a different approach compared to this patch:
docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_command.c | 3 +++ src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 1 + 6 files changed, 47 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7d058ec..d9afe36 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2183,6 +2183,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.
We certainly don't want to give the user the need to specify node names. In fact I think libvirt shouldn't expose node names in any way. The implementation should remain internal and users will interact via the backing chain 'index' element: 'This attribute is only valid in output (and ignored on input) and it can be used to refer to a specific part of the disk chain when doing block operations (such as via the virDomainBlockRebase API). For example, vda[2] refers to the backing store with index='2' of the disk with vda target.' Once we do this we should specify a node name for every backing chain element or possibly re-detect it after qemu starts and store the backing chain info internally. This will be necessary as libvirt has to model the operations with the backing chain the same way as qemu is doing it so that libvirt can ensure that qemu is not accessing files that it should not access. At any rate, node names are a very useful concept, but this patch would be a step in the wrong direction. Peter

On Thu, Apr 23, 2015 at 2:41 PM, Matthias Gatto <matthias.gatto@outscale.com> wrote:
The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html
This feature ask for 6 task:
1) Allow a _virStorageSource to contain more than one backing store.
Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation.
2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums.
3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum
4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child.
5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet.
6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs.
There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add.
2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice
3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one)
V2: -Rebase on master -Add Documentation
V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size.
V4: -Rebase on master
V5: -Rebase on master -patch 1-4/9: use patchs from John Ferlan -patch 4/9: check return of virStorageSourceSetBackingStore -patch 5/9: report type of error on virStorageSourceSetBackingStore
Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name
docs/formatdomain.html.in | 30 +++++- docs/schemas/domaincommon.rng | 26 ++++- docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c | 195 ++++++++++++++++++++++++++-------- src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++ src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 39 ++++--- src/qemu/qemu_migration.c | 1 + 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 | 11 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 3 +- src/util/virstoragefile.c | 122 ++++++++++++++++++--- src/util/virstoragefile.h | 13 ++- tests/virstoragetest.c | 18 ++-- 24 files changed, 551 insertions(+), 141 deletions(-)
-- 2.3.5
ping
participants (3)
-
John Ferlan
-
Matthias Gatto
-
Peter Krempa