| Message ID | DU0P192MB1700AA0337B5E6598E23EE0AE3332@DU0P192MB1700.EURP192.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools: init scripts: Add missing XENCONSOLED_ARGS variable to xencommons | expand |
On Wed, Mar 20, 2024 at 08:48:33AM +0100, Rafaël Kooi wrote: > The systemd xenconsoled.service uses the XENCONSOLED_ARGS variable, but > it was missing from the xencommons file. Actually, I'm tempted to go the other way and remove XENCONSOLED_ARGS from the systemd's service file. In the other service files (openrc, sysvinit) for Linux/FreeBSD/NetBSD, XENCONSOLED_ARGS is simply used to construct the command line for `xenconsoled`. For the Linux one, if you set XENCONSOLED_TRACE, XENCONSOLED_ARGS from xencommons won't be used. On the other end, with systemd, it is very easy to modified a service file, to add an argument to the command line. So removing XENCONSOLED_ARGS would make more sense. But some user might have notice it exist and start using it, so it's probably too late to remove it. So overall, I don't think it's a good idea to advertise XENCONSOLED_ARGS, first because it's kind of useless for systemd, and second because it's broken for "tools/hotplug/Linux/init.d/xencommons.in" Side note: looks like on my test machine I've used systemd instead of editing /etc/default/xencommons to change XENCONSOLED_TRACE, so there's really no need for "xencommons" config file on systemd: # /etc/systemd/system/xenconsoled.service.d/trace_all.conf [Service] Environment=XENCONSOLED_TRACE=all That "xencommons" config file is just there to be compatible with both init system. Cheers,
On 25/03/2024 11:32, Anthony PERARD wrote: > On Wed, Mar 20, 2024 at 08:48:33AM +0100, Rafaël Kooi wrote: >> The systemd xenconsoled.service uses the XENCONSOLED_ARGS variable, but >> it was missing from the xencommons file. > > Actually, I'm tempted to go the other way and remove XENCONSOLED_ARGS > from the systemd's service file. In the other service files (openrc, > sysvinit) for Linux/FreeBSD/NetBSD, XENCONSOLED_ARGS is simply used to > construct the command line for `xenconsoled`. For the Linux one, if you > set XENCONSOLED_TRACE, XENCONSOLED_ARGS from xencommons won't be used. I assume you mean the OpenRC/SysV init script? > > On the other end, with systemd, it is very easy to modified a service > file, to add an argument to the command line. So removing > XENCONSOLED_ARGS would make more sense. But some user might have notice > it exist and start using it, so it's probably too late to remove it.> > So overall, I don't think it's a good idea to advertise > XENCONSOLED_ARGS, first because it's kind of useless for systemd, and > second because it's broken for "tools/hotplug/Linux/init.d/xencommons.in" > > > > Side note: looks like on my test machine I've used systemd instead of > editing /etc/default/xencommons to change XENCONSOLED_TRACE, so there's > really no need for "xencommons" config file on systemd: > > # /etc/systemd/system/xenconsoled.service.d/trace_all.conf > [Service] > Environment=XENCONSOLED_TRACE=all > > That "xencommons" config file is just there to be compatible with both > init system. > > Cheers, > Makes sense. To be honest I don't even know what arguments one would pass to xenconsoled other than setting the log level to begin with. I would assume this to be a niche enough use case where very few users would be affected if we do decided to remove the variable. Maybe we could opt to advertise it as something that we will be removing in a later release? Maybe either through an "ExecStartPre" message, or by updating the comment in xencommons. Thanks, Rafaël Kooi
On Mon, Mar 25, 2024 at 12:09:40PM +0100, Rafaël Kooi wrote: > Makes sense. To be honest I don't even know what arguments one would pass > to xenconsoled other than setting the log level to begin with. I would > assume this to be a niche enough use case where very few users would be > affected if we do decided to remove the variable. Maybe we could opt to > advertise it as something that we will be removing in a later release? I think it's fine to leave the variable where it is. On a systemd service file, it's kind of nice to be able to add new arguments without overwriting the whole command, as it means that we get the command line on an update if it changed. Cheers,
diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in index 1bdd830d8a..42104ecaa4 100644 --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in @@ -5,6 +5,12 @@ # Log xenconsoled messages (cf xl dmesg) #XENCONSOLED_TRACE=[none|guest|hv|all] +## Type: string +## Default: "" +# +# Additional commandline arguments to start xenconsoled. +XENCONSOLED_ARGS= + ## Type: string ## Default: daemon #
The systemd xenconsoled.service uses the XENCONSOLED_ARGS variable, but it was missing from the xencommons file. Signed-off-by: Rafaël Kooi <rafael_andreas@hotmail.com> --- tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 6 ++++++ 1 file changed, 6 insertions(+)