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

[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