[libvirt] [PATCH v4 39/42] remote: handle autoprobing of driver within virtproxyd

[Jim Fehlig]

On 8/8/19 9:10 AM, Daniel P. Berrangé  wrote:
> The virtproxyd daemon is merely responsible for forwarding RPC calls to
> one of the other per-driver daemons. As such, it does not have any
> drivers loaded and so regular auto-probing logic will not work. We need
> it to be able to handle NULL URIs though, so must implement some kind of
> alternative probing logic.
> 
> When running as root this is quite crude. If a per-driver daemon is
> running, its UNIX socket will exist and we can assume it will accept
> connections. If the per-driver daemon is not running, but socket
> autostart is enabled, we again just assume it will accept connections.
> 
> The is not great, however, because a default install may well have
> all sockets available for activation. IOW, the virtxend socket may
> exist, despite the fact that the libxl driver will not actually work.
> 
> When running as non-root this is slightly easier as we only have two
> drivers, QEMU and VirtualBox. These daemons will likely not be running
> and socket activation won't be used either, as libvirt spawns the
> daemon on demand. So we just check whether the daemon actually is
> installed.
> 
> Reviewed-by: Andrea Bolognani <abologna at redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/remote/Makefile.inc.am          |   1 +
>   src/remote/remote_daemon_dispatch.c | 138 ++++++++++++++++++++++++++++
>   2 files changed, 139 insertions(+)
> 
> diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
> index 0a3aef1ec1..09535ee6bb 100644
> --- a/src/remote/Makefile.inc.am
> +++ b/src/remote/Makefile.inc.am
> @@ -235,6 +235,7 @@ virtproxyd_CFLAGS = \
>   	-DSOCK_PREFIX="\"libvirt\"" \
>   	-DDAEMON_NAME="\"virtproxyd\"" \
>   	-DENABLE_IP \
> +	-DVIRTPROXYD \
>   	$(NULL)
>   virtproxyd_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS)
>   virtproxyd_LDADD = $(REMOTE_DAEMON_LD_ADD)
> diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
> index 7a66629d5b..c8e353ebd3 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -50,6 +50,7 @@
>   #include "viraccessapicheckqemu.h"
>   #include "virpolkit.h"
>   #include "virthreadjob.h"
> +#include "configmake.h"
>   
>   #define VIR_FROM_THIS VIR_FROM_RPC
>   
> @@ -2094,6 +2095,131 @@ void *remoteClientNew(virNetServerClientPtr client,
>   
>   /*----- Functions. -----*/
>   
> +#ifdef VIRTPROXYD
> +/*
> + * When running in virtproxyd regular auto-probing of drivers
> + * does not work as we don't have any drivers present (except
> + * stateless ones inside libvirt.so). All the interesting
> + * drivers are in separate daemons. Thus when we get a NULL
> + * URI we need to simulate probing that virConnectOpen would
> + * previously do. We use the existance of the UNIX domain
> + * socket as our hook for probing.
> + *
> + * This assumes no stale sockets left over from a now dead
> + * daemon, but that's reasonable since libvirtd unlinks
> + * sockets it creates on shutdown, or uses systemd activation
> + *
> + * We only try to probe for primary hypervisor drivers,
> + * not the secondary drivers.
> + */
> +static int
> +remoteDispatchProbeURI(bool readonly,
> +                       char **probeduri)
> +{
> +    *probeduri = NULL;
> +    VIR_DEBUG("Probing for driver daemon sockets");
> +
> +    /*
> +     * If running root, either the daemon is running and the socket
> +     * exists, or we're using socket activation so the socket exists
> +     * too.
> +     *
> +     * If running non-root, chances are that the daemon won't be
> +     * running, nor any socket activation is used. We need to
> +     * be able to auto-spawn the daemon. We thus just check to
> +     * see what daemons are installed. This is not a big deal as
> +     * only QEMU & VBox run as non-root, anyway.
> +     */
> +    if (geteuid() != 0) {
> +        /* Order these the same as virDriverLoadModule
> +         * calls in daemonInitialize */
> +        const char *drivers[] = {
> +# ifdef WITH_QEMU
> +            "qemu",
> +# endif
> +# ifdef WITH_VBOX
> +            "vbox",
> +# endif
> +        };
> +        size_t i;
> +
> +        for (i = 0; i < ARRAY_CARDINALITY(drivers) && !*probeduri; i++) {

FYI, while skimming through libvirt-related mail on xen-devel I noticed the 
following build failure

remote/remote_daemon_dispatch.c:2146:23: error: comparison of unsigned 
expression < 0 is always false [-Werror=type-limits]
          for (i = 0; i < ARRAY_CARDINALITY(drivers) && !*probeduri; i++) {
                        ^

Full log

http://logs.test-lab.xenproject.org/osstest/logs/140045/build-amd64-libvirt/6.ts-libvirt-build.log

Regards,
Jim