Re: generation of virtproxd socket files

2021-02-09 Thread Jim Fehlig

On 2/9/21 6:56 AM, Martin Kletzander wrote:

On Mon, Feb 08, 2021 at 04:09:16PM -0700, Jim Fehlig wrote:

Hi All,


Thanks to everyone for the hints!


I received a report [1] and verified that virtproxyd*.socket files have broken
syntax. E.g. from virtproxyd.socket

[Unit]
Description=Libvirt proxy local socket
Before=virtproxyd.service
libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket libvirtd-tcp.socket
libvirtd-tls.socket

virtproxyd.socket should 'Conflicts' with the libvirtd sockets. I suspect this
regressed in the switch to meson. I checked a libvirt 6.0.0 installation and
indeed it has

Conflicts=libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket
libvirtd-tcp.socket libvirtd-tls.socket



It looks like the issue might have various root causes as I am unsure myself
about what exactly was the idea behind some of the actions in the code.  But all
the .socket files are generated from `src/remote/libvirtd.socket.in` and the 
line which causes trouble for you is probably this one:


   
https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/remote/libvirtd.socket.in#L4 



This is used in `src/meson.build` to generate socket files for all the services:

   
https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/meson.build#L792-833

The proxy daemon service lists the other sockets as dependencies here (even 
though it uses variable clearly named `*_conflicts`):


   
https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/remote/meson.build#L233


and I'm not sure whether that should be prefixed with "Conflicts=" or whether
the `conflicts` field should come into play, although it looks like it is not
used anywhere else:

   https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/meson.build#L204


Yeah, afaict it is not used. I can send a separate patch to remove it.


So long story short: I do not have a right solution for you, but I hope this can
help you figure out where to go from here.


Yes, it was helpful. Thanks for all the details!


Actually, let me check if the `deps` are used anywhere else for a unit... yep,
it is, for virtxend and it actually uses it for the whole line it needs in that
context.  So maybe a simple patch like this could help?


Yes, I think adding the Conflicts= when the virtproxyd unit is defined is the 
better way to go. I've sent your patch :-)


https://listman.redhat.com/archives/libvir-list/2021-February/msg00607.html

Regards,
Jim




Re: generation of virtproxd socket files

2021-02-09 Thread Martin Kletzander

On Mon, Feb 08, 2021 at 04:09:16PM -0700, Jim Fehlig wrote:

Hi All,

I received a report [1] and verified that virtproxyd*.socket files have broken
syntax. E.g. from virtproxyd.socket

[Unit]
Description=Libvirt proxy local socket
Before=virtproxyd.service
libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket libvirtd-tcp.socket
libvirtd-tls.socket

virtproxyd.socket should 'Conflicts' with the libvirtd sockets. I suspect this
regressed in the switch to meson. I checked a libvirt 6.0.0 installation and
indeed it has

Conflicts=libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket
libvirtd-tcp.socket libvirtd-tls.socket



It looks like the issue might have various root causes as I am unsure myself
about what exactly was the idea behind some of the actions in the code.  But all
the .socket files are generated from `src/remote/libvirtd.socket.in` and the 
line which causes trouble for you is probably this one:

  
https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/remote/libvirtd.socket.in#L4

This is used in `src/meson.build` to generate socket files for all the services:

  
https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/meson.build#L792-833

The proxy daemon service lists the other sockets as dependencies here (even 
though it uses variable clearly named `*_conflicts`):

  
https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/remote/meson.build#L233

and I'm not sure whether that should be prefixed with "Conflicts=" or whether
the `conflicts` field should come into play, although it looks like it is not
used anywhere else:

  https://gitlab.com/libvirt/libvirt/-/blob/a619e28dba8c/src/meson.build#L204

So long story short: I do not have a right solution for you, but I hope this can
help you figure out where to go from here.

Actually, let me check if the `deps` are used anywhere else for a unit... yep,
it is, for virtxend and it actually uses it for the whole line it needs in that
context.  So maybe a simple patch like this could help?

diff --git i/src/remote/meson.build w/src/remote/meson.build
index 9ad2f6ab1c26..0a188268b58b 100644
--- i/src/remote/meson.build
+++ w/src/remote/meson.build
@@ -230,7 +230,7 @@ if conf.has('WITH_REMOTE')
   'name': 'Libvirt proxy',
   'sockprefix': 'libvirt',
   'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
-  'deps': libvirtd_socket_conflicts,
+  'deps': 'Conflicts=' + libvirtd_socket_conflicts,
 }

 openrc_init_files += {
--

I'm not well versed in the systemd unit files, so feel free to adjust for
whatever feels right.

I hope that helps, have a nice day.
Martin


I blame it on Monday and my mind stuck in the weekend, but I spent too much time
trying to figure out how those socket files are generated before writing this
mail. It would be much appreciated if someone can give me a nudge in the right
direction :-).

Regards,
Jim

[1] https://bugzilla.opensuse.org/show_bug.cgi?id=1181838



signature.asc
Description: PGP signature


Re: generation of virtproxd socket files

2021-02-09 Thread Daniel P . Berrangé
On Mon, Feb 08, 2021 at 04:09:16PM -0700, Jim Fehlig wrote:
> Hi All,
> 
> I received a report [1] and verified that virtproxyd*.socket files have
> broken syntax. E.g. from virtproxyd.socket
> 
> [Unit]
> Description=Libvirt proxy local socket
> Before=virtproxyd.service
> libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket libvirtd-tcp.socket
> libvirtd-tls.socket
> 
> virtproxyd.socket should 'Conflicts' with the libvirtd sockets. I suspect
> this regressed in the switch to meson. I checked a libvirt 6.0.0
> installation and indeed it has
> 
> Conflicts=libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket
> libvirtd-tcp.socket libvirtd-tls.socket
> 
> I blame it on Monday and my mind stuck in the weekend, but I spent too much
> time trying to figure out how those socket files are generated before
> writing this mail. It would be much appreciated if someone can give me a
> nudge in the right direction :-).

The original src/remote/Makefile.inc.am code  did

-e 's|[@]deps[@]|Conflicts=$(LIBVIRTD_SOCKET_UNIT_FILES)|g' \

but src/remote/meson.build just does

   libvirtd_socket_conflicts = ' '.join(libvirtd_socket_unit_files)

so missing the "Conflicts=" part.


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



Re: generation of virtproxd socket files

2021-02-09 Thread Christian Ehrhardt
On Tue, Feb 9, 2021 at 12:09 AM Jim Fehlig  wrote:
>
> Hi All,
>
> I received a report [1] and verified that virtproxyd*.socket files have broken
> syntax. E.g. from virtproxyd.socket
>
> [Unit]
> Description=Libvirt proxy local socket
> Before=virtproxyd.service
> libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket libvirtd-tcp.socket
> libvirtd-tls.socket
>
> virtproxyd.socket should 'Conflicts' with the libvirtd sockets. I suspect this
> regressed in the switch to meson. I checked a libvirt 6.0.0 installation and
> indeed it has
>
> Conflicts=libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket
> libvirtd-tcp.socket libvirtd-tls.socket
>
> I blame it on Monday and my mind stuck in the weekend, but I spent too much 
> time
> trying to figure out how those socket files are generated before writing this
> mail. It would be much appreciated if someone can give me a nudge in the right
> direction :-).

*point*
-> src/remote/meson.build
227 virt_daemon_units += {
228   'service': 'virtproxyd',
229   'service_in': files('virtproxyd.service.in'),
230   'name': 'Libvirt proxy',
231   'sockprefix': 'libvirt',
232   'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
233   'deps': libvirtd_socket_conflicts,
234 }

-> src/meson.build
 195 # virt_daemon_units:
 196 #   generate libvirt daemon systemd unit files
 197 #   * service - name of the service (required)
 198 #   * service_in - service source file (required)
 199 #   * name - socket description (required)
 200 #   * sockprefix - socket prefix name (required)
 201 #   * sockets - array of additional sockets (optional, default [
'main', 'ro', 'admin' ])
 202 #   * socket_$name_in - additional socket source files (optional,
default remote/libvirtd.socket.in )
 203 #   * deps - socket dependencies (optional, default '')
 204 #   * conflicts - if the service conflicts with libvirtd
(optional, true)
 205 virt_daemon_units = []
...
 792 foreach unit : virt_daemon_units
 793   unit_conf = configuration_data()
 794   unit_conf.set('runstatedir', runstatedir)
 795   unit_conf.set('sbindir', sbindir)
 796   unit_conf.set('sysconfdir', sysconfdir)
 797   unit_conf.set('name', unit['name'])
 798   unit_conf.set('service', unit['service'])
 799   unit_conf.set('sockprefix', unit['sockprefix'])
 800   unit_conf.set('deps', unit.get('deps', ''))
 801   if conf.has('WITH_POLKIT')
 802 unit_conf.set('mode', '0666')
 803   else
 804 unit_conf.set('mode', '0600')
 805   endif
...

Also see: 
https://gitlab.com/libvirt/libvirt/-/commit/dd4f2c73ad7f9fc0eae5325d5bf5786afd3a467e

So if not just an error/mistake somewhere, then setting
socket_$name_in and providing such a file with your needs could be a
start

> Regards,
> Jim
>
> [1] https://bugzilla.opensuse.org/show_bug.cgi?id=1181838
>


-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd



generation of virtproxd socket files

2021-02-08 Thread Jim Fehlig

Hi All,

I received a report [1] and verified that virtproxyd*.socket files have broken 
syntax. E.g. from virtproxyd.socket


[Unit]
Description=Libvirt proxy local socket
Before=virtproxyd.service
libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket libvirtd-tcp.socket 
libvirtd-tls.socket


virtproxyd.socket should 'Conflicts' with the libvirtd sockets. I suspect this 
regressed in the switch to meson. I checked a libvirt 6.0.0 installation and 
indeed it has


Conflicts=libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket 
libvirtd-tcp.socket libvirtd-tls.socket


I blame it on Monday and my mind stuck in the weekend, but I spent too much time 
trying to figure out how those socket files are generated before writing this 
mail. It would be much appreciated if someone can give me a nudge in the right 
direction :-).


Regards,
Jim

[1] https://bugzilla.opensuse.org/show_bug.cgi?id=1181838