[libvirt] [PATCH 27/29] remote: switch to connect to per-driver daemons by default

[Michal Privoznik]

On 7/11/19 6:05 PM, Daniel P. Berrangé wrote:
> Historically URIs handled by the remote driver will always connect to
> the libvirtd UNIX socket. There will now be one daemon per driver, and
> each of these has its own UNIX sockets to connect to.
> 
> It will still be possible to run the traditional monolithic libvirtd too
> which will have the original UNIX socket path.
> 
> In addition there is a virproxyd daemon that doesn't run any drivers,
> but provides proxying for clients accessing libvirt over IP sockets, or
> tunnelling to the legacy libvirtd UNIX socket path.
> 
> Finally when running inside a daemon, the remote driver must not reject
> connections unconditionally. For example, the QEMU driver needs to be
> able to connect to the network driver. The remote driver must thus be
> willing to handle connections even when inside the daemon, provided no
> local driver is registered.
> 
> This refactoring causes the remote driver to prefer connecting to the
> per-driver daemons. The URI parameter "no_direct=1" can be used to
> force connecting to the legacy libvirtd UNIX socket.
> 
> The remote driver will only ever spawn the per-driver daemons, or
> the legacy libvirtd. It won't ever try to spawn virtproxyd, as
> that is only there for IP based connectivity, or for access from
> legacy remote clients.
> 
> The $HOME/.config/libvirt/libvirt.conf can also be set to have a
> parameter 'remote_direct=0|1' to hardcode the behaviour for all
> clients.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/driver.h               |   2 +
>   src/libvirt.c              |  24 +++++
>   src/remote/remote_driver.c | 202 +++++++++++++++++++++++++++++++++----
>   src/remote/remote_driver.h |   3 -
>   4 files changed, 207 insertions(+), 24 deletions(-)
> 
> diff --git a/src/driver.h b/src/driver.h
> index 898fb96df4..f7d667a03c 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -108,6 +108,8 @@ int virSetSharedNWFilterDriver(virNWFilterDriverPtr driver) ATTRIBUTE_RETURN_CHE
>   int virSetSharedSecretDriver(virSecretDriverPtr driver) ATTRIBUTE_RETURN_CHECK;
>   int virSetSharedStorageDriver(virStorageDriverPtr driver) ATTRIBUTE_RETURN_CHECK;
>   
> +bool virHasDriverForURIScheme(const char *scheme);
> +
>   int virDriverLoadModule(const char *name,
>                           const char *regfunc,
>                           bool required);
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 7e665b6cba..e21cad0e2e 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -601,6 +601,30 @@ virRegisterConnectDriver(virConnectDriverPtr driver,
>   }
>   
>   
> +/**
> + * virHasDriverForURIScheme:
> + * @scheme: the URI scheme
> + *
> + * Determine if there is a driver registered that explicitly
> + * handles URIs with the scheme @scheme.
> + *
> + * Returns: true if a driver is registered
> + */
> +bool virHasDriverForURIScheme(const char *scheme)
> +{
> +    size_t i, j;
> +    for (i = 0; i < virConnectDriverTabCount; i++) {
> +        if (!virConnectDriverTab[i]->uriSchemes)
> +            continue;
> +        for (j = 0; virConnectDriverTab[i]->uriSchemes[j]; j++) {
> +            if (STREQ(virConnectDriverTab[i]->uriSchemes[j], scheme))
> +                return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +
>   /**
>    * virRegisterStateDriver:
>    * @driver: pointer to a driver block
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index c6905dffff..98a165e163 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -72,6 +72,22 @@ VIR_ENUM_IMPL(remoteDriverTransport,
>                 REMOTE_DRIVER_TRANSPORT_LAST,
>                 "tls", "unix", "ssh", "libssh2", "ext", "tcp", "libssh");
>   
> +typedef enum {
> +    /* Prefer per-driver virt*d daemons, but fallback to legacy libvirtd */
> +    REMOTE_DRIVER_MODE_AUTO,
> +    /* Always use the legacy libvirtd */
> +    REMOTE_DRIVER_MODE_LEGACY,
> +    /* Always use the per-driver virt*d daemons */
> +    REMOTE_DRIVER_MODE_DIRECT,
> +
> +    REMOTE_DRIVER_MODE_LAST
> +} remoteDriverMode;
> +
> +VIR_ENUM_DECL(remoteDriverMode);
> +VIR_ENUM_IMPL(remoteDriverMode,
> +              REMOTE_DRIVER_MODE_LAST,
> +              "auto", "legacy", "direct");
> +
>   #if SIZEOF_LONG < 8
>   # define HYPER_TO_TYPE(_type, _to, _from) \
>       do { \
> @@ -92,6 +108,7 @@ VIR_ENUM_IMPL(remoteDriverTransport,
>   
>   static bool inside_daemon;
>   
> +
>   struct private_data {
>       virMutex lock;
>   
> @@ -740,8 +757,9 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
>   
>   
>   static char *
> -remoteGetUNIXSocket(remoteDriverTransport transport,
> -                    unsigned int flags)
> +remoteGetUNIXSocketHelper(remoteDriverTransport transport,
> +                          const char *sock_prefix,
> +                          unsigned int flags)
>   {
>       char *sockname = NULL;
>       VIR_AUTOFREE(char *userdir);
> @@ -758,21 +776,125 @@ remoteGetUNIXSocket(remoteDriverTransport transport,
>           if (!(userdir = virGetUserRuntimeDirectory()))
>               return NULL;
>   
> -        if (virAsprintf(&sockname,
> -                        "%s/" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0)
> +        if (virAsprintf(&sockname, "%s/%s-sock",
> +                        userdir, sock_prefix) < 0)
>               return NULL;
>       } else {
> -        if (VIR_STRDUP(sockname,
> -                       flags & VIR_DRV_OPEN_REMOTE_RO ?
> -                       LIBVIRTD_PRIV_UNIX_SOCKET_RO :
> -                       LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> +        if (virAsprintf(&sockname, "%s/run/libvirt/%s-%s",
> +                        LOCALSTATEDIR, sock_prefix,
> +                        flags & VIR_DRV_OPEN_REMOTE_RO ?
> +                        "sock-ro" : "sock") < 0)
>               return NULL;
>       }
>   
> -    VIR_DEBUG("Chosen UNIX sockname %s", sockname);
> +    VIR_DEBUG("Built UNIX sockname %s for transport %s prefix %s flags=0x%x",
> +              sockname, remoteDriverTransportTypeToString(transport),
> +              sock_prefix, flags);
>       return sockname;
>   }
>   
> +
> +#define DRIVER_SCHEME_CHRS "abcdefghijklmnopqrstuvwxyz"
> +static char *
> +remoteGetUNIXSocket(remoteDriverTransport transport,
> +                    remoteDriverMode mode,
> +                    const char *driver,
> +                    char **daemon,
> +                    unsigned int flags)
> +{
> +    char *sock_name = NULL;
> +    VIR_AUTOFREE(char *) direct_daemon = NULL;
> +    VIR_AUTOFREE(char *) legacy_daemon = NULL;
> +    VIR_AUTOFREE(char *) direct_sock_name = NULL;
> +    VIR_AUTOFREE(char *) legacy_sock_name = NULL;
> +
> +    if (strspn(driver, DRIVER_SCHEME_CHRS) < strlen(driver)) {

Note that if no connection URI was provided then @driver is going to be 
NULL here and thus calling strlen() over it is going to crash.

> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Invalid character in driver '%s'"), driver);
> +        return NULL;
> +    }

Michal