[PATCH v1] virtqemud: remove sysconfig file
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>:
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
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
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Jim Fehlig -
Olaf Hering