All of lore.kernel.org
 help / color / mirror / Atom feed
* pvgrub2 legacy_configfile issues: /sbin/init exec error -8
@ 2017-02-11 13:53 Andy Smith
  2017-02-11 14:46 ` Andrei Borzenkov
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Smith @ 2017-02-11 13:53 UTC (permalink / raw)
  To: grub-devel

Hi,

I'm experimenting with pvgrub2 for the first time, and having some
problems getting a grub-legacy menu.lst config to work.

Using a Debian jessie Xen dom0 and guest. In dom0, grub is built
from git:

----------------------------------------------------------------------
$ sudo apt-get build-dep grub
$ git clone git://git.savannah.gnu.org/grub.git
$ cd grub/
$ ./autogen.sh
$ ./configure --prefix=/opt/grub --target=amd64 --with-platform=xen
$ make
$ sudo make install
$ sudo mkdir -vp /opt/grub/share/memdisk
$ sudo tar -C /opt/grub/share/memdisk -cvf /opt/grub/share/memdisk.tar grub.cfg
$ sudo /opt/grub/bin/grub-mkimage -O x86_64-xen -c /opt/grub/etc/grub-bootstrap.cfg -m /opt/grub/share/memdisk.tar -o /opt/grub/lib/grub-x86_64-xen.bin -p /boot/grub /opt/grub/lib/grub/x86_64-xen/*.mod
----------------------------------------------------------------------

Contents of /opt/grub/etc/grub-bootstrap.cfg:

----------------------------------------------------------------------
normal (memdisk)/grub.cfg
----------------------------------------------------------------------

Contents of /opt/grub/share/memdisk/grub.cfg:

----------------------------------------------------------------------
if search -s -f /boot/xen/pvboot-x86_64.elf ; then
        echo "Chainloading (${root})/boot/xen/pvboot-x86_64.elf"
        read
        multiboot "/boot/xen/pvboot-x86_64.elf"
        boot
fi

if search -s -f /xen/pvboot-x86_64.elf ; then
        echo "Chainloading (${root})/xen/pvboot-x86_64.elf"
        read
        multiboot "/xen/pvboot-x86_64.elf"
        boot
fi

if search -s -f /boot/grub/grub.cfg ; then
        echo "Reading (${root})/boot/grub/grub.cfg"
        read
        configfile /boot/grub/grub.cfg
fi

if search -s -f /grub/grub.cfg ; then
        echo "Reading (${root})/grub/grub.cfg"
        read
        configfile /grub/grub.cfg
fi

if search -s -f /boot/grub/menu.lst ; then
        echo "Reading (${root})/boot/grub/menu.lst"
        read
        legacy_configfile /boot/grub/menu.lst
fi

if search -s -f /grub/menu.lst ; then
        set root=(xen/xvda,msdos1)
        echo "Reading (${root})/grub/menu.lst"
        read
        legacy_configfile /grub/menu.lst
fi
----------------------------------------------------------------------

Contents of /boot/grub/menu.lst in guest:

----------------------------------------------------------------------
default         0
timeout         5
color cyan/blue white/blue
title           Debian GNU/Linux, kernel 3.16.0-4-amd64
root            (hd0,0)
kernel          /boot/vmlinuz-3.16.0-4-amd64 root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59 ro 
initrd          /boot/initrd.img-3.16.0-4-amd64
----------------------------------------------------------------------

So, purpose of the grub.cfg in the dom0 memdisk is to try to, in
order:

1. Chainload pvgrub from guest, in boot/xen/ or in /xen/.

2. Parse guest's /boot/grub/grub.cfg or /grub/grub.cfg in pvgrub2.

3. Parse guest's legacy /boot/grub/menu.lst or /grub/menu.lst in
   pvgrub2.

(The "read" commands were used just to make it pause until I hit
return so I could read the "echo" before it.)

I've tested that outcomes (1) and (2) work. That is, chainload to
pvgrub2 in guest works and so does pvgrub parsing guest's grub.cfg.

After purging grub-xen and grub-pc packages from the guest and
installing grub-legacy, ensuring there is no /boot/xen/*, no
/boot/grub/grub.cfg, and that there is a /boot/grub/menu.lst as
above, an attempt to boot the guest results in a kernel panic at the
point where the guest kernel tries to execute /sbin/init:

----------------------------------------------------------------------
.
.
.
[    1.470403] Freeing unused kernel memory: 216K (ffff8800017ca000 - ffff880001800000)
[    1.471934] Failed to execute /init (error -8)
[    1.472673] Starting init: /sbin/init exists but couldn't execute it (error -8)
[    1.473359] Starting init: /bin/sh exists but couldn't execute it (error -8)
[    1.473365] Kernel panic - not syncing: No working init found.  Try passing init= option to kernel. See Linux Documentation/init.txt for guidance.
[    1.473369] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.16.0-4-amd64 #1 Debian 3.16.39-1
[    1.473372]  0000000000000000 ffffffff81514c11 ffffffff81705310 ffff88003e23bf40
[    1.473377]  ffffffff8151195e ffffffff00000008 ffff88003e23bf50 ffff88003e23bef0
[    1.473380]  ffff88003e23bef8 0000000000000000 0000000000000495 0000000000000495
[    1.473384] Call Trace:
----------------------------------------------------------------------

Error -8 is an exec format error, almost as if it got booted as a
32-bit guest and tried to execute a 64-bit binary.

Switching the guest config back to pygrub allows it to boot without
any alteration of the menu.lst inside the guest.

So, I feel I am missing something very simple here, but much
searching has not produced any answer. Has anyone seen this before?

I appreciate I may have sent this query to the wrong place. It
doesn't feel like a Xen issue so I haven't yet tried xen-devel list.
Yet also probably not a bug in grub, just my misunderstanding, so I
didn't put anything in your bugzilla. If you think there is a more
appropriate place to send the query please do let me know.

Cheers,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 13:53 pvgrub2 legacy_configfile issues: /sbin/init exec error -8 Andy Smith
@ 2017-02-11 14:46 ` Andrei Borzenkov
  2017-02-11 15:26   ` Andy Smith
  0 siblings, 1 reply; 12+ messages in thread
From: Andrei Borzenkov @ 2017-02-11 14:46 UTC (permalink / raw)
  To: The development of GNU GRUB

11.02.2017 16:53, Andy Smith пишет:
...
> 
> if search -s -f /grub/menu.lst ; then
>         set root=(xen/xvda,msdos1)
>         echo "Reading (${root})/grub/menu.lst"
>         read
>         legacy_configfile /grub/menu.lst
> fi
> ----------------------------------------------------------------------
> 
> Contents of /boot/grub/menu.lst in guest:
> 
> ----------------------------------------------------------------------
> default         0
> timeout         5
> color cyan/blue white/blue
> title           Debian GNU/Linux, kernel 3.16.0-4-amd64
> root            (hd0,0)
> kernel          /boot/vmlinuz-3.16.0-4-amd64 root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59 ro 
> initrd          /boot/initrd.img-3.16.0-4-amd64
> ----------------------------------------------------------------------
> 

What happens if you load the same kernel manually in dom0?

linux /boot/vmlinuz ...
initrd /boot/initrd ...

(adjust paths and $root as needed)?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 14:46 ` Andrei Borzenkov
@ 2017-02-11 15:26   ` Andy Smith
  2017-02-11 17:19     ` Andy Smith
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Smith @ 2017-02-11 15:26 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: The development of GNU GRUB

Hi Andrei,

On Sat, Feb 11, 2017 at 05:46:03PM +0300, Andrei Borzenkov wrote:
> What happens if you load the same kernel manually in dom0?

When I do:

sudo /usr/lib/xen-4.4/bin/pygrub -n /dev/myvg/debtest1_xvda

I get:

linux (kernel '<kernel:/boot/vmlinuz-3.16.0-4-amd64>')(ramdisk '<ramdisk:/boot/initrd.img-3.16.0-4-amd64>')(args 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59 ro')

(as mentioned, if I boot it with pygrub then it works)

Putting that directly in a Xen guest config file like:

$ sudo kpartx -va /dev/mapper/myvg-debtest1_xvda add map myvg-debtest1_xvda1 (253:29): 0 10483712 linear /dev/mapper/myvg-debtest1_xvda 2048
$ sudo mount /dev/mapper/myvg-debtest1_xvda1 /mnt/xen
$ sudo cp /mnt/xen/boot/vmlinuz-3.16.0-4-amd64 /tmp/kernel
$ sudo cp /mnt/xen/boot/initrd.img-3.16.0-4-amd64 /tmp/initrd
$ cat /etc/xen/debtest1.conf
name       = "debtest1"
memory     = 1024
vcpus      = 2
vif        = [ … ]
kernel     = '/tmp/kernel'
ramdisk    = '/tmp/initrd'
extra      = 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59 ro'
disk       = [ "phy:/dev/myvg/debtest1_xvda,xvda,w",
               "phy:/dev/myvg/debtest1_xvdb,xvdb,w",
               "phy:/dev/myvg/debtest1_xvdc,xvdc,w" ]
on_poweroff = "preserve"
on_reboot   = "preserve"
on_crash    = "preserve"

This boots fine.

What also works with the configuration in dom0 as described in
previous email:

- Installing grub-xen in guest and chainloading to that

- Installing grub-pc in guest and using "configfile
  /boot/grub/grub.cfg"

The only thing that I can't get working is "legacy_configfile
/boot/grub/menu.lst"

Here is what the config looks like if I interrupt the boot and go to
edit it:

set root='(hd0,1)'; set legacy_hdbias='0'
legacy_kernel   '/boot/vmlinuz-3.16.0-4-amd64' '/boot/vmlinuz-3.16.0-4-amd64' 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59' 'ro'
legacy_initrd '/boot/initrd.img-3.16.0-4-amd64' '/boot/initrd.img-3.16.0-4-amd64'

Cheers,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 15:26   ` Andy Smith
@ 2017-02-11 17:19     ` Andy Smith
  2017-02-11 18:45       ` Andrei Borzenkov
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Smith @ 2017-02-11 17:19 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: The development of GNU GRUB

Hello,

On Sat, Feb 11, 2017 at 03:26:43PM +0000, Andy Smith wrote:
> Here is what the config looks like if I interrupt the boot and go to
> edit it:
> 
> set root='(hd0,1)'; set legacy_hdbias='0'
> legacy_kernel   '/boot/vmlinuz-3.16.0-4-amd64' '/boot/vmlinuz-3.16.0-4-amd64' 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59' 'ro'
> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64' '/boot/initrd.img-3.16.0-4-amd64'

Over on the xen-users list, Sarah Newman pointed out that the
legacy_initrd line should not have a repeated argument:

    https://lists.xenproject.org/archives/html/xen-users/2017-02/msg00029.html

If I edit away the repetition, so that the config line is:

legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'

then this boots fine.

Should I report this in grub's bugzilla?

Thanks,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 17:19     ` Andy Smith
@ 2017-02-11 18:45       ` Andrei Borzenkov
  2017-02-11 21:07         ` Vladimir 'phcoder' Serbinenko
  0 siblings, 1 reply; 12+ messages in thread
From: Andrei Borzenkov @ 2017-02-11 18:45 UTC (permalink / raw)
  To: Andy Smith; +Cc: The development of GNU GRUB

11.02.2017 20:19, Andy Smith пишет:
> Hello,
> 
> On Sat, Feb 11, 2017 at 03:26:43PM +0000, Andy Smith wrote:
>> Here is what the config looks like if I interrupt the boot and go to
>> edit it:
>>
>> set root='(hd0,1)'; set legacy_hdbias='0'
>> legacy_kernel   '/boot/vmlinuz-3.16.0-4-amd64' '/boot/vmlinuz-3.16.0-4-amd64' 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59' 'ro'
>> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64' '/boot/initrd.img-3.16.0-4-amd64'
> 
> Over on the xen-users list, Sarah Newman pointed out that the
> legacy_initrd line should not have a repeated argument:
> 
>     https://lists.xenproject.org/archives/html/xen-users/2017-02/msg00029.html
> 
> If I edit away the repetition, so that the config line is:
> 
> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'
> 
> then this boots fine.
> 

This was explicitly added in commit
9fb175ed9a48102543867f8006842628cc41217c. I do not understand the
purpose of this (is there any requirement that multiboot kernel/module
gets its name as the first parameter?)

@Vladimir? Either we need to revert this, or add similar handling to
legacy_initrd, but I miss why we need it in the first place.

> Should I report this in grub's bugzilla?
> 

Sure. Although I still wonder why it fails. I.e. kernel should simply
extract the same initrd content twice, why it causes such effect?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 18:45       ` Andrei Borzenkov
@ 2017-02-11 21:07         ` Vladimir 'phcoder' Serbinenko
  2017-02-11 23:36           ` Vladimir 'phcoder' Serbinenko
  2017-02-12  7:06           ` Andrei Borzenkov
  0 siblings, 2 replies; 12+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2017-02-11 21:07 UTC (permalink / raw)
  To: The development of GNU GRUB, Andy Smith


[-- Attachment #1.1: Type: text/plain, Size: 1968 bytes --]

On Sat, 11 Feb 2017, 19:45 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> 11.02.2017 20:19, Andy Smith пишет:
> > Hello,
> >
> > On Sat, Feb 11, 2017 at 03:26:43PM +0000, Andy Smith wrote:
> >> Here is what the config looks like if I interrupt the boot and go to
> >> edit it:
> >>
> >> set root='(hd0,1)'; set legacy_hdbias='0'
> >> legacy_kernel   '/boot/vmlinuz-3.16.0-4-amd64'
> '/boot/vmlinuz-3.16.0-4-amd64'
> 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59' 'ro'
> >> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'
> '/boot/initrd.img-3.16.0-4-amd64'
> >
> > Over on the xen-users list, Sarah Newman pointed out that the
> > legacy_initrd line should not have a repeated argument:
> >
> >
> https://lists.xenproject.org/archives/html/xen-users/2017-02/msg00029.html
> >
> > If I edit away the repetition, so that the config line is:
> >
> > legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'
> >
> > then this boots fine.
> >
>
> This was explicitly added in commit
> 9fb175ed9a48102543867f8006842628cc41217c. I do not understand the
> purpose of this (is there any requirement that multiboot kernel/module
> gets its name as the first parameter?)
>
> @Vladimir? Either we need to revert this, or add similar handling to
> legacy_initrd, but I miss why we need it in the first place.
>
For multiboot module old grub was passing module file name on module
command line, so legacy_configfile should replicate this behaviour. But in
Linux case it shouldn't have ended up loading twice. We need additional
code in legacy_initrd. Please try the attached patch

>
> > Should I report this in grub's bugzilla?
> >
>
> Sure. Although I still wonder why it fails. I.e. kernel should simply
> extract the same initrd content twice, why it causes such effect?
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #1.2: Type: text/html, Size: 3654 bytes --]

[-- Attachment #2: initrd.diff --]
[-- Type: text/plain, Size: 450 bytes --]

diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
index dd9d9f1..24546bf 100644
--- a/grub-core/commands/legacycfg.c
+++ b/grub-core/commands/legacycfg.c
@@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd __attribute__ ((unused)),
 #endif
 			   );
 
-      return cmd->func (cmd, argc, args);
+      return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);
     }
   if (kernel_type == MULTIBOOT)
     {

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 21:07         ` Vladimir 'phcoder' Serbinenko
@ 2017-02-11 23:36           ` Vladimir 'phcoder' Serbinenko
  2017-02-12  7:06           ` Andrei Borzenkov
  1 sibling, 0 replies; 12+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2017-02-11 23:36 UTC (permalink / raw)
  To: The development of GNU GRUB, Andy Smith

[-- Attachment #1: Type: text/plain, Size: 2144 bytes --]

Actually patch is wrong, I'll correct and send improved version

On Sat, 11 Feb 2017, 22:07 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
wrote:

>
>
> On Sat, 11 Feb 2017, 19:45 Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>
> 11.02.2017 20:19, Andy Smith пишет:
> > Hello,
> >
> > On Sat, Feb 11, 2017 at 03:26:43PM +0000, Andy Smith wrote:
> >> Here is what the config looks like if I interrupt the boot and go to
> >> edit it:
> >>
> >> set root='(hd0,1)'; set legacy_hdbias='0'
> >> legacy_kernel   '/boot/vmlinuz-3.16.0-4-amd64'
> '/boot/vmlinuz-3.16.0-4-amd64'
> 'root=UUID=38420e46-6123-477d-ba23-baeba8ac0d59' 'ro'
> >> legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'
> '/boot/initrd.img-3.16.0-4-amd64'
> >
> > Over on the xen-users list, Sarah Newman pointed out that the
> > legacy_initrd line should not have a repeated argument:
> >
> >
> https://lists.xenproject.org/archives/html/xen-users/2017-02/msg00029.html
> >
> > If I edit away the repetition, so that the config line is:
> >
> > legacy_initrd '/boot/initrd.img-3.16.0-4-amd64'
> >
> > then this boots fine.
> >
>
> This was explicitly added in commit
> 9fb175ed9a48102543867f8006842628cc41217c. I do not understand the
> purpose of this (is there any requirement that multiboot kernel/module
> gets its name as the first parameter?)
>
> @Vladimir? Either we need to revert this, or add similar handling to
> legacy_initrd, but I miss why we need it in the first place.
>
> For multiboot module old grub was passing module file name on module
> command line, so legacy_configfile should replicate this behaviour. But in
> Linux case it shouldn't have ended up loading twice. We need additional
> code in legacy_initrd. Please try the attached patch
>
>
> > Should I report this in grub's bugzilla?
> >
>
> Sure. Although I still wonder why it fails. I.e. kernel should simply
> extract the same initrd content twice, why it causes such effect?
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>

[-- Attachment #2: Type: text/html, Size: 4200 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-11 21:07         ` Vladimir 'phcoder' Serbinenko
  2017-02-11 23:36           ` Vladimir 'phcoder' Serbinenko
@ 2017-02-12  7:06           ` Andrei Borzenkov
  2017-02-12 10:52             ` Vladimir 'phcoder' Serbinenko
  1 sibling, 1 reply; 12+ messages in thread
From: Andrei Borzenkov @ 2017-02-12  7:06 UTC (permalink / raw)
  To: The development of GNU GRUB, Andy Smith

12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет:
...
>>
> For multiboot module old grub was passing module file name on module
> command line, so legacy_configfile should replicate this behaviour. But in
> Linux case it shouldn't have ended up loading twice. We need additional
> code in legacy_initrd. Please try the attached patch
> 
...
> 
> 
> diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
> index dd9d9f1..24546bf 100644
> --- a/grub-core/commands/legacycfg.c
> +++ b/grub-core/commands/legacycfg.c
> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd __attribute__ ((unused)),
>  #endif
>  			   );
>  
> -      return cmd->func (cmd, argc, args);
> +      return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);

If the goal is to be compatible with legacy grub, it is not compatible
(nor was) - legacy grub "initrd" ignored all extra arguments to "initrd"
command while we attempt to load them as additional initrd components.

Nor did legacy grub attempt to quote command line for multiboot kernel
or modules.

>      }
>    if (kernel_type == MULTIBOOT)
>      {
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-12  7:06           ` Andrei Borzenkov
@ 2017-02-12 10:52             ` Vladimir 'phcoder' Serbinenko
  2017-02-24 23:22               ` Vladimir 'phcoder' Serbinenko
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2017-02-12 10:52 UTC (permalink / raw)
  To: The development of GNU GRUB, Andy Smith

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

On Sun, 12 Feb 2017, 08:06 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> 12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет:
> ...
> >>
> > For multiboot module old grub was passing module file name on module
> > command line, so legacy_configfile should replicate this behaviour. But
> in
> > Linux case it shouldn't have ended up loading twice. We need additional
> > code in legacy_initrd. Please try the attached patch
> >
> ...
> >
> >
> > diff --git a/grub-core/commands/legacycfg.c
> b/grub-core/commands/legacycfg.c
> > index dd9d9f1..24546bf 100644
> > --- a/grub-core/commands/legacycfg.c
> > +++ b/grub-core/commands/legacycfg.c
> > @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
> __attribute__ ((unused)),
> >  #endif
> >                          );
> >
> > -      return cmd->func (cmd, argc, args);
> > +      return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);
>
> If the goal is to be compatible with legacy grub, it is not compatible
> (nor was) - legacy grub "initrd" ignored all extra arguments to "initrd"
> command while we attempt to load them as additional initrd components.
>
> Nor did legacy grub attempt to quote command line for multiboot kernel
> or modules.
>
Agreed on both. That's why I said that previous patch is wrong and I'm
working on new one

>
> >      }
> >    if (kernel_type == MULTIBOOT)
> >      {
> >
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 3593 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-12 10:52             ` Vladimir 'phcoder' Serbinenko
@ 2017-02-24 23:22               ` Vladimir 'phcoder' Serbinenko
  2017-02-25  5:27                 ` Andrei Borzenkov
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2017-02-24 23:22 UTC (permalink / raw)
  To: The development of GNU GRUB, Andy Smith

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

On Sun, Feb 12, 2017, 11:52 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
wrote:

>
>
> On Sun, 12 Feb 2017, 08:06 Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>
> 12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет:
> ...
> >>
> > For multiboot module old grub was passing module file name on module
> > command line, so legacy_configfile should replicate this behaviour. But
> in
> > Linux case it shouldn't have ended up loading twice. We need additional
> > code in legacy_initrd. Please try the attached patch
> >
> ...
> >
> >
> > diff --git a/grub-core/commands/legacycfg.c
> b/grub-core/commands/legacycfg.c
> > index dd9d9f1..24546bf 100644
> > --- a/grub-core/commands/legacycfg.c
> > +++ b/grub-core/commands/legacycfg.c
> > @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
> __attribute__ ((unused)),
> >  #endif
> >                          );
> >
> > -      return cmd->func (cmd, argc, args);
> > +      return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);
>
> If the goal is to be compatible with legacy grub, it is not compatible
> (nor was) - legacy grub "initrd" ignored all extra arguments to "initrd"
> command while we attempt to load them as additional initrd components.
>
> Nor did legacy grub attempt to quote command line for multiboot kernel
> or modules.
>
> Agreed on both. That's why I said that previous patch is wrong and I'm
> working on new one
>
For first point, see attached patch.
Second point isn't a problem as legacycfg will already split along spaces
and hence quoting code will not be triggered.


diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
index dd9d9f1..b32f3c7 100644
--- a/grub-core/commands/legacycfg.c
+++ b/grub-core/commands/legacycfg.c
@@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
__attribute__ ((unused)),
#endif
);

- return cmd->func (cmd, argc, args);
+ return cmd->func (cmd, argc ? 1 : 0, args);
}
if (kernel_type == MULTIBOOT)
{

[-- Attachment #2: Type: text/html, Size: 3455 bytes --]

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-24 23:22               ` Vladimir 'phcoder' Serbinenko
@ 2017-02-25  5:27                 ` Andrei Borzenkov
  2017-02-25 12:19                   ` Andy Smith
  0 siblings, 1 reply; 12+ messages in thread
From: Andrei Borzenkov @ 2017-02-25  5:27 UTC (permalink / raw)
  To: The development of GNU GRUB, Andy Smith

25.02.2017 02:22, Vladimir 'phcoder' Serbinenko пишет:
> On Sun, Feb 12, 2017, 11:52 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
> wrote:
> 
>>
>>
>> On Sun, 12 Feb 2017, 08:06 Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>
>> 12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет:
>> ...
>>>>
>>> For multiboot module old grub was passing module file name on module
>>> command line, so legacy_configfile should replicate this behaviour. But
>> in
>>> Linux case it shouldn't have ended up loading twice. We need additional
>>> code in legacy_initrd. Please try the attached patch
>>>
>> ...
>>>
>>>
>>> diff --git a/grub-core/commands/legacycfg.c
>> b/grub-core/commands/legacycfg.c
>>> index dd9d9f1..24546bf 100644
>>> --- a/grub-core/commands/legacycfg.c
>>> +++ b/grub-core/commands/legacycfg.c
>>> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
>> __attribute__ ((unused)),
>>>  #endif
>>>                          );
>>>
>>> -      return cmd->func (cmd, argc, args);
>>> +      return cmd->func (cmd, argc ? argc - 1 : 0, args + 1);
>>
>> If the goal is to be compatible with legacy grub, it is not compatible
>> (nor was) - legacy grub "initrd" ignored all extra arguments to "initrd"
>> command while we attempt to load them as additional initrd components.
>>
>> Nor did legacy grub attempt to quote command line for multiboot kernel
>> or modules.
>>
>> Agreed on both. That's why I said that previous patch is wrong and I'm
>> working on new one
>>
> For first point, see attached patch.
> Second point isn't a problem as legacycfg will already split along spaces
> and hence quoting code will not be triggered.
> 

It will.

      while (*c)
        {
          if (*c == '\\' || *c == '\'' || *c == '"')
            *buf++ = '\\';
          *buf++ = *c;
          c++;
        }

In particular, this is a problem with Linux which does not interpret
these quotes at all (it only supports " .... " without any way to escape
nested `"'). That's something for post 2.02 now, but I still would like
to know what was the reason for this code in the first place.

> 
> diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
> index dd9d9f1..b32f3c7 100644
> --- a/grub-core/commands/legacycfg.c
> +++ b/grub-core/commands/legacycfg.c
> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
> __attribute__ ((unused)),
> #endif
> );
> 
> - return cmd->func (cmd, argc, args);
> + return cmd->func (cmd, argc ? 1 : 0, args);
> }
> if (kernel_type == MULTIBOOT)
> {
> 

Looks OK as stop gap. @Andy, could you test it?	

I still do not understand why initrd is parsed as TYPE_FILE_NO_CONSUME,
nor why we handle multiboot kernel in legacy_initrd in the first place.
Legacy grub only supported Linux kernel with initrd command.

  switch (kernel_type)
    {
    case KERNEL_TYPE_LINUX:
    case KERNEL_TYPE_BIG_LINUX:
      if (! load_initrd (arg))
        return 1;
      break;

    default:
      errnum = ERR_NEED_LX_KERNEL;
      return 1;
    }

Although this probably does not really matter now.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: pvgrub2 legacy_configfile issues: /sbin/init exec error -8
  2017-02-25  5:27                 ` Andrei Borzenkov
@ 2017-02-25 12:19                   ` Andy Smith
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Smith @ 2017-02-25 12:19 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: arvidjaar

Hi Andrei, Vladimir,

On Sat, Feb 25, 2017 at 08:27:55AM +0300, Andrei Borzenkov wrote:
> 25.02.2017 02:22, Vladimir 'phcoder' Serbinenko пишет:
> > diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c
> > index dd9d9f1..b32f3c7 100644
> > --- a/grub-core/commands/legacycfg.c
> > +++ b/grub-core/commands/legacycfg.c
> > @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd
> > __attribute__ ((unused)),
> > #endif
> > );
> > 
> > - return cmd->func (cmd, argc, args);
> > + return cmd->func (cmd, argc ? 1 : 0, args);
> > }
> > if (kernel_type == MULTIBOOT)
> > {
> > 
> 
> Looks OK as stop gap. @Andy, could you test it?	

Yep, this works also.

Cheers,
Andy


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-02-25 12:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-11 13:53 pvgrub2 legacy_configfile issues: /sbin/init exec error -8 Andy Smith
2017-02-11 14:46 ` Andrei Borzenkov
2017-02-11 15:26   ` Andy Smith
2017-02-11 17:19     ` Andy Smith
2017-02-11 18:45       ` Andrei Borzenkov
2017-02-11 21:07         ` Vladimir 'phcoder' Serbinenko
2017-02-11 23:36           ` Vladimir 'phcoder' Serbinenko
2017-02-12  7:06           ` Andrei Borzenkov
2017-02-12 10:52             ` Vladimir 'phcoder' Serbinenko
2017-02-24 23:22               ` Vladimir 'phcoder' Serbinenko
2017-02-25  5:27                 ` Andrei Borzenkov
2017-02-25 12:19                   ` Andy Smith

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.