[libvirt PATCH] meson: Fix libvirtd|virtproxyd socket prefixes

Erik Skultety posted 1 patch 3 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b4b8435754221113e1f33ca56618498b57e31938.1596526219.git.eskultet@redhat.com
src/remote/meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[libvirt PATCH] meson: Fix libvirtd|virtproxyd socket prefixes
Posted by Erik Skultety 3 years, 7 months ago
For the daemons in question the correct socket prefix is "libvirt-",
not "libvirtd-".

Fixes: dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/remote/meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/remote/meson.build b/src/remote/meson.build
index 25712c943b..5983238a0a 100644
--- a/src/remote/meson.build
+++ b/src/remote/meson.build
@@ -184,7 +184,7 @@ if conf.has('WITH_REMOTE')
       'service': 'libvirtd',
       'service_in': files('libvirtd.service.in'),
       'name': 'Libvirt',
-      'sockprefix': 'libvirtd',
+      'sockprefix': 'libvirt',
       'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
     }
 
@@ -218,7 +218,7 @@ if conf.has('WITH_REMOTE')
       'service': 'virtproxyd',
       'service_in': files('virtproxyd.service.in'),
       'name': 'Libvirt proxy',
-      'sockprefix': 'libvirtd',
+      'sockprefix': 'libvirt',
       'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
       'deps': libvirtd_socket_conflicts,
     }
-- 
2.26.2

Re: [libvirt PATCH] meson: Fix libvirtd|virtproxyd socket prefixes
Posted by Pavel Hrdina 3 years, 7 months ago
On Tue, Aug 04, 2020 at 09:30:42AM +0200, Erik Skultety wrote:
> For the daemons in question the correct socket prefix is "libvirt-",
> not "libvirtd-".
> 
> Fixes: dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/remote/meson.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/remote/meson.build b/src/remote/meson.build
> index 25712c943b..5983238a0a 100644
> --- a/src/remote/meson.build
> +++ b/src/remote/meson.build
> @@ -184,7 +184,7 @@ if conf.has('WITH_REMOTE')
>        'service': 'libvirtd',
>        'service_in': files('libvirtd.service.in'),
>        'name': 'Libvirt',
> -      'sockprefix': 'libvirtd',
> +      'sockprefix': 'libvirt',
>        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
>      }
>  
> @@ -218,7 +218,7 @@ if conf.has('WITH_REMOTE')
>        'service': 'virtproxyd',
>        'service_in': files('virtproxyd.service.in'),
>        'name': 'Libvirt proxy',
> -      'sockprefix': 'libvirtd',
> +      'sockprefix': 'libvirt',
>        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
>        'deps': libvirtd_socket_conflicts,

How did you figure out that the prefix is libvirt? It's not correct.
Check the autoconf version or even in your fedora:

    rpm -ql libvirt-daemon | grep ".*\.socket"

and you will see that it's libvirtd.

NACK

Pavel
Re: [libvirt PATCH] meson: Fix libvirtd|virtproxyd socket prefixes
Posted by Pavel Hrdina 3 years, 7 months ago
On Tue, Aug 04, 2020 at 09:48:58AM +0200, Pavel Hrdina wrote:
> On Tue, Aug 04, 2020 at 09:30:42AM +0200, Erik Skultety wrote:
> > For the daemons in question the correct socket prefix is "libvirt-",
> > not "libvirtd-".
> > 
> > Fixes: dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e
> > 
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  src/remote/meson.build | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/remote/meson.build b/src/remote/meson.build
> > index 25712c943b..5983238a0a 100644
> > --- a/src/remote/meson.build
> > +++ b/src/remote/meson.build
> > @@ -184,7 +184,7 @@ if conf.has('WITH_REMOTE')
> >        'service': 'libvirtd',
> >        'service_in': files('libvirtd.service.in'),
> >        'name': 'Libvirt',
> > -      'sockprefix': 'libvirtd',
> > +      'sockprefix': 'libvirt',
> >        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
> >      }
> >  
> > @@ -218,7 +218,7 @@ if conf.has('WITH_REMOTE')
> >        'service': 'virtproxyd',
> >        'service_in': files('virtproxyd.service.in'),
> >        'name': 'Libvirt proxy',
> > -      'sockprefix': 'libvirtd',
> > +      'sockprefix': 'libvirt',
> >        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
> >        'deps': libvirtd_socket_conflicts,
> 
> How did you figure out that the prefix is libvirt? It's not correct.
> Check the autoconf version or even in your fedora:
> 
>     rpm -ql libvirt-daemon | grep ".*\.socket"
> 
> and you will see that it's libvirtd.

So I double checked is as it looked strange to me and even though the
service and the systemd socket files use libvirtd the runtime unix
socket has libvirt as prefix. Sigh.

So the patch is correct, sorry about the noise.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Re: [libvirt PATCH] meson: Fix libvirtd|virtproxyd socket prefixes
Posted by Erik Skultety 3 years, 7 months ago
On Tue, Aug 04, 2020 at 10:00:19AM +0200, Pavel Hrdina wrote:
> On Tue, Aug 04, 2020 at 09:48:58AM +0200, Pavel Hrdina wrote:
> > On Tue, Aug 04, 2020 at 09:30:42AM +0200, Erik Skultety wrote:
> > > For the daemons in question the correct socket prefix is "libvirt-",
> > > not "libvirtd-".
> > > 
> > > Fixes: dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e
> > > 
> > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > ---
> > >  src/remote/meson.build | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/remote/meson.build b/src/remote/meson.build
> > > index 25712c943b..5983238a0a 100644
> > > --- a/src/remote/meson.build
> > > +++ b/src/remote/meson.build
> > > @@ -184,7 +184,7 @@ if conf.has('WITH_REMOTE')
> > >        'service': 'libvirtd',
> > >        'service_in': files('libvirtd.service.in'),
> > >        'name': 'Libvirt',
> > > -      'sockprefix': 'libvirtd',
> > > +      'sockprefix': 'libvirt',
> > >        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
> > >      }
> > >  
> > > @@ -218,7 +218,7 @@ if conf.has('WITH_REMOTE')
> > >        'service': 'virtproxyd',
> > >        'service_in': files('virtproxyd.service.in'),
> > >        'name': 'Libvirt proxy',
> > > -      'sockprefix': 'libvirtd',
> > > +      'sockprefix': 'libvirt',
> > >        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
> > >        'deps': libvirtd_socket_conflicts,
> > 
> > How did you figure out that the prefix is libvirt? It's not correct.
> > Check the autoconf version or even in your fedora:
> > 
> >     rpm -ql libvirt-daemon | grep ".*\.socket"
> > 
> > and you will see that it's libvirtd.
> 
> So I double checked is as it looked strange to me and even though the
> service and the systemd socket files use libvirtd the runtime unix
> socket has libvirt as prefix. Sigh.

Well, historical reasons... we couldn't have renamed it with the modular
daemon introduction, because that would have been a regression.

> 
> So the patch is correct, sorry about the noise.
> 
> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Thank you.

Erik

Re: [libvirt PATCH] meson: Fix libvirtd|virtproxyd socket prefixes
Posted by Erik Skultety 3 years, 7 months ago
On Tue, Aug 04, 2020 at 10:00:19AM +0200, Pavel Hrdina wrote:
> On Tue, Aug 04, 2020 at 09:48:58AM +0200, Pavel Hrdina wrote:
> > On Tue, Aug 04, 2020 at 09:30:42AM +0200, Erik Skultety wrote:
> > > For the daemons in question the correct socket prefix is "libvirt-",
> > > not "libvirtd-".
> > > 
> > > Fixes: dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e
> > > 
> > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > ---
> > >  src/remote/meson.build | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/remote/meson.build b/src/remote/meson.build
> > > index 25712c943b..5983238a0a 100644
> > > --- a/src/remote/meson.build
> > > +++ b/src/remote/meson.build
> > > @@ -184,7 +184,7 @@ if conf.has('WITH_REMOTE')
> > >        'service': 'libvirtd',
> > >        'service_in': files('libvirtd.service.in'),
> > >        'name': 'Libvirt',
> > > -      'sockprefix': 'libvirtd',
> > > +      'sockprefix': 'libvirt',
> > >        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
> > >      }
> > >  
> > > @@ -218,7 +218,7 @@ if conf.has('WITH_REMOTE')
> > >        'service': 'virtproxyd',
> > >        'service_in': files('virtproxyd.service.in'),
> > >        'name': 'Libvirt proxy',
> > > -      'sockprefix': 'libvirtd',
> > > +      'sockprefix': 'libvirt',
> > >        'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
> > >        'deps': libvirtd_socket_conflicts,
> > 
> > How did you figure out that the prefix is libvirt? It's not correct.
> > Check the autoconf version or even in your fedora:
> > 
> >     rpm -ql libvirt-daemon | grep ".*\.socket"
> > 
> > and you will see that it's libvirtd.
> 
> So I double checked is as it looked strange to me and even though the
> service and the systemd socket files use libvirtd the runtime unix
> socket has libvirt as prefix. Sigh.
> 
> So the patch is correct, sorry about the noise.

You know what? Your initial response pointed out that the commit message was
not sufficient in explaining why the change was necessary and you had to
dig further, so I'll reword the commit message accordingly to provide
more details.

Erik