| Message ID | 20220716173434.17183-2-peter@pjd.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | python/machine: Fix AF_UNIX path too long on macOS | expand |
On Sat, Jul 16, 2022 at 10:34:34AM -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 Looking at this again, I realize my suggestion for dealing with the second part of the path was mistaken. The "qemu-37331-10bacf110-monitor.sock" part is combining two pieces. First the result of f"qemu-{os.getpid()}-{id(self):02x}" is qemu-37331-10bacf110 and the code later than appends '-monitor.sock' So... > > 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 reduces the size of both paths, and removes the unique > identification information from the socket name, since it seems to be > unnecessary. > > This commit produces paths like the following: > > pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T > $ tree qemu* > qemu_oc7h7f3u > ├── qmp-console.sock > └── qmp-monitor.sock > > [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 | 2 +- > tests/avocado/avocado_qemu/__init__.py | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py > index 37191f433b..b1823966b3 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 "qmp" ...my suggestion here was wrong. We don't need the os.getpid() unoiqueness because the tmpdir already ensures that is safe, but keeping 'id(self)' is a good idea, if the test case creates multiple machines concurrently. Bearing in mind we later append '-monitor.sock' we don't need 'qmp' in the self._name. So on reflection I think I should have suggested using: self._name = name or f"{id(self):02x}" And *in addition*, a few lines later change: self._monitor_address = os.path.join( self.sock_dir, f"{self._name}-monitor.sock" ) To self._monitor_address = os.path.join( self.sock_dir, f"{self._name}.qmp" ) > 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_") This bit is fine. > 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) With regards, Daniel
On Mon, Jul 18, 2022 at 09:56:17AM +0100, Daniel P. Berrangé wrote: > On Sat, Jul 16, 2022 at 10:34:34AM -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 > > > Looking at this again, I realize my suggestion for dealing with the > second part of the path was mistaken. > > The "qemu-37331-10bacf110-monitor.sock" part is combining two > pieces. > > First the result of > > f"qemu-{os.getpid()}-{id(self):02x}" > > is > > qemu-37331-10bacf110 > > and the code later than appends '-monitor.sock' > > So... > > > > > 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 reduces the size of both paths, and removes the unique > > identification information from the socket name, since it seems to be > > unnecessary. > > > > This commit produces paths like the following: > > > > pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T > > $ tree qemu* > > qemu_oc7h7f3u > > ├── qmp-console.sock > > └── qmp-monitor.sock > > > > [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 | 2 +- > > tests/avocado/avocado_qemu/__init__.py | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py > > index 37191f433b..b1823966b3 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 "qmp" > > ...my suggestion here was wrong. > > We don't need the os.getpid() unoiqueness because the tmpdir already > ensures that is safe, but keeping 'id(self)' is a good idea, if the > test case creates multiple machines concurrently. Bearing in mind we > later append '-monitor.sock' we don't need 'qmp' in the self._name. Yeah you're right, it's a good idea to make the QEMUMachine name unique. When I made v2, I looked around to see if we were creating 2 QEMUMachine's concurrently. I didn't see anything, but looking again, I see this in tests/migration/guestperf/engine.py: if self._dst_host != "localhost": dstmonaddr = ("localhost", 9001) else: dstmonaddr = "/var/tmp/qemu-dst-%d-monitor.sock" % os.getpid() srcmonaddr = "/var/tmp/qemu-src-%d-monitor.sock" % os.getpid() src = QEMUMachine(self._binary, args=self._get_src_args(hardware), wrapper=self._get_src_wrapper(hardware), name="qemu-src-%d" % os.getpid(), monitor_address=srcmonaddr) dst = QEMUMachine(self._binary, args=self._get_dst_args(hardware, uri), wrapper=self._get_dst_wrapper(hardware), name="qemu-dst-%d" % os.getpid(), monitor_address=dstmonaddr) This test case would probably still pass with this change, because the monitor_address is specified, but I think you're right, it's a good idea to make the default name different for each QEMUMachine instance to avoid breaking anything. > > So on reflection I think I should have suggested using: > > self._name = name or f"{id(self):02x}" +1, yeah I'll change it to that > > And *in addition*, a few lines later change: > > self._monitor_address = os.path.join( > self.sock_dir, f"{self._name}-monitor.sock" > ) > > To > > self._monitor_address = os.path.join( > self.sock_dir, f"{self._name}.qmp" > ) Oh ok, yeah that looks good too. Thanks for the detailed review and suggestions! Peter > > > > 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_") > > This bit is fine. +1 > > > 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) > > 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 :| >
On Mon, Jul 18, 2022 at 09:56:17AM +0100, Daniel P. Berrangé wrote: > On Sat, Jul 16, 2022 at 10:34:34AM -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 > > > Looking at this again, I realize my suggestion for dealing with the > second part of the path was mistaken. > > The "qemu-37331-10bacf110-monitor.sock" part is combining two > pieces. > > First the result of > > f"qemu-{os.getpid()}-{id(self):02x}" > > is > > qemu-37331-10bacf110 > > and the code later than appends '-monitor.sock' > > So... > > > > > 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 reduces the size of both paths, and removes the unique > > identification information from the socket name, since it seems to be > > unnecessary. > > > > This commit produces paths like the following: > > > > pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T > > $ tree qemu* > > qemu_oc7h7f3u > > ├── qmp-console.sock > > └── qmp-monitor.sock > > > > [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 | 2 +- > > tests/avocado/avocado_qemu/__init__.py | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py > > index 37191f433b..b1823966b3 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 "qmp" > > ...my suggestion here was wrong. > > We don't need the os.getpid() unoiqueness because the tmpdir already > ensures that is safe, but keeping 'id(self)' is a good idea, if the > test case creates multiple machines concurrently. Bearing in mind we > later append '-monitor.sock' we don't need 'qmp' in the self._name. > > So on reflection I think I should have suggested using: > > self._name = name or f"{id(self):02x}" > > And *in addition*, a few lines later change: > > self._monitor_address = os.path.join( > self.sock_dir, f"{self._name}-monitor.sock" > ) > > To > > self._monitor_address = os.path.join( > self.sock_dir, f"{self._name}.qmp" > ) > Finally getting back to this (sorry, been working on other stuff), and I noticed the console socket is just below this: self._console_address = os.path.join( self.sock_dir, f"{self._name}-console.sock" ) So I probably shouldn't do the "-monitor.sock" change right? That's ok though, the other changes are sufficient to fix the problem. I'll give you a chance to reply to this email, or else I'll just send a v3 that doesn't include the -monitor.sock change. Thanks, Peter > > > > 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_") > > This bit is fine. > > > 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) > > 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 :| >
On Thu, Jul 21, 2022 at 07:44:21PM -0700, Peter Delevoryas wrote: > On Mon, Jul 18, 2022 at 09:56:17AM +0100, Daniel P. Berrangé wrote: > > On Sat, Jul 16, 2022 at 10:34:34AM -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 > > > > > > Looking at this again, I realize my suggestion for dealing with the > > second part of the path was mistaken. > > > > The "qemu-37331-10bacf110-monitor.sock" part is combining two > > pieces. > > > > First the result of > > > > f"qemu-{os.getpid()}-{id(self):02x}" > > > > is > > > > qemu-37331-10bacf110 > > > > and the code later than appends '-monitor.sock' > > > > So... > > > > > > > > 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 reduces the size of both paths, and removes the unique > > > identification information from the socket name, since it seems to be > > > unnecessary. > > > > > > This commit produces paths like the following: > > > > > > pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T > > > $ tree qemu* > > > qemu_oc7h7f3u > > > ├── qmp-console.sock > > > └── qmp-monitor.sock > > > > > > [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 | 2 +- > > > tests/avocado/avocado_qemu/__init__.py | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py > > > index 37191f433b..b1823966b3 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 "qmp" > > > > ...my suggestion here was wrong. > > > > We don't need the os.getpid() unoiqueness because the tmpdir already > > ensures that is safe, but keeping 'id(self)' is a good idea, if the > > test case creates multiple machines concurrently. Bearing in mind we > > later append '-monitor.sock' we don't need 'qmp' in the self._name. > > > > So on reflection I think I should have suggested using: > > > > self._name = name or f"{id(self):02x}" > > > > And *in addition*, a few lines later change: > > > > self._monitor_address = os.path.join( > > self.sock_dir, f"{self._name}-monitor.sock" > > ) > > > > To > > > > self._monitor_address = os.path.join( > > self.sock_dir, f"{self._name}.qmp" > > ) > > > > Finally getting back to this (sorry, been working on other stuff), and I noticed > the console socket is just below this: > > self._console_address = os.path.join( > self.sock_dir, f"{self._name}-console.sock" > ) > > So I probably shouldn't do the "-monitor.sock" change right? I'd suggest changing this one to f"{self._name}.con" at the same time. With regards, Daniel
On Fri, Jul 22, 2022 at 08:20:11AM +0100, Daniel P. Berrangé wrote: > On Thu, Jul 21, 2022 at 07:44:21PM -0700, Peter Delevoryas wrote: > > On Mon, Jul 18, 2022 at 09:56:17AM +0100, Daniel P. Berrangé wrote: > > > On Sat, Jul 16, 2022 at 10:34:34AM -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 > > > > > > > > > Looking at this again, I realize my suggestion for dealing with the > > > second part of the path was mistaken. > > > > > > The "qemu-37331-10bacf110-monitor.sock" part is combining two > > > pieces. > > > > > > First the result of > > > > > > f"qemu-{os.getpid()}-{id(self):02x}" > > > > > > is > > > > > > qemu-37331-10bacf110 > > > > > > and the code later than appends '-monitor.sock' > > > > > > So... > > > > > > > > > > > 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 reduces the size of both paths, and removes the unique > > > > identification information from the socket name, since it seems to be > > > > unnecessary. > > > > > > > > This commit produces paths like the following: > > > > > > > > pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T > > > > $ tree qemu* > > > > qemu_oc7h7f3u > > > > ├── qmp-console.sock > > > > └── qmp-monitor.sock > > > > > > > > [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 | 2 +- > > > > tests/avocado/avocado_qemu/__init__.py | 2 +- > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py > > > > index 37191f433b..b1823966b3 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 "qmp" > > > > > > ...my suggestion here was wrong. > > > > > > We don't need the os.getpid() unoiqueness because the tmpdir already > > > ensures that is safe, but keeping 'id(self)' is a good idea, if the > > > test case creates multiple machines concurrently. Bearing in mind we > > > later append '-monitor.sock' we don't need 'qmp' in the self._name. > > > > > > So on reflection I think I should have suggested using: > > > > > > self._name = name or f"{id(self):02x}" > > > > > > And *in addition*, a few lines later change: > > > > > > self._monitor_address = os.path.join( > > > self.sock_dir, f"{self._name}-monitor.sock" > > > ) > > > > > > To > > > > > > self._monitor_address = os.path.join( > > > self.sock_dir, f"{self._name}.qmp" > > > ) > > > > > > > Finally getting back to this (sorry, been working on other stuff), and I noticed > > the console socket is just below this: > > > > self._console_address = os.path.join( > > self.sock_dir, f"{self._name}-console.sock" > > ) > > > > So I probably shouldn't do the "-monitor.sock" change right? > > I'd suggest changing this one to f"{self._name}.con" at the > same time. Ohh ok, yeah that's nice and short. Ok I'll include the socket name suffix changes then. > > > 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 :| >
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 37191f433b..b1823966b3 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 "qmp" self._temp_dir: Optional[str] = None self._base_temp_dir = base_temp_dir self._sock_dir = sock_dir 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)
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 reduces the size of both paths, and removes the unique identification information from the socket name, since it seems to be unnecessary. This commit produces paths like the following: pdel@pdel-mbp:/var/folders/d7/rz20f6hd709c1ty8f6_6y_z40000gn/T $ tree qemu* qemu_oc7h7f3u ├── qmp-console.sock └── qmp-monitor.sock [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 | 2 +- tests/avocado/avocado_qemu/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)