Re: [libvirt PATCH 26/42] systemd: Switch virtchd to common templates

On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; > --- > 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`.