[PATCH] qemu_slirp: Check if helper exists before fetching its capabilities

[Laine Stump]

On 10/13/20 9:32 AM, Michal Privoznik wrote:
> I've noticed when running libvirtd in the session mode that
> whenever I start a virtual machine the following error is printed
> into logs:
>
>    error : cannot execute binary /usr/bin/slirp-helper: No such file or directory
>
> The error message is produced in qemuSlirpNewForHelper() which
> does attempt to be a NO-OP if the helper doesn't exist by
> checking if its path is NULL, but that's not usually the case
> because in the default config (in virQEMUDriverConfigNew()) its
> path is initialized to QEMU_SLIRP_HELPER. And while it is true
> that during configure we try to get the actual path of the helper
> we fallback to the path above if not found.
>
> Fix the check so that the function checks whether the helper
> exists and is executable.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>   src/qemu/qemu_slirp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
> index d2e4ed79be..4e5ab12727 100644
> --- a/src/qemu/qemu_slirp.c
> +++ b/src/qemu/qemu_slirp.c
> @@ -101,7 +101,8 @@ qemuSlirpNewForHelper(const char *helper)
>       virJSONValuePtr featuresJSON;
>       size_t i, nfeatures;
>   
> -    if (!helper)
> +    if (!helper ||
> +        !virFileIsExecutable(helper))


I would prefer if either then entire expression was on a single line 
(since it's so short), or if you're going to put it on multiple lines, 
that you surroung the "return NULL;" with { .... } (I think the coding 
standards say to do this, but the syntax-check doesn't check it)

Reviewed-by: Laine Stump <laine at redhat.com>


>           return NULL;
>   
>       slirp = qemuSlirpNew();