Bug#1016547: /etc/default/grub.d/xen.cfg: Extraneous output line causes error message at boot

[John E. Krokes]

Package: xen-hypervisor-common
Version: 4.14.5+24-g87d90d511c-1
Severity: minor
File: /etc/default/grub.d/xen.cfg

Dear Maintainer,

When invoked via grub-mkconfig, xen.cfg outputs this as its first line:
Including Xen overrides from /etc/default/grub.d/xen.cfg

The output of grub-mkconfig is expected to be redirected into a grub.cfg file.
Grub will read the grub.cfg at boot. Unfortunately, "Including" is not a
valid grub command. So when booting, grub emits this error message before
displaying its menu:
error: can't find command `Including'.


The error message is obscured very quickly. It does not affect functionality
in any way. It requires booting on a VERY slow machine in order to read
the error message at all.


If I add a '#' to the start of the "Including", the resulting grub config file
boots with no error.
echo "#Including Xen overrides from /etc/default/grub.d/xen.cfg"


I'm not sure if this line was intended to go into the generated config
file as a comment, or if it was intended to be shown to the user while
grub-mkconfig is running.


I have observed this and tested my fix against version
4.14.5+24-g87d90d511c-1 of xen-hypervisor-common. I have also checked
with the debian git at https://salsa.debian.org/xen-team/debian-xen/-/blob/master/debian/tree/xen-hypervisor-common/etc/default/grub.d/xen.cfg. This line
has not changed in a very long time.


I can also duplicate the behavior using grub-emu, with the output redirected
to a file.


I am running devuan, and originally reported this to their BTS but was
redirected to debian. So my version number does not match. Apologies for
that.


Thanks for your time.

-Mag



-- System Information:
Debian Release: 10.0
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-16-amd64 (SMP w/2 CPU threads)
Locale: LANG=C, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)
LSM: AppArmor: enabled

-- Configuration Files:
/etc/default/grub.d/xen.cfg changed:
echo "#Including Xen overrides from /etc/default/grub.d/xen.cfg"
XEN_OVERRIDE_GRUB_DEFAULT=0
if [ "$XEN_OVERRIDE_GRUB_DEFAULT" = "" ]; then
echo "WARNING: GRUB_DEFAULT changed to boot into Xen by default!"
echo " Edit /etc/default/grub.d/xen.cfg to avoid this warning."
XEN_OVERRIDE_GRUB_DEFAULT=1
fi
if [ "$XEN_OVERRIDE_GRUB_DEFAULT" = "1" ]; then
GRUB_DEFAULT=$( \
printf "$(gettext "%s, with Xen hypervisor")" \
"$GRUB_DISTRIBUTOR GNU/Linux")
fi


-- no debconf information

[Hans van Kranenburg]

Hi John,

On 8/2/22 19:50, John E. Krokes wrote:
> Package: xen-hypervisor-common
> Version: 4.14.5+24-g87d90d511c-1
> Severity: minor
> File: /etc/default/grub.d/xen.cfg
>
> Dear Maintainer,
>
> When invoked via grub-mkconfig, xen.cfg outputs this as its first line:
> Including Xen overrides from /etc/default/grub.d/xen.cfg
>
> The output of grub-mkconfig is expected to be redirected into a grub.cfg file.
> Grub will read the grub.cfg at boot. Unfortunately, "Including" is not a
> valid grub command. So when booting, grub emits this error message before
> displaying its menu:
> error: can't find command `Including'.

Aha! Nice catch. That's indeed something that should be improved.

> [...]

>
> The error message is obscured very quickly. It does not affect functionality
> in any way. It requires booting on a VERY slow machine in order to read
> the error message at all.
>
>
> If I add a '#' to the start of the "Including", the resulting grub config file
> boots with no error.
> echo "#Including Xen overrides from /etc/default/grub.d/xen.cfg"
>
> I'm not sure if this line was intended to go into the generated config
> file as a comment, or if it was intended to be shown to the user while
> grub-mkconfig is running.

I'm sure it's the latter, yes. Just some 'hey! I'm doing this now' message.

> I have observed this and tested my fix against version
> 4.14.5+24-g87d90d511c-1 of xen-hypervisor-common. I have also checked
> with the debian git at https://salsa.debian.org/xen-team/debian-xen/-/blob/master/debian/tree/xen-hypervisor-common/etc/default/grub.d/xen.cfg. This line
> has not changed in a very long time.
>
>
> I can also duplicate the behavior using grub-emu, with the output redirected
> to a file.
>
>
> I am running devuan, and originally reported this to their BTS but was
> redirected to debian. So my version number does not match. Apologies for
> that.

It's ok. The changes/improvements for this will end up in the Xen 4.16
package that's in Debian unstable now, anyway.

So, in our grub.d/xen.cfg file, there's two places that cause text
output: the 'Including Xen overrides ...' informational one, and the
notification/warning about overriding GRUB_DEFAULT.

The grub.d/* files are executed (sourced) in the context of the
grub-mkconfig itself using '.'. In there, I can see that similar status
messages are just redirected to stderr. We can do the same here. For the
warning, there's a grub_warn helper function, which we can use.

So, that results in the follow changes I have here now:

diff --git a/default/grub.d/xen.cfg b/default/grub.d/xen.cfg
index d35744e..42670eb 100644
--- a/default/grub.d/xen.cfg
+++ b/default/grub.d/xen.cfg
@@ -5,7 +5,7 @@
# The configuration in here makes it possible to have different options set
# for the linux kernel when booting with or without Xen.

-echo "Including Xen overrides from /etc/default/grub.d/xen.cfg"
+echo "Including Xen overrides from /etc/default/grub.d/xen.cfg" >&2

#######################################################################
# Xen Hypervisor Command Line Options
@@ -83,8 +83,8 @@ GRUB_CMDLINE_LINUX_XEN_REPLACE="earlyprintk=xen
console=hvc0 noresume"
#XEN_OVERRIDE_GRUB_DEFAULT=
#
if [ "$XEN_OVERRIDE_GRUB_DEFAULT" = "" ]; then
- echo "WARNING: GRUB_DEFAULT changed to boot into Xen by default!"
- echo " Edit /etc/default/grub.d/xen.cfg to avoid this
warning."
+ grub_warn "GRUB_DEFAULT changed to boot into Xen by default!" \
+ "Edit /etc/default/grub.d/xen.cfg to avoid this warning."

XEN_OVERRIDE_GRUB_DEFAULT=1
fi
if [ "$XEN_OVERRIDE_GRUB_DEFAULT" = "1" ]; then

None of this output will now be mixed with the generated config any more.

This will be in the next package upload.

https://salsa.debian.org/xen-team/debian-xen/-/commits/wip/sid

Thanks,
Hans