virdnsmasq: Lookup DNSMASQ in PATH

While it's true that our virCommand subsystem is happy with
non-absolute paths, the dnsmasq capability code is not. It stores
the path to dnsmasq within and makes it accessible via
dnsmasqCapsGetBinaryPath(). While strictly speaking no caller
necessarily needs canonicalized path, let's find dnsmasq once and
cache the result.

Therefore, when constructing the capabilities structure look up
the binary path. If DNSMASQ already contains an absolute path
then virFindFileInPath() will simply return a copy.

With this code in place, the virFileIsExecutable() check can be
removed from dnsmasqCapsRefreshInternal() because
virFindFileInPath() already made sure the binary is executable.

But introducing virFindFileInPath() means we have to mock it in
test suite because dnsmasqCaps are created in
networkxml2conftest.

Moreover, we don't need to check for dnsmasq in configure.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>

diff --git a/meson.build b/meson.build
index c2e9619f1f4729f6829fcd4a0ba13aed7b3013db..70843afcd546537dc626b94961fad9bb643ec9ee 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -813,7 +813,6 @@ endforeach
 optional_programs = [
   'augparse',
   'dmidecode',
-  'dnsmasq',
   'ebtables',
   'flake8',
   'ip',

diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 5bed8817e5d0ca7d88aad78f0c9d5ca60964f205..579c67d86a2047b798df9cc8bedfeaa4137a2fbe 100644 (file)
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -46,6 +46,7 @@
 
 VIR_LOG_INIT("util.dnsmasq");
 
+#define DNSMASQ "dnsmasq"
 #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile"
 #define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
 
@@ -650,16 +651,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCaps *caps)
     g_autoptr(virCommand) vercmd = NULL;
     g_autofree char *version = NULL;
 
-    /* Make sure the binary we are about to try exec'ing exists.
-     * Technically we could catch the exec() failure, but that's
-     * in a sub-process so it's hard to feed back a useful error.
-     */
-    if (!virFileIsExecutable(caps->binaryPath)) {
-        virReportSystemError(errno, _("dnsmasq binary %s is not executable"),
-                             caps->binaryPath);
-        return -1;
-    }
-
     vercmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
     virCommandSetOutputBuffer(vercmd, &version);
     virCommandAddEnvPassCommon(vercmd);
@@ -679,7 +670,13 @@ dnsmasqCapsNewEmpty(void)
         return NULL;
     if (!(caps = virObjectNew(dnsmasqCapsClass)))
         return NULL;
-    caps->binaryPath = g_strdup(DNSMASQ);
+
+    if (!(caps->binaryPath = virFindFileInPath(DNSMASQ))) {
+        virReportSystemError(ENOENT, "%s",
+                             _("Unable to find 'dnsmasq' binary in $PATH"));
+        return NULL;
+    }
+
     return g_steal_pointer(&caps);
 }
 

diff --git a/tests/meson.build b/tests/meson.build
index 4792220b48d68710cda1fa97adecc6c337634902..8f92f2033f63eff7192d486af859102eedf43eb9 100644 (file)
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -59,6 +59,7 @@ mock_libs = [
   { 'name': 'domaincapsmock' },
   { 'name': 'shunload', 'sources': [ 'shunloadhelper.c' ] },
   { 'name': 'vircgroupmock' },
+  { 'name': 'virdnsmasqmock' },
   { 'name': 'virfilecachemock' },
   { 'name': 'virfirewallmock' },
   { 'name': 'virgdbusmock' },

diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 6a2c70ead19d65e792f6307457e2b0e09cd46351..13257c749b2142430e57184d9eac8f2f5f50fbb5 100644 (file)
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -158,4 +158,5 @@ mymain(void)
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-VIR_TEST_MAIN(mymain)
+VIR_TEST_MAIN_PRELOAD(mymain,
+                      VIR_TEST_MOCK("virdnsmasq"))

diff --git a/tests/virdnsmasqmock.c b/tests/virdnsmasqmock.c
new file mode 100644 (file)
index 0000000..448332e
--- /dev/null
+++ b/tests/virdnsmasqmock.c
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2022 Red Hat, Inc.
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include <config.h>
+
+#include "internal.h"
+#include "virfile.h"
+
+char *
+virFindFileInPath(const char *file)
+{
+    if (STREQ_NULLABLE(file, "dnsmasq"))
+        return g_strdup("/usr/sbin/dnsmasq");
+
+    /* We should not need any other binaries so return NULL. */
+    return NULL;
+}