On Mon, Jan 03, 2022 at 11:31:20AM +0100, Olaf Hering wrote: > Thu, 23 Dec 2021 10:29:33 -0800 Andrea Bolognani <abologna@xxxxxxxxxx>: > > 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