[libvirt PATCH] qemu: Allow sockets in long or deep paths.

[Daniel P. Berrangé]

On Tue, Apr 18, 2023 at 02:59:26AM -0400, Nick Guenther wrote:
> The qemu driver creates IPC sockets using absolute paths,
> but under POSIX socket paths are constrained pretty tightly.
> On systems with homedirs on an unusual mount point, like
> network homedirs, or just particularly long usernames, this
> could make starting VMs under qemu:///session impossible.
> 
> Resolves https://gitlab.com/libvirt/libvirt/-/issues/466

Example path from that bug

  /home/WAVES.EXAMPLE.ORG/u123456/.config/libvirt/qemu/channel/target/domain-11-alpinelinux3.14/org.qemu.guest_agent.0

IMHO the key problem here is not your $HOME location, but rather
libvirt being ridiculously verbose in the nested dir structuion

Looking at the pieces

 * /home/WAVES.EXAMPLE.ORG/u123456 ->  31 chars

   $HOME, we can't change this
   

 * /.config/libvirt/qemu -> 21 chars

   Libvirt's config directory scope to the driver, we don't
   want to change this

 * /channel/target/domain-11-alpinelinux3.14 => 42 chars

   Arbitrarily inventing nesting for the sockets, we can
   change at will

 * /org.qemu.guest_agent.0 -> 23 chars

   Either user specified, or matches the port name, ideally
   don't change this


So we've got 31 + 21 + 23 == 75 chars we don't want to /can't
change, but that leaves 33 to play with

I feel like having 'domain-' in the path is redudant as
everything under /channel is for a domain.  Having 'target'
also feels redundant to me.

We could use '/channel/tgt-11-alpinelinux3.14' == 31 chars
or just /channel/11-alpinelinux3.14 == 27 chars, which
gives extra scope for a longer $HOME.

> 
> Signed-off-by: Nick Guenther <nick.guenther at polymtl.ca>
> ---
>  src/qemu/qemu_command.c | 52 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4ca93bf3dc..3f180d5fb6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -53,6 +53,7 @@
>  #include "virmdev.h"
>  #include "virutil.h"
>  
> +#include <libgen.h>
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  
> @@ -4866,6 +4867,37 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev)
>      struct sockaddr_un addr;
>      socklen_t addrlen = sizeof(addr);
>      int fd;
> +    char* wd = NULL;
> +    char* socket_dir_c = NULL;
> +    char* socket_name_c = NULL;
> +    char* socket_dir = NULL;
> +    char* socket_name = NULL;
> +
> +    /* The path length is limited to what fits in sockaddr_un.
> +     * It's pretty short: 108 on Linux, and this is too easy to hit.
> +     * Work around this limit by using a *relative path*.
> +     *
> +     * background: https://stackoverflow.com/questions/34829600/why-is-the-maximal-path-length-allowed-for-unix-sockets-on-linux-108
> +     *
> +     * docker added a different workaround: https://github.com/moby/moby/pull/13408
> +     */
> +    if ((wd = getcwd(NULL, 0)) == NULL) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to get working directory"));
> +	goto error;
> +    }
> +
> +    socket_dir_c = strdup(dev->data.nix.path); // dirname edits the string given it, so it must be copied
> +    socket_name_c = strdup(dev->data.nix.path);
> +
> +    socket_dir = dirname(socket_dir_c);
> +    socket_name = basename(socket_name_c);
> +
> +    if (chdir(socket_dir) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to get change to socket directory"));
> +	goto error;
> +    }
>  
>      if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
>          virReportSystemError(errno, "%s",
> @@ -4875,10 +4907,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev)
>  
>      memset(&addr, 0, sizeof(addr));
>      addr.sun_family = AF_UNIX;
> -    if (virStrcpyStatic(addr.sun_path, dev->data.nix.path) < 0) {
> +    if (virStrcpyStatic(addr.sun_path, socket_name) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("UNIX socket path '%1$s' too long"),
> -                       dev->data.nix.path);
> +                       _("UNIX socket name '%1$s' too long"),
> +                       socket_name);
>          goto error;
>      }
>  
> @@ -4909,9 +4941,23 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev)
>      if (virFileUpdatePerm(dev->data.nix.path, 0002, 0664) < 0)
>          goto error;
>  
> +    /* restore working directory */
> +    if (chdir(wd) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to restore working directory"));
> +        goto error;
> +    }
> +
> +    free(socket_name_c);
> +    free(socket_dir_c);
> +    free(wd);
> +
>      return fd;
>  
>   error:
> +    if (socket_name_c != NULL) { free(socket_name_c); }
> +    if (socket_dir_c != NULL) { free(socket_dir_c); }
> +    if (wd != NULL) { free(wd); }
>      VIR_FORCE_CLOSE(fd);
>      return -1;
>  }
> -- 
> 2.34.1
> 

With 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 :|