6:18 a.m.
See 2/2.
Peter Krempa (2):
qemuSecurityChownCallback: Remove 'cleanup' section
qemuSecurityChownCallback: Don't initialize storage file subsystem for
local file
src/qemu/qemu_driver.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
--
2.24.1
6:18 a.m.
New subject: [PATCH 1/2] qemuSecurityChownCallback: Remove 'cleanup' section
Treat the shortcut for chowning local files as a stand-alone section
by returning success from it and refactor the rest so that the cleanup
section is inline.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 570dc059e9..b1f7be50b4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -268,24 +268,20 @@ qemuSecurityChownCallback(const virStorageSource *src,
}
if (chown(src->path, uid, gid) < 0)
- goto cleanup;
- } else {
- if (!(cpy = virStorageSourceCopy(src, false)))
- goto cleanup;
-
- /* src file init reports errors, return -2 on failure */
- if (virStorageFileInit(cpy) < 0) {
- ret = -2;
- goto cleanup;
- }
+ return -1;
- if (virStorageFileChown(cpy, uid, gid) < 0)
- goto cleanup;
+ return 0;
}
- ret = 0;
+ if (!(cpy = virStorageSourceCopy(src, false)))
+ return -1;
+
+ /* src file init reports errors, return -2 on failure */
+ if (virStorageFileInit(cpy) < 0)
+ return -2;
+
+ ret = virStorageFileChown(cpy, uid, gid);
- cleanup:
save_errno = errno;
virStorageFileDeinit(cpy);
errno = save_errno;
--
2.24.1
6:18 a.m.
New subject: [PATCH 2/2] qemuSecurityChownCallback: Don't initialize storage file subsystem for local file
virStorageFileSupportsSecurityDriver ends up initializing the storage
file backend which after the recent changes to the daemon architecture
may end up dlopening of the backend modules.
Since this is required only for remote storage we can optimize the call
by moving the check whether the backend is supported to the branch which
deals with remote storage. This will prevent the pointless dlopen for
local files.
This is amplified by the fact that we now do most storage driver
operations in forks so the storage backends may not even be initialized
in the main daemon. On the other hand, not having gluster linked in the
main daemon is a big win, so we should keep only the forks having to
link with it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b1f7be50b4..3cbd900f7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -249,10 +249,6 @@ qemuSecurityChownCallback(const virStorageSource *src,
int rv;
g_autoptr(virStorageSource) cpy = NULL;
- rv = virStorageFileSupportsSecurityDriver(src);
- if (rv <= 0)
- return rv;
-
if (virStorageSourceIsLocalStorage(src)) {
/* use direct chown for local files so that the file doesn't
* need to be initialized */
@@ -273,6 +269,9 @@ qemuSecurityChownCallback(const virStorageSource *src,
return 0;
}
+ if ((rv = virStorageFileSupportsSecurityDriver(src)) <= 0)
+ return rv;
+
if (!(cpy = virStorageSourceCopy(src, false)))
return -1;
--
2.24.1
6:22 a.m.
New subject: [PATCH 2/2] qemuSecurityChownCallback: Don't initialize storage file subsystem for local file
On Thu, Mar 26, 2020 at 12:18:03 +0100, Peter Krempa wrote:
virStorageFileSupportsSecurityDriver ends up initializing the
storage
file backend which after the recent changes to the daemon architecture
may end up dlopening of the backend modules.
Since this is required only for remote storage we can optimize the call
by moving the check whether the backend is supported to the branch which
deals with remote storage. This will prevent the pointless dlopen for
local files.
This is amplified by the fact that we now do most storage driver
operations in forks so the storage backends may not even be initialized
in the main daemon. On the other hand, not having gluster linked in the
main daemon is a big win, so we should keep only the forks having to
link with it.
Actually, I'll redact this as standard startup of a VM still invokes
the storage file backends, so they will be linked eventually on normal
usage. I've observed this after libvirtd restart.
10:33 a.m.
On Thu, Mar 26, 2020 at 12:18:01PM +0100, Peter Krempa wrote:
See 2/2.
Peter Krempa (2):
qemuSecurityChownCallback: Remove 'cleanup' section
qemuSecurityChownCallback: Don't initialize storage file subsystem for
local file
src/qemu/qemu_driver.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
--
2.24.1
Reviewed-by: Pavel Mores <pmores(a)redhat.com>
1695
days inactive
1702
days old
4 comments
2 participants
participants (2)
-
Pavel Mores -
Peter Krempa