[libvirt] [PATCH 00/12] qemu: block: Prepare for user-specified backing chains (blockdev-add saga)
When users will specify backing chain, we need to take into account what was passed in them and/or inherit the data from the parents as we are copying the data (labels, etc...) from the parent disk source. Peter Krempa (12): storage: Extract common code to retrieve driver backend for support check storage: Add feature check for storage file backend supporting access check storage: Extract error reporting for broken chains security: selinux: Pass parent storage source into image labeling helper security: dac: Take parent security label into account security: selinux: Take parent security label into account qemu: domain: Simplify using DAC permissions of top of backing chain qemu: domain: Extract setup for disk source secrets qemu: domain: Destroy secrets for complete backing chain qemu: domain: Remove pointless alias check qemu: domain: Prepare TLS data for the whole backing chain qemu: domain: skip chain detection to end of backing chain src/qemu/qemu_domain.c | 177 ++++++++++++++++++++++++++-------------- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- src/security/security_dac.c | 38 +++++++-- src/security/security_selinux.c | 26 +++--- src/storage/storage_source.c | 110 ++++++++++++++++--------- src/storage/storage_source.h | 5 ++ 9 files changed, 246 insertions(+), 126 deletions(-) -- 2.14.1
The 'file access' module of the storage driver has few feature checks to determine whether libvirt supports given storage driver method. The code to retrieve the driver struct needed for the check is the same so it can be extracted. --- src/storage/storage_source.c | 43 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index 419fa3d43..e3c5c3285 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -44,24 +44,30 @@ virStorageFileIsInitialized(const virStorageSource *src) } -static bool -virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) +static virStorageFileBackendPtr +virStorageFileGetBackendForSupportCheck(const virStorageSource *src) { int actualType; - virStorageFileBackendPtr backend; if (!src) - return false; + return NULL; + + if (src->drv) + return src->drv->backend; + actualType = virStorageSourceGetActualType(src); - if (src->drv) { - backend = src->drv->backend; - } else { - if (!(backend = virStorageFileBackendForTypeInternal(actualType, - src->protocol, - false))) - return false; - } + return virStorageFileBackendForTypeInternal(actualType, src->protocol, false); +} + + +static bool +virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) +{ + virStorageFileBackendPtr backend; + + if (!(backend = virStorageFileGetBackendForSupportCheck(src))) + return false; return backend->storageFileGetUniqueIdentifier && backend->storageFileRead && @@ -80,21 +86,10 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) bool virStorageFileSupportsSecurityDriver(const virStorageSource *src) { - int actualType; virStorageFileBackendPtr backend; - if (!src) + if (!(backend = virStorageFileGetBackendForSupportCheck(src))) return false; - actualType = virStorageSourceGetActualType(src); - - if (src->drv) { - backend = src->drv->backend; - } else { - if (!(backend = virStorageFileBackendForTypeInternal(actualType, - src->protocol, - false))) - return false; - } return !!backend->storageFileChown; } -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
The 'file access' module of the storage driver has few feature checks to determine whether libvirt supports given storage driver method. The code to retrieve the driver struct needed for the check is the same so it can be extracted. --- src/storage/storage_source.c | 43 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 24 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
When the user provides backing chain, we don't need the full support for traversing the backing chain. This patch adds a feature check for the virStorageSourceAccess API. --- src/storage/storage_source.c | 20 ++++++++++++++++++++ src/storage/storage_source.h | 1 + 2 files changed, 21 insertions(+) diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index e3c5c3285..cced5308c 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -95,6 +95,26 @@ virStorageFileSupportsSecurityDriver(const virStorageSource *src) } +/** + * virStorageFileSupportsAccess: + * + * @src: a storage file structure + * + * Check if a storage file supports checking if the storage source is accessible + * for the given vm. + */ +bool +virStorageFileSupportsAccess(const virStorageSource *src) +{ + virStorageFileBackendPtr backend; + + if (!(backend = virStorageFileGetBackendForSupportCheck(src))) + return false; + + return !!backend->storageFileAccess; +} + + void virStorageFileDeinit(virStorageSourcePtr src) { diff --git a/src/storage/storage_source.h b/src/storage/storage_source.h index 6462baf6a..320ea3cab 100644 --- a/src/storage/storage_source.h +++ b/src/storage/storage_source.h @@ -41,6 +41,7 @@ int virStorageFileAccess(virStorageSourcePtr src, int mode); int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid); bool virStorageFileSupportsSecurityDriver(const virStorageSource *src); +bool virStorageFileSupportsAccess(const virStorageSource *src); int virStorageFileGetMetadata(virStorageSourcePtr src, uid_t uid, gid_t gid, -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
When the user provides backing chain, we don't need the full support for traversing the backing chain. This patch adds a feature check for the virStorageSourceAccess API. --- src/storage/storage_source.c | 20 ++++++++++++++++++++ src/storage/storage_source.h | 1 + 2 files changed, 21 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Simplify reporting the error if backing chain is broken for further callers by extracting it into a separate function. --- src/storage/storage_source.c | 47 +++++++++++++++++++++++++++++++------------- src/storage/storage_source.h | 4 ++++ 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index cced5308c..4586ef4ad 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -404,6 +404,38 @@ virStorageFileChown(const virStorageSource *src, } +/** + * virStorageFileReportBrokenChain: + * + * @errcode: errno when accessing @src + * @src: inaccessible file in the backing chain of @parent + * @parent: root virStorageSource being checked + * + * Reports the correct error message if @src is missing in the backing chain + * for @parent. + */ +void +virStorageFileReportBrokenChain(int errcode, + virStorageSourcePtr src, + virStorageSourcePtr parent) +{ + unsigned int access_user = src->drv->uid; + unsigned int access_group = src->drv->gid; + + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s' " + "(as uid:%u, gid:%u)"), + src->path, access_user, access_group); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s' (as uid:%u, gid:%u)"), + src->path, parent->path, access_user, access_group); + } +} + + /* Recursive workhorse for virStorageFileGetMetadata. */ static int virStorageFileGetMetadataRecurse(virStorageSourcePtr src, @@ -433,20 +465,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, return -1; if (virStorageFileAccess(src, F_OK) < 0) { - if (src == parent) { - virReportSystemError(errno, - _("Cannot access storage file '%s' " - "(as uid:%u, gid:%u)"), - src->path, (unsigned int)uid, - (unsigned int)gid); - } else { - virReportSystemError(errno, - _("Cannot access backing file '%s' " - "of storage file '%s' (as uid:%u, gid:%u)"), - src->path, parent->path, - (unsigned int)uid, (unsigned int)gid); - } - + virStorageFileReportBrokenChain(errno, src, parent); goto cleanup; } diff --git a/src/storage/storage_source.h b/src/storage/storage_source.h index 320ea3cab..0640c138e 100644 --- a/src/storage/storage_source.h +++ b/src/storage/storage_source.h @@ -52,4 +52,8 @@ int virStorageFileGetMetadata(virStorageSourcePtr src, char *virStorageFileGetBackingStoreStr(virStorageSourcePtr src) ATTRIBUTE_NONNULL(1); +void virStorageFileReportBrokenChain(int errcode, + virStorageSourcePtr src, + virStorageSourcePtr parent); + #endif /* __VIR_STORAGE_SOURCE_H__ */ -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
Simplify reporting the error if backing chain is broken for further callers by extracting it into a separate function. --- src/storage/storage_source.c | 47 +++++++++++++++++++++++++++++++------------- src/storage/storage_source.h | 4 ++++ 2 files changed, 37 insertions(+), 14 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
virSecuritySELinuxSetImageLabelInternal assigns different labels to backing chain members than to the parent image. This was done via the 'first' flag. Convert it to passing in pointer to the parent virStorageSource. This will allow us to use the parent virStorageSource in further changes. --- src/security/security_selinux.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index cd3e41193..66b3bbf1c 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1592,7 +1592,7 @@ static int virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src, - bool first) + virStorageSourcePtr parent) { virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDefPtr secdef; @@ -1614,7 +1614,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, if (disk_seclabel && disk_seclabel->relabel && disk_seclabel->label) { ret = virSecuritySELinuxSetFilecon(mgr, src->path, disk_seclabel->label); - } else if (first) { + } else if (!parent || parent == src) { if (src->shared) { ret = virSecuritySELinuxSetFileconOptional(mgr, src->path, @@ -1660,7 +1660,7 @@ virSecuritySELinuxSetImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src) { - return virSecuritySELinuxSetImageLabelInternal(mgr, def, src, true); + return virSecuritySELinuxSetImageLabelInternal(mgr, def, src, NULL); } @@ -1670,14 +1670,11 @@ virSecuritySELinuxSetDiskLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { - bool first = true; virStorageSourcePtr next; for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { - if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, first) < 0) + if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, disk->src) < 0) return -1; - - first = false; } return 0; -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
virSecuritySELinuxSetImageLabelInternal assigns different labels to backing chain members than to the parent image. This was done via the 'first' flag. Convert it to passing in pointer to the parent virStorageSource. This will allow us to use the parent virStorageSource in further changes. --- src/security/security_selinux.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Until now we ignored user-provided backing chains and while detecting the code inherited labels of the parent device. With user provided chains we should keep this functionality, so label of the parent image in the backing chain will be applied if an image-specific label is not present. --- src/security/security_dac.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 244b300a9..54120890f 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -688,12 +688,14 @@ virSecurityDACRestoreFileLabel(virSecurityDACDataPtr priv, static int -virSecurityDACSetImageLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def, - virStorageSourcePtr src) +virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src, + virStorageSourcePtr parent) { virSecurityLabelDefPtr secdef; virSecurityDeviceLabelDefPtr disk_seclabel; + virSecurityDeviceLabelDefPtr parent_seclabel = NULL; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); uid_t user; gid_t group; @@ -705,14 +707,24 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr, if (secdef && !secdef->relabel) return 0; - disk_seclabel = virStorageSourceGetSecurityLabelDef(src, - SECURITY_DAC_NAME); - if (disk_seclabel && !disk_seclabel->relabel) - return 0; + disk_seclabel = virStorageSourceGetSecurityLabelDef(src, SECURITY_DAC_NAME); + if (parent) + parent_seclabel = virStorageSourceGetSecurityLabelDef(parent, + SECURITY_DAC_NAME); + + if (disk_seclabel && (!disk_seclabel->relabel || disk_seclabel->label)) { + if (!disk_seclabel->relabel) + return 0; - if (disk_seclabel && disk_seclabel->label) { if (virParseOwnershipIds(disk_seclabel->label, &user, &group) < 0) return -1; + } else if (parent_seclabel && + (!parent_seclabel->relabel || parent_seclabel->label)) { + if (!parent_seclabel->relabel) + return 0; + + if (virParseOwnershipIds(parent_seclabel->label, &user, &group) < 0) + return -1; } else { if (virSecurityDACGetImageIds(secdef, priv, &user, &group)) return -1; @@ -722,6 +734,14 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr, } +static int +virSecurityDACSetImageLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virStorageSourcePtr src) +{ + return virSecurityDACSetImageLabelInternal(mgr, def, src, NULL); +} + static int virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, @@ -731,7 +751,7 @@ virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr, virStorageSourcePtr next; for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) { - if (virSecurityDACSetImageLabel(mgr, def, next) < 0) + if (virSecurityDACSetImageLabelInternal(mgr, def, next, disk->src) < 0) return -1; } -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
Until now we ignored user-provided backing chains and while detecting the code inherited labels of the parent device. With user provided chains we should keep this functionality, so label of the parent image in the backing chain will be applied if an image-specific label is not present. --- src/security/security_dac.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Until now we ignored user-provided backing chains and while detecting the code inherited labels of the parent device. With user provided chains we should keep this functionality, so label of the parent image in the backing chain will be applied if an image-specific label is not present. --- src/security/security_selinux.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 66b3bbf1c..ed1828a12 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1597,6 +1597,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDefPtr secdef; virSecurityDeviceLabelDefPtr disk_seclabel; + virSecurityDeviceLabelDefPtr parent_seclabel = NULL; int ret; if (!src->path || !virStorageSourceIsLocalStorage(src)) @@ -1608,12 +1609,20 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, disk_seclabel = virStorageSourceGetSecurityLabelDef(src, SECURITY_SELINUX_NAME); + if (parent) + parent_seclabel = virStorageSourceGetSecurityLabelDef(parent, + SECURITY_SELINUX_NAME); - if (disk_seclabel && !disk_seclabel->relabel) - return 0; + if (disk_seclabel && (!disk_seclabel->relabel || disk_seclabel->label)) { + if (!disk_seclabel->relabel) + return 0; - if (disk_seclabel && disk_seclabel->relabel && disk_seclabel->label) { ret = virSecuritySELinuxSetFilecon(mgr, src->path, disk_seclabel->label); + } else if (parent_seclabel && (!parent_seclabel->relabel || parent_seclabel->label)) { + if (!parent_seclabel->relabel) + return 0; + + ret = virSecuritySELinuxSetFilecon(mgr, src->path, parent_seclabel->label); } else if (!parent || parent == src) { if (src->shared) { ret = virSecuritySELinuxSetFileconOptional(mgr, -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
Until now we ignored user-provided backing chains and while detecting the code inherited labels of the parent device. With user provided chains we should keep this functionality, so label of the parent image in the backing chain will be applied if an image-specific label is not present. --- src/security/security_selinux.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
qemuDomainGetImageIds and qemuDomainStorageFileInit are helpful when trying to access a virStorageSource from the qemu driver since they figure out the correct uid and gid for the image. When accessing members of a backing chain the permissions for the top level would be used. To allow using specific permissions per backing chain level but still allow inheritance from the parent of the chain we need to add a new parameter to the image ID APIs. --- src/qemu/qemu_domain.c | 13 ++++++++++--- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 6 +++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 00610edf1..24ed61bc2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5926,6 +5926,7 @@ static void qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, virStorageSourcePtr src, + virStorageSourcePtr parentSrc, uid_t *uid, gid_t *gid) { virSecurityLabelDefPtr vmlabel; @@ -5948,6 +5949,11 @@ qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg, vmlabel->label) virParseOwnershipIds(vmlabel->label, uid, gid); + if (parentSrc && + (disklabel = virStorageSourceGetSecurityLabelDef(parentSrc, "dac")) && + disklabel->label) + virParseOwnershipIds(disklabel->label, uid, gid); + if ((disklabel = virStorageSourceGetSecurityLabelDef(src, "dac")) && disklabel->label) virParseOwnershipIds(disklabel->label, uid, gid); @@ -5957,14 +5963,15 @@ qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg, int qemuDomainStorageFileInit(virQEMUDriverPtr driver, virDomainObjPtr vm, - virStorageSourcePtr src) + virStorageSourcePtr src, + virStorageSourcePtr parent) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); uid_t uid; gid_t gid; int ret = -1; - qemuDomainGetImageIds(cfg, vm, src, &uid, &gid); + qemuDomainGetImageIds(cfg, vm, src, parent, &uid, &gid); if (virStorageFileInitAs(src, uid, gid) < 0) goto cleanup; @@ -6014,7 +6021,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, goto cleanup; } - qemuDomainGetImageIds(cfg, vm, disk->src, &uid, &gid); + qemuDomainGetImageIds(cfg, vm, disk->src, NULL, &uid, &gid); if (virStorageFileGetMetadata(disk->src, uid, gid, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 39cb68b3c..a8ad59d20 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -674,7 +674,8 @@ bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, int qemuDomainStorageFileInit(virQEMUDriverPtr driver, virDomainObjPtr vm, - virStorageSourcePtr src); + virStorageSourcePtr src, + virStorageSourcePtr parent); char *qemuDomainStorageAlias(const char *device, int depth); void qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d56992fbb..23692fedb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11495,7 +11495,7 @@ qemuDomainBlockPeek(virDomainPtr dom, goto cleanup; } - if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0) + if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) < 0) goto cleanup; if ((nread = virStorageFileRead(disk->src, offset, size, &tmpbuf)) < 0) @@ -14418,7 +14418,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0) goto error; - if (qemuDomainStorageFileInit(driver, vm, dd->src) < 0) + if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0) goto error; dd->initialized = true; @@ -17093,7 +17093,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, goto endjob; } - if (qemuDomainStorageFileInit(driver, vm, mirror) < 0) + if (qemuDomainStorageFileInit(driver, vm, mirror, NULL) < 0) goto endjob; if (qemuDomainBlockCopyValidateMirror(mirror, disk->dst, &reuse) < 0) -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
qemuDomainGetImageIds and qemuDomainStorageFileInit are helpful when trying to access a virStorageSource from the qemu driver since they figure out the correct uid and gid for the image.
When accessing members of a backing chain the permissions for the top level would be used. To allow using specific permissions per backing chain level but still allow inheritance from the parent of the chain we need to add a new parameter to the image ID APIs. --- src/qemu/qemu_domain.c | 13 ++++++++++--- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 6 +++--- 3 files changed, 15 insertions(+), 7 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Separate it so that it deals only with single virStorageSource, so that it can later be reused for full backing chain support. Two aliases are passed since authentication is more relevant to the 'storage backend' whereas encryption is more relevant to the protocol layer. When using node names, the aliases will be different. --- src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 24ed61bc2..4a2ba1761 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1368,27 +1368,19 @@ qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr src) } -/* qemuDomainSecretDiskPrepare: - * @conn: Pointer to connection - * @priv: pointer to domain private object - * @disk: Pointer to a disk definition - * - * For the right disk, generate the qemuDomainSecretInfo structure. - * - * Returns 0 on success, -1 on failure - */ -int -qemuDomainSecretDiskPrepare(virConnectPtr conn, - qemuDomainObjPrivatePtr priv, - virDomainDiskDefPtr disk) +static int +qemuDomainSecretStorageSourcePrepare(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + virStorageSourcePtr src, + const char *authalias, + const char *encalias) { - virStorageSourcePtr src = disk->src; qemuDomainStorageSourcePrivatePtr srcPriv; - if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew())) + if (!(src->privateData = qemuDomainStorageSourcePrivateNew())) return -1; - srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI; @@ -1397,7 +1389,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, usageType = VIR_SECRET_USAGE_TYPE_CEPH; if (!(srcPriv->secinfo = - qemuDomainSecretInfoNew(conn, priv, disk->info.alias, + qemuDomainSecretInfoNew(conn, priv, authalias, usageType, src->auth->username, &src->auth->seclookupdef, false))) return -1; @@ -1405,7 +1397,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, if (qemuDomainDiskHasEncryptionSecret(src)) { if (!(srcPriv->encinfo = - qemuDomainSecretInfoNew(conn, priv, disk->info.alias, + qemuDomainSecretInfoNew(conn, priv, encalias, VIR_SECRET_USAGE_TYPE_VOLUME, NULL, &src->encryption->secrets[0]->seclookupdef, true))) @@ -1416,6 +1408,27 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, } +/* qemuDomainSecretDiskPrepare: + * @conn: Pointer to connection + * @priv: pointer to domain private object + * @disk: Pointer to a disk definition + * + * For the right disk, generate the qemuDomainSecretInfo structure. + * + * Returns 0 on success, -1 on failure + */ + +int +qemuDomainSecretDiskPrepare(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + virDomainDiskDefPtr disk) +{ + return qemuDomainSecretStorageSourcePrepare(conn, priv, disk->src, + disk->info.alias, + disk->info.alias); +} + + /* qemuDomainSecretHostdevDestroy: * @disk: Pointer to a hostdev definition * -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
Separate it so that it deals only with single virStorageSource, so that it can later be reused for full backing chain support.
Two aliases are passed since authentication is more relevant to the 'storage backend' whereas encryption is more relevant to the protocol layer. When using node names, the aliases will be different. --- src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-)
FWIW: The @authalias would be the secret for the source to access the server (RBD only right now, but the iSCSI patches are on list) while the @encalias would be the LUKS secret. Does the backing chain allow different "auth" sources for different chain members? Mind boggling, but I guess possible. I suppose it would also be possible that some member of the chain doesn't use the auth, but rather it's a local LUKS encrypted file - is that the essential goal? I think perhaps it'd help to add some comments for qemuDomainSecretStorageSourcePrepare in order to describe the parameters and the "expectations" for varying levels of the chain. There's also some "assumptions" built into the hotplug code for at least the top level. In general as long as you add a bit more details as to the parameter expectations, Reviewed-by: John Ferlan <jferlan@redhat.com> John
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 24ed61bc2..4a2ba1761 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1368,27 +1368,19 @@ qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr src) }
-/* qemuDomainSecretDiskPrepare: - * @conn: Pointer to connection - * @priv: pointer to domain private object - * @disk: Pointer to a disk definition - * - * For the right disk, generate the qemuDomainSecretInfo structure. - * - * Returns 0 on success, -1 on failure - */ -int -qemuDomainSecretDiskPrepare(virConnectPtr conn, - qemuDomainObjPrivatePtr priv, - virDomainDiskDefPtr disk) +static int +qemuDomainSecretStorageSourcePrepare(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + virStorageSourcePtr src, + const char *authalias, + const char *encalias) { - virStorageSourcePtr src = disk->src; qemuDomainStorageSourcePrivatePtr srcPriv;
- if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew())) + if (!(src->privateData = qemuDomainStorageSourcePrivateNew())) return -1;
- srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI; @@ -1397,7 +1389,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, usageType = VIR_SECRET_USAGE_TYPE_CEPH;
if (!(srcPriv->secinfo = - qemuDomainSecretInfoNew(conn, priv, disk->info.alias, + qemuDomainSecretInfoNew(conn, priv, authalias, usageType, src->auth->username, &src->auth->seclookupdef, false))) return -1; @@ -1405,7 +1397,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
if (qemuDomainDiskHasEncryptionSecret(src)) { if (!(srcPriv->encinfo = - qemuDomainSecretInfoNew(conn, priv, disk->info.alias, + qemuDomainSecretInfoNew(conn, priv, encalias, VIR_SECRET_USAGE_TYPE_VOLUME, NULL, &src->encryption->secrets[0]->seclookupdef, true))) @@ -1416,6 +1408,27 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, }
+/* qemuDomainSecretDiskPrepare: + * @conn: Pointer to connection + * @priv: pointer to domain private object + * @disk: Pointer to a disk definition + * + * For the right disk, generate the qemuDomainSecretInfo structure. + * + * Returns 0 on success, -1 on failure + */ + +int +qemuDomainSecretDiskPrepare(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + virDomainDiskDefPtr disk) +{ + return qemuDomainSecretStorageSourcePrepare(conn, priv, disk->src, + disk->info.alias, + disk->info.alias); +} + + /* qemuDomainSecretHostdevDestroy: * @disk: Pointer to a hostdev definition *
On Thu, Oct 26, 2017 at 11:12:08 -0400, John Ferlan wrote:
On 10/20/2017 09:47 AM, Peter Krempa wrote:
Separate it so that it deals only with single virStorageSource, so that it can later be reused for full backing chain support.
Two aliases are passed since authentication is more relevant to the 'storage backend' whereas encryption is more relevant to the protocol layer. When using node names, the aliases will be different. --- src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-)
FWIW: The @authalias would be the secret for the source to access the server (RBD only right now, but the iSCSI patches are on list) while the @encalias would be the LUKS secret.
Does the backing chain allow different "auth" sources for different chain members? Mind boggling, but I guess possible.
I suppose it would also be possible that some member of the chain doesn't use the auth, but rather it's a local LUKS encrypted file - is that the essential goal?
I think perhaps it'd help to add some comments for qemuDomainSecretStorageSourcePrepare in order to describe the parameters and the "expectations" for varying levels of the chain. There's also some "assumptions" built into the hotplug code for at least the top level.
Yes every level can have different authentication data. (Since they can reside on completely different storage technologies). This is possible even now, but authentication will not work in that case. The aliases are separate since in qemu the storage access layer and format driver level have different node names. Authentication is relevant to the storage access level, while encryption to the format driver level. I've added a comment trying to explain this.
--- src/qemu/qemu_domain.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4a2ba1761..c689911c4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1324,6 +1324,19 @@ qemuDomainSecretInfoTLSNew(virConnectPtr conn, } +static void +qemuDomainSecretStorageSourceDestroy(virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (srcPriv && srcPriv->secinfo) + qemuDomainSecretInfoFree(&srcPriv->secinfo); + + if (srcPriv && srcPriv->encinfo) + qemuDomainSecretInfoFree(&srcPriv->encinfo); +} + + /* qemuDomainSecretDiskDestroy: * @disk: Pointer to a disk definition * @@ -1332,13 +1345,10 @@ qemuDomainSecretInfoTLSNew(virConnectPtr conn, void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) { - qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); - - if (srcPriv && srcPriv->secinfo) - qemuDomainSecretInfoFree(&srcPriv->secinfo); + virStorageSourcePtr next; - if (srcPriv && srcPriv->encinfo) - qemuDomainSecretInfoFree(&srcPriv->encinfo); + for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) + qemuDomainSecretStorageSourceDestroy(next); } -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
--- src/qemu/qemu_domain.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
When attaching the disks, aliases are always generated. --- src/qemu/qemu_domain.c | 8 -------- src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c689911c4..aebe24e7b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7833,7 +7833,6 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, /* qemuProcessPrepareDiskSourceTLS: * @source: pointer to host interface data for disk device - * @diskAlias: alias use for the disk device * @cfg: driver configuration * * Updates host interface TLS encryption setting based on qemu.conf @@ -7844,7 +7843,6 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, */ int qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, - const char *diskAlias, virQEMUDriverConfigPtr cfg) { @@ -7863,12 +7861,6 @@ qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, } if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { - if (!diskAlias) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("disk does not have an alias")); - return -1; - } - /* Grab the vxhsTLSx509certdir and set the verify/listen values. * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */ if (VIR_STRDUP(src->tlsCertdir, cfg->vxhsTLSx509certdir) < 0) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a8ad59d20..6615dabf9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -882,9 +882,8 @@ void qemuDomainPrepareChardevSource(virDomainDefPtr def, int qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, - const char *diskAlias, virQEMUDriverConfigPtr cfg) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 91f7f9ed6..e4157f631 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -394,7 +394,7 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn, if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error; - if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) < 0) + if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) goto error; if (disk->src->haveTLS && diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 66e81bbe5..9bbfabcde 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5301,7 +5301,7 @@ qemuProcessPrepareDomainStorage(virConnectPtr conn, continue; } - if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) < 0) + if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) return -1; } -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
When attaching the disks, aliases are always generated. --- src/qemu/qemu_domain.c | 8 -------- src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- 4 files changed, 3 insertions(+), 12 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Iterate through the backing chain when setting up TLS for disks. --- src/qemu/qemu_domain.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index aebe24e7b..3560cdd29 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7845,28 +7845,31 @@ int qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, virQEMUDriverConfigPtr cfg) { + virStorageSourcePtr next; - /* VxHS uses only client certificates and thus has no need for - * the server-key.pem nor a secret that could be used to decrypt - * the it, so no need to add a secinfo for a secret UUID. */ - if (src->type == VIR_STORAGE_TYPE_NETWORK && - src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) { - - if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT) { - if (cfg->vxhsTLS) - src->haveTLS = VIR_TRISTATE_BOOL_YES; - else - src->haveTLS = VIR_TRISTATE_BOOL_NO; - src->tlsFromConfig = true; - } + for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) { + /* VxHS uses only client certificates and thus has no need for + * the server-key.pem nor a secret that could be used to decrypt + * the it, so no need to add a secinfo for a secret UUID. */ + if (next->type == VIR_STORAGE_TYPE_NETWORK && + next->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) { + + if (next->haveTLS == VIR_TRISTATE_BOOL_ABSENT) { + if (cfg->vxhsTLS) + next->haveTLS = VIR_TRISTATE_BOOL_YES; + else + next->haveTLS = VIR_TRISTATE_BOOL_NO; + next->tlsFromConfig = true; + } - if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { - /* Grab the vxhsTLSx509certdir and set the verify/listen values. - * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */ - if (VIR_STRDUP(src->tlsCertdir, cfg->vxhsTLSx509certdir) < 0) - return -1; + if (next->haveTLS == VIR_TRISTATE_BOOL_YES) { + /* Grab the vxhsTLSx509certdir and set the verify/listen values. + * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */ + if (VIR_STRDUP(next->tlsCertdir, cfg->vxhsTLSx509certdir) < 0) + return -1; - src->tlsVerify = true; + next->tlsVerify = true; + } } } -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
Iterate through the backing chain when setting up TLS for disks. --- src/qemu/qemu_domain.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-)
So (for now) a VxHS device could be at some depth within the backing chain and we'd need to make sure we can use TLS for it, but not necessarily use TLS for all levels - is that the basic premise? I don't even want to think about having different target TLS types within the chain - pffttt, splat goes the brain. Reviewed-by: John Ferlan <jferlan@redhat.com> John
When a user provides the backing chain, we will not need to re-detect all the backing stores again, but should move to the end of the user specified chain. Additionally if a user provides a full terminated chain we should not attempt any further detection. --- src/qemu/qemu_domain.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3560cdd29..5973474ca 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, bool report_broken) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - int ret = 0; + virStorageSourcePtr src = disk->src; + int ret = -1; uid_t uid; gid_t gid; - if (virStorageSourceIsEmpty(disk->src)) + if (virStorageSourceIsEmpty(src)) { + ret = 0; goto cleanup; + } if (virStorageSourceHasBacking(disk->src)) { - if (force_probe) - virStorageSourceBackingStoreClear(disk->src); - else - goto cleanup; + if (force_probe) { + virStorageSourceBackingStoreClear(src); + } else { + /* skip to the end of the chain */ + while (virStorageSourceIsBacking(src)) { + if (report_broken && + virStorageFileSupportsAccess(src)) { + + if (qemuDomainStorageFileInit(driver, vm, src, disk->src) < 0) + goto cleanup; + + if (virStorageFileAccess(src, F_OK) < 0) { + virStorageFileReportBrokenChain(errno, src, disk->src); + virStorageFileDeinit(src); + goto cleanup; + } + + virStorageFileDeinit(src); + } + src = src->backingStore; + } + } } - qemuDomainGetImageIds(cfg, vm, disk->src, NULL, &uid, &gid); + /* We skipped to the end of the chain. Skip detection if there's the + * terminator. (An allocated but empty backingStore) */ + if (src->backingStore) { + ret = 0; + goto cleanup; + } + + qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid); - if (virStorageFileGetMetadata(disk->src, + if (virStorageFileGetMetadata(src, uid, gid, cfg->allowDiskFormatProbing, report_broken) < 0) - ret = -1; + goto cleanup; + + ret = 0; cleanup: virObjectUnref(cfg); -- 2.14.1
On 10/20/2017 09:47 AM, Peter Krempa wrote:
When a user provides the backing chain, we will not need to re-detect all the backing stores again, but should move to the end of the user specified chain. Additionally if a user provides a full terminated chain we should not attempt any further detection. --- src/qemu/qemu_domain.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3560cdd29..5973474ca 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, bool report_broken) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - int ret = 0; + virStorageSourcePtr src = disk->src; + int ret = -1; uid_t uid; gid_t gid;
- if (virStorageSourceIsEmpty(disk->src)) + if (virStorageSourceIsEmpty(src)) { + ret = 0; goto cleanup; + }
if (virStorageSourceHasBacking(disk->src)) {
could this one be just @src?
- if (force_probe) - virStorageSourceBackingStoreClear(disk->src); - else - goto cleanup; + if (force_probe) { + virStorageSourceBackingStoreClear(src); + } else { + /* skip to the end of the chain */ + while (virStorageSourceIsBacking(src)) { + if (report_broken && + virStorageFileSupportsAccess(src)) { + + if (qemuDomainStorageFileInit(driver, vm, src, disk->src) < 0) + goto cleanup; + + if (virStorageFileAccess(src, F_OK) < 0) { + virStorageFileReportBrokenChain(errno, src, disk->src); + virStorageFileDeinit(src); + goto cleanup; + } + + virStorageFileDeinit(src); + } + src = src->backingStore; + } + } }
- qemuDomainGetImageIds(cfg, vm, disk->src, NULL, &uid, &gid); + /* We skipped to the end of the chain. Skip detection if there's the + * terminator. (An allocated but empty backingStore) */ + if (src->backingStore) { + ret = 0; + goto cleanup; + } + + qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid);
- if (virStorageFileGetMetadata(disk->src, + if (virStorageFileGetMetadata(src, uid, gid, cfg->allowDiskFormatProbing, report_broken) < 0) - ret = -1; + goto cleanup;
+ + ret = 0;
cleanup: virObjectUnref(cfg);
On 10/20/2017 09:47 AM, Peter Krempa wrote:
When a user provides the backing chain, we will not need to re-detect all the backing stores again, but should move to the end of the user specified chain. Additionally if a user provides a full terminated chain we should not attempt any further detection. --- src/qemu/qemu_domain.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-)
Looks like some magic combination of keys caused an premature send. In any case, IDC if you change the noted disk->src to src, but figured I'd ask anyway... Reviewed-by: John Ferlan <jferlan@redhat.com>
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3560cdd29..5973474ca 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, bool report_broken) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - int ret = 0; + virStorageSourcePtr src = disk->src; + int ret = -1; uid_t uid; gid_t gid;
- if (virStorageSourceIsEmpty(disk->src)) + if (virStorageSourceIsEmpty(src)) { + ret = 0; goto cleanup; + }
if (virStorageSourceHasBacking(disk->src)) { - if (force_probe) - virStorageSourceBackingStoreClear(disk->src); - else - goto cleanup; + if (force_probe) { + virStorageSourceBackingStoreClear(src); + } else { + /* skip to the end of the chain */ + while (virStorageSourceIsBacking(src)) { + if (report_broken && + virStorageFileSupportsAccess(src)) { + + if (qemuDomainStorageFileInit(driver, vm, src, disk->src) < 0) + goto cleanup; + + if (virStorageFileAccess(src, F_OK) < 0) { + virStorageFileReportBrokenChain(errno, src, disk->src); + virStorageFileDeinit(src); + goto cleanup; + } + + virStorageFileDeinit(src); + } + src = src->backingStore; + } + } }
- qemuDomainGetImageIds(cfg, vm, disk->src, NULL, &uid, &gid); + /* We skipped to the end of the chain. Skip detection if there's the + * terminator. (An allocated but empty backingStore) */ + if (src->backingStore) { + ret = 0; + goto cleanup; + } + + qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid);
- if (virStorageFileGetMetadata(disk->src, + if (virStorageFileGetMetadata(src, uid, gid, cfg->allowDiskFormatProbing, report_broken) < 0) - ret = -1; + goto cleanup;
This looks familiar to the recent upstream question...
+ + ret = 0;
cleanup: virObjectUnref(cfg);
participants (2)
-
John Ferlan -
Peter Krempa