[PATCH v1] virdnsmasq: fix runtime search for executable
dnsmasq is an optional binary which does not neccessary exist during build. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- src/util/virdnsmasq.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index f2f606913f..06d192c99d 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -729,8 +729,26 @@ dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force) return ret; } +static char * +dnsmasqGetBinaryPath(void) +{ + static const char binary[] = DNSMASQ; + char *binary_path; + + if (g_path_is_absolute(binary)) + return g_strdup(binary); + + binary_path = virFindFileInPath(binary); + if (!binary_path) { + virReportSystemError(ENOENT, _("Cannot find '%s' in path"), binary); + binary_path = g_strdup(binary); + } + + return binary_path; +} + static dnsmasqCaps * -dnsmasqCapsNewEmpty(const char *binaryPath) +dnsmasqCapsNewEmpty(void) { dnsmasqCaps *caps; @@ -739,14 +757,14 @@ dnsmasqCapsNewEmpty(const char *binaryPath) if (!(caps = virObjectNew(dnsmasqCapsClass))) return NULL; caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST); - caps->binaryPath = g_strdup(binaryPath ? binaryPath : DNSMASQ); + caps->binaryPath = dnsmasqGetBinaryPath(); return caps; } dnsmasqCaps * dnsmasqCapsNewFromBuffer(const char *buf) { - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ); + dnsmasqCaps *caps = dnsmasqCapsNewEmpty(); if (!caps) return NULL; @@ -761,7 +779,7 @@ dnsmasqCapsNewFromBuffer(const char *buf) dnsmasqCaps * dnsmasqCapsNewFromBinary(void) { - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ); + dnsmasqCaps *caps = dnsmasqCapsNewEmpty(); if (!caps) return NULL; @@ -776,7 +794,7 @@ dnsmasqCapsNewFromBinary(void) const char * dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps) { - return caps ? caps->binaryPath : DNSMASQ; + return caps ? caps->binaryPath : dnsmasqGetBinaryPath(); } unsigned long
On Wed, Aug 11, 2021 at 11:47:13AM +0200, Olaf Hering wrote:
dnsmasq is an optional binary which does not neccessary exist during build.
Signed-off-by: Olaf Hering <olaf@aepfle.de> --- src/util/virdnsmasq.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
Is there any error or incorrect behavior that you are trying to fix? I did audit the code and within the virdnsmasq.c file we use the binaryPath with virCommandRun() which will do this same check before acutally execing, see virExec(). Pavel
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index f2f606913f..06d192c99d 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -729,8 +729,26 @@ dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force) return ret; }
+static char * +dnsmasqGetBinaryPath(void) +{ + static const char binary[] = DNSMASQ; + char *binary_path; + + if (g_path_is_absolute(binary)) + return g_strdup(binary); +
No need for this check, virFindFileInPath calls g_find_program_in_path which will do the correct thing if the path is already absolute.
+ binary_path = virFindFileInPath(binary); + if (!binary_path) { + virReportSystemError(ENOENT, _("Cannot find '%s' in path"), binary); + binary_path = g_strdup(binary); + } + + return binary_path; +} + static dnsmasqCaps * -dnsmasqCapsNewEmpty(const char *binaryPath) +dnsmasqCapsNewEmpty(void) { dnsmasqCaps *caps;
@@ -739,14 +757,14 @@ dnsmasqCapsNewEmpty(const char *binaryPath) if (!(caps = virObjectNew(dnsmasqCapsClass))) return NULL; caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST); - caps->binaryPath = g_strdup(binaryPath ? binaryPath : DNSMASQ); + caps->binaryPath = dnsmasqGetBinaryPath(); return caps; }
dnsmasqCaps * dnsmasqCapsNewFromBuffer(const char *buf) { - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ); + dnsmasqCaps *caps = dnsmasqCapsNewEmpty();
if (!caps) return NULL; @@ -761,7 +779,7 @@ dnsmasqCapsNewFromBuffer(const char *buf) dnsmasqCaps * dnsmasqCapsNewFromBinary(void) { - dnsmasqCaps *caps = dnsmasqCapsNewEmpty(DNSMASQ); + dnsmasqCaps *caps = dnsmasqCapsNewEmpty();
if (!caps) return NULL; @@ -776,7 +794,7 @@ dnsmasqCapsNewFromBinary(void) const char * dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps) { - return caps ? caps->binaryPath : DNSMASQ; + return caps ? caps->binaryPath : dnsmasqGetBinaryPath(); }
unsigned long
Tue, 17 Aug 2021 15:13:05 +0200 Pavel Hrdina <phrdina@redhat.com>:
On Wed, Aug 11, 2021 at 11:47:13AM +0200, Olaf Hering wrote:
dnsmasq is an optional binary which does not neccessary exist during build.
Signed-off-by: Olaf Hering <olaf@aepfle.de> --- src/util/virdnsmasq.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
Is there any error or incorrect behavior that you are trying to fix? I did audit the code and within the virdnsmasq.c file we use the binaryPath with virCommandRun() which will do this same check before acutally execing, see virExec().
The error was, and still is today, like this: Jan 10 12:53:36 rdma03 libvirtd[1725]: Cannot check dnsmasq binary dnsmasq: No such file or directory How do you intent to fix this? Olaf
On 1/10/22 14:29, Olaf Hering wrote:
Tue, 17 Aug 2021 15:13:05 +0200 Pavel Hrdina <phrdina@redhat.com>:
On Wed, Aug 11, 2021 at 11:47:13AM +0200, Olaf Hering wrote:
dnsmasq is an optional binary which does not neccessary exist during build.
Signed-off-by: Olaf Hering <olaf@aepfle.de> --- src/util/virdnsmasq.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
Is there any error or incorrect behavior that you are trying to fix? I did audit the code and within the virdnsmasq.c file we use the binaryPath with virCommandRun() which will do this same check before acutally execing, see virExec().
The error was, and still is today, like this:
Jan 10 12:53:36 rdma03 libvirtd[1725]: Cannot check dnsmasq binary dnsmasq: No such file or directory
How do you intent to fix this?
Olaf
Yeah, this is a real problem. I think I have a small, non-intrusive patch. Let me post it. Michal
On 1/10/22 16:21, Michal Prívozník wrote:
Yeah, this is a real problem. I think I have a small, non-intrusive patch. Let me post it.
Huh, so my patches turned out to be similar to yours. https://listman.redhat.com/archives/libvir-list/2022-January/msg00386.html Let me know if you want me to add your SoB line. Michal
participants (3)
-
Michal Prívozník -
Olaf Hering -
Pavel Hrdina