All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/1] python/machine: Fix AF_UNIX path too long on macOS
@ 2022-07-22 18:25 Peter Delevoryas
  2022-07-22 18:25 ` [PATCH v3 1/1] " Peter Delevoryas
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Delevoryas @ 2022-07-22 18:25 UTC (permalink / raw)
  Cc: jsnow, crosa, bleal, f4bug, wainersm, qemu-devel, berrange, peter

v1: https://lore.kernel.org/qemu-devel/20220705214659.73369-1-peter@pjd.dev/
v2: https://lore.kernel.org/qemu-devel/20220716173434.17183-1-peter@pjd.dev/
v3:
    - Changed QEMUMachine._name to f"{id(self):x}". Suggestion was to do
      f"{id(self):02x}", but the id's look like they are probably just the
      object address (8-byte pointer), so the "02" had no effect.
    - Changed QMP socket name suffix from "-monitor.sock" to ".qmp".
    - Changed console socket name suffix from "-console.sock" to ".con".

Thanks for all the comments and suggestions! Glad to be fixing this.
Peter

Peter Delevoryas (1):
  python/machine: Fix AF_UNIX path too long on macOS

 python/qemu/machine/machine.py         | 6 +++---
 tests/avocado/avocado_qemu/__init__.py | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.37.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/1] python/machine: Fix AF_UNIX path too long on macOS
  2022-07-22 18:25 [PATCH v3 0/1] python/machine: Fix AF_UNIX path too long on macOS Peter Delevoryas
@ 2022-07-22 18:25 ` Peter Delevoryas
  2022-07-25  9:06   ` Daniel P. Berrangé
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Delevoryas @ 2022-07-22 18:25 UTC (permalink / raw)
  Cc: jsnow, crosa, bleal, f4bug, wainersm, qemu-devel, berrange, peter

On macOS, private $TMPDIR's are the default. These $TMPDIR's are
generated from a user's unix UID and UUID [1], which can create a
relatively long path:

    /var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/

QEMU's avocado tests create a temporary directory prefixed by
"avo_qemu_sock_", and create QMP sockets within _that_ as well.
The QMP socket is unnecessarily long, because a temporary directory
is created for every QEMUMachine object.

    /avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock

The path limit for unix sockets on macOS is 104: [2]

    /*
     * [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) */
    };

This results in avocado tests failing on macOS because the QMP unix
socket can't be created, because the path is too long:

    ERROR| Failed to establish connection: OSError: AF_UNIX path too long

This change resolves by reducing the size of the socket directory prefix
and the suffix on the QMP and console socket names.

The result is paths like this:

    pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T
    $ tree qemu*
    qemu_df4evjeq
    qemu_jbxel3gy
    qemu_ml9s_gg7
    qemu_oc7h7f3u
    qemu_oqb1yf97
    ├── 10a004050.con
    └── 10a004050.qmp

[1] https://apple.stackexchange.com/questions/353832/why-is-mac-osx-temp-directory-in-weird-path
[2] /Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include/sys/un.h

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 python/qemu/machine/machine.py         | 6 +++---
 tests/avocado/avocado_qemu/__init__.py | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 37191f433b..5df210c810 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"{id(self):x}"
         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}.qmp"
             )
 
         self._console_log_path = console_log
@@ -192,7 +192,7 @@ def __init__(self,
         self._console_set = False
         self._console_device_type: Optional[str] = None
         self._console_address = os.path.join(
-            self.sock_dir, f"{self._name}-console.sock"
+            self.sock_dir, f"{self._name}.con"
         )
         self._console_socket: Optional[socket.socket] = None
         self._remove_files: List[str] = []
diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index ed4853c805..43b8c8848c 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -296,7 +296,7 @@ def require_accelerator(self, accelerator):
                         "available" % accelerator)
 
     def _new_vm(self, name, *args):
-        self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
+        self._sd = tempfile.TemporaryDirectory(prefix="qemu_")
         vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir,
                          sock_dir=self._sd.name, log_dir=self.logdir)
         self.log.debug('QEMUMachine "%s" created', name)
-- 
2.37.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] python/machine: Fix AF_UNIX path too long on macOS
  2022-07-22 18:25 ` [PATCH v3 1/1] " Peter Delevoryas
@ 2022-07-25  9:06   ` Daniel P. Berrangé
  2023-01-09 21:09     ` John Snow
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel P. Berrangé @ 2022-07-25  9:06 UTC (permalink / raw)
  To: Peter Delevoryas; +Cc: jsnow, crosa, bleal, f4bug, wainersm, qemu-devel

On Fri, Jul 22, 2022 at 11:25:08AM -0700, Peter Delevoryas wrote:
> On macOS, private $TMPDIR's are the default. These $TMPDIR's are
> generated from a user's unix UID and UUID [1], which can create a
> relatively long path:
> 
>     /var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/
> 
> QEMU's avocado tests create a temporary directory prefixed by
> "avo_qemu_sock_", and create QMP sockets within _that_ as well.
> The QMP socket is unnecessarily long, because a temporary directory
> is created for every QEMUMachine object.
> 
>     /avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock
> 
> The path limit for unix sockets on macOS is 104: [2]
> 
>     /*
>      * [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) */
>     };
> 
> This results in avocado tests failing on macOS because the QMP unix
> socket can't be created, because the path is too long:
> 
>     ERROR| Failed to establish connection: OSError: AF_UNIX path too long
> 
> This change resolves by reducing the size of the socket directory prefix
> and the suffix on the QMP and console socket names.
> 
> The result is paths like this:
> 
>     pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T
>     $ tree qemu*
>     qemu_df4evjeq
>     qemu_jbxel3gy
>     qemu_ml9s_gg7
>     qemu_oc7h7f3u
>     qemu_oqb1yf97
>     ├── 10a004050.con
>     └── 10a004050.qmp
> 
> [1] https://apple.stackexchange.com/questions/353832/why-is-mac-osx-temp-directory-in-weird-path
> [2] /Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include/sys/un.h
> 
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> ---
>  python/qemu/machine/machine.py         | 6 +++---
>  tests/avocado/avocado_qemu/__init__.py | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] python/machine: Fix AF_UNIX path too long on macOS
  2022-07-25  9:06   ` Daniel P. Berrangé
@ 2023-01-09 21:09     ` John Snow
  2023-01-10  8:35       ` Peter Delevoryas
  0 siblings, 1 reply; 5+ messages in thread
From: John Snow @ 2023-01-09 21:09 UTC (permalink / raw)
  To: Peter Delevoryas
  Cc: Daniel P. Berrangé, crosa, bleal, f4bug, wainersm, qemu-devel

On Mon, Jul 25, 2022 at 5:06 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Jul 22, 2022 at 11:25:08AM -0700, Peter Delevoryas wrote:
> > On macOS, private $TMPDIR's are the default. These $TMPDIR's are
> > generated from a user's unix UID and UUID [1], which can create a
> > relatively long path:
> >
> >     /var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/
> >
> > QEMU's avocado tests create a temporary directory prefixed by
> > "avo_qemu_sock_", and create QMP sockets within _that_ as well.
> > The QMP socket is unnecessarily long, because a temporary directory
> > is created for every QEMUMachine object.
> >
> >     /avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock
> >
> > The path limit for unix sockets on macOS is 104: [2]
> >
> >     /*
> >      * [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) */
> >     };
> >
> > This results in avocado tests failing on macOS because the QMP unix
> > socket can't be created, because the path is too long:
> >
> >     ERROR| Failed to establish connection: OSError: AF_UNIX path too long
> >
> > This change resolves by reducing the size of the socket directory prefix
> > and the suffix on the QMP and console socket names.
> >
> > The result is paths like this:
> >
> >     pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T
> >     $ tree qemu*
> >     qemu_df4evjeq
> >     qemu_jbxel3gy
> >     qemu_ml9s_gg7
> >     qemu_oc7h7f3u
> >     qemu_oqb1yf97
> >     ├── 10a004050.con
> >     └── 10a004050.qmp
> >
> > [1] https://apple.stackexchange.com/questions/353832/why-is-mac-osx-temp-directory-in-weird-path
> > [2] /Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include/sys/un.h
> >
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > ---
> >  python/qemu/machine/machine.py         | 6 +++---
> >  tests/avocado/avocado_qemu/__init__.py | 2 +-
> >  2 files changed, 4 insertions(+), 4 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

My apologies, I missed this update because it appeared in a thread
underneath the old version.

Peter, may I please ask for you to kindly re-submit this patch with an
incremented version number?

--js



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] python/machine: Fix AF_UNIX path too long on macOS
  2023-01-09 21:09     ` John Snow
@ 2023-01-10  8:35       ` Peter Delevoryas
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Delevoryas @ 2023-01-10  8:35 UTC (permalink / raw)
  To: John Snow
  Cc: Daniel P. Berrangé, crosa, bleal, f4bug, wainersm, qemu-devel

On Mon, Jan 09, 2023 at 04:09:32PM -0500, John Snow wrote:
> On Mon, Jul 25, 2022 at 5:06 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Fri, Jul 22, 2022 at 11:25:08AM -0700, Peter Delevoryas wrote:
> > > On macOS, private $TMPDIR's are the default. These $TMPDIR's are
> > > generated from a user's unix UID and UUID [1], which can create a
> > > relatively long path:
> > >
> > >     /var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T/
> > >
> > > QEMU's avocado tests create a temporary directory prefixed by
> > > "avo_qemu_sock_", and create QMP sockets within _that_ as well.
> > > The QMP socket is unnecessarily long, because a temporary directory
> > > is created for every QEMUMachine object.
> > >
> > >     /avo_qemu_sock_uh3w_dgc/qemu-37331-10bacf110-monitor.sock
> > >
> > > The path limit for unix sockets on macOS is 104: [2]
> > >
> > >     /*
> > >      * [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) */
> > >     };
> > >
> > > This results in avocado tests failing on macOS because the QMP unix
> > > socket can't be created, because the path is too long:
> > >
> > >     ERROR| Failed to establish connection: OSError: AF_UNIX path too long
> > >
> > > This change resolves by reducing the size of the socket directory prefix
> > > and the suffix on the QMP and console socket names.
> > >
> > > The result is paths like this:
> > >
> > >     pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T
> > >     $ tree qemu*
> > >     qemu_df4evjeq
> > >     qemu_jbxel3gy
> > >     qemu_ml9s_gg7
> > >     qemu_oc7h7f3u
> > >     qemu_oqb1yf97
> > >     ├── 10a004050.con
> > >     └── 10a004050.qmp
> > >
> > > [1] https://apple.stackexchange.com/questions/353832/why-is-mac-osx-temp-directory-in-weird-path
> > > [2] /Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include/sys/un.h
> > >
> > > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > > ---
> > >  python/qemu/machine/machine.py         | 6 +++---
> > >  tests/avocado/avocado_qemu/__init__.py | 2 +-
> > >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> My apologies, I missed this update because it appeared in a thread
> underneath the old version.

Oh, that's ok, nice catch digging this up! I completely forgot about it, and
haven't been developing QEMU stuff on macOS in a little while

> 
> Peter, may I please ask for you to kindly re-submit this patch with an
> incremented version number?

Sure! I've resubmitted it as v4 and v5, v4 might not have been sent correctly
(I have so many problems with sending emails lol)

v4: https://lore.kernel.org/qemu-devel/20230110080756.38271-1-peter@pjd.dev/T/#t
v5: < will probably show up on the mailing list soon >


- Peter

> 
> --js
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-10  9:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-22 18:25 [PATCH v3 0/1] python/machine: Fix AF_UNIX path too long on macOS Peter Delevoryas
2022-07-22 18:25 ` [PATCH v3 1/1] " Peter Delevoryas
2022-07-25  9:06   ` Daniel P. Berrangé
2023-01-09 21:09     ` John Snow
2023-01-10  8:35       ` Peter Delevoryas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.