7:17 a.m.
Hi,
I'm really unhappy about commit 8eb4461645c5 ("remove sysconfig files",
2022-01-17), first included in release v8.1.0.
The (a) well-documented and (b) easily editable config file
"/etc/sysconfig/libvirt-guests" is now gone. So if I want to do now on
Fedora 36 the same thing that I used to do on up to and including Fedora
35, I now need to consult a new manual page (from grandparent commit
161727417a91, "docs: Add man page for libvirt-guests", 2022-01-17), and
collect a bunch of options manually.
The message on commit 8eb4461645c5 says,
Remove the sysconfig file and place the current desired default into
the service file.
which I briefly considered a consolation, figuring I'd just copy the
collected bunch of options (and hopefully their comments!) to the same
place as before, from the "service file" -- "libvirt-guests.service".
However, the actual commit does not live up to its promise; for example,
the important ON_SHUTDOWN knob is only *removed* from the codebase by
the commit; it is not reintroduced anywhere (certainly not in
"libvirt-guests.service"). Well, the manual page, two commits up the
branch, documents it, but that's totally no viable replacement.
As of f8ebb5816350:
$ git grep -w ON_SHUTDOWN
docs/manpages/libvirt-guests.rst:- ON_SHUTDOWN=suspend
docs/manpages/libvirt-guests.rst: time to shutdown. When setting ON_SHUTDOWN=shutdown,
you must also set
docs/manpages/libvirt-guests.rst: "ON_SHUTDOWN" is set to
"shutdown". If Set to 0, guests will be shutdown one
tools/libvirt-guests.sh.in:ON_SHUTDOWN="suspend"
tools/libvirt-guests.sh.in: if [ "x$ON_SHUTDOWN" = xshutdown ]; then
tools/libvirt-guests.sh.in: ON_SHUTDOWN="shutdown"
... It seems that "tools/libvirt-guests.sh.in" does have some built-in
defaults (going back as far as to 66823690e469, "Init script for
handling guests on shutdown/boot", 2010-05-21), which I could copy and
modify presumably; however, those defaults still lack the previously
directly adjacent documentation.
Please consider remedying this. Readily editable config files with
documentation and defaults included are very powerful. They are not
suitable for all config formats of course (especially hierarchical ones:
consider the domain XML for example), but for flat or otherwise simply
structured config files, offering that productivity boost to end-users
is a no-brainer, IMO. Please restore it if you can.
Thanks
Laszlo
11:01 a.m.
Mon, 7 Nov 2022 14:17:00 +0100 Laszlo Ersek <lersek(a)redhat.com>:
The (a) well-documented and (b) easily editable config file
"/etc/sysconfig/libvirt-guests" is now gone.
Right, these admin-owned files are not installed anymore.
The runtime code still uses the files, in case they are created by the admin.
The message on commit 8eb4461645c5 says,
Remove the sysconfig file and place the current desired default into
the service file.
Yes, and right in the next paragraph the commit messages states the files
are (still) recognized.
As of f8b6c7e5, libvirt-guests.sh initializes some internal defaults, then
it loads the admin-owned sysconfig file, in case it was created.
The documentation about this specific tool (libvirt-guests(1)) states what
values from /etc/sysconfig/libvirt-guests will be recognized.
While browsing various user mailing lists in the past years I often saw
messages like "system was upgraded to new version, and as a result an
expected file somewhere in /etc does not exist anymore. What now?".
Apparently some folks expect files to be present before they can be edited.
In my experience it is always possible to create the required files and
fill them with the desired content, based on available documentation.
Olaf
1:46 a.m.
On 11/07/22 18:01, Olaf Hering wrote:
Mon, 7 Nov 2022 14:17:00 +0100 Laszlo Ersek
<lersek(a)redhat.com>:
> The (a) well-documented and (b) easily editable config file
> "/etc/sysconfig/libvirt-guests" is now gone.
Right, these admin-owned files are not installed anymore.
The runtime code still uses the files, in case they are created by the admin.
> The message on commit 8eb4461645c5 says,
> Remove the sysconfig file and place the current desired default into
> the service file.
Yes, and right in the next paragraph the commit messages states the files
are (still) recognized.
As of f8b6c7e5, libvirt-guests.sh initializes some internal defaults, then
it loads the admin-owned sysconfig file, in case it was created.
The documentation about this specific tool (libvirt-guests(1)) states what
values from /etc/sysconfig/libvirt-guests will be recognized.
While browsing various user mailing lists in the past years I often saw
messages like "system was upgraded to new version, and as a result an
expected file somewhere in /etc does not exist anymore. What now?".
Apparently some folks expect files to be present before they can be edited.
In my experience it is always possible to create the required files and
fill them with the desired content, based on available documentation.
It's obviously possible; the question is how comfortable it is, how much
time the user now needs to spend on something that used to be much
easier / faster before.
Can you at least include the previously shipped *documented* config file
as a template in a contrib or docs directory or something?
Thanks
Laszlo
9:58 a.m.
Tue, 8 Nov 2022 08:46:34 +0100 Laszlo Ersek <lersek(a)redhat.com>:
Can you at least include the previously shipped *documented* config
file
as a template in a contrib or docs directory or something?
This is up to the maintainers of libvirt.git to decide.
If I were in charge, nothing would be installed into /etc.
What needs to go there, in case it is required, is already reasonably well documented.
Olaf
12:07 p.m.
On Tue, Nov 08, 2022 at 04:58:37PM +0100, Olaf Hering wrote:
Tue, 8 Nov 2022 08:46:34 +0100 Laszlo Ersek
<lersek(a)redhat.com>:
> Can you at least include the previously shipped *documented* config file
> as a template in a contrib or docs directory or something?
This is up to the maintainers of libvirt.git to decide.
If I were in charge, nothing would be installed into /etc.
What needs to go there, in case it is required, is already reasonably well documented.
Laszlo,
I feel your frustration.
I don't think a revert to the previous situation is going to happen,
because there are some real advantages resulting from not shipping
admin-owned files in our packages.
That said, the current situation is clearly not ideal either, so
let's try to find a way to make things at least a bit better :)
I think a reasonable compromise would be to add
Environment=URIS="default"
Environment=ON_BOOT="start"
...
to libvirt-guests.service. Maybe instead of having the extended
documentation that was originally in the defaults file we could have
a shorter, one-line version? Maybe a pointer to the manual page?
Whatever comments we put there will show up when running 'systemctl
edit libvirt-guests', which makes them fairly discoverable IMO.
That's really the key point, because even today you can change the
behavior both with a defaults file and a systemd unit override.
What do you think? Would that work for you?
--
Andrea Bolognani / Red Hat / Virtualization
12:49 p.m.
Tue, 8 Nov 2022 10:07:21 -0800 Andrea Bolognani <abologna(a)redhat.com>:
I think a reasonable compromise would be to add
Environment=URIS="default"
Environment=ON_BOOT="start"
to libvirt-guests.service.
The script needs to be adjusted to recognize existing environment variables.
Olaf
12:26 a.m.
On 11/08/22 19:07, Andrea Bolognani wrote:
On Tue, Nov 08, 2022 at 04:58:37PM +0100, Olaf Hering wrote:
> Tue, 8 Nov 2022 08:46:34 +0100 Laszlo Ersek <lersek(a)redhat.com>:
>> Can you at least include the previously shipped *documented* config file
>> as a template in a contrib or docs directory or something?
>
> This is up to the maintainers of libvirt.git to decide.
>
> If I were in charge, nothing would be installed into /etc.
> What needs to go there, in case it is required, is already reasonably well
documented.
Laszlo,
I feel your frustration.
I don't think a revert to the previous situation is going to happen,
because there are some real advantages resulting from not shipping
admin-owned files in our packages.
That said, the current situation is clearly not ideal either, so
let's try to find a way to make things at least a bit better :)
I think a reasonable compromise would be to add
Environment=URIS="default"
Environment=ON_BOOT="start"
...
to libvirt-guests.service. Maybe instead of having the extended
documentation that was originally in the defaults file we could have
a shorter, one-line version? Maybe a pointer to the manual page?
Whatever comments we put there will show up when running 'systemctl
edit libvirt-guests', which makes them fairly discoverable IMO.
That's really the key point, because even today you can change the
behavior both with a defaults file and a systemd unit override.
What do you think? Would that work for you?
I'd prefer an (otherwise initially inactive) file somewhere in the
distribution that followed the style of (say) "/etc/libvirt/qemu.conf".
I could copy that in-place, and then edit it.
If this still counts as duplication, then let's stick with the status quo.
Thanks,
Laszlo
11:36 a.m.
On Tue, Nov 08, 2022 at 08:46:34AM +0100, Laszlo Ersek wrote:
On 11/07/22 18:01, Olaf Hering wrote:
> Mon, 7 Nov 2022 14:17:00 +0100 Laszlo Ersek <lersek(a)redhat.com>:
>
>> The (a) well-documented and (b) easily editable config file
>> "/etc/sysconfig/libvirt-guests" is now gone.
>
> Right, these admin-owned files are not installed anymore.
> The runtime code still uses the files, in case they are created by the admin.
>
>> The message on commit 8eb4461645c5 says,
>> Remove the sysconfig file and place the current desired default into
>> the service file.
>
> Yes, and right in the next paragraph the commit messages states the files
> are (still) recognized.
>
> As of f8b6c7e5, libvirt-guests.sh initializes some internal defaults, then
> it loads the admin-owned sysconfig file, in case it was created.
>
> The documentation about this specific tool (libvirt-guests(1)) states what
> values from /etc/sysconfig/libvirt-guests will be recognized.
>
>
> While browsing various user mailing lists in the past years I often saw
> messages like "system was upgraded to new version, and as a result an
> expected file somewhere in /etc does not exist anymore. What now?".
> Apparently some folks expect files to be present before they can be edited.
> In my experience it is always possible to create the required files and
> fill them with the desired content, based on available documentation.
It's obviously possible; the question is how comfortable it is, how much
time the user now needs to spend on something that used to be much
easier / faster before.
Can you at least include the previously shipped *documented* config file
as a template in a contrib or docs directory or something?
That would just be duplicating information already provideed in the
manpage, where there is much more detailed description, so I don't
think that would be useful.
With 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 :|
12:24 a.m.
On 11/08/22 18:36, Daniel P. Berrangé wrote:
On Tue, Nov 08, 2022 at 08:46:34AM +0100, Laszlo Ersek wrote:
> On 11/07/22 18:01, Olaf Hering wrote:
>> Mon, 7 Nov 2022 14:17:00 +0100 Laszlo Ersek <lersek(a)redhat.com>:
>>
>>> The (a) well-documented and (b) easily editable config file
>>> "/etc/sysconfig/libvirt-guests" is now gone.
>>
>> Right, these admin-owned files are not installed anymore.
>> The runtime code still uses the files, in case they are created by the admin.
>>
>>> The message on commit 8eb4461645c5 says,
>>> Remove the sysconfig file and place the current desired default into
>>> the service file.
>>
>> Yes, and right in the next paragraph the commit messages states the files
>> are (still) recognized.
>>
>> As of f8b6c7e5, libvirt-guests.sh initializes some internal defaults, then
>> it loads the admin-owned sysconfig file, in case it was created.
>>
>> The documentation about this specific tool (libvirt-guests(1)) states what
>> values from /etc/sysconfig/libvirt-guests will be recognized.
>>
>>
>> While browsing various user mailing lists in the past years I often saw
>> messages like "system was upgraded to new version, and as a result an
>> expected file somewhere in /etc does not exist anymore. What now?".
>> Apparently some folks expect files to be present before they can be edited.
>> In my experience it is always possible to create the required files and
>> fill them with the desired content, based on available documentation.
>
> It's obviously possible; the question is how comfortable it is, how much
> time the user now needs to spend on something that used to be much
> easier / faster before.
>
> Can you at least include the previously shipped *documented* config file
> as a template in a contrib or docs directory or something?
That would just be duplicating information already provideed in the
manpage, where there is much more detailed description, so I don't
think that would be useful.
The following files:
/etc/libvirt/libvirt-admin.conf
/etc/libvirt/libvirt.conf
/etc/libvirt/libvirtd.conf
/etc/libvirt/libxl.conf
/etc/libvirt/libxl-lockd.conf
/etc/libvirt/lxc.conf
/etc/libvirt/qemu.conf
/etc/libvirt/qemu-lockd.conf
/etc/libvirt/virtinterfaced.conf
/etc/libvirt/virtlockd.conf
/etc/libvirt/virtlogd.conf
/etc/libvirt/virtlxcd.conf
/etc/libvirt/virtnetworkd.conf
/etc/libvirt/virtnodedevd.conf
/etc/libvirt/virtnwfilterd.conf
/etc/libvirt/virtproxyd.conf
/etc/libvirt/virtqemud.conf
/etc/libvirt/virtsecretd.conf
/etc/libvirt/virtstoraged.conf
/etc/libvirt/virtvboxd.conf
/etc/libvirt/virtxend.conf
all exist out of the box too, and follow the style that I'm requesting
(= they are heavily documented in-line).
Laszlo
11:04 a.m.
On Wed, Nov 09, 2022 at 07:24:08AM +0100, Laszlo Ersek wrote:
On 11/08/22 18:36, Daniel P. Berrangé wrote:
> On Tue, Nov 08, 2022 at 08:46:34AM +0100, Laszlo Ersek wrote:
>> Can you at least include the previously shipped *documented* config file
>> as a template in a contrib or docs directory or something?
>
> That would just be duplicating information already provideed in the
> manpage, where there is much more detailed description, so I don't
> think that would be useful.
The following files:
/etc/libvirt/libvirt-admin.conf
/etc/libvirt/libvirt.conf
[...]
/etc/libvirt/virtvboxd.conf
/etc/libvirt/virtxend.conf
all exist out of the box too, and follow the style that I'm requesting
(= they are heavily documented in-line).
Uh. Good point.
Olaf, can you please remind me why the files we dropped were
problematic but these ones apparently aren't?
--
Andrea Bolognani / Red Hat / Virtualization
2:17 p.m.
Wed, 9 Nov 2022 09:04:12 -0800 Andrea Bolognani <abologna(a)redhat.com>:
Olaf, can you please remind me why the files we dropped were
problematic but these ones apparently aren't?
These are equally problematic because they are owned by the admin as well.
They are essentially empty, their content is a duplication from doc/ (I hope).
They should not be part of libvirt.git.
The upgrade needs to be handled like it is done with
libvirt_sysconfig_pre/libvirt_sysconfig_posttrans in libvirt.spec.
Olaf
2:57 a.m.
On Wed, Nov 09, 2022 at 09:17:08PM +0100, Olaf Hering wrote:
Wed, 9 Nov 2022 09:04:12 -0800 Andrea Bolognani
<abologna(a)redhat.com>:
> Olaf, can you please remind me why the files we dropped were
> problematic but these ones apparently aren't?
These are equally problematic because they are owned by the admin as well.
They are essentially empty, their content is a duplication from doc/ (I hope).
They should not be part of libvirt.git.
We are not going to remove these files from git or from the RPM, it would
be incredibly user hostile. I didn't mind removing the sysconfig files
because they were legacy configs from initscripts era which are obsolete
with systemd and so should not really be used except for existing
deployments.
With 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 :|
4:32 a.m.
On Thu, Nov 10, 2022 at 08:57:25AM +0000, Daniel P. Berrangé wrote:
On Wed, Nov 09, 2022 at 09:17:08PM +0100, Olaf Hering wrote:
> Wed, 9 Nov 2022 09:04:12 -0800 Andrea Bolognani <abologna(a)redhat.com>:
> > Olaf, can you please remind me why the files we dropped were
> > problematic but these ones apparently aren't?
>
> These are equally problematic because they are owned by the admin as well.
>
> They are essentially empty, their content is a duplication from doc/ (I hope).
> They should not be part of libvirt.git.
We are not going to remove these files from git or from the RPM, it would
be incredibly user hostile. I didn't mind removing the sysconfig files
because they were legacy configs from initscripts era which are obsolete
with systemd and so should not really be used except for existing
deployments.
Should we reconsider the fate of libvirt-guests's sysconfig file
then?
The other sysconfig files basically only influenced the way each
service is spawned (VIRT*_ARGS, mostly used for --timeout) but in the
case of libvirt-guests the sysconfig file is used to configure the
behavior in ways that don't really fall under the purview of systemd.
Maybe a proper /etc/libvirt/libvirt-guests.conf would make sense, for
consistency with other services? But then, you don't want to be
parsing libvirt's config format from a shell script. Could we rewrite
libvirt-guests in C, possibly improving on the hairier parts of it in
the process? Perhaps even take the opportunity to reconsider the way
some of its settings interact with each other.
I might be getting a bit ahead of myself here :)
--
Andrea Bolognani / Red Hat / Virtualization
7:53 a.m.
On 11/10/22 11:32, Andrea Bolognani wrote:
On Thu, Nov 10, 2022 at 08:57:25AM +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 09, 2022 at 09:17:08PM +0100, Olaf Hering wrote:
>> Wed, 9 Nov 2022 09:04:12 -0800 Andrea Bolognani <abologna(a)redhat.com>:
>>> Olaf, can you please remind me why the files we dropped were
>>> problematic but these ones apparently aren't?
>>
>> These are equally problematic because they are owned by the admin as well.
>>
>> They are essentially empty, their content is a duplication from doc/ (I hope).
>> They should not be part of libvirt.git.
>
> We are not going to remove these files from git or from the RPM, it would
> be incredibly user hostile. I didn't mind removing the sysconfig files
> because they were legacy configs from initscripts era which are obsolete
> with systemd and so should not really be used except for existing
> deployments.
Hmm. That's a good distinction actually. So I guess my original
complaint should be rephrased as one against systemd style service
configuration -- then again, that ship has sailed ;)
Thanks for the enlightenment!
Should we reconsider the fate of libvirt-guests's sysconfig file
then?
The other sysconfig files basically only influenced the way each
service is spawned (VIRT*_ARGS, mostly used for --timeout) but in the
case of libvirt-guests the sysconfig file is used to configure the
behavior in ways that don't really fall under the purview of systemd.
I'm not sure if I know enough to claim this myself -- the libvirt-guests
service's configuration may actually fall in systemd territory. I just
find the override files
/etc/systemd/system/NAME.service.d/override.conf
from "systemctl edit" much less intuitive than the previous way.
BTW, now that I've retried "systemctl edit", it seems that the override
file is "primed" from the central service file
(/usr/lib/systemd/system/NAME.service) -- the latter is included,
commented out, when $EDITOR starts up. So if all the documentation and
the default knob assignments were included in
"/usr/lib/systemd/system/libvirt-guests.service", then "systemctl
edit"
would indeed be a mostly complete replacement for the sysconfig file.
In fact, commit 8eb4461645c5 does indicate *just this* method -- but as
I pointed out up-thread, the central "libvirt-guests.service" file does
not include the actual knobs and their docs; those are (at best) in
"tools/libvirt-guests.sh.in".
Instead, the central service file currently contains (excerpt):
[Service]
EnvironmentFile=-/etc/sysconfig/libvirt-guests
# Hack just call traditional service until we factor
# out the code
ExecStart=/usr/libexec/libvirt-guests.sh start
ExecStop=/usr/libexec/libvirt-guests.sh stop
I guess *that* is the actual problem -- it's technical debt. This
"factoring out" (= the migration of the knobs and the docs to the
service file) should have *preceded* commit 8eb4461645c5. Commit
8eb4461645c5 alluded to the proper (modern) way for configuring
services, and with that argument removed the sysconfig file for
"libvirt-guests" -- but the libvirt-guests service had not converted
sufficiently for that.
Therefore I do claim that commit 8eb4461645c5 is buggy -- it was too early.
Olaf said up-thread, "the script needs to be adjusted to recognize
existing environment variables", and technically, that may indeed be the
solution; I'm just saying it was actualy a *pre-requisite* (missed or
ignored) for commit 8eb4461645c5.
Maybe a proper /etc/libvirt/libvirt-guests.conf would make sense,
for
consistency with other services? But then, you don't want to be
parsing libvirt's config format from a shell script. Could we rewrite
libvirt-guests in C, possibly improving on the hairier parts of it in
the process? Perhaps even take the opportunity to reconsider the way
some of its settings interact with each other.
I might be getting a bit ahead of myself here :)
I think the logic being implemented as a shell script is fine, but its
configuration (env vars), and the related documentation, needs to be
moved / added to the central service file, so that "systemctl edit" can
present them as a crutch to the user, formatted in comments.
The manual page is *not* a substitute for this.
Laszlo
743
days inactive
746
days old
13 comments
4 participants
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Laszlo Ersek -
Olaf Hering