[libvirt] [PATCH 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. Therefore we have to treat the field virStorageSourcePtr backingStores as an array instead of a pointer. But doing that, most of the backingStore field would be an array contening only one element. So I've decide to allocate the array only if there is more than 1 backing store in a _virStorageSource. 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) 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: Treat backingStore as a pointer or an array virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name src/conf/domain_conf.c | 193 ++++++++++++++++++++++++++-------- src/conf/storage_conf.c | 7 +- src/libvirt_private.syms | 3 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++ src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 20 ++-- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 +-- src/storage/storage_backend_fs.c | 12 +-- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 136 +++++++++++++++++++++--- src/util/virstoragefile.h | 12 +++ tests/virstoragetest.c | 18 ++-- 17 files changed, 445 insertions(+), 103 deletions(-) -- 1.8.3.1
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 1853a9c..12dd399 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1992,6 +1992,7 @@ virStorageSourceClear; virStorageSourceCopy; virStorageSourceFree; virStorageSourceGetActualType; +virStorageSourceGetBackingStore; virStorageSourceGetSecurityLabelDef; virStorageSourceInitChainElement; virStorageSourceIsEmpty; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c424251..ebcf7c3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1814,6 +1814,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 e05b843..fe07dde 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -288,6 +288,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, + size_t pos); + int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); int virStorageFileProbeFormatFromBuf(const char *path, char *buf, -- 1.8.3.1
Uniformize backing store usage by calling virStorageSourceGetBackingStore instead of setting backing store manually. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/conf/domain_conf.c | 7 ++++--- src/conf/storage_conf.c | 4 ++-- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 ++++++------ src/storage/storage_backend_fs.c | 8 ++++---- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 20 ++++++++++---------- tests/virstoragetest.c | 14 +++++++------- 13 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2d81c37..b790fc5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16473,7 +16473,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; @@ -16595,7 +16595,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_XML_INACTIVE) && - virDomainDiskBackingStoreFormat(buf, def->src->backingStore, + virDomainDiskBackingStoreFormat(buf, + virStorageSourceGetBackingStore(def->src, 0), def->src->backingStoreRaw, 1) < 0) return -1; @@ -20554,7 +20555,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 3987470..a8a90b4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1642,9 +1642,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 0e94cae..f878f4b 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -120,7 +120,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; @@ -138,7 +138,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 220304f..9157e4d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2708,7 +2708,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 9152cf5..9ed5035 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13496,13 +13496,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); } @@ -15637,7 +15637,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto endjob; } - if (virStorageFileGetRelativeBackingPath(disk->src->backingStore, + if (virStorageFileGetRelativeBackingPath(virStorageSourceGetBackingStore(disk->src, 0), baseSource, &backingPath) < 0) goto endjob; @@ -16359,7 +16359,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); @@ -16367,14 +16367,14 @@ qemuDomainBlockCommit(virDomainPtr dom, } if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) - baseSource = topSource->backingStore; + baseSource = virStorageSourceGetBackingStore(topSource, 0); else if (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 || !(baseSource = virStorageFileChainLookup(disk->src, topSource, base, baseIndex, NULL))) goto endjob; if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) && - baseSource != topSource->backingStore) { + baseSource != virStorageSourceGetBackingStore(topSource, 0)) { virReportError(VIR_ERR_INVALID_ARG, _("base '%s' is not immediately below '%s' in chain " "for '%s'"), diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..ffff9d7 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -374,7 +374,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 f96be50..b6f206f 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1140,7 +1140,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 readoly/shared disks, because other VMs may @@ -1270,7 +1270,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 869bf18..3bd82bb 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -941,7 +941,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)) { bool probe = ctl->allowDiskFormatProbing; virStorageFileGetMetadata(disk->src, -1, -1, probe, false); } diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b990a82..266a59f 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -873,7 +873,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, } - if (vol->target.backingStore) { + if (virStorageSourceGetBackingStore(&(vol->target), 0)) { int accessRetCode = -1; char *absolutePath = NULL; @@ -890,7 +890,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, * backing store, not really sure what use it serves though, and it * may cause issues with lvm. Untested essentially. */ - if (inputvol && inputvol->target.backingStore && + if (inputvol && virStorageSourceGetBackingStore(&(inputvol->target), 0) && STRNEQ_NULLABLE(inputvol->target.backingStore->path, vol->target.backingStore->path)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -959,7 +959,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, cmd = virCommandNew(create_tool); convert = !!inputvol; - backing = !inputvol && vol->target.backingStore; + backing = !inputvol && virStorageSourceGetBackingStore(&(vol->target), 0); if (convert) virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, NULL); @@ -1086,7 +1086,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED, vol->target.format); return -1; } - if (vol->target.backingStore != NULL) { + if (virStorageSourceGetBackingStore(&(vol->target), 0) != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("copy-on-write image not supported with " "qcow-create")); @@ -1491,8 +1491,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, openflags)) < 0) return ret; - if (vol->target.backingStore && - (ret = virStorageBackendUpdateVolTargetInfo(vol->target.backingStore, + if (virStorageSourceGetBackingStore(&(vol->target), 0) && + (ret = virStorageBackendUpdateVolTargetInfo(virStorageSourceGetBackingStore(&(vol->target), 0), updateCapacity, withBlockVolFormat, VIR_STORAGE_VOL_OPEN_DEFAULT | diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 34f2153..d7bd1fb 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -104,8 +104,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, /* 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(virStorageSourceGetBackingStore(target, 0))) { + virStorageSourceFree(virStorageSourceGetBackingStore(target, 0)); if (VIR_ALLOC(target->backingStore) < 0) goto cleanup; @@ -900,8 +900,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, if (vol->target.format == VIR_STORAGE_FILE_DIR) vol->type = VIR_STORAGE_VOL_DIR; - if (vol->target.backingStore) { - ignore_value(virStorageBackendUpdateVolTargetInfo(vol->target.backingStore, + if (virStorageSourceGetBackingStore(&(vol->target), 0)) { + ignore_value(virStorageBackendUpdateVolTargetInfo(virStorageSourceGetBackingStore(&(vol->target), 0), true, false, VIR_STORAGE_VOL_OPEN_DEFAULT)); /* If this failed, the backing file is currently unavailable, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4959985..300c990 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -763,7 +763,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, } virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity, 1024)); - if (vol->target.backingStore) + if (virStorageSourceGetBackingStore(&(vol->target), 0)) virCommandAddArgList(cmd, "-s", vol->target.backingStore->path, NULL); else virCommandAddArg(cmd, pool->def->source.name); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ebcf7c3..fd0e68c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1079,10 +1079,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; @@ -1350,8 +1350,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain, *parent = NULL; if (startFrom) { - while (chain && chain != startFrom->backingStore) { - chain = chain->backingStore; + while (chain && chain != virStorageSourceGetBackingStore(startFrom, 0)) { + chain = virStorageSourceGetBackingStore(chain, 0); i++; } *parent = startFrom; @@ -1359,7 +1359,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); @@ -1394,7 +1394,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain, } } *parent = chain; - chain = chain->backingStore; + chain = virStorageSourceGetBackingStore(chain, 0); i++; } @@ -1895,8 +1895,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; } @@ -2847,7 +2847,7 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, *relpath = NULL; - for (next = top; next; next = next->backingStore) { + for (next = top; next; next = virStorageSourceGetBackingStore(next, 0)) { if (!next->relPath) { ret = 1; goto cleanup; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2601edc..12670da 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -403,7 +403,7 @@ testStorageChain(const void *args) } VIR_FREE(expect); VIR_FREE(actual); - elt = elt->backingStore; + elt = virStorageSourceGetBackingStore(elt, 0); i++; } if (i != data->nfiles) { @@ -1029,8 +1029,8 @@ mymain(void) ret = -1; goto cleanup; } - chain2 = chain->backingStore; - chain3 = chain2->backingStore; + chain2 = virStorageSourceGetBackingStore(chain, 0); + chain3 = virStorageSourceGetBackingStore(chain2, 0); #define TEST_LOOKUP_TARGET(id, target, from, name, index, result, \ meta, parent) \ @@ -1095,8 +1095,8 @@ mymain(void) ret = -1; goto cleanup; } - chain2 = chain->backingStore; - chain3 = chain2->backingStore; + chain2 = virStorageSourceGetBackingStore(chain, 0); + chain3 = virStorageSourceGetBackingStore(chain2, 0); TEST_LOOKUP(28, NULL, "bogus", NULL, NULL, NULL); TEST_LOOKUP(29, chain, "bogus", NULL, NULL, NULL); @@ -1142,8 +1142,8 @@ mymain(void) ret = -1; goto cleanup; } - chain2 = chain->backingStore; - chain3 = chain2->backingStore; + chain2 = virStorageSourceGetBackingStore(chain, 0); + chain3 = virStorageSourceGetBackingStore(chain2, 0); TEST_LOOKUP(56, NULL, "bogus", NULL, NULL, NULL); TEST_LOOKUP(57, NULL, "sub/link2", chain->path, chain, NULL); -- 1.8.3.1
On 08.12.2014 19:31, Matthias Gatto wrote:
Uniformize backing store usage by calling virStorageSourceGetBackingStore instead of setting backing store manually.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/conf/domain_conf.c | 7 ++++--- src/conf/storage_conf.c | 4 ++-- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++------ src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 ++-- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 ++++++------ src/storage/storage_backend_fs.c | 8 ++++---- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 20 ++++++++++---------- tests/virstoragetest.c | 14 +++++++------- 13 files changed, 47 insertions(+), 46 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 869bf18..3bd82bb 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -941,7 +941,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)) {
s/disk->src/disk->src, 0/
bool probe = ctl->allowDiskFormatProbing; virStorageFileGetMetadata(disk->src, -1, -1, probe, false); }
Michal
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> --- 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 12dd399..dd83518 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2002,6 +2002,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fd0e68c..88b0ce6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1822,6 +1822,16 @@ virStorageSourceGetBackingStore(const virStorageSource *src, } +virStorageSourcePtr +virStorageSourceSetBackingStore(virStorageSourcePtr src, + virStorageSourcePtr backingStore, + size_t pos ATTRIBUTE_UNUSED) +{ + src->backingStore = backingStore; + return src->backingStore; +} + + /** * virStorageSourcePtr: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index fe07dde..4cba66b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -288,6 +288,9 @@ struct _virStorageSource { # define DEV_BSIZE 512 # endif +virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, + virStorageSourcePtr backingStore, + size_t pos); virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos); -- 1.8.3.1
Replace the parts of the code where a backing store is set manually with virStorageSourceSetBackingStore Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/conf/domain_conf.c | 2 +- src/qemu/qemu_domain.c | 1 - src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 8 +++++--- tests/virstoragetest.c | 4 ++-- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b790fc5..00e0470 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5523,7 +5523,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) goto cleanup; - src->backingStore = backingStore; + virStorageSourceSetBackingStore(src, backingStore, 0); ret = 0; cleanup: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9157e4d..458ceb0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2534,7 +2534,6 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int ret = -1; size_t i; - VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { size_t idx = i - 1; virDomainDiskDefPtr disk = vm->def->disks[idx]; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9ed5035..8e3a492 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13497,13 +13497,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, /* Update vm in place to match changes. */ tmp = disk->src; disk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + virStorageSourceSetBackingStore(tmp, NULL, 0); virStorageSourceFree(tmp); if (persistDisk) { tmp = persistDisk->src; persistDisk->src = virStorageSourceGetBackingStore(tmp, 0); - tmp->backingStore = NULL; + virStorageSourceSetBackingStore(tmp, NULL, 0); virStorageSourceFree(tmp); } } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d7bd1fb..07fd6ee 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -96,7 +96,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup; if (meta->backingStoreRaw) { - if (!(target->backingStore = virStorageSourceNewFromBacking(meta))) + if (!virStorageSourceSetBackingStore(target, virStorageSourceNewFromBacking(meta), 0)) goto cleanup; target->backingStore->format = backingStoreFormat; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 66dc994..eeb2a4c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2899,7 +2899,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; } - src->backingStore = backingStore; + virStorageSourceSetBackingStore(src, backingStore, 0); backingStore = NULL; ret = 0; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 88b0ce6..20f45f3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1906,8 +1906,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 12670da..b7820d4 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -583,9 +583,9 @@ testPathRelativePrepare(void) for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) { if (i < ARRAY_CARDINALITY(backingchain) - 1) - backingchain[i].backingStore = &backingchain[i + 1]; + virStorageSourceSetBackingStore(&backingchain[i], &backingchain[i + 1], 0); else - backingchain[i].backingStore = NULL; + virStorageSourceSetBackingStore(&backingchain[i], NULL, 0); backingchain[i].relPath = NULL; } -- 1.8.3.1
On 08.12.2014 19:31, 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> --- src/conf/domain_conf.c | 2 +- src/qemu/qemu_domain.c | 1 - src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 8 +++++--- tests/virstoragetest.c | 4 ++-- 7 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b790fc5..00e0470 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5523,7 +5523,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) goto cleanup;
- src->backingStore = backingStore; + virStorageSourceSetBackingStore(src, backingStore, 0); ret = 0;
cleanup: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9157e4d..458ceb0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2534,7 +2534,6 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int ret = -1; size_t i;
- VIR_DEBUG("Checking for disk presence");
This is rather unrelated.
for (i = vm->def->ndisks; i > 0; i--) { size_t idx = i - 1; virDomainDiskDefPtr disk = vm->def->disks[idx];
Michal
As explain in the former patchs, backingStore can be treat an array or a pointer. If we have only one backingStore we have to use it as a normal ptr but if there is more backing store, we use it as a pointer's array. Because it would be complicated to expend backingStore manually, and do the convertion from a pointer to an array, I've created the virStorageSourcePushBackingStore function to help. This function allocate an array of pointer only if there is more than 1 bs. Because we can not remove a child from a quorum, i didn't create the function virStorageSourcePopBackingStore. I've changed virStorageSourceBackingStoreClear, virStorageSourceSetBackingStore and virStorageSourceGetBackingStore to handle the case where backingStore is an array. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/conf/storage_conf.c | 3 +- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 87 ++++++++++++++++++++++++++++++++--- src/util/virstoragefile.h | 2 + 6 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a8a90b4..fc51d47 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1340,8 +1340,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(ret->target.backingStore) < 0) + if (VIR_ALLOC_N(ret->target.backingStore, 1) < 0) goto error; + ret->target.nBackingStores = 1; ret->target.backingStore->path = backingStore; backingStore = NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dd83518..71e8625 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2002,6 +2002,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourcePushBackingStore; virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 07fd6ee..979abd3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -107,7 +107,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (!virStorageSourceIsLocalStorage(virStorageSourceGetBackingStore(target, 0))) { virStorageSourceFree(virStorageSourceGetBackingStore(target, 0)); - if (VIR_ALLOC(target->backingStore) < 0) + if (virStorageSourcePushBackingStore(target) == false) goto cleanup; target->backingStore->type = VIR_STORAGE_TYPE_NETWORK; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 300c990..edec124 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -148,7 +148,7 @@ virStorageBackendLogicalMakeVol(char **const groups, * lv is created with "--virtualsize"). */ if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (virStorageSourcePushBackingStore(&vol->target) == false) goto cleanup; if (virAsprintf(&vol->target.backingStore->path, "%s/%s", diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 20f45f3..4217a80 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1814,21 +1814,86 @@ 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->backingStore || (pos > 1 && pos >= src->nBackingStores)) + return NULL; + if (src->nBackingStores < 2) + return src->backingStore; + return ((virStorageSourcePtr *)src->backingStore)[pos]; } +/** + * virStorageSourcePushBackingStore: + * @src: virStorageSourcePtr to allocate the new backing store + * + * Allocate size for a new backing store in src->backingStore + * and update src->nBackingStores + * If we have less than 2 backing stores, we treat src->backingStore + * as a pointer otherwise we treat it as an array of virStorageSourcePtr + */ +bool +virStorageSourcePushBackingStore(virStorageSourcePtr src) +{ + virStorageSourcePtr tmp; + virStorageSourcePtr *tmp2; + + if (src->nBackingStores == 1) { +/* If we need more than one backing store we need an array + * Because we don't want to lose our data from the old Backing Store + * we copy the pointer from src->backingStore to src->backingStore[0] */ + tmp = src->backingStore; + if (VIR_ALLOC_N(tmp2, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + src->nBackingStores += 1; + virStorageSourceSetBackingStore(src, tmp, 0); + } else if (src->nBackingStores > 1) { + tmp2 = ((virStorageSourcePtr *)src->backingStore); + if (VIR_EXPAND_N(tmp2, src->nBackingStores, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + } else { +/* Most of the time we use only one backingStore + * So we don't need to allocate an array */ + src->nBackingStores += 1; + } + return true; +} + + +/** + * virStorageSourceSetBackingStore: + * @src: virStorageSourcePtr to hold @backingStore + * @backingStore: backingStore to store + * @pos: position of the backing store to store + * + * Set @backingStore at @pos in src->backingStore + */ virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, virStorageSourcePtr backingStore, - size_t pos ATTRIBUTE_UNUSED) + size_t pos) { - src->backingStore = backingStore; - return src->backingStore; + if (!src || (pos > 1 && pos >= src->nBackingStores)) + goto error; + if (src->nBackingStores > 1) { + ((virStorageSourcePtr *)src->backingStore)[pos] = backingStore; + } else { + src->backingStore = backingStore; + } + return backingStore; + error: + return NULL; } @@ -2038,6 +2103,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) void virStorageSourceBackingStoreClear(virStorageSourcePtr def) { + size_t i; + if (!def) return; @@ -2045,7 +2112,13 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); + for (i = 0; i < def->nBackingStores; ++i) + virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); + if (def->nBackingStores > 1) { + /* in this case def->backingStores is treat as an array so we have to free it*/ + VIR_FREE(def->backingStore); + } + def->nBackingStores = 0; virStorageSourceSetBackingStore(def, NULL, 0); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4cba66b..d28138f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -270,6 +270,7 @@ struct _virStorageSource { /* backing chain of the storage source */ virStorageSourcePtr backingStore; + size_t nBackingStores; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; @@ -293,6 +294,7 @@ virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, size_t pos); virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos); +bool virStorageSourcePushBackingStore(virStorageSourcePtr src); int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid); int virStorageFileProbeFormatFromBuf(const char *path, -- 1.8.3.1
On 08.12.2014 19:31, Matthias Gatto wrote:
As explain in the former patchs, backingStore can be treat an array or a pointer. If we have only one backingStore we have to use it as a normal ptr but if there is more backing store, we use it as a pointer's array.
Because it would be complicated to expend backingStore manually, and do the convertion from a pointer to an array, I've created the virStorageSourcePushBackingStore function to help.
This function allocate an array of pointer only if there is more than 1 bs.
Because we can not remove a child from a quorum, i didn't create the function virStorageSourcePopBackingStore.
I've changed virStorageSourceBackingStoreClear, virStorageSourceSetBackingStore and virStorageSourceGetBackingStore to handle the case where backingStore is an array.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/conf/storage_conf.c | 3 +- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 87 ++++++++++++++++++++++++++++++++--- src/util/virstoragefile.h | 2 + 6 files changed, 87 insertions(+), 10 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 300c990..edec124 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c
+/** + * virStorageSourcePushBackingStore: + * @src: virStorageSourcePtr to allocate the new backing store + * + * Allocate size for a new backing store in src->backingStore + * and update src->nBackingStores + * If we have less than 2 backing stores, we treat src->backingStore + * as a pointer otherwise we treat it as an array of virStorageSourcePtr + */ +bool +virStorageSourcePushBackingStore(virStorageSourcePtr src) +{ + virStorageSourcePtr tmp; + virStorageSourcePtr *tmp2; + + if (src->nBackingStores == 1) { +/* If we need more than one backing store we need an array + * Because we don't want to lose our data from the old Backing Store + * we copy the pointer from src->backingStore to src->backingStore[0] */
I'd expect the comments to be indented with the code.
+ tmp = src->backingStore; + if (VIR_ALLOC_N(tmp2, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + src->nBackingStores += 1; + virStorageSourceSetBackingStore(src, tmp, 0); + } else if (src->nBackingStores > 1) { + tmp2 = ((virStorageSourcePtr *)src->backingStore); + if (VIR_EXPAND_N(tmp2, src->nBackingStores, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + } else { +/* Most of the time we use only one backingStore + * So we don't need to allocate an array */ + src->nBackingStores += 1; + } + return true; +} +
Michal
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> --- 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 +++ 6 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 00e0470..1add21f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5445,6 +5445,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, @@ -16412,6 +16413,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 1831323..525a49c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3086,6 +3086,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 8e3a492..1dc7558 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12977,6 +12977,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: @@ -13040,6 +13041,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: @@ -13064,6 +13066,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: @@ -13183,6 +13186,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 0acbb57..5efd753 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1515,6 +1515,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_NETWORK: + 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 4217a80..ad87e80 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -57,7 +57,8 @@ VIR_ENUM_IMPL(virStorage, VIR_STORAGE_TYPE_LAST, "block", "dir", "network", - "volume") + "volume", + "quorum") VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, @@ -66,7 +67,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") @@ -1911,6 +1912,7 @@ virStorageSourceCopy(const virStorageSource *src, bool backingChain) { virStorageSourcePtr ret = NULL; + size_t i; if (VIR_ALLOC(ret) < 0) return NULL; @@ -1922,6 +1924,8 @@ virStorageSourceCopy(const virStorageSource *src, ret->capacity = src->capacity; 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; @@ -1970,12 +1974,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; @@ -2059,6 +2065,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 d28138f..40d221c 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 */ @@ -271,6 +273,7 @@ struct _virStorageSource { /* backing chain of the storage source */ virStorageSourcePtr backingStore; size_t nBackingStores; + size_t threshold; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; -- 1.8.3.1
On 08.12.2014 19:31, Matthias Gatto wrote:
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> --- 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 +++ 6 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 00e0470..1add21f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5445,6 +5445,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, @@ -16412,6 +16413,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 1831323..525a49c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3086,6 +3086,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 8e3a492..1dc7558 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12977,6 +12977,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: @@ -13040,6 +13041,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: @@ -13064,6 +13066,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: @@ -13183,6 +13186,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 0acbb57..5efd753 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1515,6 +1515,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_NETWORK: + 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 4217a80..ad87e80 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -57,7 +57,8 @@ VIR_ENUM_IMPL(virStorage, VIR_STORAGE_TYPE_LAST, "block", "dir", "network", - "volume") + "volume", + "quorum")
This adds new type to domain disk. Any change to XML must go hand in hand with documentation and XML schema changes.
VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, @@ -66,7 +67,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")
Michal
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 | 165 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 120 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1add21f..9164cf3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5469,20 +5469,58 @@ virDomainDiskSourceParse(xmlNodePtr node, } +static bool +virDomainDiskThresholdParse(virStorageSourcePtr src, + xmlNodePtr node) +{ + char *threshold = virXMLPropString(node, "threshold"); + int ret; + + if (!threshold) { + virReportError(VIR_ERR_XML_ERROR, + "%s", _("missing threshold in quorum")); + return false; + } + ret = virStrToLong_ul(threshold, NULL, 10, &src->threshold); + if (ret < 0 || src->threshold < 2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unexpected threshold %s"), + "threshold must be a decimal number superior to 2 " + "and inferior to the number of children"); + VIR_FREE(threshold); + return false; + } + VIR_FREE(threshold); + return true; +} + + static int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, - virStorageSourcePtr src) + virStorageSourcePtr src, + xmlNodePtr node, + size_t pos) { virStorageSourcePtr backingStore = NULL; xmlNodePtr save_ctxt = ctxt->node; - xmlNodePtr source; + xmlNodePtr source = NULL; + xmlNodePtr cur = NULL; char *type = NULL; char *format = NULL; int ret = -1; + size_t i = 0; - if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { - ret = 0; - goto cleanup; + if (src->type == VIR_STORAGE_TYPE_QUORUM) { + if (!node) { + ret = 0; + goto cleanup; + } + ctxt->node = node; + } else { + if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { + ret = 0; + goto cleanup; + } } if (VIR_ALLOC(backingStore) < 0) @@ -5514,6 +5552,22 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { + if (!virDomainDiskThresholdParse(backingStore, node)) + goto cleanup; + + for (cur = node->children, i = 0; cur != NULL; cur = cur->next) { + if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if (virStorageSourcePushBackingStore(backingStore) == false) + goto cleanup; + if ((virDomainDiskBackingStoreParse(ctxt, backingStore, cur, i) < 0)) + goto cleanup; + ++i; + } + } + goto exit; + } + if (!(source = virXPathNode("./source", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store source")); @@ -5521,10 +5575,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, } if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) + virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) goto cleanup; - virStorageSourceSetBackingStore(src, backingStore, 0); + exit: + virStorageSourceSetBackingStore(src, backingStore, pos); ret = 0; cleanup: @@ -5536,7 +5591,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, return ret; } - #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -6030,6 +6084,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ + } else if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { + if (virStorageSourcePushBackingStore(def->src) == false) + goto error; + if (virDomainDiskBackingStoreParse(ctxt, def->src, cur, + def->src->nBackingStores - 1) < 0) + goto error; } } cur = cur->next; @@ -6053,12 +6113,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def->device = VIR_DOMAIN_DISK_DEVICE_DISK; } + if (def->src->type == VIR_STORAGE_TYPE_QUORUM && + !virDomainDiskThresholdParse(def->src, node)) + goto error; + + snapshot = virXMLPropString(node, "snapshot"); + /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ if (source == NULL && def->src->hosts == NULL && !def->src->srcpool && (def->device == VIR_DOMAIN_DISK_DEVICE_DISK || - (flags & VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE))) { + (flags & VIR_DOMAIN_XML_INTERNAL_DISK_SOURCE)) && + def->src->type != VIR_STORAGE_TYPE_QUORUM) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); goto error; @@ -6402,9 +6469,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: @@ -16438,12 +16502,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) @@ -16451,37 +16517,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; } @@ -16551,6 +16623,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); @@ -16596,10 +16672,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_XML_INACTIVE) && - virDomainDiskBackingStoreFormat(buf, - virStorageSourceGetBackingStore(def->src, 0), - def->src->backingStoreRaw, 1) < 0) + if ((!(flags & VIR_DOMAIN_XML_INACTIVE) || + def->src->type == VIR_STORAGE_TYPE_QUORUM) && + virDomainDiskBackingStoreFormat(buf, def->src, 1) < 0) return -1; virDomainDiskGeometryDefFormat(buf, def); -- 1.8.3.1
On 08.12.2014 19:31, 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 | 165 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 120 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1add21f..9164cf3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5469,20 +5469,58 @@ virDomainDiskSourceParse(xmlNodePtr node, }
+static bool +virDomainDiskThresholdParse(virStorageSourcePtr src, + xmlNodePtr node) +{ + char *threshold = virXMLPropString(node, "threshold");
Yet again, new attribute, documentation and schema adjustment required.
+ 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; +}
Michal
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 525a49c..03ff1b7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3167,6 +3167,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 @@ -3639,6 +3744,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; -- 1.8.3.1
Add nodename inside virstoragefile During xml backingStore parsing, look for a nodename attribute in the disk declaration if this one is a quorum, if a nodename is found, add it to the virStorageSource otherwise create a new one with a random name. Take inspiration from this patch to create the nodename: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html Durring xml backingStore formating, look for a nodename attribute inside the virStorageSource struct, and add it to the disk element. Use the nodename to create the quorum in qemuBuildQuorumStr. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/conf/domain_conf.c | 25 +++++++++++++++++++++++++ src/qemu/qemu_command.c | 3 +++ src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 1 + 4 files changed, 33 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9164cf3..a572516 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -53,6 +53,7 @@ #include "device_conf.h" #include "virtpm.h" #include "virstring.h" +#include "virrandom.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -5495,6 +5496,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, @@ -5506,6 +5509,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, xmlNodePtr source = NULL; xmlNodePtr cur = NULL; char *type = NULL; + char *nodeName = NULL; char *format = NULL; int ret = -1; size_t i = 0; @@ -5539,6 +5543,23 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } + if (src->type == VIR_STORAGE_TYPE_QUORUM) { + if ((nodeName = virXMLPropString(ctxt->node, "nodename"))) { + backingStore->nodeName = nodeName; + } else { + int len; + if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN) < 0) + goto cleanup; + snprintf(nodeName, GEN_NODE_NAME_MAX_LEN, + "%s%08x", GEN_NODE_NAME_PREFIX, (int)pos); + len = strlen(nodeName); + while (len < GEN_NODE_NAME_MAX_LEN - 1) + nodeName[len++] = virRandomInt('Z' - 'A') + 'A'; + nodeName[GEN_NODE_NAME_MAX_LEN - 1] = '\0'; + backingStore->nodeName = nodeName; + } + } + if (!(format = virXPathString("string(./format/@type)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing disk backing store format")); @@ -5590,6 +5611,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 @@ -16539,6 +16562,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 03ff1b7..77c38c4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3255,6 +3255,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 ad87e80..cd0098c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1974,6 +1974,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, @@ -2119,6 +2122,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 40d221c..b53dd70 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -274,6 +274,7 @@ struct _virStorageSource { virStorageSourcePtr backingStore; size_t nBackingStores; size_t threshold; + char *nodeName; /* metadata for storage driver access to remote and local volumes */ virStorageDriverDataPtr drv; -- 1.8.3.1
On Mon, Dec 8, 2014 at 7:31 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.
Therefore we have to treat the field virStorageSourcePtr backingStores as an array instead of a pointer. But doing that, most of the backingStore field would be an array contening only one element. So I've decide to allocate the array only if there is more than 1 backing store in a _virStorageSource. 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)
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: Treat backingStore as a pointer or an array virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name
src/conf/domain_conf.c | 193 ++++++++++++++++++++++++++-------- src/conf/storage_conf.c | 7 +- src/libvirt_private.syms | 3 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++ src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 20 ++-- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 +-- src/storage/storage_backend_fs.c | 12 +-- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 136 +++++++++++++++++++++--- src/util/virstoragefile.h | 12 +++ tests/virstoragetest.c | 18 ++-- 17 files changed, 445 insertions(+), 103 deletions(-)
-- 1.8.3.1
ping
On Mon, Dec 8, 2014 at 7:31 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.
Therefore we have to treat the field virStorageSourcePtr backingStores as an array instead of a pointer. But doing that, most of the backingStore field would be an array contening only one element. So I've decide to allocate the array only if there is more than 1 backing store in a _virStorageSource. 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)
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: Treat backingStore as a pointer or an array virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name
src/conf/domain_conf.c | 193 ++++++++++++++++++++++++++-------- src/conf/storage_conf.c | 7 +- src/libvirt_private.syms | 3 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++ src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 20 ++-- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 +-- src/storage/storage_backend_fs.c | 12 +-- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 136 +++++++++++++++++++++--- src/util/virstoragefile.h | 12 +++ tests/virstoragetest.c | 18 ++-- 17 files changed, 445 insertions(+), 103 deletions(-)
-- 1.8.3.1
ping
On Wed, Dec 17, 2014 at 10:53 AM, Matthias Gatto <matthias.gatto@outscale.com> wrote: ping
On 08.12.2014 19:31, Matthias Gatto 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.
Therefore we have to treat the field virStorageSourcePtr backingStores as an array instead of a pointer. But doing that, most of the backingStore field would be an array contening only one element. So I've decide to allocate the array only if there is more than 1 backing store in a _virStorageSource. 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)
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: Treat backingStore as a pointer or an array virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name
src/conf/domain_conf.c | 193 ++++++++++++++++++++++++++-------- src/conf/storage_conf.c | 7 +- src/libvirt_private.syms | 3 + src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++ src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 20 ++-- src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 12 +-- src/storage/storage_backend_fs.c | 12 +-- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 136 +++++++++++++++++++++--- src/util/virstoragefile.h | 12 +++ tests/virstoragetest.c | 18 ++-- 17 files changed, 445 insertions(+), 103 deletions(-)
In general, looks good to me. However, I can't ACK it due to missing documentation. Moreover, you may want to rebase and resend - the code has changed and there are new places where access backingStore directly. Michal
participants (2)
-
Matthias Gatto -
Michal Privoznik