On Mon, Jan 03, 2022 at 11:31:20AM +0100, Olaf Hering wrote:
Thu, 23 Dec 2021 10:29:33 -0800 Andrea Bolognani <abologna@redhat.com>:
On Tue, Dec 21, 2021 at 12:22:44PM +0100, Olaf Hering wrote:
-# If systemd socket activation is disabled, then the following -# can be used to listen on TCP/TLS sockets -#LIBVIRTD_ARGS="--listen"
This bit about passing --listen to libvirtd got lost during the move to the unit file. Please make sure you preserve it.
It was wrong from day one to have this fragment here. Documentation belongs to the doc directory.
The fact that we still QEMU_AUDIO_DRV and SDL_AUDIODRIVER in the service file even after your changes goes against this principle. But I don't necessarily disagree, given that --listen is documented both in the manual page for libvirtd and elsewhere.
After this change, libvirt-guests' configuration becomes entirely opaque: the only way the admin can learn how to configure the service is by somehow realizing that it's a shell script as opposed to a binary and looking inside it.
Can we do better?
libvirt-guests is undocumented. The file type is easy to guess, based on the filename in libvirt-guests.service. The Documentation= setting in this file is wrong, libvirtd(8) says nothing about this functionality.
Initial documentation has to be provided as a separate change, by creating a new docs/whatever.rst.
This sounds very good in theory, but in practice we had some sort of documentation in a place where the admin could reasonably be expected to see it, and with your change we're now asking them to go poke inside a script to find that same information, with no clear indication that doing so is necessary. I believe that's a significant downgrade in user experience which is not justified by the desire to have a perfect separation between configuration files and documentation. Please either adopt the mitigation I suggested in my previous message (add a short note to the service file and make sure configuration variables are documented near the very top of the script) or introduce proper documentation for libvirt-guests before making the one we currently have much harder to locate. -- Andrea Bolognani / Red Hat / Virtualization