8:47 a.m.
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
8:47 a.m.
New subject: [libvirt] [PATCH 01/12] storage: Extract common code to retrieve driver backend for support check
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
9:11 a.m.
New subject: [libvirt] [PATCH 01/12] storage: Extract common code to retrieve driver backend for support check
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 02/12] storage: Add feature check for storage file backend supporting access check
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
9:12 a.m.
New subject: [libvirt] [PATCH 02/12] storage: Add feature check for storage file backend supporting access check
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 03/12] storage: Extract error reporting for broken chains
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
9:12 a.m.
New subject: [libvirt] [PATCH 03/12] storage: Extract error reporting for broken chains
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 04/12] security: selinux: Pass parent storage source into image labeling helper
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
9:13 a.m.
New subject: [libvirt] [PATCH 04/12] security: selinux: Pass parent storage source into image labeling helper
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 05/12] security: dac: Take parent security label into account
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
9:23 a.m.
New subject: [libvirt] [PATCH 05/12] security: dac: Take parent security label into account
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 06/12] security: selinux: Take parent security label into account
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
9:25 a.m.
New subject: [libvirt] [PATCH 06/12] security: selinux: Take parent security label into account
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 07/12] qemu: domain: Simplify using DAC permissions of top of backing chain
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
9:40 a.m.
New subject: [libvirt] [PATCH 07/12] qemu: domain: Simplify using DAC permissions of top of backing chain
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
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
10:12 a.m.
New subject: [libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
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(a)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
*
4:25 a.m.
New subject: [libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
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.
8:47 a.m.
New subject: [libvirt] [PATCH 09/12] qemu: domain: Destroy secrets for complete backing chain
---
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
10:13 a.m.
New subject: [libvirt] [PATCH 09/12] qemu: domain: Destroy secrets for complete backing chain
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 10/12] qemu: domain: Remove pointless alias check
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
10:18 a.m.
New subject: [libvirt] [PATCH 10/12] qemu: domain: Remove pointless alias check
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 11/12] qemu: domain: Prepare TLS data for the whole backing chain
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
10:23 a.m.
New subject: [libvirt] [PATCH 11/12] qemu: domain: Prepare TLS data for the whole backing chain
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(a)redhat.com>
John
8:47 a.m.
New subject: [libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain
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
10:26 a.m.
New subject: [libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain
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);
10:29 a.m.
New subject: [libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain
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(a)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);
2575
days inactive
2589
days old
26 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa