On Tue, Jan 11, 2022 at 09:28:45AM +0100, Michal Prívozník wrote:
> On 1/10/22 18:41, Andrea Bolognani wrote:
> > On Mon, Jan 10, 2022 at 04:44:55PM +0100, Michal Privoznik wrote:
> >> While it's true that our virCommand subsystem is happy with
> >> non-absolute paths, the dnsmasq capability code is not. For
> >> instance, it does call stat() over the binary to learn its mtime
> >> (and thus decide whether capabilities need to be fetched again or
> >> not).
> >>
> >> Therefore, when constructing the capabilities structure look up
> >> the binary path. If DNSMASQ already contains an absolute path
> >> then it is returned (and virFindFileInPath() is a NOP).
> >
> > Saying that virFindFileInPath() is a NOP is not quite correct: if you
> > pass an absolute path, it will return a copy. So I'd rewrite the
> > above as
> >
> > If DNSMASQ already contains an absolute path then
> > virFindFileInPath() will simply return a copy.
> >
>
> Yes, this makes sense.
>
> > Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> >
>
> Thanks, but as I was looking through virFindFileInPath() I've noticed
> that if the file exists but is not executable then NULL is returned, so
> we will need a fallback case. Effectively this needs to be squashed in:
Why do we need a fallback ? If someone has put 'dnsmasq' in $PATH
without the execute bit set, surely that's just a broken deployment
and returning NULL is correct.
> diff --git c/src/util/virdnsmasq.c w/src/util/virdnsmasq.c
> index b6ccb9d0a4..7792a65bc3 100644
> --- c/src/util/virdnsmasq.c
> +++ w/src/util/virdnsmasq.c
> @@ -703,12 +703,17 @@ static dnsmasqCaps *
> dnsmasqCapsNewEmpty(void)
> {
> dnsmasqCaps *caps;
> + g_autofree char *binaryPath = NULL;
>
> if (dnsmasqCapsInitialize() < 0)
> return NULL;
> if (!(caps = virObjectNew(dnsmasqCapsClass)))
> return NULL;
> - caps->binaryPath = virFindFileInPath(DNSMASQ);
> +
> + if (!(binaryPath = virFindFileInPath(DNSMASQ)))
> + binaryPath = g_strdup(DNSMASQ);
> +
> + caps->binaryPath = g_steal_pointer(&binaryPath);
> return caps;
> }
>
> I'll post v2 of this patch shortly.
>
> Michal
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
