Re: [PATCH] pass kernel command line as verbatim

[Michael Chang <mchang@suse.com>]

On Tue, Apr 24, 2018 at 12:03:21PM +0200, Daniel Kiper wrote:
> On Wed, Apr 18, 2018 at 03:07:15PM +0800, Michael Chang wrote:
> > On Tue, Apr 17, 2018 at 06:37:06PM +0200, Daniel Kiper wrote:
> > > On Wed, Apr 11, 2018 at 05:17:03PM +0800, Michael Chang wrote:
> > > > And this bug report seems relevant ..
> > > >
> > > > https://savannah.gnu.org/bugs/?49937
> > > >
> > > > On Wed, Apr 11, 2018 at 04:58:54PM +0800, Michael Chang wrote:
> > > > > The command line has been processed by grub shell, then the result is expected
> > > > > to be passed to kernel command line as verbatim according to the grub manual
> > > > > [1][2].
> > > > >
> > > > > This patch removes extra escape character added as it helps nothing but only
> > > > > creates trouble as you want them to be literal. Besides the surrounding
> > > > > double-quotes added is kept as it used to protect space.
> > >
> > > CC-ing Vladmir.
> > >
> > > Hmmm... Have you tested this patch on all platforms supported by GRUB2?
> >
> > AFAICS, it affects boot protocols of which grub supports in commands like
> > linux, multiboot, xen_hypervisor and so on. Non of them will intepret escape
> > sequence and are pretty straightforward in composition -- In general, use space
> > to delimit options and quote the options by double quotes once they contain any
> > space.
> >
> > > I understand that current behavior is not accepted on some but I have
> > > a feeling that somebody make it work in that way due to some reason.
> > > Sadly I cannot find anything about that in git log. So, I have to think
> > > how to cope with that. Or...
> >
> > As a side note, we had a down stream bug report that udev generates symlink
> > names containing '\' character, and with the current grub command processing
> > it's never possible to use those symlinks as kernel command line option, as the
> > character is either stripped or doubled.
> 
> I understand your problem but I am afraid about compatibility. I would
> agree if you change the behavior here and introduce a shell variable to
> disable that change. If nobody will complain for some time then we can
> drop that variable a leave new desired behavior. Does it make sense?

Yes, it makes sense. But here I have another idea that can be helpful if we
want to have one grub.cfg working on all versions. With the shell variable is
not possible as the result is either breaking in one way or not.

It borrows the same idea of using feature variable to determine what current
build can support at run time, and thus allow to use another set of settings
like GRUB_CMDLINE_LINUX_V2 to override cmdline for the new sytax. It is fine to
not specify GRUB_CMDLINE_LINUX_V2, in that we will use GRUB_CMDLINE_LINUX.

> 
> And I have a question about space (what about tab?): why it is quoted
> if, AIUI, kernel command line should not be processed at this point?
> Should not it be created earlier properly? I mean with required quotes.

The command line arguments will be processed by grub shell before passing them
to kernel command line. They will evaluate quotes (among other things) and
extract string from them. As a helper, the result can be observed by "echo"
command.

  grub> echo abc="foo bar"
  abc=foo bar

As you can see, we have to add the quotes back here then it can be passed to
kernel command line as one option.

  "abc=foo bar"

There could have other cases needs additional processing, but as a rule of
thumb I think we should avoid them as much as possible unless strickly
necessary. We need a "preditable" result that stays the same by means of echo
command's output.

For the question of tab, I did not investigate. I think it will be the same
with other characters unless it possessed special meaning to grub's shell
syntax...

> 
> Hmmm... What about modules command lines?

I think it in common with others, Why is it special ?

Thanks,
Michael

> 
> Daniel