sysconfig files are owned by the admin of the host. He has the liberty
to put anything he wants into these files. This makes it difficult to
provide different defaults.

Remove the sysconfig file and place the current desired default into
the service file.

Local customizations can now go either into /etc/sysconfig/virtqemud
or /etc/systemd/system/virtqemud.service.d/my-knobs.conf

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---

Untested, just to demonstrate how it could be done for every sysconfig file.

 libvirt.spec.in               |  1 -
 src/qemu/meson.build          |  5 -----
 src/qemu/virtqemud.service.in |  1 +
 src/qemu/virtqemud.sysconf    | 12 ------------
 4 files changed, 1 insertion(+), 18 deletions(-)
 delete mode 100644 src/qemu/virtqemud.sysconf

diff --git a/libvirt.spec.in b/libvirt.spec.in
index cb48dd0be0..2cc3e61e3c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1689,7 +1689,6 @@ exit 0
 
 %if %{with_qemu}
 %files daemon-driver-qemu
-%config(noreplace) %{_sysconfdir}/sysconfig/virtqemud
 %config(noreplace) %{_sysconfdir}/libvirt/virtqemud.conf
 %{_datadir}/augeas/lenses/virtqemud.aug
 %{_datadir}/augeas/lenses/tests/test_virtqemud.aug
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 3898d23877..6b4dd58309 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -165,11 +165,6 @@ if conf.has('WITH_QEMU')
     'in_file': files('virtqemud.init.in'),
   }
 
-  sysconf_files += {
-    'name': 'virtqemud',
-    'file': files('virtqemud.sysconf'),
-  }
-
   virt_install_dirs += [
     localstatedir / 'lib' / 'libvirt' / 'qemu',
     runstatedir / 'libvirt' / 'qemu',
diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in
index 8abc9d3a7f..a5cb145668 100644
--- a/src/qemu/virtqemud.service.in
+++ b/src/qemu/virtqemud.service.in
@@ -18,6 +18,7 @@ Documentation=https://libvirt.org
 
 [Service]
 Type=notify
+Environment=VIRTQEMUD_ARGS="--timeout 120"
 EnvironmentFile=-@sysconfdir@/sysconfig/virtqemud
 ExecStart=@sbindir@/virtqemud $VIRTQEMUD_ARGS
 ExecReload=/bin/kill -HUP $MAINPID
diff --git a/src/qemu/virtqemud.sysconf b/src/qemu/virtqemud.sysconf
deleted file mode 100644
index 87b626e3ed..0000000000
--- a/src/qemu/virtqemud.sysconf
+++ /dev/null
@@ -1,12 +0,0 @@
-# Customizations for the virtqemud.service systemd unit
-
-VIRTQEMUD_ARGS="--timeout 120"
-
-# Override the QEMU/SDL default audio driver probing when
-# starting virtual machines using SDL graphics
-#
-# NB these have no effect for VMs using VNC, unless vnc_allow_host_audio
-# is enabled in /etc/libvirt/qemu.conf
-#QEMU_AUDIO_DRV=sdl
-#
-#SDL_AUDIODRIVER=pulse

On Thu, Jul 15, 2021 at 11:23:37PM +0200, Olaf Hering wrote:
> sysconfig files are owned by the admin of the host. He has the liberty
> to put anything he wants into these files. This makes it difficult to
> provide different defaults.
>
> Remove the sysconfig file and place the current desired default into
> the service file.
>
> Local customizations can now go either into /etc/sysconfig/virtqemud
> or /etc/systemd/system/virtqemud.service.d/my-knobs.conf

I'm unclear on what exactly you're trying to achieve here.

The sysconfig files shipped with libvirt contain the defaults, and
the admin is absolutely welcome to tweak them however they might like
after installation, just as is the case for all the configuration
files in /etc/libvirt.

I expect the distro's package manager will then do the right thing
when it comes to preserving these local modifications across libvirt
upgrades - I know for sure that's the case at least in Debian.

This arrangement appears to be very common, too: on my Fedora 34
machine, /etc/sysconfig contains ~25 files, all owned by some package
and most consisting of at least one non-comment lines.

Can you please elaborate on why you feel that changing the status quo
is necessary / desirable?

-- 
Andrea Bolognani / Red Hat / Virtualization

Am Fri, 16 Jul 2021 00:58:43 -0700
schrieb Andrea Bolognani <abologna@redhat.com>:

> This arrangement appears to be very common, too: on my Fedora 34
> machine, /etc/sysconfig contains ~25 files, all owned by some package
> and most consisting of at least one non-comment lines.

Forgot to reply to this paragraph:

None of the files in /etc have to be owned by a package.
We are slowly getting there.

/etc/sysconfig is not owned by any package, it is entirely maintained by the admin.

Olaf

On Fri, Jul 16, 2021 at 12:04:00PM +0200, Olaf Hering wrote:
> Am Fri, 16 Jul 2021 00:58:43 -0700
> schrieb Andrea Bolognani <abologna@redhat.com>:
>
> > This arrangement appears to be very common, too: on my Fedora 34
> > machine, /etc/sysconfig contains ~25 files, all owned by some package
> > and most consisting of at least one non-comment lines.
>
> Forgot to reply to this paragraph:
>
> None of the files in /etc have to be owned by a package.
> We are slowly getting there.

Is this goal of having packages not touch /etc connected to the idea
of transactionally-updated read-only OS images? If there is an
initiative tracking progress in this area, either in the context of a
single distro or more generally, can you please point me to it?

> /etc/sysconfig is not owned by any package, it is entirely maintained by the admin.

That doesn't seem to be the case at least in Fedora 34:

  $ rpm -qf /etc/sysconfig/ /etc/sysconfig/*
  filesystem-3.14-5.fc34.x86_64
  file /etc/sysconfig/anaconda is not owned by any package
  chrony-4.1-1.fc34.x86_64
  initscripts-10.09-1.fc34.x86_64
  moby-engine-20.10.6-1.fc34.x86_64
  firewalld-0.9.4-1.fc34.noarch
  grub2-tools-extra-2.06-2.fc34.x86_64
  httpd-2.4.48-1.fc34.x86_64
  kexec-tools-2.0.21-5.fc34.x86_64
  file /etc/sysconfig/kernel is not owned by any package
  libvirt-daemon-7.5.0-1.fc34.x86_64
  libvirt-daemon-7.5.0-1.fc34.x86_64
  man-db-2.9.3-3.fc34.x86_64
  initscripts-10.09-1.fc34.x86_64
  file /etc/sysconfig/network is not owned by any package
  NetworkManager-1.30.6-1.fc34.x86_64
  nftables-0.9.8-2.fc34.x86_64
  radvd-2.19-2.fc34.x86_64
  mdadm-4.1-7.fc34.x86_64
  rpcbind-1.2.6-0.fc34.x86_64
  samba-common-4.14.6-0.fc34.noarch
  cyrus-sasl-2.1.27-8.fc34.x86_64
  selinux-policy-34.14-1.fc34.noarch
  sheepdog-1.0.1-14.fc34.x86_64
  openssh-server-8.6p1-3.fc34.x86_64
  subversion-1.14.1-1.fc34.x86_64
  libvirt-daemon-driver-interface-7.5.0-1.fc34.x86_64
  libvirt-daemon-7.5.0-1.fc34.x86_64
  libvirt-daemon-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-lxc-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-network-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-nodedev-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-nwfilter-7.5.0-1.fc34.x86_64
  libvirt-daemon-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-qemu-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-secret-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-storage-core-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-vbox-7.5.0-1.fc34.x86_64
  libvirt-daemon-driver-libxl-7.5.0-1.fc34.x86_64
  wpa_supplicant-2.9-12.fc34.x86_64
  zfs-fuse-0.7.2.2-18.fc34.x86_64

-- 
Andrea Bolognani / Red Hat / Virtualization

Am Mon, 19 Jul 2021 12:01:57 -0400
schrieb Andrea Bolognani <abologna@redhat.com>:

> On Fri, Jul 16, 2021 at 12:04:00PM +0200, Olaf Hering wrote:
> > None of the files in /etc have to be owned by a package.
> > We are slowly getting there.  
> Is this goal of having packages not touch /etc connected to the idea
> of transactionally-updated read-only OS images? If there is an
> initiative tracking progress in this area, either in the context of a
> single distro or more generally, can you please point me to it?

There is some effort in this area in Tumbleweed, right now one can get still up to 10k more or less bogus rpm-owned files in /etc. A /etc that is now owned by rpm certainly helps transactional updates as far as I understand it.


But more importantly it is bad to install empty configuration files:
They often just duplicate what is already written in /usr/share/man or doc.
There is no way to get the delta between a .rpmnew file and the base version.
They just waste space and time during backup and restore.
Ideally /etc should contain just the tiny amount that is really required.

> > /etc/sysconfig is not owned by any package, it is entirely maintained by the admin.  
> That doesn't seem to be the case at least in Fedora 34:

That might be. On a SUSE system it is driven by fillup, based on fillup-templates.


Olaf

Am Fri, 16 Jul 2021 00:58:43 -0700
schrieb Andrea Bolognani <abologna@redhat.com>:

> The sysconfig files shipped with libvirt contain the defaults, and
> the admin is absolutely welcome to tweak them however they might like
> after installation, just as is the case for all the configuration
> files in /etc/libvirt.

Right. Once they are modified for whatever reason things will go downhill.

Just recently the default (for libvirtd) changed from --listen to --timeout.
This is an incompatible change. There is very little info on the system to 
workout if the admin actually wanted --listen, and just append some other knob,
or if he dirtied the sysconfig file in other ways. Therefore %post or %posttrans
scripts have no easy way to decide what to do with the existing sysconfig file
to switch to the new default.

If --listen would have been configured like I proposed, any admin changes are
clear and obvious, and he has to keep the pieces if it falls apart because
some default changed. In this case from .service to .socket activation.


Furthermore my change should also add every undocumented variable to the service file.
Configuration files are not documentation files.
This is also violated, but should probably be discussed on another Friday.

Olaf

On 7/16/21 2:31 AM, Olaf Hering wrote:
> Am Fri, 16 Jul 2021 00:58:43 -0700
> schrieb Andrea Bolognani <abologna@redhat.com>:
> 
>> The sysconfig files shipped with libvirt contain the defaults, and
>> the admin is absolutely welcome to tweak them however they might like
>> after installation, just as is the case for all the configuration
>> files in /etc/libvirt.
> 
> Right. Once they are modified for whatever reason things will go downhill.
> 
> Just recently the default (for libvirtd) changed from --listen to --timeout.

To clarify: that was a not-so-recent downstream change. I have an *old* 
downstream patch for libvirtd.sysconf that adds some (likely no longer required) 
SUSE metadata. I don't really recall why or how '--listen' ended up in 
LIBVIRTD_ARGS in that patch. And even though I removed it from the patch nearly 
2 years ago, it is still biting me in the ass today :-).

That said, I like the trend of moving stuff owned by packages out of /etc and 
might remove these files from the downstream SUSE package regardless of the 
outcome here.

Regards,
Jim

On Fri, Jul 16, 2021 at 10:31:11AM +0200, Olaf Hering wrote:
> Am Fri, 16 Jul 2021 00:58:43 -0700
> schrieb Andrea Bolognani <abologna@redhat.com>:
> 
> > The sysconfig files shipped with libvirt contain the defaults, and
> > the admin is absolutely welcome to tweak them however they might like
> > after installation, just as is the case for all the configuration
> > files in /etc/libvirt.
> 
> Right. Once they are modified for whatever reason things will go downhill.
> 
> Just recently the default (for libvirtd) changed from --listen to --timeout.
> This is an incompatible change.

The --listen parameter was never the default, it only ever existed in
commented out form, similarly today the tcp/tls socket units should
not be enabled by default, because they require auth configuration
before use.

The addition of --timeout parameter was combined with introdution of
socket activation, so should be essentially transparent

> If --listen would have been configured like I proposed, any admin changes are
> clear and obvious, and he has to keep the pieces if it falls apart because
> some default changed. In this case from .service to .socket activation.

I don't see that would have made any change since --listen was not enabled
either before / after the changes.

Originally the sysconfig file provided a way to override the initscripts
config, and was kept with systemd units essentially because it already
exists. Now that initscripts are completely gone, the sysconfig possibly
doesn't need to exist, as it is already posible to override unit files in
a reasonably simple manner.

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

Am Mon, 19 Jul 2021 09:55:37 +0100
schrieb Daniel P. Berrangé <berrange@redhat.com>:

> I don't see that would have made any change since --listen was not enabled
> either before / after the changes.

It was the default in the SUSE package.
I should have checked libvirt.git if it has been used.


> Originally the sysconfig file provided a way to override the initscripts
> config, and was kept with systemd units essentially because it already
> exists. Now that initscripts are completely gone, the sysconfig possibly
> doesn't need to exist, as it is already posible to override unit files in
> a reasonably simple manner.

I have seen sysv scripts that do "test -f sysconfig/file && . $_", so in some
sense sysconfig was already optional before. I have not checked if the sysv
scripts from libvirt.git had such code.

So, do you or anyone else agree to get rid of sysconfig files in libvirt.git
and put the defaults provided by libvirt.git into the systemd unit files?
Just as I proposed in my patch?

If yes, should it be one monolithic change that touches also libvirt.spec
and the NEWS file?


Olaf

On Mon, Jul 19, 2021 at 02:07:10PM +0200, Olaf Hering wrote:
> Am Mon, 19 Jul 2021 09:55:37 +0100
> schrieb Daniel P. Berrangé <berrange@redhat.com>:
> 
> > I don't see that would have made any change since --listen was not enabled
> > either before / after the changes.
> 
> It was the default in the SUSE package.
> I should have checked libvirt.git if it has been used.
> 
> 
> > Originally the sysconfig file provided a way to override the initscripts
> > config, and was kept with systemd units essentially because it already
> > exists. Now that initscripts are completely gone, the sysconfig possibly
> > doesn't need to exist, as it is already posible to override unit files in
> > a reasonably simple manner.
> 
> I have seen sysv scripts that do "test -f sysconfig/file && . $_", so in some
> sense sysconfig was already optional before. I have not checked if the sysv
> scripts from libvirt.git had such code.
> 
> So, do you or anyone else agree to get rid of sysconfig files in libvirt.git
> and put the defaults provided by libvirt.git into the systemd unit files?
> Just as I proposed in my patch?

The challenge i see here is that of managing upgrades to the proposed
state.

By putting '--timeout=xxx' into the .service file, everyone is going
get that change installed, even if they modified their .sysconfig
file previously and thus don't currently have it.

<hand waving>
I think we would need some task that looks at the .sysconfig file
that exists, and then drops in a /etc/systemd/system/libvirtd.service.d/notimeout.conf
file to disable the --timeout usage, if it was not currently in use.
</hand waving>

> If yes, should it be one monolithic change that touches also libvirt.spec
> and the NEWS file?

Yes, if we do this it would need to be for everything I think.

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

Am Mon, 19 Jul 2021 13:21:39 +0100
schrieb Daniel P. Berrangé <berrange@redhat.com>:

> The challenge i see here is that of managing upgrades to the proposed
> state.

This is as easy as this for each modified file that was marked as %config(noreplace):

%pre
test -f /etc/sysconfig/file.rpmsave && mv -v "$_" "$_".old
%posttrans
test -f /etc/sysconfig/file.rpmsave && mv -v "$_" /etc/sysconfig/file


Olaf