[libvirt] [PATCH 37/41] remote: handle autoprobing of driver within virtproxyd

[Andrea Bolognani]

On Tue, 2019-07-23 at 17:03 +0100, Daniel P. Berrangé wrote:
[...]
> +++ b/src/remote/remote_daemon_dispatch.c
> +static int
> +remoteDispatchProbeURI(bool readonly, char **probeduri)

Once again, there should be one argument per line and a personal
preference I'd rather have readonly as the last one, but you can
safely ignore the latter comment if you feel otherwise.

[...]
> +        for (i = 0; i < ARRAY_CARDINALITY(drivers) && !*probeduri; i++) {
> +            VIR_AUTOFREE(char *)daemonname = NULL;

Missing space between "(char *)" and "daemonname".

> +            if (!(daemonname = virFileFindResource("virtqemud", "src", BINDIR)))
> +                return -1;

Err, no :)

First of all, you need to use virAsprintf() or similar to build the
name of the daemon, since the above will not work for the vbox
driver; additionally, we install the daemons in SBINDIR, so you need
to look for them there rather than in BINDIR.

[...]
> +            VIR_DEBUG("Probed URI %s via sock %s", *probeduri, daemonname);

s/sock/daemon/

[...]
> +    /* Even if we didn't probe any socket, we won't
> +     * return error. Just let virConnectOpen's normal
> +     * logic run which will likely return an error anyway
> +     */

I didn't verify this myself, but I hope that when you wrote "likely"
above what you really meant was "definitely", "certainly",
"absolutely", "without any doubt" or something along those lines ;)

[...]
> @@ -2065,6 +2186,9 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
>                            struct remote_connect_open_args *args)
>  {
>      const char *name;
> +#ifdef VIRTPROXYD
> +    VIR_AUTOFREE(char *)probeduri = NULL;
> +#endif

Missing space between "(char *)" and "probeduri".

-- 
Andrea Bolognani / Red Hat / Virtualization