Re: [PATCH v2 1/1] qemu: Allow sockets in long or deep paths.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/28/23 07:14, 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
> 
> Signed-off-by: Nick Guenther <nick.guenther@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c | 72 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 57 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4ca93bf3dc..4bedbb515f 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4860,12 +4860,64 @@ qemuBuildSCSIHostdevDevProps(const virDomainDef *def,
>      return g_steal_pointer(&props);
>  }
>  
> +struct qemuBindSocketData {
> +    int fd;
> +    char* path;
> +};
> +
> +static int
> +qemuBindSocket(pid_t ppid G_GNUC_UNUSED, void *opaque)
> +{
> +    /* The path length of a unix socket 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*, by chdir()ing first.
> +     * But chdir() isn't thread-safe, so run it in a *subprocess* (this function)
> +     * where the chdir() will be instantly forgotten once it has helped configure fd.
> +     *
> +     * background: https://stackoverflow.com/questions/34829600/why-is-the-maximal-path-length-allowed-for-unix-sockets-on-linux-108
> +     */
> +
> +    g_autofree char *dir = NULL;
> +    g_autofree char *name = NULL;
> +
> +    struct sockaddr_un addr;
> +    struct qemuBindSocketData *data = opaque;
> +
> +    dir = g_path_get_dirname(data->path);
> +    name = g_path_get_basename(data->path);
> +
> +    memset(&addr, 0, sizeof(addr));
> +    addr.sun_family = AF_UNIX;
> +    if (virStrcpyStatic(addr.sun_path, name) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("UNIX socket name '%1$s' too long"),
> +                       name);
> +        return -1;
> +    }
> +
> +    if (chdir(dir) < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to chdir to containing directory '%1$s' while binding UNIX socket '%2$s'"),
> +                             dir, name);
> +        return -1;
> +    }
> +
> +    if (bind(data->fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to bind UNIX socket '%1$s/%2$s'"),
> +                             dir, name);
> +        return -1;
> +    }

This only fixes the part that creates the socket. In some cases, libvirt
still needs to connect to them (e.g. monitor/guest agent sockets). That
code path still suffers from the limitation.

And for other sockets, mgmt apps are expected to connect to them. And
surely we shouldn't expect them to do this kind of tricks only to
connect.

I believe these commits solve the problem better:

https://gitlab.com/MichalPrivoznik/libvirt/-/commits/qemu_long_unix_paths/

I haven't sent them yet, wanted to wait for your v2. Can you please give
them a test since you have the env set up? Thanks,

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux