Re: [PATCH RESEND] python/machine: Fix AF_UNIX path too long on macOS

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jul 05, 2022 at 02:46:59PM -0700, Peter Delevoryas wrote:
> I noticed that I can't run any avocado tests on macOS because the QMP
> unix socket path is too long:


> I think the path limit for unix sockets on macOS might be 104 [1]

All platforms have a very limited path limit, so it isn't really
a macOS specific problem, rather....

> 
> /*
>  * [XSI] Definitions for UNIX IPC domain.
>  */
> struct  sockaddr_un {
>     unsigned char   sun_len;        /* sockaddr len including null */
>     sa_family_t     sun_family;     /* [XSI] AF_UNIX */
>     char            sun_path[104];  /* [XSI] path name (gag) */
> };
> 
> The path we're using is exactly 105 characters:
> 
> $ python
> Python 2.7.10 (default, Jan 19 2016, 22:24:01)
> [GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> len('/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock')

It is a problem related to where the test suite is creating the
paths.

/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/avo_qemu_sock_uh3w_dgc/

is way too deep a directory location.

It seems we just create this location using 'tempfile.TemporyDirectory'
to get a standard tmp dir.

Do you know why python is choosing

  /var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/

as the temp dir ? Is that a standard location on macOS or is it
from some env variable you have set ?

> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 37191f433b..93451774e3 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -157,7 +157,7 @@ def __init__(self,
>          self._wrapper = wrapper
>          self._qmp_timer = qmp_timer
>  
> -        self._name = name or f"qemu-{os.getpid()}-{id(self):02x}"
> +        self._name = name or f"{os.getpid()}{id(self):02x}"

I don't think this is the right fix really, because IMHO the problem
is the hugely long path, rather than the final socket name.

That said, there is redundancy in the path - avocado is passing in
a dierctory created using 'tempfile.TemporyDirectory' so there is no
reason why we need to add more entropy via the POD and the 'id(self)'
hex string.

IMHO avocado should pass in the 'name' parameter explicitly, using a
plain name and thus get a shorter string.

>          self._temp_dir: Optional[str] = None
>          self._base_temp_dir = base_temp_dir
>          self._sock_dir = sock_dir
> @@ -167,7 +167,7 @@ def __init__(self,
>              self._monitor_address = monitor_address
>          else:
>              self._monitor_address = os.path.join(
> -                self.sock_dir, f"{self._name}-monitor.sock"
> +                self.sock_dir, f"{self._name}.sock"
>              )
>  
>          self._console_log_path = console_log
> -- 
> 2.37.0
> 
> 

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