[libvirt PATCH 2/8] Revert "remote: move timeout arg into sysconf file"

[Ján Tomko]

On a Wednesday in 2020, Andrea Bolognani wrote:
>There is nothing really systemd-specific about passing extra
>arguments to daemons so it's reasonable, although not currently the
>case, that startup scripts written for other init systems might want
>to source these sysconf files; for those init systems, which likely
>do not support socket activation, making the daemon quit after a
>timeout has expired is probably not a good idea.

The sysconf file is documented to be a list of customizations for the
systemd unit file, so having systemd-specific options in it is OK.

Unless you actually have plans to use this in a different init system,
I'd say you can drop this paragraph.

>
>More generally, the sysconf files should not reflect the default
>behavior, but only contain overrides explicitly put in place by the
>admin;

If that's a rule, it seems to be widely broken on my Fedora machine.

>now that we have a mechanism to disable timeouts regardless
>of the default set in the service file, that argument for having the
>default timeout in the sysconf file is moot as well.
>
>This reverts commit 581767a98ab5f674ac335d6c270efa8576bfdfbf.

Please drop the trailing period.

>
>Signed-off-by: Andrea Bolognani <abologna at redhat.com>
>---
> src/remote/libvirtd.service.in |  6 +++++-
> src/remote/libvirtd.sysconf    | 12 +++---------
> 2 files changed, 8 insertions(+), 10 deletions(-)
>
>diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
>index 90b2cad5b0..3e2d716af7 100644
>--- a/src/remote/libvirtd.service.in
>+++ b/src/remote/libvirtd.service.in
>@@ -26,7 +26,11 @@ Documentation=https://libvirt.org
> [Service]
> Type=notify
> EnvironmentFile=- at sysconfdir@/sysconfig/libvirtd
>-ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
>+# libvirtd.service is set to run on boot so that autostart of
>+# VMs can be performed. We don't want it to stick around if
>+# unused though, so we set a timeout. The socket activation
>+# then ensures it gets started again if anything needs it
>+ExecStart=@sbindir@/libvirtd --timeout 120 $LIBVIRTD_ARGS
> ExecReload=/bin/kill -HUP $MAINPID
> KillMode=process
> Restart=on-failure
>diff --git a/src/remote/libvirtd.sysconf b/src/remote/libvirtd.sysconf
>index ee9db22bab..5969518bf2 100644
>--- a/src/remote/libvirtd.sysconf
>+++ b/src/remote/libvirtd.sysconf
>@@ -1,14 +1,8 @@
> # Customizations for the libvirtd.service systemd unit
>
>-# Default behaviour is for libvirtd.service to start on boot
>-# so that VM autostart can be performed. We then want it to
>-# shutdown again if nothing was started and rely on systemd
>-# socket activation to start it again when some client app
>-# connects.
>-LIBVIRTD_ARGS="--timeout 120"
>-
>-# If systemd socket activation is disabled, then the following
>-# can be used to listen on TCP/TLS sockets
>+# Listen for TCP/IP connections. This is not required if using systemd
>+# socket activation.
>+# NB. must setup TLS/SSL keys prior to using this
> #LIBVIRTD_ARGS="--listen"

But I'm happy to get rid of two assignments to the same variable in one
file.

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200402/fcf555da/attachment-0001.sig>