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