[libvirt PATCH 00/42] systemd: Improve units for services and sockets
A grab bag of changes, ranging from very much functional ones to purely aesthetical ones. Patches 01-05 fix a few inconsistencies that the units for virtlogd/virtlockd had compared to the ones for all other services. Patches 06-16 prepare the stage for the mass conversion of unit files to set of common templates; patches 17-32 are where the conversion actually happen. After this point, most units are based on the common templates, which will hopefully make it less painful to maintain them going forward. The remaining patches should prove this point. Patches 33-34 reorganize conflict relationships so that the nastiest interactions between modular daemons and libvirtd can no longer happen. Patches 35-42 contain a few additional improvements and cleanups. Andrea Bolognani (42): systemd: Add missing Also for admin socket systemd: Add missing WantedBy for virtlogd/virtlockd systemd: Add missing Service for virtlogd/virtlockd systemd: Set Type=notify for virtlogd/virtlockd systemd: Set @name@ for virtlogd/virtlockd systemd: Rename socket_in_def -> socket_in_default systemd: Rename @mode@ -> @sockmode@ systemd: Only set @sockmode@ once systemd: Drop unnecessary uses of @sockets@ systemd: Make @sockprefix@ optional systemd: Drop unnecessary uses of @sockprefix@ systemd: Make @service_in@ optional systemd: Introduce temporary libvirtd_socket*_in values systemd: Provide all input files explicitly systemd: Introduce common templates systemd: Use common templates by default systemd: Switch virtnodedevd to common templates systemd: Switch virtinterfaced to common templates systemd: Switch virtnwfilterd to common templates systemd: Switch virtsecretd to common templates systemd: Switch virtnetworkd to common templates systemd: Switch virtstoraged to common templates systemd: Switch virtproxyd to common templates systemd: Switch virtvboxd to common templates systemd: Switch virtvzd to common templates systemd: Switch virtchd to common templates systemd: Switch virtxend to common templates systemd: Switch virtlxcd to common templates systemd: Switch virtqemud to common templates systemd: Drop libvirtd_socket*_in values systemd: Drop @deps@ systemd: Drop parametrization from libvirtd sockets systemd: Drop Conflicts from virtproxyd sockets systemd: Make modular daemons conflict with libvirtd systemd: Replace Requires with BindTo+After for sockets systemd: Augment Requires/Wants with After systemd: Drop Before=libvirtd from virtlogd/virtlockd systemd: Drop Before=foo.service from sockets systemd: Add Also between sockets systemd: Drop BindTo/After between sockets systemd: Improve and unify unit descriptions systemd: Move Documentation lines src/ch/meson.build | 28 ++++++++++++--- src/ch/virtchd.service.in | 44 ----------------------- src/interface/meson.build | 5 +-- src/interface/virtinterfaced.service.in | 25 ------------- src/libxl/meson.build | 23 +++++++++--- src/libxl/virtxend.service.in | 32 ----------------- src/locking/meson.build | 3 +- src/locking/virtlockd-admin.socket.in | 6 ++-- src/locking/virtlockd.service.in | 14 +++++--- src/locking/virtlockd.socket.in | 5 +-- src/logging/meson.build | 3 +- src/logging/virtlogd-admin.socket.in | 6 ++-- src/logging/virtlogd.service.in | 14 +++++--- src/logging/virtlogd.socket.in | 5 +-- src/lxc/meson.build | 28 ++++++++++++--- src/lxc/virtlxcd.service.in | 44 ----------------------- src/meson.build | 46 ++++++++++++++---------- src/network/meson.build | 8 ++--- src/network/virtnetworkd.service.in | 26 -------------- src/node_device/meson.build | 5 +-- src/node_device/virtnodedevd.service.in | 25 ------------- src/nwfilter/meson.build | 5 +-- src/nwfilter/virtnwfilterd.service.in | 25 ------------- src/qemu/meson.build | 36 ++++++++++++++++--- src/qemu/virtqemud.service.in | 48 ------------------------- src/remote/libvirtd-admin.socket.in | 12 +++---- src/remote/libvirtd-ro.socket.in | 12 +++---- src/remote/libvirtd-tcp.socket.in | 8 ++--- src/remote/libvirtd-tls.socket.in | 8 ++--- src/remote/libvirtd.service.in | 14 +++++--- src/remote/libvirtd.socket.in | 12 +++---- src/remote/meson.build | 13 +++---- src/remote/virtproxyd.service.in | 25 ------------- src/secret/meson.build | 5 +-- src/secret/virtsecretd.service.in | 25 ------------- src/storage/meson.build | 9 ++--- src/storage/virtstoraged.service.in | 27 -------------- src/vbox/meson.build | 8 ++--- src/vbox/virtvboxd.service.in | 26 -------------- src/virtd-admin.socket.in | 17 +++++++++ src/virtd-ro.socket.in | 17 +++++++++ src/virtd-tcp.socket.in | 14 ++++++++ src/virtd-tls.socket.in | 14 ++++++++ src/virtd.service.in | 32 +++++++++++++++++ src/virtd.socket.in | 18 ++++++++++ src/vz/meson.build | 8 ++--- src/vz/virtvzd.service.in | 26 -------------- 47 files changed, 326 insertions(+), 533 deletions(-) delete mode 100644 src/ch/virtchd.service.in delete mode 100644 src/interface/virtinterfaced.service.in delete mode 100644 src/libxl/virtxend.service.in delete mode 100644 src/lxc/virtlxcd.service.in delete mode 100644 src/network/virtnetworkd.service.in delete mode 100644 src/node_device/virtnodedevd.service.in delete mode 100644 src/nwfilter/virtnwfilterd.service.in delete mode 100644 src/qemu/virtqemud.service.in delete mode 100644 src/remote/virtproxyd.service.in delete mode 100644 src/secret/virtsecretd.service.in delete mode 100644 src/storage/virtstoraged.service.in delete mode 100644 src/vbox/virtvboxd.service.in create mode 100644 src/virtd-admin.socket.in create mode 100644 src/virtd-ro.socket.in create mode 100644 src/virtd-tcp.socket.in create mode 100644 src/virtd-tls.socket.in create mode 100644 src/virtd.service.in create mode 100644 src/virtd.socket.in delete mode 100644 src/vz/virtvzd.service.in -- 2.41.0
When libvirtd, virtlog and virtlockd are enabled, we want their admin sockets to be enabled as well. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 1 + src/logging/virtlogd.service.in | 1 + src/remote/libvirtd.service.in | 1 + 3 files changed, 3 insertions(+) diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index dd0bbab083..18873f86a6 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -22,3 +22,4 @@ LimitNOFILE=1024:524288 [Install] Also=virtlockd.socket +Also=virtlockd-admin.socket diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 8e245ddb43..14a991f348 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -22,3 +22,4 @@ LimitNOFILE=1024:524288 [Install] Also=virtlogd.socket +Also=virtlogd-admin.socket diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 84f1613adc..8839c00a15 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -50,3 +50,4 @@ Also=virtlockd.socket Also=virtlogd.socket Also=libvirtd.socket Also=libvirtd-ro.socket +Also=libvirtd-admin.socket -- 2.41.0
On Mon, Sep 25, 2023 at 08:57:59PM +0200, Andrea Bolognani wrote:
When libvirtd, virtlog and virtlockd are enabled, we want their admin sockets to be enabled as well.
s/enabled/enabled for socket activation/ because these admin sockets were enabled automatically when the service eventually starts already.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 1 + src/logging/virtlogd.service.in | 1 + src/remote/libvirtd.service.in | 1 + 3 files changed, 3 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index dd0bbab083..18873f86a6 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -22,3 +22,4 @@ LimitNOFILE=1024:524288
[Install] Also=virtlockd.socket +Also=virtlockd-admin.socket diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 8e245ddb43..14a991f348 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -22,3 +22,4 @@ LimitNOFILE=1024:524288
[Install] Also=virtlogd.socket +Also=virtlogd-admin.socket diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 84f1613adc..8839c00a15 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -50,3 +50,4 @@ Also=virtlockd.socket Also=virtlogd.socket Also=libvirtd.socket Also=libvirtd-ro.socket +Also=libvirtd-admin.socket -- 2.41.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 :|
This annotation being missing resulted in virtlogd and virtlockd being marked as "indirect" services, i.e. services that cannot be started directly but have to be socket activated instead. While this is our preferred configuration, we shouldn't prevent the admin to start them at boot if they want to. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 1 + src/logging/virtlogd.service.in | 1 + 2 files changed, 2 insertions(+) diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index 18873f86a6..f12c3040e9 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -21,5 +21,6 @@ OOMScoreAdjust=-900 LimitNOFILE=1024:524288 [Install] +WantedBy=multi-user.target Also=virtlockd.socket Also=virtlockd-admin.socket diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 14a991f348..e665e8a02e 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -21,5 +21,6 @@ OOMScoreAdjust=-900 LimitNOFILE=1024:524288 [Install] +WantedBy=multi-user.target Also=virtlogd.socket Also=virtlogd-admin.socket -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:00PM +0200, Andrea Bolognani wrote:
This annotation being missing resulted in virtlogd and virtlockd being marked as "indirect" services, i.e. services that cannot be started directly but have to be socket activated instead.
While this is our preferred configuration, we shouldn't prevent the admin to start them at boot if they want to.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 1 + src/logging/virtlogd.service.in | 1 + 2 files changed, 2 insertions(+)
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 :|
While systemd will automatically match foo.socket with foo.service based on their names, it's nicer to connect the two explicitly. This is what we do for all services, with virtlogd and virtlockd being the only exceptions. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.socket.in | 1 + src/logging/virtlogd.socket.in | 1 + 2 files changed, 2 insertions(+) diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in index 52014f45ec..4ce75391ae 100644 --- a/src/locking/virtlockd.socket.in +++ b/src/locking/virtlockd.socket.in @@ -4,6 +4,7 @@ Before=libvirtd.service [Socket] ListenStream=@runstatedir@/libvirt/virtlockd-sock +Service=virtlockd.service SocketMode=0600 [Install] diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in index 9749a33197..ff3e66e09b 100644 --- a/src/logging/virtlogd.socket.in +++ b/src/logging/virtlogd.socket.in @@ -4,6 +4,7 @@ Before=libvirtd.service [Socket] ListenStream=@runstatedir@/libvirt/virtlogd-sock +Service=virtlogd.service SocketMode=0600 [Install] -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:01PM +0200, Andrea Bolognani wrote:
While systemd will automatically match foo.socket with foo.service based on their names, it's nicer to connect the two explicitly.
This is what we do for all services, with virtlogd and virtlockd being the only exceptions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.socket.in | 1 + src/logging/virtlogd.socket.in | 1 + 2 files changed, 2 insertions(+)
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 :|
This tells systemd that the services in question support the native socket activation protocol. virtlogd and virtlockd, just like all the other daemons, implement the necessary handshake. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 1 + src/logging/virtlogd.service.in | 1 + 2 files changed, 2 insertions(+) diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index f12c3040e9..9e91fa3261 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -7,6 +7,7 @@ Documentation=man:virtlockd(8) Documentation=https://libvirt.org [Service] +Type=notify Environment=VIRTLOCKD_ARGS= EnvironmentFile=-@initconfdir@/virtlockd ExecStart=@sbindir@/virtlockd $VIRTLOCKD_ARGS diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index e665e8a02e..97c942ffb0 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -7,6 +7,7 @@ Documentation=man:virtlogd(8) Documentation=https://libvirt.org [Service] +Type=notify Environment=VIRTLOGD_ARGS= EnvironmentFile=-@initconfdir@/virtlogd ExecStart=@sbindir@/virtlogd $VIRTLOGD_ARGS -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:02PM +0200, Andrea Bolognani wrote:
This tells systemd that the services in question support the native socket activation protocol.
virtlogd and virtlockd, just like all the other daemons, implement the necessary handshake.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 1 + src/logging/virtlogd.service.in | 1 + 2 files changed, 2 insertions(+)
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 :|
The information is not used anywhere right now, but the documentation for virt_daemon_units claims it's mandatory. More importantly, we're going to start actually using it later on. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/meson.build | 2 +- src/logging/meson.build | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locking/meson.build b/src/locking/meson.build index 57764b0da6..b7ba342171 100644 --- a/src/locking/meson.build +++ b/src/locking/meson.build @@ -144,7 +144,7 @@ if conf.has('WITH_LIBVIRTD') virt_daemon_units += { 'service': 'virtlockd', 'service_in': files('virtlockd.service.in'), - 'name': '', + 'name': 'Libvirt locking', 'sockprefix': '', 'sockets': [ 'main', 'admin' ], 'socket_in': files('virtlockd.socket.in'), diff --git a/src/logging/meson.build b/src/logging/meson.build index fa6af50fa6..aa8affb52c 100644 --- a/src/logging/meson.build +++ b/src/logging/meson.build @@ -91,7 +91,7 @@ if conf.has('WITH_LIBVIRTD') virt_daemon_units += { 'service': 'virtlogd', 'service_in': files('virtlogd.service.in'), - 'name': '', + 'name': 'Libvirt logging', 'sockprefix': '', 'sockets': [ 'main', 'admin' ], 'socket_in': files('virtlogd.socket.in'), -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:03PM +0200, Andrea Bolognani wrote:
The information is not used anywhere right now, but the documentation for virt_daemon_units claims it's mandatory.
More importantly, we're going to start actually using it later on.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/meson.build | 2 +- src/logging/meson.build | 2 +- 2 files changed, 2 insertions(+), 2 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 :|
The meaning of the _def suffix might not be immediately obvious, especially since it's also used to refer to the output of the meson-gen-def.py script elsewhere in the same file. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/meson.build b/src/meson.build index 28e4d0cc4e..7b6ce6d32f 100644 --- a/src/meson.build +++ b/src/meson.build @@ -826,12 +826,12 @@ if conf.has('WITH_LIBVIRTD') foreach socket : unit.get('sockets', [ 'main', 'ro', 'admin' ]) if socket == 'main' - socket_in_def = 'remote' / 'libvirtd.socket.in' - socket_in = unit.get('socket_in', socket_in_def) + socket_in_default = 'remote' / 'libvirtd.socket.in' + socket_in = unit.get('socket_in', socket_in_default) socket_out = '@0@.socket'.format(unit['service']) else - socket_in_def = 'remote' / 'libvirtd-@0@.socket.in'.format(socket) - socket_in = unit.get('socket_@0@_in'.format(socket), socket_in_def) + socket_in_default = 'remote' / 'libvirtd-@0@.socket.in'.format(socket) + socket_in = unit.get('socket_@0@_in'.format(socket), socket_in_default) socket_out = '@0@-@1@.socket'.format(unit['service'], socket) endif configure_file( -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:04PM +0200, Andrea Bolognani wrote:
The meaning of the _def suffix might not be immediately obvious, especially since it's also used to refer to the output of the meson-gen-def.py script elsewhere in the same file.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 8 ++++---- 1 file 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 :|
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 6 +++--- src/remote/libvirtd.socket.in | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/meson.build b/src/meson.build index 7b6ce6d32f..3fae80d67c 100644 --- a/src/meson.build +++ b/src/meson.build @@ -799,9 +799,9 @@ if conf.has('WITH_LIBVIRTD') foreach unit : virt_daemon_units if conf.has('WITH_POLKIT') - mode = '0666' + sockmode = '0666' else - mode = '0600' + sockmode = '0600' endif unit_conf = configuration_data({ @@ -813,7 +813,7 @@ if conf.has('WITH_LIBVIRTD') 'service': unit['service'], 'sockprefix': unit['sockprefix'], 'deps': unit.get('deps', ''), - 'mode': mode, + 'sockmode': sockmode, }) configure_file( diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index 4842712648..e6e903a8ce 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -6,7 +6,7 @@ Before=@service@.service [Socket] ListenStream=@runstatedir@/libvirt/@sockprefix@-sock Service=@service@.service -SocketMode=@mode@ +SocketMode=@sockmode@ RemoveOnStop=yes [Install] -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:05PM +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 6 +++--- src/remote/libvirtd.socket.in | 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 :|
The decision is based only on whether Polkit support is enabled, so there's no need to go through it again for every single service. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/meson.build b/src/meson.build index 3fae80d67c..9d5085a8aa 100644 --- a/src/meson.build +++ b/src/meson.build @@ -797,13 +797,13 @@ if conf.has('WITH_LIBVIRTD') install_dir: systemd_unit_dir, ) - foreach unit : virt_daemon_units - if conf.has('WITH_POLKIT') - sockmode = '0666' - else - sockmode = '0600' - endif + if conf.has('WITH_POLKIT') + sockmode = '0666' + else + sockmode = '0600' + endif + foreach unit : virt_daemon_units unit_conf = configuration_data({ 'runstatedir': runstatedir, 'sbindir': sbindir, -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:06PM +0200, Andrea Bolognani wrote:
The decision is based only on whether Polkit support is enabled, so there's no need to go through it again for every single service.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 12 ++++++------ 1 file changed, 6 insertions(+), 6 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 :|
For most services, the value provided explicitly matches the documented default. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 1 - src/interface/meson.build | 1 - src/libxl/meson.build | 1 - src/lxc/meson.build | 1 - src/network/meson.build | 1 - src/node_device/meson.build | 1 - src/nwfilter/meson.build | 1 - src/qemu/meson.build | 1 - src/secret/meson.build | 1 - src/storage/meson.build | 1 - src/vbox/meson.build | 1 - src/vz/meson.build | 1 - 12 files changed, 12 deletions(-) diff --git a/src/ch/meson.build b/src/ch/meson.build index 66b77907b0..5d1c3f6ca0 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -60,7 +60,6 @@ if conf.has('WITH_CH') 'service_in': files('virtchd.service.in'), 'name': 'Libvirt ch', 'sockprefix': 'virtchd', - 'sockets': [ 'main', 'ro', 'admin' ], } virt_install_dirs += [ diff --git a/src/interface/meson.build b/src/interface/meson.build index 828f274422..3df9bbc9a4 100644 --- a/src/interface/meson.build +++ b/src/interface/meson.build @@ -47,7 +47,6 @@ if conf.has('WITH_INTERFACE') 'service_in': files('virtinterfaced.service.in'), 'name': 'Libvirt interface', 'sockprefix': 'virtinterfaced', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/libxl/meson.build b/src/libxl/meson.build index 0cc277db82..967b6b587c 100644 --- a/src/libxl/meson.build +++ b/src/libxl/meson.build @@ -69,7 +69,6 @@ if conf.has('WITH_LIBXL') 'service_in': files('virtxend.service.in'), 'name': 'Libvirt libxl', 'sockprefix': 'virtxend', - 'sockets': [ 'main', 'ro', 'admin' ], 'deps': 'ConditionPathExists=/proc/xen/capabilities', } diff --git a/src/lxc/meson.build b/src/lxc/meson.build index 99d4a34213..49cc5e6b26 100644 --- a/src/lxc/meson.build +++ b/src/lxc/meson.build @@ -167,7 +167,6 @@ if conf.has('WITH_LXC') 'service_in': files('virtlxcd.service.in'), 'name': 'Libvirt lxc', 'sockprefix': 'virtlxcd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/network/meson.build b/src/network/meson.build index 0888d1beac..121172cafe 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -65,7 +65,6 @@ if conf.has('WITH_NETWORK') 'service_in': files('virtnetworkd.service.in'), 'name': 'Libvirt network', 'sockprefix': 'virtnetworkd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/node_device/meson.build b/src/node_device/meson.build index 1c95975c37..2610822d28 100644 --- a/src/node_device/meson.build +++ b/src/node_device/meson.build @@ -55,7 +55,6 @@ if conf.has('WITH_NODE_DEVICES') 'service_in': files('virtnodedevd.service.in'), 'name': 'Libvirt nodedev', 'sockprefix': 'virtnodedevd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build index 55cf8fcce4..bb03a47f8f 100644 --- a/src/nwfilter/meson.build +++ b/src/nwfilter/meson.build @@ -53,7 +53,6 @@ if conf.has('WITH_NWFILTER') 'service_in': files('virtnwfilterd.service.in'), 'name': 'Libvirt nwfilter', 'sockprefix': 'virtnwfilterd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 607b597c8c..2d55cd30cb 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -186,7 +186,6 @@ if conf.has('WITH_QEMU') 'service_in': files('virtqemud.service.in'), 'name': 'Libvirt qemu', 'sockprefix': 'virtqemud', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/secret/meson.build b/src/secret/meson.build index 1bda59849b..882ed2ac70 100644 --- a/src/secret/meson.build +++ b/src/secret/meson.build @@ -36,7 +36,6 @@ if conf.has('WITH_SECRETS') 'service_in': files('virtsecretd.service.in'), 'name': 'Libvirt secret', 'sockprefix': 'virtsecretd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/storage/meson.build b/src/storage/meson.build index a8b173db96..49fd4072d5 100644 --- a/src/storage/meson.build +++ b/src/storage/meson.build @@ -114,7 +114,6 @@ if conf.has('WITH_STORAGE') 'service_in': files('virtstoraged.service.in'), 'name': 'Libvirt storage', 'sockprefix': 'virtstoraged', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/vbox/meson.build b/src/vbox/meson.build index 1b0dad3336..f19d375c4b 100644 --- a/src/vbox/meson.build +++ b/src/vbox/meson.build @@ -60,7 +60,6 @@ if conf.has('WITH_VBOX') 'service_in': files('virtvboxd.service.in'), 'name': 'Libvirt vbox', 'sockprefix': 'virtvboxd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { diff --git a/src/vz/meson.build b/src/vz/meson.build index d102696943..db3d8bb203 100644 --- a/src/vz/meson.build +++ b/src/vz/meson.build @@ -51,7 +51,6 @@ if conf.has('WITH_VZ') 'service_in': files('virtvzd.service.in'), 'name': 'Libvirt vz', 'sockprefix': 'virtvzd', - 'sockets': [ 'main', 'ro', 'admin' ], } openrc_init_files += { -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:07PM +0200, Andrea Bolognani wrote:
For most services, the value provided explicitly matches the documented default.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 1 - src/interface/meson.build | 1 - src/libxl/meson.build | 1 - src/lxc/meson.build | 1 - src/network/meson.build | 1 - src/node_device/meson.build | 1 - src/nwfilter/meson.build | 1 - src/qemu/meson.build | 1 - src/secret/meson.build | 1 - src/storage/meson.build | 1 - src/vbox/meson.build | 1 - src/vz/meson.build | 1 - 12 files changed, 12 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 :|
For most services, the socket paths can be derived trivially from the name of the daemon: for virtqemud, for example, they will be /run/libvirt/virtqemud-sock /run/libvirt/virtqemud-sock-ro /run/libvirt/virtqemud-admin-sock libvirtd and virtproxyd are the exceptions, since their socket paths will be /run/libvirt/libvirt-sock /run/libvirt/libvirt-sock-ro /run/libvirt/libvirt-admin-sock So we still need to be able to provide a custom @sockprefix@ in those cases, but in the most common scenario we can do away with the requirement by introducing a sensible default. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/meson.build b/src/meson.build index 9d5085a8aa..6c85cc9b9b 100644 --- a/src/meson.build +++ b/src/meson.build @@ -196,7 +196,7 @@ guest_unit_files = [] # * service - name of the service (required) # * service_in - service source file (required) # * name - socket description (required) -# * sockprefix - socket prefix name (required) +# * sockprefix - socket prefix name (optional, default unit['service']) # * sockets - array of additional sockets (optional, default [ 'main', 'ro', 'admin' ]) # * socket_$name_in - additional socket source files (optional, default remote/libvirtd.socket.in ) # * deps - socket dependencies (optional, default '') @@ -811,7 +811,7 @@ if conf.has('WITH_LIBVIRTD') 'initconfdir': initconfdir, 'name': unit['name'], 'service': unit['service'], - 'sockprefix': unit['sockprefix'], + 'sockprefix': unit.get('sockprefix', unit['service']), 'deps': unit.get('deps', ''), 'sockmode': sockmode, }) -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:08PM +0200, Andrea Bolognani wrote:
For most services, the socket paths can be derived trivially from the name of the daemon: for virtqemud, for example, they will be
/run/libvirt/virtqemud-sock /run/libvirt/virtqemud-sock-ro /run/libvirt/virtqemud-admin-sock
libvirtd and virtproxyd are the exceptions, since their socket paths will be
/run/libvirt/libvirt-sock /run/libvirt/libvirt-sock-ro /run/libvirt/libvirt-admin-sock
So we still need to be able to provide a custom @sockprefix@ in those cases, but in the most common scenario we can do away with the requirement by introducing a sensible default.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 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 :|
Now that providing the value is optional, we can remove almost all uses. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 1 - src/interface/meson.build | 1 - src/libxl/meson.build | 1 - src/locking/meson.build | 1 - src/logging/meson.build | 1 - src/lxc/meson.build | 1 - src/network/meson.build | 1 - src/node_device/meson.build | 1 - src/nwfilter/meson.build | 1 - src/qemu/meson.build | 1 - src/secret/meson.build | 1 - src/storage/meson.build | 1 - src/vbox/meson.build | 1 - src/vz/meson.build | 1 - 14 files changed, 14 deletions(-) diff --git a/src/ch/meson.build b/src/ch/meson.build index 5d1c3f6ca0..936b9bc95a 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -59,7 +59,6 @@ if conf.has('WITH_CH') 'service': 'virtchd', 'service_in': files('virtchd.service.in'), 'name': 'Libvirt ch', - 'sockprefix': 'virtchd', } virt_install_dirs += [ diff --git a/src/interface/meson.build b/src/interface/meson.build index 3df9bbc9a4..06c5241fa3 100644 --- a/src/interface/meson.build +++ b/src/interface/meson.build @@ -46,7 +46,6 @@ if conf.has('WITH_INTERFACE') 'service': 'virtinterfaced', 'service_in': files('virtinterfaced.service.in'), 'name': 'Libvirt interface', - 'sockprefix': 'virtinterfaced', } openrc_init_files += { diff --git a/src/libxl/meson.build b/src/libxl/meson.build index 967b6b587c..db8ccde38e 100644 --- a/src/libxl/meson.build +++ b/src/libxl/meson.build @@ -68,7 +68,6 @@ if conf.has('WITH_LIBXL') 'service': 'virtxend', 'service_in': files('virtxend.service.in'), 'name': 'Libvirt libxl', - 'sockprefix': 'virtxend', 'deps': 'ConditionPathExists=/proc/xen/capabilities', } diff --git a/src/locking/meson.build b/src/locking/meson.build index b7ba342171..2ccc822ed3 100644 --- a/src/locking/meson.build +++ b/src/locking/meson.build @@ -145,7 +145,6 @@ if conf.has('WITH_LIBVIRTD') 'service': 'virtlockd', 'service_in': files('virtlockd.service.in'), 'name': 'Libvirt locking', - 'sockprefix': '', 'sockets': [ 'main', 'admin' ], 'socket_in': files('virtlockd.socket.in'), 'socket_admin_in': files('virtlockd-admin.socket.in'), diff --git a/src/logging/meson.build b/src/logging/meson.build index aa8affb52c..95d2ef2a3f 100644 --- a/src/logging/meson.build +++ b/src/logging/meson.build @@ -92,7 +92,6 @@ if conf.has('WITH_LIBVIRTD') 'service': 'virtlogd', 'service_in': files('virtlogd.service.in'), 'name': 'Libvirt logging', - 'sockprefix': '', 'sockets': [ 'main', 'admin' ], 'socket_in': files('virtlogd.socket.in'), 'socket_admin_in': files('virtlogd-admin.socket.in'), diff --git a/src/lxc/meson.build b/src/lxc/meson.build index 49cc5e6b26..a8773f64a5 100644 --- a/src/lxc/meson.build +++ b/src/lxc/meson.build @@ -166,7 +166,6 @@ if conf.has('WITH_LXC') 'service': 'virtlxcd', 'service_in': files('virtlxcd.service.in'), 'name': 'Libvirt lxc', - 'sockprefix': 'virtlxcd', } openrc_init_files += { diff --git a/src/network/meson.build b/src/network/meson.build index 121172cafe..40abfaef7e 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -64,7 +64,6 @@ if conf.has('WITH_NETWORK') 'service': 'virtnetworkd', 'service_in': files('virtnetworkd.service.in'), 'name': 'Libvirt network', - 'sockprefix': 'virtnetworkd', } openrc_init_files += { diff --git a/src/node_device/meson.build b/src/node_device/meson.build index 2610822d28..47d9f63600 100644 --- a/src/node_device/meson.build +++ b/src/node_device/meson.build @@ -54,7 +54,6 @@ if conf.has('WITH_NODE_DEVICES') 'service': 'virtnodedevd', 'service_in': files('virtnodedevd.service.in'), 'name': 'Libvirt nodedev', - 'sockprefix': 'virtnodedevd', } openrc_init_files += { diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build index bb03a47f8f..5efdee7189 100644 --- a/src/nwfilter/meson.build +++ b/src/nwfilter/meson.build @@ -52,7 +52,6 @@ if conf.has('WITH_NWFILTER') 'service': 'virtnwfilterd', 'service_in': files('virtnwfilterd.service.in'), 'name': 'Libvirt nwfilter', - 'sockprefix': 'virtnwfilterd', } openrc_init_files += { diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 2d55cd30cb..afa9139d9a 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -185,7 +185,6 @@ if conf.has('WITH_QEMU') 'service': 'virtqemud', 'service_in': files('virtqemud.service.in'), 'name': 'Libvirt qemu', - 'sockprefix': 'virtqemud', } openrc_init_files += { diff --git a/src/secret/meson.build b/src/secret/meson.build index 882ed2ac70..49f6972f36 100644 --- a/src/secret/meson.build +++ b/src/secret/meson.build @@ -35,7 +35,6 @@ if conf.has('WITH_SECRETS') 'service': 'virtsecretd', 'service_in': files('virtsecretd.service.in'), 'name': 'Libvirt secret', - 'sockprefix': 'virtsecretd', } openrc_init_files += { diff --git a/src/storage/meson.build b/src/storage/meson.build index 49fd4072d5..d0d0b72228 100644 --- a/src/storage/meson.build +++ b/src/storage/meson.build @@ -113,7 +113,6 @@ if conf.has('WITH_STORAGE') 'service': 'virtstoraged', 'service_in': files('virtstoraged.service.in'), 'name': 'Libvirt storage', - 'sockprefix': 'virtstoraged', } openrc_init_files += { diff --git a/src/vbox/meson.build b/src/vbox/meson.build index f19d375c4b..9f2fb0f938 100644 --- a/src/vbox/meson.build +++ b/src/vbox/meson.build @@ -59,7 +59,6 @@ if conf.has('WITH_VBOX') 'service': 'virtvboxd', 'service_in': files('virtvboxd.service.in'), 'name': 'Libvirt vbox', - 'sockprefix': 'virtvboxd', } openrc_init_files += { diff --git a/src/vz/meson.build b/src/vz/meson.build index db3d8bb203..4c8747e3eb 100644 --- a/src/vz/meson.build +++ b/src/vz/meson.build @@ -50,7 +50,6 @@ if conf.has('WITH_VZ') 'service': 'virtvzd', 'service_in': files('virtvzd.service.in'), 'name': 'Libvirt vz', - 'sockprefix': 'virtvzd', } openrc_init_files += { -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:09PM +0200, Andrea Bolognani wrote:
Now that providing the value is optional, we can remove almost all uses.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 1 - src/interface/meson.build | 1 - src/libxl/meson.build | 1 - src/locking/meson.build | 1 - src/logging/meson.build | 1 - src/lxc/meson.build | 1 - src/network/meson.build | 1 - src/node_device/meson.build | 1 - src/nwfilter/meson.build | 1 - src/qemu/meson.build | 1 - src/secret/meson.build | 1 - src/storage/meson.build | 1 - src/vbox/meson.build | 1 - src/vz/meson.build | 1 - 14 files changed, 14 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 :|
It is currently considered required, but we're soon going to provide a default that will be suitable for most services. Since all services currently provide a value explicitly, we can implement a default without breaking anything. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/meson.build b/src/meson.build index 6c85cc9b9b..b1dd9e5422 100644 --- a/src/meson.build +++ b/src/meson.build @@ -194,10 +194,10 @@ guest_unit_files = [] # virt_daemon_units: # generate libvirt daemon systemd unit files # * service - name of the service (required) -# * service_in - service source file (required) # * name - socket description (required) # * sockprefix - socket prefix name (optional, default unit['service']) # * sockets - array of additional sockets (optional, default [ 'main', 'ro', 'admin' ]) +# * service_in - service source file (optional, default remote/libvirtd.service.in) # * socket_$name_in - additional socket source files (optional, default remote/libvirtd.socket.in ) # * deps - socket dependencies (optional, default '') virt_daemon_units = [] @@ -803,6 +803,8 @@ if conf.has('WITH_LIBVIRTD') sockmode = '0600' endif + service_in_default = 'remote' / 'libvirtd.service.in' + foreach unit : virt_daemon_units unit_conf = configuration_data({ 'runstatedir': runstatedir, @@ -817,7 +819,7 @@ if conf.has('WITH_LIBVIRTD') }) configure_file( - input: unit['service_in'], + input: unit.get('service_in', service_in_default), output: '@0@.service'.format(unit['service']), configuration: unit_conf, install: true, -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:10PM +0200, Andrea Bolognani wrote:
It is currently considered required, but we're soon going to provide a default that will be suitable for most services.
Since all services currently provide a value explicitly, we can implement a default without breaking anything.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 :|
These will be useful during the upcoming migration to common templates for systemd units and will be dropped as soon as all services have been converted. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/meson.build b/src/meson.build index b1dd9e5422..fdd15906b9 100644 --- a/src/meson.build +++ b/src/meson.build @@ -191,6 +191,10 @@ virt_test_aug_dir = datadir / 'augeas' / 'lenses' / 'tests' # guest unit files to install guest_unit_files = [] +libvirtd_socket_in = files('remote' / 'libvirtd.socket.in') +libvirtd_socket_ro_in = files('remote' / 'libvirtd-ro.socket.in') +libvirtd_socket_admin_in = files('remote' / 'libvirtd-admin.socket.in') + # virt_daemon_units: # generate libvirt daemon systemd unit files # * service - name of the service (required) -- 2.41.0
We're about to change the defaults and start migrating to common templates: in order to be able to switch units over one at a time, make the input files that are currently used explicit rather than implicit. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 3 +++ src/interface/meson.build | 3 +++ src/libxl/meson.build | 3 +++ src/lxc/meson.build | 3 +++ src/network/meson.build | 3 +++ src/node_device/meson.build | 3 +++ src/nwfilter/meson.build | 3 +++ src/qemu/meson.build | 3 +++ src/remote/meson.build | 10 ++++++++++ src/secret/meson.build | 3 +++ src/storage/meson.build | 3 +++ src/vbox/meson.build | 3 +++ src/vz/meson.build | 3 +++ 13 files changed, 46 insertions(+) diff --git a/src/ch/meson.build b/src/ch/meson.build index 936b9bc95a..dc08069dcd 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -59,6 +59,9 @@ if conf.has('WITH_CH') 'service': 'virtchd', 'service_in': files('virtchd.service.in'), 'name': 'Libvirt ch', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } virt_install_dirs += [ diff --git a/src/interface/meson.build b/src/interface/meson.build index 06c5241fa3..6fa65117c3 100644 --- a/src/interface/meson.build +++ b/src/interface/meson.build @@ -46,6 +46,9 @@ if conf.has('WITH_INTERFACE') 'service': 'virtinterfaced', 'service_in': files('virtinterfaced.service.in'), 'name': 'Libvirt interface', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/libxl/meson.build b/src/libxl/meson.build index db8ccde38e..a1553dbe27 100644 --- a/src/libxl/meson.build +++ b/src/libxl/meson.build @@ -68,6 +68,9 @@ if conf.has('WITH_LIBXL') 'service': 'virtxend', 'service_in': files('virtxend.service.in'), 'name': 'Libvirt libxl', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, 'deps': 'ConditionPathExists=/proc/xen/capabilities', } diff --git a/src/lxc/meson.build b/src/lxc/meson.build index a8773f64a5..531078448c 100644 --- a/src/lxc/meson.build +++ b/src/lxc/meson.build @@ -166,6 +166,9 @@ if conf.has('WITH_LXC') 'service': 'virtlxcd', 'service_in': files('virtlxcd.service.in'), 'name': 'Libvirt lxc', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/network/meson.build b/src/network/meson.build index 40abfaef7e..2e51d5d47b 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -64,6 +64,9 @@ if conf.has('WITH_NETWORK') 'service': 'virtnetworkd', 'service_in': files('virtnetworkd.service.in'), 'name': 'Libvirt network', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/node_device/meson.build b/src/node_device/meson.build index 47d9f63600..dd60b1f819 100644 --- a/src/node_device/meson.build +++ b/src/node_device/meson.build @@ -54,6 +54,9 @@ if conf.has('WITH_NODE_DEVICES') 'service': 'virtnodedevd', 'service_in': files('virtnodedevd.service.in'), 'name': 'Libvirt nodedev', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build index 5efdee7189..de672bb827 100644 --- a/src/nwfilter/meson.build +++ b/src/nwfilter/meson.build @@ -52,6 +52,9 @@ if conf.has('WITH_NWFILTER') 'service': 'virtnwfilterd', 'service_in': files('virtnwfilterd.service.in'), 'name': 'Libvirt nwfilter', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/qemu/meson.build b/src/qemu/meson.build index afa9139d9a..b52497bdf0 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -185,6 +185,9 @@ if conf.has('WITH_QEMU') 'service': 'virtqemud', 'service_in': files('virtqemud.service.in'), 'name': 'Libvirt qemu', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/remote/meson.build b/src/remote/meson.build index eb4f7a0068..365d632095 100644 --- a/src/remote/meson.build +++ b/src/remote/meson.build @@ -196,6 +196,11 @@ if conf.has('WITH_REMOTE') 'name': 'Libvirt', 'sockprefix': 'libvirt', 'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ], + 'socket_in': files('libvirtd.socket.in'), + 'socket_ro_in': files('libvirtd-ro.socket.in'), + 'socket_admin_in': files('libvirtd-admin.socket.in'), + 'socket_tcp_in': files('libvirtd-tcp.socket.in'), + 'socket_tls_in': files('libvirtd-tls.socket.in'), } openrc_init_files += { @@ -225,6 +230,11 @@ if conf.has('WITH_REMOTE') 'name': 'Libvirt proxy', 'sockprefix': 'libvirt', 'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ], + 'socket_in': files('libvirtd.socket.in'), + 'socket_ro_in': files('libvirtd-ro.socket.in'), + 'socket_admin_in': files('libvirtd-admin.socket.in'), + 'socket_tcp_in': files('libvirtd-tcp.socket.in'), + 'socket_tls_in': files('libvirtd-tls.socket.in'), 'deps': 'Conflicts=' + libvirtd_socket_conflicts, } diff --git a/src/secret/meson.build b/src/secret/meson.build index 49f6972f36..58e47c22e8 100644 --- a/src/secret/meson.build +++ b/src/secret/meson.build @@ -35,6 +35,9 @@ if conf.has('WITH_SECRETS') 'service': 'virtsecretd', 'service_in': files('virtsecretd.service.in'), 'name': 'Libvirt secret', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/storage/meson.build b/src/storage/meson.build index d0d0b72228..e0a1e9f4de 100644 --- a/src/storage/meson.build +++ b/src/storage/meson.build @@ -113,6 +113,9 @@ if conf.has('WITH_STORAGE') 'service': 'virtstoraged', 'service_in': files('virtstoraged.service.in'), 'name': 'Libvirt storage', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/vbox/meson.build b/src/vbox/meson.build index 9f2fb0f938..2d6b71ab8f 100644 --- a/src/vbox/meson.build +++ b/src/vbox/meson.build @@ -59,6 +59,9 @@ if conf.has('WITH_VBOX') 'service': 'virtvboxd', 'service_in': files('virtvboxd.service.in'), 'name': 'Libvirt vbox', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/vz/meson.build b/src/vz/meson.build index 4c8747e3eb..9c2eb90463 100644 --- a/src/vz/meson.build +++ b/src/vz/meson.build @@ -50,6 +50,9 @@ if conf.has('WITH_VZ') 'service': 'virtvzd', 'service_in': files('virtvzd.service.in'), 'name': 'Libvirt vz', + 'socket_in': libvirtd_socket_in, + 'socket_ro_in': libvirtd_socket_ro_in, + 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { -- 2.41.0
These contain the part that is common to all existing service and socket definitions. Each section of each template ends with a placeholder, which marks the spot where additional, service-specific lines should be injected. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 10 ++++++++++ src/virtd-admin.socket.in | 16 ++++++++++++++++ src/virtd-ro.socket.in | 16 ++++++++++++++++ src/virtd-tcp.socket.in | 15 +++++++++++++++ src/virtd-tls.socket.in | 15 +++++++++++++++ src/virtd.service.in | 28 ++++++++++++++++++++++++++++ src/virtd.socket.in | 15 +++++++++++++++ 7 files changed, 115 insertions(+) create mode 100644 src/virtd-admin.socket.in create mode 100644 src/virtd-ro.socket.in create mode 100644 src/virtd-tcp.socket.in create mode 100644 src/virtd-tls.socket.in create mode 100644 src/virtd.service.in create mode 100644 src/virtd.socket.in diff --git a/src/meson.build b/src/meson.build index fdd15906b9..d7133d1293 100644 --- a/src/meson.build +++ b/src/meson.build @@ -203,6 +203,8 @@ libvirtd_socket_admin_in = files('remote' / 'libvirtd-admin.socket.in') # * sockets - array of additional sockets (optional, default [ 'main', 'ro', 'admin' ]) # * service_in - service source file (optional, default remote/libvirtd.service.in) # * socket_$name_in - additional socket source files (optional, default remote/libvirtd.socket.in ) +# * service_$name_extra - additional lines for service's [$name] section (optional, default []) +# * socket_$name_extra - additional lines for socket's [$name] section (optional, default []) # * deps - socket dependencies (optional, default '') virt_daemon_units = [] @@ -817,11 +819,19 @@ if conf.has('WITH_LIBVIRTD') 'initconfdir': initconfdir, 'name': unit['name'], 'service': unit['service'], + 'SERVICE': unit['service'].to_upper(), 'sockprefix': unit.get('sockprefix', unit['service']), 'deps': unit.get('deps', ''), 'sockmode': sockmode, }) + foreach extra : [ 'service_unit', 'service_service', 'service_install', + 'socket_unit', 'socket_socket', 'socket_install' ] + extra_key = '@0@_extra'.format(extra) + extra_value = '\n'.join(unit.get(extra_key, [])) + unit_conf.set(extra_key, extra_value) + endforeach + configure_file( input: unit.get('service_in', service_in_default), output: '@0@.service'.format(unit['service']), diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in new file mode 100644 index 0000000000..3a09951b12 --- /dev/null +++ b/src/virtd-admin.socket.in @@ -0,0 +1,16 @@ +[Unit] +Description=@name@ admin socket +Before=@service@.service +BindsTo=@service@.socket +After=@service@.socket +@socket_unit_extra@ + +[Socket] +ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock +Service=@service@.service +SocketMode=0600 +@socket_socket_extra@ + +[Install] +WantedBy=sockets.target +@socket_install_extra@ diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in new file mode 100644 index 0000000000..e882f25a7b --- /dev/null +++ b/src/virtd-ro.socket.in @@ -0,0 +1,16 @@ +[Unit] +Description=@name@ local read-only socket +Before=@service@.service +BindsTo=@service@.socket +After=@service@.socket +@socket_unit_extra@ + +[Socket] +ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro +Service=@service@.service +SocketMode=0666 +@socket_socket_extra@ + +[Install] +WantedBy=sockets.target +@socket_install_extra@ diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in new file mode 100644 index 0000000000..26c6dfa75b --- /dev/null +++ b/src/virtd-tcp.socket.in @@ -0,0 +1,15 @@ +[Unit] +Description=@name@ non-TLS IP socket +Before=@service@.service +BindsTo=@service@.socket +After=@service@.socket +@socket_unit_extra@ + +[Socket] +ListenStream=16509 +Service=@service@.service +@socket_socket_extra@ + +[Install] +WantedBy=sockets.target +@socket_install_extra@ diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in new file mode 100644 index 0000000000..077c320cce --- /dev/null +++ b/src/virtd-tls.socket.in @@ -0,0 +1,15 @@ +[Unit] +Description=@name@ TLS IP socket +Before=@service@.service +BindsTo=@service@.socket +After=@service@.socket +@socket_unit_extra@ + +[Socket] +ListenStream=16514 +Service=@service@.service +@socket_socket_extra@ + +[Install] +WantedBy=sockets.target +@socket_install_extra@ diff --git a/src/virtd.service.in b/src/virtd.service.in new file mode 100644 index 0000000000..c9afecad73 --- /dev/null +++ b/src/virtd.service.in @@ -0,0 +1,28 @@ +[Unit] +Description=@name@ daemon +Conflicts=libvirtd.service +Requires=@service@.socket +Requires=@service@-ro.socket +Requires=@service@-admin.socket +After=network.target +After=dbus.service +After=apparmor.service +Documentation=man:@service@(8) +Documentation=https://libvirt.org +@service_unit_extra@ + +[Service] +Type=notify +Environment=@SERVICE@_ARGS="--timeout 120" +EnvironmentFile=-@initconfdir@/@service@ +ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS +ExecReload=/bin/kill -HUP $MAINPID +Restart=on-failure +@service_service_extra@ + +[Install] +WantedBy=multi-user.target +Also=@service@.socket +Also=@service@-ro.socket +Also=@service@-admin.socket +@service_install_extra@ diff --git a/src/virtd.socket.in b/src/virtd.socket.in new file mode 100644 index 0000000000..278f59ef1c --- /dev/null +++ b/src/virtd.socket.in @@ -0,0 +1,15 @@ +[Unit] +Description=@name@ local socket +Before=@service@.service +@socket_unit_extra@ + +[Socket] +ListenStream=@runstatedir@/libvirt/@sockprefix@-sock +Service=@service@.service +SocketMode=@sockmode@ +RemoveOnStop=yes +@socket_socket_extra@ + +[Install] +WantedBy=sockets.target +@socket_install_extra@ -- 2.41.0
All services are still listing their input files explicitly, so no changes to the output files will occur yet. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/meson.build b/src/meson.build index d7133d1293..b00895fd07 100644 --- a/src/meson.build +++ b/src/meson.build @@ -201,8 +201,8 @@ libvirtd_socket_admin_in = files('remote' / 'libvirtd-admin.socket.in') # * name - socket description (required) # * sockprefix - socket prefix name (optional, default unit['service']) # * sockets - array of additional sockets (optional, default [ 'main', 'ro', 'admin' ]) -# * service_in - service source file (optional, default remote/libvirtd.service.in) -# * socket_$name_in - additional socket source files (optional, default remote/libvirtd.socket.in ) +# * service_in - service source file (optional, default virtd.service.in) +# * socket_$name_in - additional socket source files (optional, default virtd.socket.in or virtd-$name.socket.in) # * service_$name_extra - additional lines for service's [$name] section (optional, default []) # * socket_$name_extra - additional lines for socket's [$name] section (optional, default []) # * deps - socket dependencies (optional, default '') @@ -809,7 +809,7 @@ if conf.has('WITH_LIBVIRTD') sockmode = '0600' endif - service_in_default = 'remote' / 'libvirtd.service.in' + service_in_default = 'virtd.service.in' foreach unit : virt_daemon_units unit_conf = configuration_data({ @@ -842,11 +842,11 @@ if conf.has('WITH_LIBVIRTD') foreach socket : unit.get('sockets', [ 'main', 'ro', 'admin' ]) if socket == 'main' - socket_in_default = 'remote' / 'libvirtd.socket.in' + socket_in_default = 'virtd.socket.in' socket_in = unit.get('socket_in', socket_in_default) socket_out = '@0@.socket'.format(unit['service']) else - socket_in_default = 'remote' / 'libvirtd-@0@.socket.in'.format(socket) + socket_in_default = 'virtd-@0@.socket.in'.format(socket) socket_in = unit.get('socket_@0@_in'.format(socket), socket_in_default) socket_out = '@0@-@1@.socket'.format(unit['service'], socket) endif -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/node_device/meson.build | 4 ---- src/node_device/virtnodedevd.service.in | 25 ------------------------- 2 files changed, 29 deletions(-) delete mode 100644 src/node_device/virtnodedevd.service.in diff --git a/src/node_device/meson.build b/src/node_device/meson.build index dd60b1f819..2614ff8b9c 100644 --- a/src/node_device/meson.build +++ b/src/node_device/meson.build @@ -52,11 +52,7 @@ if conf.has('WITH_NODE_DEVICES') virt_daemon_units += { 'service': 'virtnodedevd', - 'service_in': files('virtnodedevd.service.in'), 'name': 'Libvirt nodedev', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/node_device/virtnodedevd.service.in b/src/node_device/virtnodedevd.service.in deleted file mode 100644 index 2ac41db32e..0000000000 --- a/src/node_device/virtnodedevd.service.in +++ /dev/null @@ -1,25 +0,0 @@ -[Unit] -Description=Virtualization nodedev daemon -Conflicts=libvirtd.service -Requires=virtnodedevd.socket -Requires=virtnodedevd-ro.socket -Requires=virtnodedevd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -Documentation=man:virtnodedevd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTNODEDEVD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtnodedevd -ExecStart=@sbindir@/virtnodedevd $VIRTNODEDEVD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtnodedevd.socket -Also=virtnodedevd-ro.socket -Also=virtnodedevd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/interface/meson.build | 4 ---- src/interface/virtinterfaced.service.in | 25 ------------------------- 2 files changed, 29 deletions(-) delete mode 100644 src/interface/virtinterfaced.service.in diff --git a/src/interface/meson.build b/src/interface/meson.build index 6fa65117c3..54c0b1a935 100644 --- a/src/interface/meson.build +++ b/src/interface/meson.build @@ -44,11 +44,7 @@ if conf.has('WITH_INTERFACE') virt_daemon_units += { 'service': 'virtinterfaced', - 'service_in': files('virtinterfaced.service.in'), 'name': 'Libvirt interface', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/interface/virtinterfaced.service.in b/src/interface/virtinterfaced.service.in deleted file mode 100644 index 5cb2cd19dc..0000000000 --- a/src/interface/virtinterfaced.service.in +++ /dev/null @@ -1,25 +0,0 @@ -[Unit] -Description=Virtualization interface daemon -Conflicts=libvirtd.service -Requires=virtinterfaced.socket -Requires=virtinterfaced-ro.socket -Requires=virtinterfaced-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -Documentation=man:virtinterfaced(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTINTERFACED_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtinterfaced -ExecStart=@sbindir@/virtinterfaced $VIRTINTERFACED_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtinterfaced.socket -Also=virtinterfaced-ro.socket -Also=virtinterfaced-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/nwfilter/meson.build | 4 ---- src/nwfilter/virtnwfilterd.service.in | 25 ------------------------- 2 files changed, 29 deletions(-) delete mode 100644 src/nwfilter/virtnwfilterd.service.in diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build index de672bb827..c091bc3f1b 100644 --- a/src/nwfilter/meson.build +++ b/src/nwfilter/meson.build @@ -50,11 +50,7 @@ if conf.has('WITH_NWFILTER') virt_daemon_units += { 'service': 'virtnwfilterd', - 'service_in': files('virtnwfilterd.service.in'), 'name': 'Libvirt nwfilter', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/nwfilter/virtnwfilterd.service.in b/src/nwfilter/virtnwfilterd.service.in deleted file mode 100644 index d6e98240a8..0000000000 --- a/src/nwfilter/virtnwfilterd.service.in +++ /dev/null @@ -1,25 +0,0 @@ -[Unit] -Description=Virtualization nwfilter daemon -Conflicts=libvirtd.service -Requires=virtnwfilterd.socket -Requires=virtnwfilterd-ro.socket -Requires=virtnwfilterd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -Documentation=man:virtnwfilterd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTNWFILTERD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtnwfilterd -ExecStart=@sbindir@/virtnwfilterd $VIRTNWFILTERD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtnwfilterd.socket -Also=virtnwfilterd-ro.socket -Also=virtnwfilterd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/secret/meson.build | 4 ---- src/secret/virtsecretd.service.in | 25 ------------------------- 2 files changed, 29 deletions(-) delete mode 100644 src/secret/virtsecretd.service.in diff --git a/src/secret/meson.build b/src/secret/meson.build index 58e47c22e8..e05b46abea 100644 --- a/src/secret/meson.build +++ b/src/secret/meson.build @@ -33,11 +33,7 @@ if conf.has('WITH_SECRETS') virt_daemon_units += { 'service': 'virtsecretd', - 'service_in': files('virtsecretd.service.in'), 'name': 'Libvirt secret', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, } openrc_init_files += { diff --git a/src/secret/virtsecretd.service.in b/src/secret/virtsecretd.service.in deleted file mode 100644 index 3804fe553b..0000000000 --- a/src/secret/virtsecretd.service.in +++ /dev/null @@ -1,25 +0,0 @@ -[Unit] -Description=Virtualization secret daemon -Conflicts=libvirtd.service -Requires=virtsecretd.socket -Requires=virtsecretd-ro.socket -Requires=virtsecretd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -Documentation=man:virtsecretd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTSECRETD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtsecretd -ExecStart=@sbindir@/virtsecretd $VIRTSECRETD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtsecretd.socket -Also=virtsecretd-ro.socket -Also=virtsecretd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/network/meson.build | 7 +++---- src/network/virtnetworkd.service.in | 26 -------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) delete mode 100644 src/network/virtnetworkd.service.in diff --git a/src/network/meson.build b/src/network/meson.build index 2e51d5d47b..d1a2338d1b 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -62,11 +62,10 @@ if conf.has('WITH_NETWORK') virt_daemon_units += { 'service': 'virtnetworkd', - 'service_in': files('virtnetworkd.service.in'), 'name': 'Libvirt network', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_service_extra': [ + 'KillMode=process', + ], } openrc_init_files += { diff --git a/src/network/virtnetworkd.service.in b/src/network/virtnetworkd.service.in deleted file mode 100644 index 3d7374715d..0000000000 --- a/src/network/virtnetworkd.service.in +++ /dev/null @@ -1,26 +0,0 @@ -[Unit] -Description=Virtualization network daemon -Conflicts=libvirtd.service -Requires=virtnetworkd.socket -Requires=virtnetworkd-ro.socket -Requires=virtnetworkd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -Documentation=man:virtnetworkd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTNETWORKD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtnetworkd -ExecStart=@sbindir@/virtnetworkd $VIRTNETWORKD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure -KillMode=process - -[Install] -WantedBy=multi-user.target -Also=virtnetworkd.socket -Also=virtnetworkd-ro.socket -Also=virtnetworkd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/storage/meson.build | 8 ++++---- src/storage/virtstoraged.service.in | 27 --------------------------- 2 files changed, 4 insertions(+), 31 deletions(-) delete mode 100644 src/storage/virtstoraged.service.in diff --git a/src/storage/meson.build b/src/storage/meson.build index e0a1e9f4de..90b8af4e41 100644 --- a/src/storage/meson.build +++ b/src/storage/meson.build @@ -111,11 +111,11 @@ if conf.has('WITH_STORAGE') virt_daemon_units += { 'service': 'virtstoraged', - 'service_in': files('virtstoraged.service.in'), 'name': 'Libvirt storage', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'After=iscsid.service', + 'After=remote-fs.target', + ], } openrc_init_files += { diff --git a/src/storage/virtstoraged.service.in b/src/storage/virtstoraged.service.in deleted file mode 100644 index 235fbc6798..0000000000 --- a/src/storage/virtstoraged.service.in +++ /dev/null @@ -1,27 +0,0 @@ -[Unit] -Description=Virtualization storage daemon -Conflicts=libvirtd.service -Requires=virtstoraged.socket -Requires=virtstoraged-ro.socket -Requires=virtstoraged-admin.socket -After=network.target -After=dbus.service -After=iscsid.service -After=apparmor.service -After=remote-fs.target -Documentation=man:virtstoraged(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTSTORAGED_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtstoraged -ExecStart=@sbindir@/virtstoraged $VIRTSTORAGED_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtstoraged.socket -Also=virtstoraged-ro.socket -Also=virtstoraged-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/meson.build | 10 +++------- src/remote/virtproxyd.service.in | 25 ------------------------- 2 files changed, 3 insertions(+), 32 deletions(-) delete mode 100644 src/remote/virtproxyd.service.in diff --git a/src/remote/meson.build b/src/remote/meson.build index 365d632095..73a9f0a986 100644 --- a/src/remote/meson.build +++ b/src/remote/meson.build @@ -226,16 +226,12 @@ if conf.has('WITH_REMOTE') virt_daemon_units += { 'service': 'virtproxyd', - 'service_in': files('virtproxyd.service.in'), 'name': 'Libvirt proxy', 'sockprefix': 'libvirt', 'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ], - 'socket_in': files('libvirtd.socket.in'), - 'socket_ro_in': files('libvirtd-ro.socket.in'), - 'socket_admin_in': files('libvirtd-admin.socket.in'), - 'socket_tcp_in': files('libvirtd-tcp.socket.in'), - 'socket_tls_in': files('libvirtd-tls.socket.in'), - 'deps': 'Conflicts=' + libvirtd_socket_conflicts, + 'socket_unit_extra': [ + 'Conflicts=' + libvirtd_socket_conflicts, + ], } openrc_init_files += { diff --git a/src/remote/virtproxyd.service.in b/src/remote/virtproxyd.service.in deleted file mode 100644 index 9b829641f7..0000000000 --- a/src/remote/virtproxyd.service.in +++ /dev/null @@ -1,25 +0,0 @@ -[Unit] -Description=Virtualization daemon -Conflicts=libvirtd.service -Requires=virtproxyd.socket -Requires=virtproxyd-ro.socket -Requires=virtproxyd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -Documentation=man:virtproxyd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTPROXYD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtproxyd -ExecStart=@sbindir@/virtproxyd $VIRTPROXYD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtproxyd.socket -Also=virtproxyd-ro.socket -Also=virtproxyd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/vbox/meson.build | 7 +++---- src/vbox/virtvboxd.service.in | 26 -------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) delete mode 100644 src/vbox/virtvboxd.service.in diff --git a/src/vbox/meson.build b/src/vbox/meson.build index 2d6b71ab8f..e07c87eaaa 100644 --- a/src/vbox/meson.build +++ b/src/vbox/meson.build @@ -57,11 +57,10 @@ if conf.has('WITH_VBOX') virt_daemon_units += { 'service': 'virtvboxd', - 'service_in': files('virtvboxd.service.in'), 'name': 'Libvirt vbox', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'After=remote-fs.target', + ], } openrc_init_files += { diff --git a/src/vbox/virtvboxd.service.in b/src/vbox/virtvboxd.service.in deleted file mode 100644 index a567ed2443..0000000000 --- a/src/vbox/virtvboxd.service.in +++ /dev/null @@ -1,26 +0,0 @@ -[Unit] -Description=Virtualization vbox daemon -Conflicts=libvirtd.service -Requires=virtvboxd.socket -Requires=virtvboxd-ro.socket -Requires=virtvboxd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -Documentation=man:virtvboxd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTVBOXD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtvboxd -ExecStart=@sbindir@/virtvboxd $VIRTVBOXD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtvboxd.socket -Also=virtvboxd-ro.socket -Also=virtvboxd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/vz/meson.build | 7 +++---- src/vz/virtvzd.service.in | 26 -------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) delete mode 100644 src/vz/virtvzd.service.in diff --git a/src/vz/meson.build b/src/vz/meson.build index 9c2eb90463..dceea1aaac 100644 --- a/src/vz/meson.build +++ b/src/vz/meson.build @@ -48,11 +48,10 @@ if conf.has('WITH_VZ') virt_daemon_units += { 'service': 'virtvzd', - 'service_in': files('virtvzd.service.in'), 'name': 'Libvirt vz', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'After=remote-fs.target', + ], } openrc_init_files += { diff --git a/src/vz/virtvzd.service.in b/src/vz/virtvzd.service.in deleted file mode 100644 index 5521e89e10..0000000000 --- a/src/vz/virtvzd.service.in +++ /dev/null @@ -1,26 +0,0 @@ -[Unit] -Description=Virtualization vz daemon -Conflicts=libvirtd.service -Requires=virtvzd.socket -Requires=virtvzd-ro.socket -Requires=virtvzd-admin.socket -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -Documentation=man:virtvzd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTVZD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtvzd -ExecStart=@sbindir@/virtvzd $VIRTVZD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure - -[Install] -WantedBy=multi-user.target -Also=virtvzd.socket -Also=virtvzd-ro.socket -Also=virtvzd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 27 ++++++++++++++++++++---- src/ch/virtchd.service.in | 44 --------------------------------------- 2 files changed, 23 insertions(+), 48 deletions(-) delete mode 100644 src/ch/virtchd.service.in diff --git a/src/ch/meson.build b/src/ch/meson.build index dc08069dcd..f6c443f3c6 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -57,11 +57,30 @@ if conf.has('WITH_CH') virt_daemon_units += { 'service': 'virtchd', - 'service_in': files('virtchd.service.in'), 'name': 'Libvirt ch', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ], } virt_install_dirs += [ diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in deleted file mode 100644 index 351eee312b..0000000000 --- a/src/ch/virtchd.service.in +++ /dev/null @@ -1,44 +0,0 @@ -[Unit] -Description=Virtualization Cloud-Hypervisor daemon -Conflicts=libvirtd.service -Requires=virtchd.socket -Requires=virtchd-ro.socket -Requires=virtchd-admin.socket -Wants=systemd-machined.service -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -After=systemd-machined.service -Documentation=man:virtchd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTCHD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtchd -ExecStart=@sbindir@/virtchd $VIRTCHD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -KillMode=process -Restart=on-failure -# Raise hard limits to match behaviour of systemd >= 240. -# During startup, daemon will set soft limit to match hard limit -# per systemd recommendations -LimitNOFILE=1024:524288 -# The cgroups pids controller can limit the number of tasks started by -# the daemon, which can limit the number of domains for some hypervisors. -# A conservative default of 8 tasks per guest results in a TasksMax of -# 32k to support 4096 guests. -TasksMax=32768 -# With cgroups v2 there is no devices controller anymore, we have to use -# eBPF to control access to devices. In order to do that we create a eBPF -# hash MAP which locks memory. The default map size for 64 devices together -# with program takes 12k per guest. After rounding up we will get 64M to -# support 4096 guests. -LimitMEMLOCK=64M - -[Install] -WantedBy=multi-user.target -Also=virtchd.socket -Also=virtchd-ro.socket -Also=virtchd-admin.socket -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 27 ++++++++++++++++++++---- src/ch/virtchd.service.in | 44 --------------------------------------- 2 files changed, 23 insertions(+), 48 deletions(-) delete mode 100644 src/ch/virtchd.service.in
diff --git a/src/ch/meson.build b/src/ch/meson.build index dc08069dcd..f6c443f3c6 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -57,11 +57,30 @@ if conf.has('WITH_CH')
virt_daemon_units += { 'service': 'virtchd', - 'service_in': files('virtchd.service.in'), 'name': 'Libvirt ch', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ],
This feels wrong to have it in meson.build file. In addition it is the same as for virtlxcd and virtqemud so we are basically duplicating the data and which makes it easy to make inconsistent changes not affecting all places. IMHO it would be better to have additional file that will be included into the template for services where we need it. I'm not sure about the `service_unit_extra` as well if we want to have it in meson.build files as it is not strictly related to the build process and there is more data compared to the old `deps`. Pavel
}
virt_install_dirs += [ diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in deleted file mode 100644 index 351eee312b..0000000000 --- a/src/ch/virtchd.service.in +++ /dev/null @@ -1,44 +0,0 @@ -[Unit] -Description=Virtualization Cloud-Hypervisor daemon -Conflicts=libvirtd.service -Requires=virtchd.socket -Requires=virtchd-ro.socket -Requires=virtchd-admin.socket -Wants=systemd-machined.service -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -After=systemd-machined.service -Documentation=man:virtchd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTCHD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtchd -ExecStart=@sbindir@/virtchd $VIRTCHD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -KillMode=process -Restart=on-failure -# Raise hard limits to match behaviour of systemd >= 240. -# During startup, daemon will set soft limit to match hard limit -# per systemd recommendations -LimitNOFILE=1024:524288 -# The cgroups pids controller can limit the number of tasks started by -# the daemon, which can limit the number of domains for some hypervisors. -# A conservative default of 8 tasks per guest results in a TasksMax of -# 32k to support 4096 guests. -TasksMax=32768 -# With cgroups v2 there is no devices controller anymore, we have to use -# eBPF to control access to devices. In order to do that we create a eBPF -# hash MAP which locks memory. The default map size for 64 devices together -# with program takes 12k per guest. After rounding up we will get 64M to -# support 4096 guests. -LimitMEMLOCK=64M - -[Install] -WantedBy=multi-user.target -Also=virtchd.socket -Also=virtchd-ro.socket -Also=virtchd-admin.socket -- 2.41.0
On Tue, Sep 26, 2023 at 11:09:44AM +0200, Pavel Hrdina wrote:
On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote:
+ 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ],
This feels wrong to have it in meson.build file. In addition it is the same as for virtlxcd and virtqemud so we are basically duplicating the data and which makes it easy to make inconsistent changes not affecting all places.
You're right, it would make sense to deduplicate this further.
IMHO it would be better to have additional file that will be included into the template for services where we need it.
Wouldn't a variable be enough? In order to use a file, I can see two ways. First one is to have a separate virtd-hypervisor.service.in that contains the same stuff as virtd.service.in plus these comments, but that means introducing duplication on a different axis and risking the two files going out of sync. Second one is to have a virtd-comments.txt or whatever that gets included conditionally from virtd.service.in, but that means adding an extra processing step. Neither really feels an outright improvement over what we have here. Can you explain what did you have in mind? Maybe I'm just not seeing it :)
I'm not sure about the `service_unit_extra` as well if we want to have it in meson.build files as it is not strictly related to the build process and there is more data compared to the old `deps`.
That's because the various services and sockets have tiny differences between them. Having a single template is IMO stictly better for maintenability than carrying around more than a dozen copies of the same basic information, which is what we have today. It's true that this is going a bit overboard compared to what we're using configuration data for elsewhere, but I don't think it's too much of a stretch or something that feels too out of place. That said, if you have an idea for an alternative approach to achieving the same result, please do share it! I'm not married to this specific implementation :) -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Sep 26, 2023 at 11:09:44AM +0200, Pavel Hrdina wrote:
On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 27 ++++++++++++++++++++---- src/ch/virtchd.service.in | 44 --------------------------------------- 2 files changed, 23 insertions(+), 48 deletions(-) delete mode 100644 src/ch/virtchd.service.in
diff --git a/src/ch/meson.build b/src/ch/meson.build index dc08069dcd..f6c443f3c6 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -57,11 +57,30 @@ if conf.has('WITH_CH')
virt_daemon_units += { 'service': 'virtchd', - 'service_in': files('virtchd.service.in'), 'name': 'Libvirt ch', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ],
This feels wrong to have it in meson.build file. In addition it is the same as for virtlxcd and virtqemud so we are basically duplicating the data and which makes it easy to make inconsistent changes not affecting all places.
IMHO it would be better to have additional file that will be included into the template for services where we need it.
I'm not sure about the `service_unit_extra` as well if we want to have it in meson.build files as it is not strictly related to the build process and there is more data compared to the old `deps`.
If anything I'd reverse the model. The 'virtchd.service.in' file should be the primary template, the common bits the injected data. ie cat virtchd.service.in [Unit] Description=Virtualization Cloud-Hypervisor daemon ::common-unit:: Wants=systemd-machined.service After=remote-fs.target After=systemd-machined.service Documentation=man:virtchd(8) [Service] ::common-service:: KillMode=process # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of # 32k to support 4096 guests. TasksMax=32768 # With cgroups v2 there is no devices controller anymore, we have to use # eBPF to control access to devices. In order to do that we create a eBPF # hash MAP which locks memory. The default map size for 64 devices together # with program takes 12k per guest. After rounding up we will get 64M to # support 4096 guests. LimitMEMLOCK=64M [Install] ::common-install:: arguably we don't even need the '::common-XXX::' lines in there. We can simply see the headers [Unit], [Service], etc and inject the common bits under each header. 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 Tue, Sep 26, 2023 at 11:23:51AM +0100, Daniel P. Berrangé wrote:
On Tue, Sep 26, 2023 at 11:09:44AM +0200, Pavel Hrdina wrote:
On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote:
+ 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ],
This feels wrong to have it in meson.build file. In addition it is the same as for virtlxcd and virtqemud so we are basically duplicating the data and which makes it easy to make inconsistent changes not affecting all places.
IMHO it would be better to have additional file that will be included into the template for services where we need it.
I'm not sure about the `service_unit_extra` as well if we want to have it in meson.build files as it is not strictly related to the build process and there is more data compared to the old `deps`.
If anything I'd reverse the model. The 'virtchd.service.in' file should be the primary template, the common bits the injected data.
ie
cat virtchd.service.in [Unit] Description=Virtualization Cloud-Hypervisor daemon ::common-unit:: Wants=systemd-machined.service After=remote-fs.target After=systemd-machined.service Documentation=man:virtchd(8)
[Service] ::common-service:: KillMode=process # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of # 32k to support 4096 guests. TasksMax=32768 # With cgroups v2 there is no devices controller anymore, we have to use # eBPF to control access to devices. In order to do that we create a eBPF # hash MAP which locks memory. The default map size for 64 devices together # with program takes 12k per guest. After rounding up we will get 64M to # support 4096 guests. LimitMEMLOCK=64M
[Install] ::common-install::
This doesn't address the problem with duplication that Pavel pointed out. I don't think it helps much with not storing additional data inside the build system, unless we want to store the contents of the various common snippets in separate files? Something like common_service = fs.read('common_service.inc') unit_conf = configuration_data({ 'common_service' = common_service, }) We'd have to fake fs.read() because it was introduced in 0.57 though. And we'd have to run the contents of the common parts through variable substitution anyway, because they will contain a bunch of lines like Also=@service@.socket Also=@service@-ro.socket Also=@service@-admin.socket I'm not sure the result would look much better, but I can give it a try.
arguably we don't even need the '::common-XXX::' lines in there. We can simply see the headers [Unit], [Service], etc and inject the common bits under each header.
I think markers make things both easier to implement and more obvious (whoever looks at the file can immediately tell that some sort of post-processing is going to happen and probably even make a fairly accurate guess as what it will entail), so I'd prefer having them. But this is a fairly minor detail compared to the above. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Sep 26, 2023 at 07:02:19AM -0500, Andrea Bolognani wrote:
On Tue, Sep 26, 2023 at 11:23:51AM +0100, Daniel P. Berrangé wrote:
On Tue, Sep 26, 2023 at 11:09:44AM +0200, Pavel Hrdina wrote:
On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote:
+ 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ],
This feels wrong to have it in meson.build file. In addition it is the same as for virtlxcd and virtqemud so we are basically duplicating the data and which makes it easy to make inconsistent changes not affecting all places.
IMHO it would be better to have additional file that will be included into the template for services where we need it.
I'm not sure about the `service_unit_extra` as well if we want to have it in meson.build files as it is not strictly related to the build process and there is more data compared to the old `deps`.
If anything I'd reverse the model. The 'virtchd.service.in' file should be the primary template, the common bits the injected data.
ie
cat virtchd.service.in [Unit] Description=Virtualization Cloud-Hypervisor daemon ::common-unit:: Wants=systemd-machined.service After=remote-fs.target After=systemd-machined.service Documentation=man:virtchd(8)
[Service] ::common-service:: KillMode=process # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of # 32k to support 4096 guests. TasksMax=32768 # With cgroups v2 there is no devices controller anymore, we have to use # eBPF to control access to devices. In order to do that we create a eBPF # hash MAP which locks memory. The default map size for 64 devices together # with program takes 12k per guest. After rounding up we will get 64M to # support 4096 guests. LimitMEMLOCK=64M
[Install] ::common-install::
This doesn't address the problem with duplication that Pavel pointed out.
I don't think it helps much with not storing additional data inside the build system, unless we want to store the contents of the various common snippets in separate files? Something like
common_service = fs.read('common_service.inc') unit_conf = configuration_data({ 'common_service' = common_service, })
We'd have to fake fs.read() because it was introduced in 0.57 though. And we'd have to run the contents of the common parts through variable substitution anyway, because they will contain a bunch of lines like
Also=@service@.socket Also=@service@-ro.socket Also=@service@-admin.socket
I'm not sure the result would look much better, but I can give it a try.
Don't try to do any of this in meson. We should just have a standalone python script that can combine the daemon specific unit file contents with the common unit file contents. eg scripts/merge-unit-file.py \ src/qemu/virtqemud.service.in \ src/rpc/virtd.service.in \ build/src/virtqemud.service
arguably we don't even need the '::common-XXX::' lines in there. We can simply see the headers [Unit], [Service], etc and inject the common bits under each header.
I think markers make things both easier to implement and more obvious (whoever looks at the file can immediately tell that some sort of post-processing is going to happen and probably even make a fairly accurate guess as what it will entail), so I'd prefer having them. But this is a fairly minor detail compared to the above.
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 Tue, Sep 26, 2023 at 01:14:33PM +0100, Daniel P. Berrangé wrote:
On Tue, Sep 26, 2023 at 07:02:19AM -0500, Andrea Bolognani wrote:
I don't think it helps much with not storing additional data inside the build system, unless we want to store the contents of the various common snippets in separate files? Something like
common_service = fs.read('common_service.inc') unit_conf = configuration_data({ 'common_service' = common_service, })
We'd have to fake fs.read() because it was introduced in 0.57 though. And we'd have to run the contents of the common parts through variable substitution anyway, because they will contain a bunch of lines like
Also=@service@.socket Also=@service@-ro.socket Also=@service@-admin.socket
I'm not sure the result would look much better, but I can give it a try.
Don't try to do any of this in meson. We should just have a standalone python script that can combine the daemon specific unit file contents with the common unit file contents. eg
scripts/merge-unit-file.py \ src/qemu/virtqemud.service.in \ src/rpc/virtd.service.in \ build/src/virtqemud.service
It feels a bit silly to shell out to Python to perform what is ultimately a bunch of variable substitutions, as if that wasn't part of Meson's core feature set... But I'll give it a try and see how it turns out. Can you please take a look at the remaining patches in the meantime, and provide feedback on the changes that are made to the various services and sockets as part of them? Thanks in advance :) -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Sep 26, 2023 at 08:12:43AM -0500, Andrea Bolognani wrote:
On Tue, Sep 26, 2023 at 01:14:33PM +0100, Daniel P. Berrangé wrote:
On Tue, Sep 26, 2023 at 07:02:19AM -0500, Andrea Bolognani wrote:
I don't think it helps much with not storing additional data inside the build system, unless we want to store the contents of the various common snippets in separate files? Something like
common_service = fs.read('common_service.inc') unit_conf = configuration_data({ 'common_service' = common_service, })
We'd have to fake fs.read() because it was introduced in 0.57 though. And we'd have to run the contents of the common parts through variable substitution anyway, because they will contain a bunch of lines like
Also=@service@.socket Also=@service@-ro.socket Also=@service@-admin.socket
I'm not sure the result would look much better, but I can give it a try.
Don't try to do any of this in meson. We should just have a standalone python script that can combine the daemon specific unit file contents with the common unit file contents. eg
scripts/merge-unit-file.py \ src/qemu/virtqemud.service.in \ src/rpc/virtd.service.in \ build/src/virtqemud.service
It feels a bit silly to shell out to Python to perform what is ultimately a bunch of variable substitutions, as if that wasn't part of Meson's core feature set... But I'll give it a try and see how it turns out.
IMHO Meson's job is to control the build process, rather than to actually be the build process. I think of this as "compiling" the unit files and the python sript is our compiler, which meson is to control.
Can you please take a look at the remaining patches in the meantime, and provide feedback on the changes that are made to the various services and sockets as part of them? Thanks in advance :)
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 :|
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/libxl/meson.build | 21 ++++++++++++++++----- src/libxl/virtxend.service.in | 32 -------------------------------- 2 files changed, 16 insertions(+), 37 deletions(-) delete mode 100644 src/libxl/virtxend.service.in diff --git a/src/libxl/meson.build b/src/libxl/meson.build index a1553dbe27..e84999e14d 100644 --- a/src/libxl/meson.build +++ b/src/libxl/meson.build @@ -66,12 +66,23 @@ if conf.has('WITH_LIBXL') virt_daemon_units += { 'service': 'virtxend', - 'service_in': files('virtxend.service.in'), 'name': 'Libvirt libxl', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, - 'deps': 'ConditionPathExists=/proc/xen/capabilities', + 'service_unit_extra': [ + 'Wants=virtlockd.socket', + 'After=remote-fs.target', + 'After=xencommons.service', + 'Conflicts=xendomains.service', + 'ConditionPathExists=/proc/xen/capabilities', + ], + 'service_service_extra': [ + 'KillMode=process', + ], + 'service_install_extra': [ + 'Also=virtlockd.socket', + ], + 'socket_unit_extra': [ + 'ConditionPathExists=/proc/xen/capabilities', + ], } openrc_init_files += { diff --git a/src/libxl/virtxend.service.in b/src/libxl/virtxend.service.in deleted file mode 100644 index c6a88f7fe9..0000000000 --- a/src/libxl/virtxend.service.in +++ /dev/null @@ -1,32 +0,0 @@ -[Unit] -Description=Virtualization xen daemon -Conflicts=libvirtd.service -Requires=virtxend.socket -Requires=virtxend-ro.socket -Requires=virtxend-admin.socket -Wants=virtlockd.socket -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -After=xencommons.service -Conflicts=xendomains.service -Documentation=man:virtxend(8) -Documentation=https://libvirt.org -ConditionPathExists=/proc/xen/capabilities - -[Service] -Type=notify -Environment=VIRTXEND_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtxend -ExecStart=@sbindir@/virtxend $VIRTXEND_ARGS -ExecReload=/bin/kill -HUP $MAINPID -Restart=on-failure -KillMode=process - -[Install] -WantedBy=multi-user.target -Also=virtlockd.socket -Also=virtxend.socket -Also=virtxend-ro.socket -Also=virtxend-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/lxc/meson.build | 27 +++++++++++++++++++---- src/lxc/virtlxcd.service.in | 44 ------------------------------------- 2 files changed, 23 insertions(+), 48 deletions(-) delete mode 100644 src/lxc/virtlxcd.service.in diff --git a/src/lxc/meson.build b/src/lxc/meson.build index 531078448c..9b933023dd 100644 --- a/src/lxc/meson.build +++ b/src/lxc/meson.build @@ -164,11 +164,30 @@ if conf.has('WITH_LXC') virt_daemon_units += { 'service': 'virtlxcd', - 'service_in': files('virtlxcd.service.in'), 'name': 'Libvirt lxc', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ], } openrc_init_files += { diff --git a/src/lxc/virtlxcd.service.in b/src/lxc/virtlxcd.service.in deleted file mode 100644 index ee3a7f1083..0000000000 --- a/src/lxc/virtlxcd.service.in +++ /dev/null @@ -1,44 +0,0 @@ -[Unit] -Description=Virtualization lxc daemon -Conflicts=libvirtd.service -Requires=virtlxcd.socket -Requires=virtlxcd-ro.socket -Requires=virtlxcd-admin.socket -Wants=systemd-machined.service -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -After=systemd-machined.service -Documentation=man:virtlxcd(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTLXCD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtlxcd -ExecStart=@sbindir@/virtlxcd $VIRTLXCD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -KillMode=process -Restart=on-failure -# Raise hard limits to match behaviour of systemd >= 240. -# During startup, daemon will set soft limit to match hard limit -# per systemd recommendations -LimitNOFILE=1024:524288 -# The cgroups pids controller can limit the number of tasks started by -# the daemon, which can limit the number of domains for some hypervisors. -# A conservative default of 8 tasks per guest results in a TasksMax of -# 32k to support 4096 guests. -TasksMax=32768 -# With cgroups v2 there is no devices controller anymore, we have to use -# eBPF to control access to devices. In order to do that we create a eBPF -# hash MAP which locks memory. The default map size for 64 devices together -# with program takes 12k per guest. After rounding up we will get 64M to -# support 4096 guests. -LimitMEMLOCK=64M - -[Install] -WantedBy=multi-user.target -Also=virtlxcd.socket -Also=virtlxcd-ro.socket -Also=virtlxcd-admin.socket -- 2.41.0
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/meson.build | 33 +++++++++++++++++++++--- src/qemu/virtqemud.service.in | 48 ----------------------------------- 2 files changed, 29 insertions(+), 52 deletions(-) delete mode 100644 src/qemu/virtqemud.service.in diff --git a/src/qemu/meson.build b/src/qemu/meson.build index b52497bdf0..7e5db09e0c 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -183,11 +183,36 @@ if conf.has('WITH_QEMU') virt_daemon_units += { 'service': 'virtqemud', - 'service_in': files('virtqemud.service.in'), 'name': 'Libvirt qemu', - 'socket_in': libvirtd_socket_in, - 'socket_ro_in': libvirtd_socket_ro_in, - 'socket_admin_in': libvirtd_socket_admin_in, + 'service_unit_extra': [ + 'Requires=virtlogd.socket', + 'Wants=virtlockd.socket', + 'Wants=systemd-machined.service', + 'After=systemd-machined.service', + 'After=remote-fs.target', + ], + 'service_service_extra': [ + 'KillMode=process', + '# Raise hard limits to match behaviour of systemd >= 240.', + '# During startup, daemon will set soft limit to match hard limit', + '# per systemd recommendations', + 'LimitNOFILE=1024:524288', + '# The cgroups pids controller can limit the number of tasks started by', + '# the daemon, which can limit the number of domains for some hypervisors.', + '# A conservative default of 8 tasks per guest results in a TasksMax of', + '# 32k to support 4096 guests.', + 'TasksMax=32768', + '# With cgroups v2 there is no devices controller anymore, we have to use', + '# eBPF to control access to devices. In order to do that we create a eBPF', + '# hash MAP which locks memory. The default map size for 64 devices together', + '# with program takes 12k per guest. After rounding up we will get 64M to', + '# support 4096 guests.', + 'LimitMEMLOCK=64M', + ], + 'service_install_extra': [ + 'Also=virtlogd.socket', + 'Also=virtlockd.socket', + ], } openrc_init_files += { diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in deleted file mode 100644 index e79670ca95..0000000000 --- a/src/qemu/virtqemud.service.in +++ /dev/null @@ -1,48 +0,0 @@ -[Unit] -Description=Virtualization qemu daemon -Conflicts=libvirtd.service -Requires=virtlogd.socket -Requires=virtqemud.socket -Requires=virtqemud-ro.socket -Requires=virtqemud-admin.socket -Wants=virtlockd.socket -Wants=systemd-machined.service -After=network.target -After=dbus.service -After=apparmor.service -After=remote-fs.target -After=systemd-machined.service -Documentation=man:virtqemud(8) -Documentation=https://libvirt.org - -[Service] -Type=notify -Environment=VIRTQEMUD_ARGS="--timeout 120" -EnvironmentFile=-@initconfdir@/virtqemud -ExecStart=@sbindir@/virtqemud $VIRTQEMUD_ARGS -ExecReload=/bin/kill -HUP $MAINPID -KillMode=process -Restart=on-failure -# Raise hard limits to match behaviour of systemd >= 240. -# During startup, daemon will set soft limit to match hard limit -# per systemd recommendations -LimitNOFILE=1024:524288 -# The cgroups pids controller can limit the number of tasks started by -# the daemon, which can limit the number of domains for some hypervisors. -# A conservative default of 8 tasks per guest results in a TasksMax of -# 32k to support 4096 guests. -TasksMax=32768 -# With cgroups v2 there is no devices controller anymore, we have to use -# eBPF to control access to devices. In order to do that we create a eBPF -# hash MAP which locks memory. The default map size for 64 devices together -# with program takes 12k per guest. After rounding up we will get 64M to -# support 4096 guests. -LimitMEMLOCK=64M - -[Install] -WantedBy=multi-user.target -Also=virtlogd.socket -Also=virtlockd.socket -Also=virtqemud.socket -Also=virtqemud-ro.socket -Also=virtqemud-admin.socket -- 2.41.0
Now that the migration to common templates has been completed, we no longer need these. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/meson.build b/src/meson.build index b00895fd07..4e837c1e6d 100644 --- a/src/meson.build +++ b/src/meson.build @@ -191,10 +191,6 @@ virt_test_aug_dir = datadir / 'augeas' / 'lenses' / 'tests' # guest unit files to install guest_unit_files = [] -libvirtd_socket_in = files('remote' / 'libvirtd.socket.in') -libvirtd_socket_ro_in = files('remote' / 'libvirtd-ro.socket.in') -libvirtd_socket_admin_in = files('remote' / 'libvirtd-admin.socket.in') - # virt_daemon_units: # generate libvirt daemon systemd unit files # * service - name of the service (required) -- 2.41.0
It's no longer used anywhere. @socket_unit_extra@ would be its equivalent when using common templates. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 2 -- src/remote/libvirtd-admin.socket.in | 1 - src/remote/libvirtd-ro.socket.in | 1 - src/remote/libvirtd-tcp.socket.in | 1 - src/remote/libvirtd-tls.socket.in | 1 - src/remote/libvirtd.socket.in | 1 - 6 files changed, 7 deletions(-) diff --git a/src/meson.build b/src/meson.build index 4e837c1e6d..7d45e23a37 100644 --- a/src/meson.build +++ b/src/meson.build @@ -201,7 +201,6 @@ guest_unit_files = [] # * socket_$name_in - additional socket source files (optional, default virtd.socket.in or virtd-$name.socket.in) # * service_$name_extra - additional lines for service's [$name] section (optional, default []) # * socket_$name_extra - additional lines for socket's [$name] section (optional, default []) -# * deps - socket dependencies (optional, default '') virt_daemon_units = [] # openrc_init_files @@ -817,7 +816,6 @@ if conf.has('WITH_LIBVIRTD') 'service': unit['service'], 'SERVICE': unit['service'].to_upper(), 'sockprefix': unit.get('sockprefix', unit['service']), - 'deps': unit.get('deps', ''), 'sockmode': sockmode, }) diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 01e1a08939..39bb0badea 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -3,7 +3,6 @@ Description=@name@ admin socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket -@deps@ [Socket] ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index 58ae1beb95..b7b7ae0dd8 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -3,7 +3,6 @@ Description=@name@ local read-only socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket -@deps@ [Socket] ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index 6949df315e..7c8bcdb525 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -3,7 +3,6 @@ Description=@name@ non-TLS IP socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket -@deps@ [Socket] ListenStream=16509 diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index ada2b871f0..c6dceb2d4e 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -3,7 +3,6 @@ Description=@name@ TLS IP socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket -@deps@ [Socket] ListenStream=16514 diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index e6e903a8ce..aec0708fd4 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -1,7 +1,6 @@ [Unit] Description=@name@ local socket Before=@service@.service -@deps@ [Socket] ListenStream=@runstatedir@/libvirt/@sockprefix@-sock -- 2.41.0
Up until now the files have been used as template for most services, but now that those have been converted to common templates we can drop parametrization and make it clear that these files are for libvirtd only. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/libvirtd-admin.socket.in | 10 +++++----- src/remote/libvirtd-ro.socket.in | 10 +++++----- src/remote/libvirtd-tcp.socket.in | 8 ++++---- src/remote/libvirtd-tls.socket.in | 8 ++++---- src/remote/libvirtd.socket.in | 6 +++--- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 39bb0badea..8d927db63b 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -1,12 +1,12 @@ [Unit] Description=@name@ admin socket -Before=@service@.service -BindsTo=@service@.socket -After=@service@.socket +Before=libvirtd.service +BindsTo=libvirtd.socket +After=libvirtd.socket [Socket] -ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock -Service=@service@.service +ListenStream=@runstatedir@/libvirt/libvirt-admin-sock +Service=libvirtd.service SocketMode=0600 [Install] diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index b7b7ae0dd8..cc10190ab4 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -1,12 +1,12 @@ [Unit] Description=@name@ local read-only socket -Before=@service@.service -BindsTo=@service@.socket -After=@service@.socket +Before=libvirtd.service +BindsTo=libvirtd.socket +After=libvirtd.socket [Socket] -ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro -Service=@service@.service +ListenStream=@runstatedir@/libvirt/libvirt-sock-ro +Service=libvirtd.service SocketMode=0666 [Install] diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index 7c8bcdb525..bc35f19c06 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -1,12 +1,12 @@ [Unit] Description=@name@ non-TLS IP socket -Before=@service@.service -BindsTo=@service@.socket -After=@service@.socket +Before=libvirtd.service +BindsTo=libvirtd.socket +After=libvirtd.socket [Socket] ListenStream=16509 -Service=@service@.service +Service=libvirtd.service [Install] WantedBy=sockets.target diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index c6dceb2d4e..868a0be318 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -1,12 +1,12 @@ [Unit] Description=@name@ TLS IP socket -Before=@service@.service -BindsTo=@service@.socket -After=@service@.socket +Before=libvirtd.service +BindsTo=libvirtd.socket +After=libvirtd.socket [Socket] ListenStream=16514 -Service=@service@.service +Service=libvirtd.service [Install] WantedBy=sockets.target diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index aec0708fd4..ea0554546a 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -1,10 +1,10 @@ [Unit] Description=@name@ local socket -Before=@service@.service +Before=libvirtd.service [Socket] -ListenStream=@runstatedir@/libvirt/@sockprefix@-sock -Service=@service@.service +ListenStream=@runstatedir@/libvirt/libvirt-sock +Service=libvirtd.service SocketMode=@sockmode@ RemoveOnStop=yes -- 2.41.0
The idea behind these is to prevent running both modular daemons and monolithic daemon at the same time. We will implement a more effective solution for that shortly. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/meson.build | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/remote/meson.build b/src/remote/meson.build index 73a9f0a986..5ee6d4e61c 100644 --- a/src/remote/meson.build +++ b/src/remote/meson.build @@ -128,8 +128,6 @@ libvirtd_socket_unit_files = [ 'libvirtd-tls.socket', ] -libvirtd_socket_conflicts = ' '.join(libvirtd_socket_unit_files) - logrotate_files = [ 'libvirtd.qemu', 'libvirtd.lxc', @@ -229,9 +227,6 @@ if conf.has('WITH_REMOTE') 'name': 'Libvirt proxy', 'sockprefix': 'libvirt', 'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ], - 'socket_unit_extra': [ - 'Conflicts=' + libvirtd_socket_conflicts, - ], } openrc_init_files += { -- 2.41.0
We want to make sure that, at any given time, we have either the modular daemons or the monolithic one running, never both. In order to achieve that, make every single modular unit conflict with the corresponding libvirtd unit. We set both Conflicts=libvirtd.unit and After=libvirtd.unit: this tells systemd that, whenever virtfood.unit and libvirtd.unit are part of the same transaction, the former should win out. Thanks to this, if both the modular daemons and the monolithic one have been enabled because of outdated automation or a simple mistake of the administrator, the request to start libvirtd at boot will be ignored and the result will be a regular modular deployment. If the request to start libvirtd is made when the modular daemons are already running, we have no way to prevent systemd from complying with that request; however, thanks to the way the conflict relationship has been declared, they will be shut down cleanly before libvirtd is started. From the user's point of view, the transition from modular to monolithic will be completely transparent: it's basically the same scenario as a regular package upgrade, just with an extra twist. Note that, while switching from modular to monolithic at runtime happens automatically, going back requires manual intervention, i.e. starting all the necessary sockets one by one. That's okay: the goal here is to prevent misconfiguration and force of habit to accidentally disrupt a working setup, not to encourage the scenario. In a correctly configured and managed host, it should never occur. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/virtd-admin.socket.in | 2 ++ src/virtd-ro.socket.in | 2 ++ src/virtd-tcp.socket.in | 2 ++ src/virtd-tls.socket.in | 2 ++ src/virtd.service.in | 3 ++- src/virtd.socket.in | 2 ++ 6 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in index 3a09951b12..e43f4df82d 100644 --- a/src/virtd-admin.socket.in +++ b/src/virtd-admin.socket.in @@ -3,6 +3,8 @@ Description=@name@ admin socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket +Conflicts=libvirtd-admin.socket +After=libvirtd-admin.socket @socket_unit_extra@ [Socket] diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in index e882f25a7b..7b91f0d657 100644 --- a/src/virtd-ro.socket.in +++ b/src/virtd-ro.socket.in @@ -3,6 +3,8 @@ Description=@name@ local read-only socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket +Conflicts=libvirtd-ro.socket +After=libvirtd-ro.socket @socket_unit_extra@ [Socket] diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in index 26c6dfa75b..d715e2d00d 100644 --- a/src/virtd-tcp.socket.in +++ b/src/virtd-tcp.socket.in @@ -3,6 +3,8 @@ Description=@name@ non-TLS IP socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket +Conflicts=libvirtd-tcp.socket +After=libvirtd-tcp.socket @socket_unit_extra@ [Socket] diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in index 077c320cce..5b2550fb1d 100644 --- a/src/virtd-tls.socket.in +++ b/src/virtd-tls.socket.in @@ -3,6 +3,8 @@ Description=@name@ TLS IP socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket +Conflicts=libvirt-tls.socket +After=libvirt-tls.socket @socket_unit_extra@ [Socket] diff --git a/src/virtd.service.in b/src/virtd.service.in index c9afecad73..21391a65b0 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -1,9 +1,10 @@ [Unit] Description=@name@ daemon -Conflicts=libvirtd.service Requires=@service@.socket Requires=@service@-ro.socket Requires=@service@-admin.socket +Conflicts=libvirtd.service +After=libvirtd.service After=network.target After=dbus.service After=apparmor.service diff --git a/src/virtd.socket.in b/src/virtd.socket.in index 278f59ef1c..e4dc94b277 100644 --- a/src/virtd.socket.in +++ b/src/virtd.socket.in @@ -1,6 +1,8 @@ [Unit] Description=@name@ local socket Before=@service@.service +Conflicts=libvirtd.socket +After=libvirtd.socket @socket_unit_extra@ [Socket] -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:32PM +0200, Andrea Bolognani wrote:
We want to make sure that, at any given time, we have either the modular daemons or the monolithic one running, never both. In order to achieve that, make every single modular unit conflict with the corresponding libvirtd unit.
We set both Conflicts=libvirtd.unit and After=libvirtd.unit: this tells systemd that, whenever virtfood.unit and libvirtd.unit are part of the same transaction, the former should win out.
Thanks to this, if both the modular daemons and the monolithic one have been enabled because of outdated automation or a simple mistake of the administrator, the request to start libvirtd at boot will be ignored and the result will be a regular modular deployment.
If the request to start libvirtd is made when the modular daemons are already running, we have no way to prevent systemd from complying with that request; however, thanks to the way the conflict relationship has been declared, they will be shut down cleanly before libvirtd is started. From the user's point of view, the transition from modular to monolithic will be completely transparent: it's basically the same scenario as a regular package upgrade, just with an extra twist.
Note that, while switching from modular to monolithic at runtime happens automatically, going back requires manual intervention, i.e. starting all the necessary sockets one by one. That's okay: the goal here is to prevent misconfiguration and force of habit to accidentally disrupt a working setup, not to encourage the scenario. In a correctly configured and managed host, it should never occur.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/virtd-admin.socket.in | 2 ++ src/virtd-ro.socket.in | 2 ++ src/virtd-tcp.socket.in | 2 ++ src/virtd-tls.socket.in | 2 ++ src/virtd.service.in | 3 ++- src/virtd.socket.in | 2 ++ 6 files changed, 12 insertions(+), 1 deletion(-)
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 :|
On Mon, Sep 25, 2023 at 08:58:32PM +0200, Andrea Bolognani wrote:
+++ b/src/virtd-tls.socket.in @@ -3,6 +3,8 @@ Description=@name@ TLS IP socket Before=@service@.service BindsTo=@service@.socket After=@service@.socket +Conflicts=libvirt-tls.socket +After=libvirt-tls.socket @socket_unit_extra@
These should obviously have been libvirt*d*-tls.socket. -- Andrea Bolognani / Red Hat / Virtualization
This is the strongest relationship that can be declared between two units, and causes the service to be terminated immediately if any of its sockets disappear. This is the behavior we want. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 6 ++++-- src/logging/virtlogd.service.in | 6 ++++-- src/virtd.service.in | 9 ++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index 9e91fa3261..a21a2c2c19 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -1,7 +1,9 @@ [Unit] Description=Virtual machine lock manager -Requires=virtlockd.socket -Requires=virtlockd-admin.socket +BindsTo=virtlockd.socket +BindsTo=virtlockd-admin.socket +After=virtlockd.socket +After=virtlockd-admin.socket Before=libvirtd.service Documentation=man:virtlockd(8) Documentation=https://libvirt.org diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 97c942ffb0..f3bd576301 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -1,7 +1,9 @@ [Unit] Description=Virtual machine log manager -Requires=virtlogd.socket -Requires=virtlogd-admin.socket +BindsTo=virtlogd.socket +BindsTo=virtlogd-admin.socket +After=virtlogd.socket +After=virtlogd-admin.socket Before=libvirtd.service Documentation=man:virtlogd(8) Documentation=https://libvirt.org diff --git a/src/virtd.service.in b/src/virtd.service.in index 21391a65b0..b9e6345e8c 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -1,8 +1,11 @@ [Unit] Description=@name@ daemon -Requires=@service@.socket -Requires=@service@-ro.socket -Requires=@service@-admin.socket +BindsTo=@service@.socket +BindsTo=@service@-ro.socket +BindsTo=@service@-admin.socket +After=@service@.socket +After=@service@-ro.socket +After=@service@-admin.socket Conflicts=libvirtd.service After=libvirtd.service After=network.target -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
This is the strongest relationship that can be declared between two units, and causes the service to be terminated immediately if any of its sockets disappear. This is the behavior we want.
IIUC, this prevents running the service with /only/ the main socket, and ro/admin sockets disabled. Running without the ro socket in particular was something we wanted to allow to reduce exposure to unprivileged services (there have been a number of CVEs where the read-only socket was the way in)
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 6 ++++-- src/logging/virtlogd.service.in | 6 ++++-- src/virtd.service.in | 9 ++++++--- 3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index 9e91fa3261..a21a2c2c19 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -1,7 +1,9 @@ [Unit] Description=Virtual machine lock manager -Requires=virtlockd.socket -Requires=virtlockd-admin.socket +BindsTo=virtlockd.socket +BindsTo=virtlockd-admin.socket +After=virtlockd.socket +After=virtlockd-admin.socket Before=libvirtd.service Documentation=man:virtlockd(8) Documentation=https://libvirt.org diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 97c942ffb0..f3bd576301 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -1,7 +1,9 @@ [Unit] Description=Virtual machine log manager -Requires=virtlogd.socket -Requires=virtlogd-admin.socket +BindsTo=virtlogd.socket +BindsTo=virtlogd-admin.socket +After=virtlogd.socket +After=virtlogd-admin.socket Before=libvirtd.service Documentation=man:virtlogd(8) Documentation=https://libvirt.org diff --git a/src/virtd.service.in b/src/virtd.service.in index 21391a65b0..b9e6345e8c 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -1,8 +1,11 @@ [Unit] Description=@name@ daemon -Requires=@service@.socket -Requires=@service@-ro.socket -Requires=@service@-admin.socket +BindsTo=@service@.socket +BindsTo=@service@-ro.socket +BindsTo=@service@-admin.socket +After=@service@.socket +After=@service@-ro.socket +After=@service@-admin.socket Conflicts=libvirtd.service After=libvirtd.service After=network.target -- 2.41.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 :|
On Tue, Sep 26, 2023 at 09:44:52AM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
This is the strongest relationship that can be declared between two units, and causes the service to be terminated immediately if any of its sockets disappear. This is the behavior we want.
IIUC, this prevents running the service with /only/ the main socket, and ro/admin sockets disabled. Running without the ro socket in particular was something we wanted to allow to reduce exposure to unprivileged services (there have been a number of CVEs where the read-only socket was the way in)
This doesn't work today either AFAICT, since the ro/admin sockets are marked as Required by the various services. If we want to support this configuration, then we need # foo.service [Unit] BindsTo=foo.socket Wants=foo-ro.socket Wants=foo-admin.socket After=foo.socket In the default scenario, things will work just the same as they do here, but it will also be possible to mask foo{-ro,-admin}.socket to obtain the hardened setup you describe. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Sep 26, 2023 at 04:09:17AM -0500, Andrea Bolognani wrote:
On Tue, Sep 26, 2023 at 09:44:52AM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
This is the strongest relationship that can be declared between two units, and causes the service to be terminated immediately if any of its sockets disappear. This is the behavior we want.
IIUC, this prevents running the service with /only/ the main socket, and ro/admin sockets disabled. Running without the ro socket in particular was something we wanted to allow to reduce exposure to unprivileged services (there have been a number of CVEs where the read-only socket was the way in)
This doesn't work today either AFAICT, since the ro/admin sockets are marked as Required by the various services.
Doh, yes, I've confirmed. I'm sure it used to work, but we must have broken it at some point as we tweaked the deps countless times over to finese the setup.
If we want to support this configuration, then we need
# foo.service [Unit] BindsTo=foo.socket Wants=foo-ro.socket Wants=foo-admin.socket After=foo.socket
In the default scenario, things will work just the same as they do here, but it will also be possible to mask foo{-ro,-admin}.socket to obtain the hardened setup you describe.
Or we just decide to keep life simple, and if people want to harden things they can change permissions on the socket via a system unit override locally. 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 Tue, Sep 26, 2023 at 01:36:39PM +0100, Daniel P. Berrangé wrote:
On Tue, Sep 26, 2023 at 04:09:17AM -0500, Andrea Bolognani wrote:
On Tue, Sep 26, 2023 at 09:44:52AM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
This is the strongest relationship that can be declared between two units, and causes the service to be terminated immediately if any of its sockets disappear. This is the behavior we want.
IIUC, this prevents running the service with /only/ the main socket, and ro/admin sockets disabled. Running without the ro socket in particular was something we wanted to allow to reduce exposure to unprivileged services (there have been a number of CVEs where the read-only socket was the way in)
This doesn't work today either AFAICT, since the ro/admin sockets are marked as Required by the various services.
Doh, yes, I've confirmed. I'm sure it used to work, but we must have broken it at some point as we tweaked the deps countless times over to finese the setup.
If we want to support this configuration, then we need
# foo.service [Unit] BindsTo=foo.socket Wants=foo-ro.socket Wants=foo-admin.socket After=foo.socket
In the default scenario, things will work just the same as they do here, but it will also be possible to mask foo{-ro,-admin}.socket to obtain the hardened setup you describe.
Or we just decide to keep life simple, and if people want to harden things they can change permissions on the socket via a system unit override locally.
I don't think this is any more complicated than the version that uses BindsTo/After for all sockets, and it shouldn't make things any worse for people who stick with the defaults, so I don't mind trying to integrate this requirement into v2. -- Andrea Bolognani / Red Hat / Virtualization
Requires/Wants only tells systemd that the corresponding unit should be started when the current one is, but that could very well happen in parallel. For virtlogd/virtlockd, we want the socket to be already active when the hypervisor driver is started. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/libxl/meson.build | 1 + src/qemu/meson.build | 2 ++ src/remote/libvirtd.service.in | 7 ++++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libxl/meson.build b/src/libxl/meson.build index e84999e14d..ad8d9b757f 100644 --- a/src/libxl/meson.build +++ b/src/libxl/meson.build @@ -69,6 +69,7 @@ if conf.has('WITH_LIBXL') 'name': 'Libvirt libxl', 'service_unit_extra': [ 'Wants=virtlockd.socket', + 'After=virtlockd.socket', 'After=remote-fs.target', 'After=xencommons.service', 'Conflicts=xendomains.service', diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 7e5db09e0c..bf900e3f14 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -187,6 +187,8 @@ if conf.has('WITH_QEMU') 'service_unit_extra': [ 'Requires=virtlogd.socket', 'Wants=virtlockd.socket', + 'After=virtlogd.socket', + 'After=virtlockd.socket', 'Wants=systemd-machined.service', 'After=systemd-machined.service', 'After=remote-fs.target', diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 8839c00a15..a2c3c8f8fa 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -1,13 +1,18 @@ [Unit] Description=Virtualization daemon -Requires=virtlogd.socket # Use Wants instead of Requires so that users # can disable these three .socket units to revert # to a traditional non-activation deployment setup Wants=libvirtd.socket Wants=libvirtd-ro.socket Wants=libvirtd-admin.socket +After=libvirtd.socket +After=libvirtd-ro.socket +After=libvirtd-admin.socket +Requires=virtlogd.socket Wants=virtlockd.socket +After=virtlogd.socket +After=virtlockd.socket Wants=systemd-machined.service After=network.target After=dbus.service -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:34PM +0200, Andrea Bolognani wrote:
Requires/Wants only tells systemd that the corresponding unit should be started when the current one is, but that could very well happen in parallel. For virtlogd/virtlockd, we want the socket to be already active when the hypervisor driver is started.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/libxl/meson.build | 1 + src/qemu/meson.build | 2 ++ src/remote/libvirtd.service.in | 7 ++++++- 3 files changed, 9 insertions(+), 1 deletion(-)
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 :|
We have already declared the mirror relationship, so this one is now redundant. Moreover, this version was incomplete: it only ever worked for the monolithic daemon, but the modular daemons for QEMU and Xen also want the sockets to be active. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd-admin.socket.in | 1 - src/locking/virtlockd.service.in | 1 - src/locking/virtlockd.socket.in | 1 - src/logging/virtlogd-admin.socket.in | 1 - src/logging/virtlogd.service.in | 1 - src/logging/virtlogd.socket.in | 1 - 6 files changed, 6 deletions(-) diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in index c66e0f9693..d5ebd7f60b 100644 --- a/src/locking/virtlockd-admin.socket.in +++ b/src/locking/virtlockd-admin.socket.in @@ -1,6 +1,5 @@ [Unit] Description=Virtual machine lock manager admin socket -Before=libvirtd.service BindsTo=virtlockd.socket After=virtlockd.socket diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index a21a2c2c19..ebf8cb9d5a 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -4,7 +4,6 @@ BindsTo=virtlockd.socket BindsTo=virtlockd-admin.socket After=virtlockd.socket After=virtlockd-admin.socket -Before=libvirtd.service Documentation=man:virtlockd(8) Documentation=https://libvirt.org diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in index 4ce75391ae..d2cc2a06a3 100644 --- a/src/locking/virtlockd.socket.in +++ b/src/locking/virtlockd.socket.in @@ -1,6 +1,5 @@ [Unit] Description=Virtual machine lock manager socket -Before=libvirtd.service [Socket] ListenStream=@runstatedir@/libvirt/virtlockd-sock diff --git a/src/logging/virtlogd-admin.socket.in b/src/logging/virtlogd-admin.socket.in index 5c0fb1880e..67259803ca 100644 --- a/src/logging/virtlogd-admin.socket.in +++ b/src/logging/virtlogd-admin.socket.in @@ -1,6 +1,5 @@ [Unit] Description=Virtual machine log manager socket -Before=libvirtd.service BindsTo=virtlogd.socket After=virtlogd.socket diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index f3bd576301..72743a61ae 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -4,7 +4,6 @@ BindsTo=virtlogd.socket BindsTo=virtlogd-admin.socket After=virtlogd.socket After=virtlogd-admin.socket -Before=libvirtd.service Documentation=man:virtlogd(8) Documentation=https://libvirt.org diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in index ff3e66e09b..7b3fc73773 100644 --- a/src/logging/virtlogd.socket.in +++ b/src/logging/virtlogd.socket.in @@ -1,6 +1,5 @@ [Unit] Description=Virtual machine log manager socket -Before=libvirtd.service [Socket] ListenStream=@runstatedir@/libvirt/virtlogd-sock -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:35PM +0200, Andrea Bolognani wrote:
We have already declared the mirror relationship, so this one is now redundant.
Moreover, this version was incomplete: it only ever worked for the monolithic daemon, but the modular daemons for QEMU and Xen also want the sockets to be active.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd-admin.socket.in | 1 - src/locking/virtlockd.service.in | 1 - src/locking/virtlockd.socket.in | 1 - src/logging/virtlogd-admin.socket.in | 1 - src/logging/virtlogd.service.in | 1 - src/logging/virtlogd.socket.in | 1 - 6 files changed, 6 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 :|
systemd will automatically infer this dependency based on the socket's Service=foo.service setting. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/libvirtd-admin.socket.in | 1 - src/remote/libvirtd-ro.socket.in | 1 - src/remote/libvirtd-tcp.socket.in | 1 - src/remote/libvirtd-tls.socket.in | 1 - src/remote/libvirtd.socket.in | 1 - src/virtd-admin.socket.in | 1 - src/virtd-ro.socket.in | 1 - src/virtd-tcp.socket.in | 1 - src/virtd-tls.socket.in | 1 - src/virtd.socket.in | 1 - 10 files changed, 10 deletions(-) diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 8d927db63b..098e372971 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ admin socket -Before=libvirtd.service BindsTo=libvirtd.socket After=libvirtd.socket diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index cc10190ab4..101555e8a0 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ local read-only socket -Before=libvirtd.service BindsTo=libvirtd.socket After=libvirtd.socket diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index bc35f19c06..8b8fbcd01a 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ non-TLS IP socket -Before=libvirtd.service BindsTo=libvirtd.socket After=libvirtd.socket diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index 868a0be318..fefda22c6b 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ TLS IP socket -Before=libvirtd.service BindsTo=libvirtd.socket After=libvirtd.socket diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index ea0554546a..3019821df3 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ local socket -Before=libvirtd.service [Socket] ListenStream=@runstatedir@/libvirt/libvirt-sock diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in index e43f4df82d..8851dde1bc 100644 --- a/src/virtd-admin.socket.in +++ b/src/virtd-admin.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ admin socket -Before=@service@.service BindsTo=@service@.socket After=@service@.socket Conflicts=libvirtd-admin.socket diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in index 7b91f0d657..89b8d80763 100644 --- a/src/virtd-ro.socket.in +++ b/src/virtd-ro.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ local read-only socket -Before=@service@.service BindsTo=@service@.socket After=@service@.socket Conflicts=libvirtd-ro.socket diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in index d715e2d00d..2873c35135 100644 --- a/src/virtd-tcp.socket.in +++ b/src/virtd-tcp.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ non-TLS IP socket -Before=@service@.service BindsTo=@service@.socket After=@service@.socket Conflicts=libvirtd-tcp.socket diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in index 5b2550fb1d..2d4d589c8a 100644 --- a/src/virtd-tls.socket.in +++ b/src/virtd-tls.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ TLS IP socket -Before=@service@.service BindsTo=@service@.socket After=@service@.socket Conflicts=libvirt-tls.socket diff --git a/src/virtd.socket.in b/src/virtd.socket.in index e4dc94b277..df4a619dd4 100644 --- a/src/virtd.socket.in +++ b/src/virtd.socket.in @@ -1,6 +1,5 @@ [Unit] Description=@name@ local socket -Before=@service@.service Conflicts=libvirtd.socket After=libvirtd.socket @socket_unit_extra@ -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:36PM +0200, Andrea Bolognani wrote:
systemd will automatically infer this dependency based on the socket's Service=foo.service setting.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/libvirtd-admin.socket.in | 1 - src/remote/libvirtd-ro.socket.in | 1 - src/remote/libvirtd-tcp.socket.in | 1 - src/remote/libvirtd-tls.socket.in | 1 - src/remote/libvirtd.socket.in | 1 - src/virtd-admin.socket.in | 1 - src/virtd-ro.socket.in | 1 - src/virtd-tcp.socket.in | 1 - src/virtd-tls.socket.in | 1 - src/virtd.socket.in | 1 - 10 files changed, 10 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 :|
This results in all sockets for a service being enabled when a single one of them is. The -tcp and -tls sockets are intentionally excluded, because enabling them should require explicit action on the administrator's part; moreover, disabling them should not result in the local sockets being disabled too. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd-admin.socket.in | 1 + src/locking/virtlockd.socket.in | 1 + src/logging/virtlogd-admin.socket.in | 1 + src/logging/virtlogd.socket.in | 1 + src/remote/libvirtd-admin.socket.in | 2 ++ src/remote/libvirtd-ro.socket.in | 2 ++ src/remote/libvirtd.socket.in | 2 ++ src/virtd-admin.socket.in | 2 ++ src/virtd-ro.socket.in | 2 ++ src/virtd.socket.in | 2 ++ 10 files changed, 16 insertions(+) diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in index d5ebd7f60b..63f78a02da 100644 --- a/src/locking/virtlockd-admin.socket.in +++ b/src/locking/virtlockd-admin.socket.in @@ -10,3 +10,4 @@ SocketMode=0600 [Install] WantedBy=sockets.target +Also=@service@.socket diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in index d2cc2a06a3..1cd50c70ec 100644 --- a/src/locking/virtlockd.socket.in +++ b/src/locking/virtlockd.socket.in @@ -8,3 +8,4 @@ SocketMode=0600 [Install] WantedBy=sockets.target +Also=@service@-admin.socket diff --git a/src/logging/virtlogd-admin.socket.in b/src/logging/virtlogd-admin.socket.in index 67259803ca..1d18fe6f56 100644 --- a/src/logging/virtlogd-admin.socket.in +++ b/src/logging/virtlogd-admin.socket.in @@ -10,3 +10,4 @@ SocketMode=0600 [Install] WantedBy=sockets.target +Also=@service@.socket diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in index 7b3fc73773..0b85af927b 100644 --- a/src/logging/virtlogd.socket.in +++ b/src/logging/virtlogd.socket.in @@ -8,3 +8,4 @@ SocketMode=0600 [Install] WantedBy=sockets.target +Also=@service@-admin.socket diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 098e372971..6df038d95a 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -10,3 +10,5 @@ SocketMode=0600 [Install] WantedBy=sockets.target +Also=libvirtd.socket +Also=libvirtd-ro.socket diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index 101555e8a0..6797517c50 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -10,3 +10,5 @@ SocketMode=0666 [Install] WantedBy=sockets.target +Also=libvirtd.socket +Also=libvirtd-admin.socket diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index 3019821df3..f483facdf3 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -9,3 +9,5 @@ RemoveOnStop=yes [Install] WantedBy=sockets.target +Also=libvirtd-ro.socket +Also=libvirtd-admin.socket diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in index 8851dde1bc..a4faeb7da8 100644 --- a/src/virtd-admin.socket.in +++ b/src/virtd-admin.socket.in @@ -14,4 +14,6 @@ SocketMode=0600 [Install] WantedBy=sockets.target +Also=@service@.socket +Also=@service@-ro.socket @socket_install_extra@ diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in index 89b8d80763..829c2e8b1f 100644 --- a/src/virtd-ro.socket.in +++ b/src/virtd-ro.socket.in @@ -14,4 +14,6 @@ SocketMode=0666 [Install] WantedBy=sockets.target +Also=@service@.socket +Also=@service@-admin.socket @socket_install_extra@ diff --git a/src/virtd.socket.in b/src/virtd.socket.in index df4a619dd4..dc25e4d781 100644 --- a/src/virtd.socket.in +++ b/src/virtd.socket.in @@ -13,4 +13,6 @@ RemoveOnStop=yes [Install] WantedBy=sockets.target +Also=@service@-ro.socket +Also=@service@-admin.socket @socket_install_extra@ -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:37PM +0200, Andrea Bolognani wrote:
This results in all sockets for a service being enabled when a single one of them is.
The -tcp and -tls sockets are intentionally excluded, because enabling them should require explicit action on the administrator's part; moreover, disabling them should not result in the local sockets being disabled too.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd-admin.socket.in | 1 + src/locking/virtlockd.socket.in | 1 + src/logging/virtlogd-admin.socket.in | 1 + src/logging/virtlogd.socket.in | 1 + src/remote/libvirtd-admin.socket.in | 2 ++ src/remote/libvirtd-ro.socket.in | 2 ++ src/remote/libvirtd.socket.in | 2 ++ src/virtd-admin.socket.in | 2 ++ src/virtd-ro.socket.in | 2 ++ src/virtd.socket.in | 2 ++ 10 files changed, 16 insertions(+)
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 :|
They are unnecessary, since all sockets for a service are now enabled as soon as one of them is and each service has a very strong dependency on all of its sockets. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd-admin.socket.in | 2 -- src/logging/virtlogd-admin.socket.in | 2 -- src/remote/libvirtd-admin.socket.in | 2 -- src/remote/libvirtd-ro.socket.in | 2 -- src/remote/libvirtd-tcp.socket.in | 2 -- src/remote/libvirtd-tls.socket.in | 2 -- src/virtd-admin.socket.in | 2 -- src/virtd-ro.socket.in | 2 -- src/virtd-tcp.socket.in | 2 -- src/virtd-tls.socket.in | 2 -- 10 files changed, 20 deletions(-) diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in index 63f78a02da..a773b511bd 100644 --- a/src/locking/virtlockd-admin.socket.in +++ b/src/locking/virtlockd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=Virtual machine lock manager admin socket -BindsTo=virtlockd.socket -After=virtlockd.socket [Socket] ListenStream=@runstatedir@/libvirt/virtlockd-admin-sock diff --git a/src/logging/virtlogd-admin.socket.in b/src/logging/virtlogd-admin.socket.in index 1d18fe6f56..e0d35cbcf3 100644 --- a/src/logging/virtlogd-admin.socket.in +++ b/src/logging/virtlogd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=Virtual machine log manager socket -BindsTo=virtlogd.socket -After=virtlogd.socket [Socket] ListenStream=@runstatedir@/libvirt/virtlogd-admin-sock diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 6df038d95a..ba060eaea4 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ admin socket -BindsTo=libvirtd.socket -After=libvirtd.socket [Socket] ListenStream=@runstatedir@/libvirt/libvirt-admin-sock diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index 6797517c50..d2ab7ba4f2 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ local read-only socket -BindsTo=libvirtd.socket -After=libvirtd.socket [Socket] ListenStream=@runstatedir@/libvirt/libvirt-sock-ro diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index 8b8fbcd01a..e32daddf25 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ non-TLS IP socket -BindsTo=libvirtd.socket -After=libvirtd.socket [Socket] ListenStream=16509 diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index fefda22c6b..2f34e8e0cd 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ TLS IP socket -BindsTo=libvirtd.socket -After=libvirtd.socket [Socket] ListenStream=16514 diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in index a4faeb7da8..dc2cb737ce 100644 --- a/src/virtd-admin.socket.in +++ b/src/virtd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ admin socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirtd-admin.socket After=libvirtd-admin.socket @socket_unit_extra@ diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in index 829c2e8b1f..ef1716e3f3 100644 --- a/src/virtd-ro.socket.in +++ b/src/virtd-ro.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ local read-only socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirtd-ro.socket After=libvirtd-ro.socket @socket_unit_extra@ diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in index 2873c35135..26ead32789 100644 --- a/src/virtd-tcp.socket.in +++ b/src/virtd-tcp.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ non-TLS IP socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirtd-tcp.socket After=libvirtd-tcp.socket @socket_unit_extra@ diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in index 2d4d589c8a..47da9317d6 100644 --- a/src/virtd-tls.socket.in +++ b/src/virtd-tls.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ TLS IP socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirt-tls.socket After=libvirt-tls.socket @socket_unit_extra@ -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:38PM +0200, Andrea Bolognani wrote:
They are unnecessary, since all sockets for a service are now enabled as soon as one of them is and each service has a very strong dependency on all of its sockets.
You earlier modified the .service units to have BindsTo= for each of the sockets it depends to. Thus if any one of the .sockets is stopped, this means the .service is stopped too. The logic removed here though was doing a different job. That said that that if $FOO.socket is stopped, it would force stop the $FOO-admin.socket and $FOO-ro.socket too. IOW, it prevented having only the RO/admin sockets running, without the primary socket. I believe that's still needed Also, you didn't add BindsTo on the libvirtd.service, because that has to be able to run without socket activation for upgrade scenarios. So we shouldn't be modifying the libvirtd sockets anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd-admin.socket.in | 2 -- src/logging/virtlogd-admin.socket.in | 2 -- src/remote/libvirtd-admin.socket.in | 2 -- src/remote/libvirtd-ro.socket.in | 2 -- src/remote/libvirtd-tcp.socket.in | 2 -- src/remote/libvirtd-tls.socket.in | 2 -- src/virtd-admin.socket.in | 2 -- src/virtd-ro.socket.in | 2 -- src/virtd-tcp.socket.in | 2 -- src/virtd-tls.socket.in | 2 -- 10 files changed, 20 deletions(-)
diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in index 63f78a02da..a773b511bd 100644 --- a/src/locking/virtlockd-admin.socket.in +++ b/src/locking/virtlockd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=Virtual machine lock manager admin socket -BindsTo=virtlockd.socket -After=virtlockd.socket
[Socket] ListenStream=@runstatedir@/libvirt/virtlockd-admin-sock diff --git a/src/logging/virtlogd-admin.socket.in b/src/logging/virtlogd-admin.socket.in index 1d18fe6f56..e0d35cbcf3 100644 --- a/src/logging/virtlogd-admin.socket.in +++ b/src/logging/virtlogd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=Virtual machine log manager socket -BindsTo=virtlogd.socket -After=virtlogd.socket
[Socket] ListenStream=@runstatedir@/libvirt/virtlogd-admin-sock diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 6df038d95a..ba060eaea4 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ admin socket -BindsTo=libvirtd.socket -After=libvirtd.socket
[Socket] ListenStream=@runstatedir@/libvirt/libvirt-admin-sock diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index 6797517c50..d2ab7ba4f2 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ local read-only socket -BindsTo=libvirtd.socket -After=libvirtd.socket
[Socket] ListenStream=@runstatedir@/libvirt/libvirt-sock-ro diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index 8b8fbcd01a..e32daddf25 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ non-TLS IP socket -BindsTo=libvirtd.socket -After=libvirtd.socket
[Socket] ListenStream=16509 diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index fefda22c6b..2f34e8e0cd 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ TLS IP socket -BindsTo=libvirtd.socket -After=libvirtd.socket
[Socket] ListenStream=16514 diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in index a4faeb7da8..dc2cb737ce 100644 --- a/src/virtd-admin.socket.in +++ b/src/virtd-admin.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ admin socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirtd-admin.socket After=libvirtd-admin.socket @socket_unit_extra@ diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in index 829c2e8b1f..ef1716e3f3 100644 --- a/src/virtd-ro.socket.in +++ b/src/virtd-ro.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ local read-only socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirtd-ro.socket After=libvirtd-ro.socket @socket_unit_extra@ diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in index 2873c35135..26ead32789 100644 --- a/src/virtd-tcp.socket.in +++ b/src/virtd-tcp.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ non-TLS IP socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirtd-tcp.socket After=libvirtd-tcp.socket @socket_unit_extra@ diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in index 2d4d589c8a..47da9317d6 100644 --- a/src/virtd-tls.socket.in +++ b/src/virtd-tls.socket.in @@ -1,7 +1,5 @@ [Unit] Description=@name@ TLS IP socket -BindsTo=@service@.socket -After=@service@.socket Conflicts=libvirt-tls.socket After=libvirt-tls.socket @socket_unit_extra@ -- 2.41.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 :|
On Wed, Sep 27, 2023 at 10:55:04AM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 25, 2023 at 08:58:38PM +0200, Andrea Bolognani wrote:
They are unnecessary, since all sockets for a service are now enabled as soon as one of them is and each service has a very strong dependency on all of its sockets.
You earlier modified the .service units to have BindsTo= for each of the sockets it depends to.
Thus if any one of the .sockets is stopped, this means the .service is stopped too.
The logic removed here though was doing a different job. That said that that if $FOO.socket is stopped, it would force stop the $FOO-admin.socket and $FOO-ro.socket too.
IOW, it prevented having only the RO/admin sockets running, without the primary socket.
I believe that's still needed
Also, you didn't add BindsTo on the libvirtd.service, because that has to be able to run without socket activation for upgrade scenarios. So we shouldn't be modifying the libvirtd sockets anyway.
I'll perform some testing just to make sure, but I think you're right and I will most likely drop this patch in v2. -- Andrea Bolognani / Red Hat / Virtualization
Hypervisors are referred to by their user-facing name rather than the name of their libvirt driver, the monolithic daemon is explicitly referred to as legacy, and a consistent format is used throughout. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 2 +- src/interface/meson.build | 2 +- src/libxl/meson.build | 2 +- src/locking/meson.build | 2 +- src/locking/virtlockd-admin.socket.in | 2 +- src/locking/virtlockd.service.in | 2 +- src/locking/virtlockd.socket.in | 2 +- src/logging/meson.build | 2 +- src/logging/virtlogd-admin.socket.in | 2 +- src/logging/virtlogd.service.in | 2 +- src/logging/virtlogd.socket.in | 2 +- src/lxc/meson.build | 2 +- src/network/meson.build | 2 +- src/node_device/meson.build | 2 +- src/nwfilter/meson.build | 2 +- src/qemu/meson.build | 2 +- src/remote/libvirtd-admin.socket.in | 2 +- src/remote/libvirtd-ro.socket.in | 2 +- src/remote/libvirtd-tcp.socket.in | 2 +- src/remote/libvirtd-tls.socket.in | 2 +- src/remote/libvirtd.service.in | 2 +- src/remote/libvirtd.socket.in | 2 +- src/remote/meson.build | 4 ++-- src/secret/meson.build | 2 +- src/storage/meson.build | 2 +- src/vbox/meson.build | 2 +- src/virtd-admin.socket.in | 2 +- src/virtd-ro.socket.in | 2 +- src/virtd-tcp.socket.in | 2 +- src/virtd-tls.socket.in | 2 +- src/virtd.service.in | 2 +- src/virtd.socket.in | 2 +- src/vz/meson.build | 2 +- 33 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/ch/meson.build b/src/ch/meson.build index f6c443f3c6..cbac6e2de6 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -57,7 +57,7 @@ if conf.has('WITH_CH') virt_daemon_units += { 'service': 'virtchd', - 'name': 'Libvirt ch', + 'name': 'Cloud Hypervisor', 'service_unit_extra': [ 'Wants=systemd-machined.service', 'After=systemd-machined.service', diff --git a/src/interface/meson.build b/src/interface/meson.build index 54c0b1a935..b1617d83e6 100644 --- a/src/interface/meson.build +++ b/src/interface/meson.build @@ -44,7 +44,7 @@ if conf.has('WITH_INTERFACE') virt_daemon_units += { 'service': 'virtinterfaced', - 'name': 'Libvirt interface', + 'name': 'interface', } openrc_init_files += { diff --git a/src/libxl/meson.build b/src/libxl/meson.build index ad8d9b757f..5affd1e7c5 100644 --- a/src/libxl/meson.build +++ b/src/libxl/meson.build @@ -66,7 +66,7 @@ if conf.has('WITH_LIBXL') virt_daemon_units += { 'service': 'virtxend', - 'name': 'Libvirt libxl', + 'name': 'Xen', 'service_unit_extra': [ 'Wants=virtlockd.socket', 'After=virtlockd.socket', diff --git a/src/locking/meson.build b/src/locking/meson.build index 2ccc822ed3..6b3cd781d1 100644 --- a/src/locking/meson.build +++ b/src/locking/meson.build @@ -144,7 +144,7 @@ if conf.has('WITH_LIBVIRTD') virt_daemon_units += { 'service': 'virtlockd', 'service_in': files('virtlockd.service.in'), - 'name': 'Libvirt locking', + 'name': 'locking', 'sockets': [ 'main', 'admin' ], 'socket_in': files('virtlockd.socket.in'), 'socket_admin_in': files('virtlockd-admin.socket.in'), diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in index a773b511bd..90077b4915 100644 --- a/src/locking/virtlockd-admin.socket.in +++ b/src/locking/virtlockd-admin.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine lock manager admin socket +Description=libvirt @name@ daemon admin socket [Socket] ListenStream=@runstatedir@/libvirt/virtlockd-admin-sock diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index ebf8cb9d5a..d1ebbda2b7 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine lock manager +Description=libvirt @name@ daemon BindsTo=virtlockd.socket BindsTo=virtlockd-admin.socket After=virtlockd.socket diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in index 1cd50c70ec..5dba9dea00 100644 --- a/src/locking/virtlockd.socket.in +++ b/src/locking/virtlockd.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine lock manager socket +Description=libvirt @name@ daemon socket [Socket] ListenStream=@runstatedir@/libvirt/virtlockd-sock diff --git a/src/logging/meson.build b/src/logging/meson.build index 95d2ef2a3f..1527f91faf 100644 --- a/src/logging/meson.build +++ b/src/logging/meson.build @@ -91,7 +91,7 @@ if conf.has('WITH_LIBVIRTD') virt_daemon_units += { 'service': 'virtlogd', 'service_in': files('virtlogd.service.in'), - 'name': 'Libvirt logging', + 'name': 'logging', 'sockets': [ 'main', 'admin' ], 'socket_in': files('virtlogd.socket.in'), 'socket_admin_in': files('virtlogd-admin.socket.in'), diff --git a/src/logging/virtlogd-admin.socket.in b/src/logging/virtlogd-admin.socket.in index e0d35cbcf3..34f27154a2 100644 --- a/src/logging/virtlogd-admin.socket.in +++ b/src/logging/virtlogd-admin.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine log manager socket +Description=libvirt @name@ daemon admin socket [Socket] ListenStream=@runstatedir@/libvirt/virtlogd-admin-sock diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 72743a61ae..bfcc43d1b7 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine log manager +Description=libvirt @name@ daemon BindsTo=virtlogd.socket BindsTo=virtlogd-admin.socket After=virtlogd.socket diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in index 0b85af927b..218d30e5ec 100644 --- a/src/logging/virtlogd.socket.in +++ b/src/logging/virtlogd.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine log manager socket +Description=libvirt @name@ daemon socket [Socket] ListenStream=@runstatedir@/libvirt/virtlogd-sock diff --git a/src/lxc/meson.build b/src/lxc/meson.build index 9b933023dd..8e513c870d 100644 --- a/src/lxc/meson.build +++ b/src/lxc/meson.build @@ -164,7 +164,7 @@ if conf.has('WITH_LXC') virt_daemon_units += { 'service': 'virtlxcd', - 'name': 'Libvirt lxc', + 'name': 'LXC', 'service_unit_extra': [ 'Wants=systemd-machined.service', 'After=systemd-machined.service', diff --git a/src/network/meson.build b/src/network/meson.build index d1a2338d1b..1974385ca9 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -62,7 +62,7 @@ if conf.has('WITH_NETWORK') virt_daemon_units += { 'service': 'virtnetworkd', - 'name': 'Libvirt network', + 'name': 'network', 'service_service_extra': [ 'KillMode=process', ], diff --git a/src/node_device/meson.build b/src/node_device/meson.build index 2614ff8b9c..d1e349bc5e 100644 --- a/src/node_device/meson.build +++ b/src/node_device/meson.build @@ -52,7 +52,7 @@ if conf.has('WITH_NODE_DEVICES') virt_daemon_units += { 'service': 'virtnodedevd', - 'name': 'Libvirt nodedev', + 'name': 'nodedev', } openrc_init_files += { diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build index c091bc3f1b..1b914f2360 100644 --- a/src/nwfilter/meson.build +++ b/src/nwfilter/meson.build @@ -50,7 +50,7 @@ if conf.has('WITH_NWFILTER') virt_daemon_units += { 'service': 'virtnwfilterd', - 'name': 'Libvirt nwfilter', + 'name': 'nwfilter', } openrc_init_files += { diff --git a/src/qemu/meson.build b/src/qemu/meson.build index bf900e3f14..2a4a8836e2 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -183,7 +183,7 @@ if conf.has('WITH_QEMU') virt_daemon_units += { 'service': 'virtqemud', - 'name': 'Libvirt qemu', + 'name': 'QEMU', 'service_unit_extra': [ 'Requires=virtlogd.socket', 'Wants=virtlockd.socket', diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index ba060eaea4..7d7416618c 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ admin socket +Description=libvirt @name@ daemon admin socket [Socket] ListenStream=@runstatedir@/libvirt/libvirt-admin-sock diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index d2ab7ba4f2..cdbebcd2e8 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ local read-only socket +Description=libvirt @name@ daemon read-only socket [Socket] ListenStream=@runstatedir@/libvirt/libvirt-sock-ro diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index e32daddf25..d0da35b8bb 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ non-TLS IP socket +Description=libvirt @name@ daemon non-TLS IP socket [Socket] ListenStream=16509 diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index 2f34e8e0cd..d95da1558b 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ TLS IP socket +Description=libvirt @name@ daemon TLS IP socket [Socket] ListenStream=16514 diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index a2c3c8f8fa..3b9819c3bc 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtualization daemon +Description=libvirt @name@ daemon # Use Wants instead of Requires so that users # can disable these three .socket units to revert # to a traditional non-activation deployment setup diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index f483facdf3..cab3cfe971 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ local socket +Description=libvirt @name@ daemon socket [Socket] ListenStream=@runstatedir@/libvirt/libvirt-sock diff --git a/src/remote/meson.build b/src/remote/meson.build index 5ee6d4e61c..e14541f09e 100644 --- a/src/remote/meson.build +++ b/src/remote/meson.build @@ -191,7 +191,7 @@ if conf.has('WITH_REMOTE') virt_daemon_units += { 'service': 'libvirtd', 'service_in': files('libvirtd.service.in'), - 'name': 'Libvirt', + 'name': 'legacy monolithic', 'sockprefix': 'libvirt', 'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ], 'socket_in': files('libvirtd.socket.in'), @@ -224,7 +224,7 @@ if conf.has('WITH_REMOTE') virt_daemon_units += { 'service': 'virtproxyd', - 'name': 'Libvirt proxy', + 'name': 'proxy', 'sockprefix': 'libvirt', 'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ], } diff --git a/src/secret/meson.build b/src/secret/meson.build index e05b46abea..791ce1a024 100644 --- a/src/secret/meson.build +++ b/src/secret/meson.build @@ -33,7 +33,7 @@ if conf.has('WITH_SECRETS') virt_daemon_units += { 'service': 'virtsecretd', - 'name': 'Libvirt secret', + 'name': 'secret', } openrc_init_files += { diff --git a/src/storage/meson.build b/src/storage/meson.build index 90b8af4e41..b01e8acd18 100644 --- a/src/storage/meson.build +++ b/src/storage/meson.build @@ -111,7 +111,7 @@ if conf.has('WITH_STORAGE') virt_daemon_units += { 'service': 'virtstoraged', - 'name': 'Libvirt storage', + 'name': 'storage', 'service_unit_extra': [ 'After=iscsid.service', 'After=remote-fs.target', diff --git a/src/vbox/meson.build b/src/vbox/meson.build index e07c87eaaa..ad41ec21ee 100644 --- a/src/vbox/meson.build +++ b/src/vbox/meson.build @@ -57,7 +57,7 @@ if conf.has('WITH_VBOX') virt_daemon_units += { 'service': 'virtvboxd', - 'name': 'Libvirt vbox', + 'name': 'VirtualBox', 'service_unit_extra': [ 'After=remote-fs.target', ], diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in index dc2cb737ce..6c2d7eed7a 100644 --- a/src/virtd-admin.socket.in +++ b/src/virtd-admin.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ admin socket +Description=libvirt @name@ daemon admin socket Conflicts=libvirtd-admin.socket After=libvirtd-admin.socket @socket_unit_extra@ diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in index ef1716e3f3..316468bf54 100644 --- a/src/virtd-ro.socket.in +++ b/src/virtd-ro.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ local read-only socket +Description=libvirt @name@ daemon read-only socket Conflicts=libvirtd-ro.socket After=libvirtd-ro.socket @socket_unit_extra@ diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in index 26ead32789..928760b872 100644 --- a/src/virtd-tcp.socket.in +++ b/src/virtd-tcp.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ non-TLS IP socket +Description=libvirt @name@ daemon non-TLS IP socket Conflicts=libvirtd-tcp.socket After=libvirtd-tcp.socket @socket_unit_extra@ diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in index 47da9317d6..4532038c8d 100644 --- a/src/virtd-tls.socket.in +++ b/src/virtd-tls.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ TLS IP socket +Description=libvirt @name@ daemon TLS IP socket Conflicts=libvirt-tls.socket After=libvirt-tls.socket @socket_unit_extra@ diff --git a/src/virtd.service.in b/src/virtd.service.in index b9e6345e8c..e940b05b19 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ daemon +Description=libvirt @name@ daemon BindsTo=@service@.socket BindsTo=@service@-ro.socket BindsTo=@service@-admin.socket diff --git a/src/virtd.socket.in b/src/virtd.socket.in index dc25e4d781..920d537e06 100644 --- a/src/virtd.socket.in +++ b/src/virtd.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=@name@ local socket +Description=libvirt @name@ daemon socket Conflicts=libvirtd.socket After=libvirtd.socket @socket_unit_extra@ diff --git a/src/vz/meson.build b/src/vz/meson.build index dceea1aaac..077c1f37fd 100644 --- a/src/vz/meson.build +++ b/src/vz/meson.build @@ -48,7 +48,7 @@ if conf.has('WITH_VZ') virt_daemon_units += { 'service': 'virtvzd', - 'name': 'Libvirt vz', + 'name': 'vz', 'service_unit_extra': [ 'After=remote-fs.target', ], -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:39PM +0200, Andrea Bolognani wrote:
Hypervisors are referred to by their user-facing name rather than the name of their libvirt driver, the monolithic daemon is explicitly referred to as legacy, and a consistent format is used throughout.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/ch/meson.build | 2 +- src/interface/meson.build | 2 +- src/libxl/meson.build | 2 +- src/locking/meson.build | 2 +- src/locking/virtlockd-admin.socket.in | 2 +- src/locking/virtlockd.service.in | 2 +- src/locking/virtlockd.socket.in | 2 +- src/logging/meson.build | 2 +- src/logging/virtlogd-admin.socket.in | 2 +- src/logging/virtlogd.service.in | 2 +- src/logging/virtlogd.socket.in | 2 +- src/lxc/meson.build | 2 +- src/network/meson.build | 2 +- src/node_device/meson.build | 2 +- src/nwfilter/meson.build | 2 +- src/qemu/meson.build | 2 +- src/remote/libvirtd-admin.socket.in | 2 +- src/remote/libvirtd-ro.socket.in | 2 +- src/remote/libvirtd-tcp.socket.in | 2 +- src/remote/libvirtd-tls.socket.in | 2 +- src/remote/libvirtd.service.in | 2 +- src/remote/libvirtd.socket.in | 2 +- src/remote/meson.build | 4 ++-- src/secret/meson.build | 2 +- src/storage/meson.build | 2 +- src/vbox/meson.build | 2 +- src/virtd-admin.socket.in | 2 +- src/virtd-ro.socket.in | 2 +- src/virtd-tcp.socket.in | 2 +- src/virtd-tls.socket.in | 2 +- src/virtd.service.in | 2 +- src/virtd.socket.in | 2 +- src/vz/meson.build | 2 +- 33 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in index a773b511bd..90077b4915 100644 --- a/src/locking/virtlockd-admin.socket.in +++ b/src/locking/virtlockd-admin.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine lock manager admin socket +Description=libvirt @name@ daemon admin socket
Using a subsitution here does not add any value IMHO, it just obscures the final text. Likewise for the similar changes that follow. 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 Wed, Sep 27, 2023 at 10:57:13AM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 25, 2023 at 08:58:39PM +0200, Andrea Bolognani wrote:
+++ b/src/locking/virtlockd-admin.socket.in @@ -1,5 +1,5 @@ [Unit] -Description=Virtual machine lock manager admin socket +Description=libvirt @name@ daemon admin socket
Using a subsitution here does not add any value IMHO, it just obscures the final text. Likewise for the similar changes that follow.
Point taken for libvirtd/virtlogd/virtlockd, which are special and don't follow the same process as other daemons. I'll drop that part. -- Andrea Bolognani / Red Hat / Virtualization
Like the Description, these are intended to be displayed to the user, so it makes sense to have them towards the top of the file before all the information that systemd will parse to calculate dependencies. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 4 ++-- src/logging/virtlogd.service.in | 4 ++-- src/remote/libvirtd.service.in | 4 ++-- src/virtd.service.in | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index d1ebbda2b7..ed3b7c7792 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -1,11 +1,11 @@ [Unit] Description=libvirt @name@ daemon +Documentation=man:virtlockd(8) +Documentation=https://libvirt.org/ BindsTo=virtlockd.socket BindsTo=virtlockd-admin.socket After=virtlockd.socket After=virtlockd-admin.socket -Documentation=man:virtlockd(8) -Documentation=https://libvirt.org [Service] Type=notify diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index bfcc43d1b7..417770b139 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -1,11 +1,11 @@ [Unit] Description=libvirt @name@ daemon +Documentation=man:virtlogd(8) +Documentation=https://libvirt.org/ BindsTo=virtlogd.socket BindsTo=virtlogd-admin.socket After=virtlogd.socket After=virtlogd-admin.socket -Documentation=man:virtlogd(8) -Documentation=https://libvirt.org [Service] Type=notify diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 3b9819c3bc..2970e855d6 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -1,5 +1,7 @@ [Unit] Description=libvirt @name@ daemon +Documentation=man:libvirtd(8) +Documentation=https://libvirt.org/ # Use Wants instead of Requires so that users # can disable these three .socket units to revert # to a traditional non-activation deployment setup @@ -22,8 +24,6 @@ After=remote-fs.target After=systemd-machined.service After=xencommons.service Conflicts=xendomains.service -Documentation=man:libvirtd(8) -Documentation=https://libvirt.org [Service] Type=notify diff --git a/src/virtd.service.in b/src/virtd.service.in index e940b05b19..6b59803574 100644 --- a/src/virtd.service.in +++ b/src/virtd.service.in @@ -1,5 +1,7 @@ [Unit] Description=libvirt @name@ daemon +Documentation=man:@service@(8) +Documentation=https://libvirt.org/ BindsTo=@service@.socket BindsTo=@service@-ro.socket BindsTo=@service@-admin.socket @@ -11,8 +13,6 @@ After=libvirtd.service After=network.target After=dbus.service After=apparmor.service -Documentation=man:@service@(8) -Documentation=https://libvirt.org @service_unit_extra@ [Service] -- 2.41.0
On Mon, Sep 25, 2023 at 08:58:40PM +0200, Andrea Bolognani wrote:
Like the Description, these are intended to be displayed to the user, so it makes sense to have them towards the top of the file before all the information that systemd will parse to calculate dependencies.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/virtlockd.service.in | 4 ++-- src/logging/virtlogd.service.in | 4 ++-- src/remote/libvirtd.service.in | 4 ++-- src/virtd.service.in | 4 ++-- 4 files changed, 8 insertions(+), 8 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 :|
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Pavel Hrdina