[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RESEND] python/machine: Fix AF_UNIX path too long on macOS
From: |
Peter Delevoryas |
Subject: |
Re: [PATCH RESEND] python/machine: Fix AF_UNIX path too long on macOS |
Date: |
Wed, 6 Jul 2022 17:52:48 -0700 |
On Wed, Jul 06, 2022 at 09:46:48AM -0700, Peter Delevoryas wrote:
> On Wed, Jul 06, 2022 at 09:02:14AM +0100, Daniel P. Berrangé wrote:
> > 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.
>
> That's a good point.
>
> >
> > 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 ?
>
> Hmmm yeah it is odd, I'm not really sure why it's created there or if
> standard glibc tmpfile creation goes there too, I'll go experiment and
> report back. And yeah, maybe I'll double check any environment variables or
> other things.
>
> The macOS system I use happens to be a Facebook work laptop, which could
> also be related now that I think about it.
Hmmm yeah looks like this is because my TMPDIR is weird.
$ echo $TMPDIR
/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/
I didn't think to check this cause I wasn't familiar with TMPDIR. 🤷
Thanks for responding, I'll just use TMPDIR=/tmp for now. It's probably
something wrong with the Facebook development environment.
Peter
>
> >
> > > 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.
>
> True, yeah let me try to investigate the directory placement.
>
> >
> > 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.
>
> Oh good point, I hadn't thought about that.
>
> >
> > IMHO avocado should pass in the 'name' parameter explicitly, using a
> > plain name and thus get a shorter string.
>
> I see, yeah that makes sense. Maybe I can include a couple patches for this,
> one fixing the directory location, and one refactoring the temporary file
> name template (If I'm understanding your suggestion correctly).
>
> Thanks for the review! I really appreciate it.
> Peter
>
> >
> > > 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
> > :|
> >
>