1:01 p.m.
This removes the need to have them present in the build environment
and makes things more flexible.
Note that we currently *do not* have the helpers available in most CI
environments, or have BuildRequires for them in the spec file. That
only works for Fedora and RHEL because the hardcoded fallback paths
happen to match those used on those distributions: everywhere else,
the choice is to either ensure that the additional packages are
installed in the build environment or to produce a build of libvirt
that can't use the corresponding features out of the box.
Andrea Bolognani (5):
util: Small refactor
util: Introduce virFileFindInPathFull()
qemu: Find helpers at runtime
meson: Stop looking for QEMU helpers
qemu: Update documentation for qemu.conf keys
meson.build | 24 ---------------
src/libvirt_private.syms | 1 +
src/qemu/qemu.conf.in | 8 +++--
src/qemu/qemu_conf.c | 4 +++
src/qemu/qemu_interface.c | 15 ++++++++--
src/qemu/qemu_process.c | 17 ++++++++---
src/qemu/test_libvirtd_qemu.aug.in | 4 +--
src/util/virfile.c | 47 ++++++++++++++++++++++++++----
src/util/virfile.h | 3 ++
9 files changed, 83 insertions(+), 40 deletions(-)
--
2.40.1
1:01 p.m.
New subject: [libvirt PATCH 1/5] util: Small refactor
Prepare for further changes.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/util/virfile.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 228482e8f8..8e94d19e45 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1743,13 +1743,15 @@ virFindFileInPath(const char *file)
return NULL;
path = g_find_program_in_path(file);
- if (!path)
- return NULL;
- /* Workaround for a bug in g_find_program_in_path() not returning absolute
- * path as documented. TODO drop it once we require GLib >= 2.69.0
- */
- return g_canonicalize_filename(path, NULL);
+ if (path) {
+ /* Workaround for a bug in g_find_program_in_path() not returning absolute
+ * path as documented. TODO drop it once we require GLib >= 2.69.0
+ */
+ return g_canonicalize_filename(path, NULL);
+ }
+
+ return NULL;
}
--
2.40.1
1:01 p.m.
New subject: [libvirt PATCH 2/5] util: Introduce virFileFindInPathFull()
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virfile.c | 33 +++++++++++++++++++++++++++++++++
src/util/virfile.h | 3 +++
3 files changed, 37 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 62c296bf5f..27b8e111aa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2358,6 +2358,7 @@ virFileWrapperFdFree;
virFileWrapperFdNew;
virFileWriteStr;
virFindFileInPath;
+virFindFileInPathFull;
# util/virfilecache.h
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 8e94d19e45..6b4d4c3522 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1737,6 +1737,25 @@ virFileIsLink(const char *linkpath)
*/
char *
virFindFileInPath(const char *file)
+{
+ return virFindFileInPathFull(file, NULL);
+}
+
+/* virFindFileInPathFull:
+ * @file: name of the program
+ * @extraDirs: NULL-terminated list of additional directories
+ *
+ * Like virFindFileInPath(), but in addition to searching $PATH also
+ * looks into all directories listed in @extraDirs. This is useful to
+ * locate helpers that are installed outside of $PATH.
+ *
+ * The returned path must be freed by the caller.
+ *
+ * Returns: absolute path of the program or NULL
+ */
+char *
+virFindFileInPathFull(const char *file,
+ const char *const *extraDirs)
{
g_autofree char *path = NULL;
if (file == NULL)
@@ -1751,6 +1770,20 @@ virFindFileInPath(const char *file)
return g_canonicalize_filename(path, NULL);
}
+ if (extraDirs) {
+ while (*extraDirs) {
+ g_autofree char *extraPath = NULL;
+
+ extraPath = g_strdup_printf("%s/%s", *extraDirs, file);
+
+ if (virFileIsExecutable(extraPath)) {
+ return g_steal_pointer(&extraPath);
+ }
+
+ extraDirs++;
+ }
+ }
+
return NULL;
}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index f7a31d9f57..6a14173625 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -189,6 +189,9 @@ int virFileIsLink(const char *linkpath)
char *virFindFileInPath(const char *file)
G_NO_INLINE;
+char *virFindFileInPathFull(const char *file,
+ const char *const *extraDirs)
+ G_NO_INLINE;
char *virFileFindResource(const char *filename,
const char *builddir,
--
2.40.1
1:01 p.m.
New subject: [libvirt PATCH 3/5] qemu: Find helpers at runtime
Use the recently introduced virFindFileInPathFull() function to
discover the path for qemu-bridge-helper and qemu-pr-helper at
runtime.
Note that it's still possible for the administrator to prevent
this lookup and use arbitrary binaries by setting the
appropriate keys in qemu.conf: this simply removes the need to
perform the lookup at build time, and thus to have the helpers
installed in the build environment.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_interface.c | 15 +++++++++++++--
src/qemu/qemu_process.c | 17 +++++++++++++----
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index e395bfcc5b..e875de48ee 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -327,7 +327,14 @@ qemuCreateInBridgePortWithHelper(virQEMUDriverConfig *cfg,
int *tapfd,
unsigned int flags)
{
+ const char *const bridgeHelperDirs[] = {
+ "/usr/libexec",
+ "/usr/lib/qemu",
+ "/usr/lib",
+ NULL,
+ };
g_autoptr(virCommand) cmd = NULL;
+ g_autofree char *bridgeHelperPath = NULL;
char *errbuf = NULL, *cmdstr = NULL;
int pair[2] = { -1, -1 };
@@ -339,13 +346,17 @@ qemuCreateInBridgePortWithHelper(virQEMUDriverConfig *cfg,
return -1;
}
- if (!virFileIsExecutable(cfg->bridgeHelperName)) {
+ bridgeHelperPath = virFindFileInPathFull(cfg->bridgeHelperName,
bridgeHelperDirs);
+
+ if (!bridgeHelperPath) {
virReportSystemError(errno, _("'%1$s' is not a suitable bridge
helper"),
cfg->bridgeHelperName);
return -1;
}
- cmd = virCommandNew(cfg->bridgeHelperName);
+ VIR_DEBUG("Using qemu-bridge-helper: %s", bridgeHelperPath);
+
+ cmd = virCommandNew(bridgeHelperPath);
if (flags & VIR_NETDEV_TAP_CREATE_VNET_HDR)
virCommandAddArgFormat(cmd, "--use-vnet");
virCommandAddArgFormat(cmd, "--br=%s", brname);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 952814d663..bb4cf35226 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2818,10 +2818,15 @@ qemuProcessStartPRDaemonHook(void *opaque)
int
qemuProcessStartManagedPRDaemon(virDomainObj *vm)
{
+ const char *const prHelperDirs[] = {
+ "/usr/libexec",
+ NULL,
+ };
qemuDomainObjPrivate *priv = vm->privateData;
virQEMUDriver *driver = priv->driver;
g_autoptr(virQEMUDriverConfig) cfg = NULL;
int errfd = -1;
+ g_autofree char *prHelperPath = NULL;
g_autofree char *pidfile = NULL;
g_autofree char *socketPath = NULL;
pid_t cpid = -1;
@@ -2832,12 +2837,16 @@ qemuProcessStartManagedPRDaemon(virDomainObj *vm)
cfg = virQEMUDriverGetConfig(driver);
- if (!virFileIsExecutable(cfg->prHelperName)) {
+ prHelperPath = virFindFileInPathFull(cfg->prHelperName, prHelperDirs);
+
+ if (!prHelperPath) {
virReportSystemError(errno, _("'%1$s' is not a suitable pr
helper"),
cfg->prHelperName);
goto cleanup;
}
+ VIR_DEBUG("Using qemu-pr-helper: %s", prHelperPath);
+
if (!(pidfile = qemuProcessBuildPRHelperPidfilePath(vm)))
goto cleanup;
@@ -2853,7 +2862,7 @@ qemuProcessStartManagedPRDaemon(virDomainObj *vm)
goto cleanup;
}
- if (!(cmd = virCommandNewArgList(cfg->prHelperName,
+ if (!(cmd = virCommandNewArgList(prHelperPath,
"-k", socketPath,
NULL)))
goto cleanup;
@@ -2881,7 +2890,7 @@ qemuProcessStartManagedPRDaemon(virDomainObj *vm)
if (virPidFileReadPath(pidfile, &cpid) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("pr helper %1$s didn't show up"),
- cfg->prHelperName);
+ prHelperPath);
goto cleanup;
}
@@ -2899,7 +2908,7 @@ qemuProcessStartManagedPRDaemon(virDomainObj *vm)
if (saferead(errfd, errbuf, sizeof(errbuf) - 1) < 0) {
virReportSystemError(errno,
_("pr helper %1$s died unexpectedly"),
- cfg->prHelperName);
+ prHelperPath);
} else {
virReportError(VIR_ERR_OPERATION_FAILED,
_("pr helper died and reported: %1$s"), errbuf);
--
2.40.1
1:01 p.m.
New subject: [libvirt PATCH 4/5] meson: Stop looking for QEMU helpers
Now that we're performing the lookup at runtime, doing it at
build time is no longer necessary.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
meson.build | 24 ------------------------
src/qemu/qemu_conf.c | 4 ++++
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/meson.build b/meson.build
index 24f52113fa..6e65e9eec3 100644
--- a/meson.build
+++ b/meson.build
@@ -1655,30 +1655,6 @@ if not get_option('driver_qemu').disabled()
conf.set_quoted('QEMU_USER', qemu_user)
conf.set_quoted('QEMU_GROUP', qemu_group)
- qemu_bridge_prog = find_program(
- 'qemu-bridge-helper',
- dirs: [ '/usr/libexec', '/usr/lib/qemu', '/usr/lib' ],
- required: false
- )
- if qemu_bridge_prog.found()
- qemu_bridge_path = qemu_bridge_prog.full_path()
- else
- qemu_bridge_path = '/usr/libexec/qemu-bridge-helper'
- endif
- conf.set_quoted('QEMU_BRIDGE_HELPER', qemu_bridge_path)
-
- qemu_pr_prog = find_program(
- 'qemu-pr-helper',
- dirs: [ '/usr/bin', '/usr/libexec' ],
- required: false
- )
- if qemu_pr_prog.found()
- qemu_pr_path = qemu_pr_prog.full_path()
- else
- qemu_pr_path = '/usr/bin/qemu-pr-helper'
- endif
- conf.set_quoted('QEMU_PR_HELPER', qemu_pr_path)
-
qemu_slirp_prog = find_program(
'slirp-helper',
dirs: [ '/usr/bin', '/usr/libexec' ],
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d9460d82e0..c76ae7ac93 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -97,6 +97,10 @@ VIR_ONCE_GLOBAL_INIT(virQEMUConfig);
#endif
+#define QEMU_BRIDGE_HELPER "qemu-bridge-helper"
+#define QEMU_PR_HELPER "qemu-pr-helper"
+
+
virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
const char *root)
{
--
2.40.1
1:01 p.m.
New subject: [libvirt PATCH 5/5] qemu: Update documentation for qemu.conf keys
Reflect the new default value, and explain that a runtime
lookup will be performed if the value is not an absolute path.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu.conf.in | 8 ++++++--
src/qemu/test_libvirtd_qemu.aug.in | 4 ++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index 3895d42514..e1a9a5e56d 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -650,7 +650,9 @@
# is used to create <source type='bridge'> interfaces when libvirtd is
# running unprivileged. libvirt invokes the helper directly, instead
# of using "-netdev bridge", for security reasons.
-#bridge_helper = "/usr/libexec/qemu-bridge-helper"
+# If this is not an absolute path, the program will be searched for
+# in $PATH as well as a few additional directories.
+#bridge_helper = "qemu-bridge-helper"
# If enabled, libvirt will have QEMU set its process name to
@@ -899,7 +901,9 @@
# Path to the SCSI persistent reservations helper. This helper is
# used whenever <reservations/> are enabled for SCSI LUN devices.
-#pr_helper = "/usr/bin/qemu-pr-helper"
+# If this is not an absolute path, the program will be searched for
+# in $PATH as well as a few additional directories.
+#pr_helper = "qemu-pr-helper"
# Path to the SLIRP networking helper.
#slirp_helper = "/usr/bin/slirp-helper"
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 1dbd692921..af99331886 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -75,7 +75,7 @@ module Test_libvirtd_qemu =
{ "auto_dump_bypass_cache" = "0" }
{ "auto_start_bypass_cache" = "0" }
{ "hugetlbfs_mount" = "/dev/hugepages" }
-{ "bridge_helper" = "/usr/libexec/qemu-bridge-helper" }
+{ "bridge_helper" = "qemu-bridge-helper" }
{ "set_process_name" = "1" }
{ "max_processes" = "0" }
{ "max_files" = "0" }
@@ -107,7 +107,7 @@ module Test_libvirtd_qemu =
{ "1" = "mount" }
}
{ "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
-{ "pr_helper" = "/usr/bin/qemu-pr-helper" }
+{ "pr_helper" = "qemu-pr-helper" }
{ "slirp_helper" = "/usr/bin/slirp-helper" }
{ "dbus_daemon" = "/usr/bin/dbus-daemon" }
{ "swtpm_user" = "tss" }
--
2.40.1
3:28 a.m.
On Fri, May 05, 2023 at 08:01:41PM +0200, Andrea Bolognani wrote:
This removes the need to have them present in the build environment
and makes things more flexible.
Note that we currently *do not* have the helpers available in most CI
environments, or have BuildRequires for them in the spec file. That
only works for Fedora and RHEL because the hardcoded fallback paths
happen to match those used on those distributions: everywhere else,
the choice is to either ensure that the additional packages are
installed in the build environment or to produce a build of libvirt
that can't use the corresponding features out of the box.
Andrea Bolognani (5):
util: Small refactor
util: Introduce virFileFindInPathFull()
qemu: Find helpers at runtime
meson: Stop looking for QEMU helpers
qemu: Update documentation for qemu.conf keys
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
meson.build | 24 ---------------
src/libvirt_private.syms | 1 +
src/qemu/qemu.conf.in | 8 +++--
src/qemu/qemu_conf.c | 4 +++
src/qemu/qemu_interface.c | 15 ++++++++--
src/qemu/qemu_process.c | 17 ++++++++---
src/qemu/test_libvirtd_qemu.aug.in | 4 +--
src/util/virfile.c | 47 ++++++++++++++++++++++++++----
src/util/virfile.h | 3 ++
9 files changed, 83 insertions(+), 40 deletions(-)
--
2.40.1
596
days inactive
601
days old
6 comments
2 participants
participants (2)
-
Andrea Bolognani -
Martin Kletzander