pvgrub2 and old grub menu.lst

pvgrub2 and old grub menu.lst

[Michael Chang]

While trying to boot xen pv guest with old grub config (menu.lst),
the pvgrub2 loads it's menu successfully, but when you attemp to boot
the kernel it failed with the following message. 

  error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
  error: you need to load the kernel first.

  Press any key to continue..."

The kernel is there and boots fine with pygrub.

After some investigation, it boils down to the menu.lst has explicitly
set it's root device to (hd0,1) and therefore the root device is not
recognized by pvgrub2.

title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
  root (hd0,1)
  kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen

I'd like to know whether old grub configuration is considered or planned
to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
valid xen disk name because it's quite common in some existing configs ?

Thanks,
Michael

Re: pvgrub2 and old grub menu.lst

[Andrey Borzenkov]

В Tue, 1 Apr 2014 15:46:22 +0800
Michael Chang <mchang@suse.com> пишет:

> While trying to boot xen pv guest with old grub config (menu.lst),
> the pvgrub2 loads it's menu successfully, but when you attemp to boot
> the kernel it failed with the following message. 
> 
>   error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
>   error: you need to load the kernel first.
> 
>   Press any key to continue..."
> 
> The kernel is there and boots fine with pygrub.
> 
> After some investigation, it boils down to the menu.lst has explicitly
> set it's root device to (hd0,1) and therefore the root device is not
> recognized by pvgrub2.
> 
> title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
>   root (hd0,1)
>   kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
> splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen
> 
> I'd like to know whether old grub configuration is considered or planned
> to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
> valid xen disk name because it's quite common in some existing configs ?
> 

I believe this technically should work; but this needs someone who
knows how Xen disks is mapped to hd number in legacy grub.

It would be something like (completely untested)

From: Andrey Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] accept also hdX as alias to native Xen disk name

This allows reusing legacy pvgrub menu.lst which is using hdX names.

Suggested-By: Michael Chang <mchang@suse.com>

---
 grub-core/disk/xen/xendisk.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/grub-core/disk/xen/xendisk.c b/grub-core/disk/xen/xendisk.c
index 2b11c2a..c6c8ecc 100644
--- a/grub-core/disk/xen/xendisk.c
+++ b/grub-core/disk/xen/xendisk.c
@@ -70,9 +70,20 @@ grub_virtdisk_open (const char *name, grub_disk_t disk)
   grub_uint32_t secsize;
   char fdir[200];
   char *buf;
+  int num = -1;
 
+  if (name[0] == 'h' && name[1] == 'd')
+    {
+      num = grub_strtoul (hd+2, 0, 10);
+      if (grub_errno)
+	{
+	  grub_errno = 0;
+	  num = -1;
+	}
+    }
   for (i = 0; i < vdiskcnt; i++)
-    if (grub_strcmp (name, virtdisks[i].fullname) == 0)
+    if ((num != -1 && virtdisks[i].handle == num) ||
+	 grub_strcmp (name, virtdisks[i].fullname) == 0)
       break;
   if (i == vdiskcnt)
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a virtdisk");
-- 
tg: (4afd010..) e/xen/hdX-compat (depends on: master)

Re: pvgrub2 and old grub menu.lst

[Michael Chang]

On Tue, Apr 01, 2014 at 08:06:44PM +0400, Andrey Borzenkov wrote:
> В Tue, 1 Apr 2014 15:46:22 +0800
> Michael Chang <mchang@suse.com> пишет:
> 
> > While trying to boot xen pv guest with old grub config (menu.lst),
> > the pvgrub2 loads it's menu successfully, but when you attemp to boot
> > the kernel it failed with the following message. 
> > 
> >   error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
> >   error: you need to load the kernel first.
> > 
> >   Press any key to continue..."
> > 
> > The kernel is there and boots fine with pygrub.
> > 
> > After some investigation, it boils down to the menu.lst has explicitly
> > set it's root device to (hd0,1) and therefore the root device is not
> > recognized by pvgrub2.
> > 
> > title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
> >   root (hd0,1)
> >   kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
> > splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen
> > 
> > I'd like to know whether old grub configuration is considered or planned
> > to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
> > valid xen disk name because it's quite common in some existing configs ?
> > 
> 
> I believe this technically should work; but this needs someone who
> knows how Xen disks is mapped to hd number in legacy grub.
> 
> It would be something like (completely untested)

Thank you for the patch. I applied it and tried to test it. The
virtdisks[i].handle never matched with num as it's id of the disk (on
my system it is 51712).

Please pardon my ignorant, can num be the index to the virtdisks array
to get the mapped xen disk ?

Thanks,
Michael

> 
> From: Andrey Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] accept also hdX as alias to native Xen disk name
> 
> This allows reusing legacy pvgrub menu.lst which is using hdX names.
> 
> Suggested-By: Michael Chang <mchang@suse.com>
> 
> ---
>  grub-core/disk/xen/xendisk.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/disk/xen/xendisk.c b/grub-core/disk/xen/xendisk.c
> index 2b11c2a..c6c8ecc 100644
> --- a/grub-core/disk/xen/xendisk.c
> +++ b/grub-core/disk/xen/xendisk.c
> @@ -70,9 +70,20 @@ grub_virtdisk_open (const char *name, grub_disk_t disk)
>    grub_uint32_t secsize;
>    char fdir[200];
>    char *buf;
> +  int num = -1;
>  
> +  if (name[0] == 'h' && name[1] == 'd')
> +    {
> +      num = grub_strtoul (hd+2, 0, 10);
> +      if (grub_errno)
> +	{
> +	  grub_errno = 0;
> +	  num = -1;
> +	}
> +    }
>    for (i = 0; i < vdiskcnt; i++)
> -    if (grub_strcmp (name, virtdisks[i].fullname) == 0)
> +    if ((num != -1 && virtdisks[i].handle == num) ||
> +	 grub_strcmp (name, virtdisks[i].fullname) == 0)
>        break;
>    if (i == vdiskcnt)
>      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a virtdisk");
> -- 
> tg: (4afd010..) e/xen/hdX-compat (depends on: master)

Re: pvgrub2 and old grub menu.lst

[Andrey Borzenkov]

On Wed, Apr 2, 2014 at 12:43 PM, Michael Chang <mchang@suse.com> wrote:
> On Tue, Apr 01, 2014 at 08:06:44PM +0400, Andrey Borzenkov wrote:
>> В Tue, 1 Apr 2014 15:46:22 +0800
>> Michael Chang <mchang@suse.com> пишет:
>>
>> > While trying to boot xen pv guest with old grub config (menu.lst),
>> > the pvgrub2 loads it's menu successfully, but when you attemp to boot
>> > the kernel it failed with the following message.
>> >
>> >   error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
>> >   error: you need to load the kernel first.
>> >
>> >   Press any key to continue..."
>> >
>> > The kernel is there and boots fine with pygrub.
>> >
>> > After some investigation, it boils down to the menu.lst has explicitly
>> > set it's root device to (hd0,1) and therefore the root device is not
>> > recognized by pvgrub2.
>> >
>> > title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
>> >   root (hd0,1)
>> >   kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
>> > splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen
>> >
>> > I'd like to know whether old grub configuration is considered or planned
>> > to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
>> > valid xen disk name because it's quite common in some existing configs ?
>> >
>>
>> I believe this technically should work; but this needs someone who
>> knows how Xen disks is mapped to hd number in legacy grub.
>>
>> It would be something like (completely untested)
>
> Thank you for the patch. I applied it and tried to test it. The
> virtdisks[i].handle never matched with num as it's id of the disk (on
> my system it is 51712).
>

As I said, completely untested :)

> Please pardon my ignorant, can num be the index to the virtdisks array
> to get the mapped xen disk ?
>

pv-grub additionally sorts by handle:
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=stubdom/grub/mini-os.c;h=9d4bcc76d5f3e9907889948ee0dcea32e19c185b;hb=HEAD#l73

So to actually map pv-grub hdX we need to do the same.

Re: pvgrub2 and old grub menu.lst

[Andrey Borzenkov]

В Wed, 2 Apr 2014 14:29:50 +0400
Andrey Borzenkov <arvidjaar@gmail.com> пишет:

> On Wed, Apr 2, 2014 at 12:43 PM, Michael Chang <mchang@suse.com> wrote:
> > On Tue, Apr 01, 2014 at 08:06:44PM +0400, Andrey Borzenkov wrote:
> >> В Tue, 1 Apr 2014 15:46:22 +0800
> >> Michael Chang <mchang@suse.com> пишет:
> >>
> >> > While trying to boot xen pv guest with old grub config (menu.lst),
> >> > the pvgrub2 loads it's menu successfully, but when you attemp to boot
> >> > the kernel it failed with the following message.
> >> >
> >> >   error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
> >> >   error: you need to load the kernel first.
> >> >
> >> >   Press any key to continue..."
> >> >
> >> > The kernel is there and boots fine with pygrub.
> >> >
> >> > After some investigation, it boils down to the menu.lst has explicitly
> >> > set it's root device to (hd0,1) and therefore the root device is not
> >> > recognized by pvgrub2.
> >> >
> >> > title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
> >> >   root (hd0,1)
> >> >   kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
> >> > splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen
> >> >
> >> > I'd like to know whether old grub configuration is considered or planned
> >> > to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
> >> > valid xen disk name because it's quite common in some existing configs ?
> >> >
> >>
> >> I believe this technically should work; but this needs someone who
> >> knows how Xen disks is mapped to hd number in legacy grub.
> >>
> >> It would be something like (completely untested)
> >
> > Thank you for the patch. I applied it and tried to test it. The
> > virtdisks[i].handle never matched with num as it's id of the disk (on
> > my system it is 51712).
> >
> 
> As I said, completely untested :)
> 
> > Please pardon my ignorant, can num be the index to the virtdisks array
> > to get the mapped xen disk ?
> >
> 
> pv-grub additionally sorts by handle:
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=stubdom/grub/mini-os.c;h=9d4bcc76d5f3e9907889948ee0dcea32e19c185b;hb=HEAD#l73
> 
> So to actually map pv-grub hdX we need to do the same.

Could you test patch below (compile-tested)?

From: Andrey Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] accept also hdX as alias to native Xen disk name

To assign correct disk numbers, sort disks by increasing order of handle
value.  This allows reusing legacy pv-grub menu.lst which is using hdX names.

Suggested-By: Michael Chang <mchang@suse.com>

---
 grub-core/disk/xen/xendisk.c | 45 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/grub-core/disk/xen/xendisk.c b/grub-core/disk/xen/xendisk.c
index 2b11c2a..b18a923 100644
--- a/grub-core/disk/xen/xendisk.c
+++ b/grub-core/disk/xen/xendisk.c
@@ -40,6 +40,7 @@ struct virtdisk
   grub_xen_evtchn_t evtchn;
   void *dma_page;
   grub_xen_grant_t dma_grant;
+  struct virtdisk *compat_next;
 };
 
 #define xen_wmb() mb()
@@ -47,6 +48,7 @@ struct virtdisk
 
 static struct virtdisk *virtdisks;
 static grub_size_t vdiskcnt;
+struct virtdisk *compat_head;
 
 static int
 grub_virtdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
@@ -66,20 +68,32 @@ grub_virtdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
 static grub_err_t
 grub_virtdisk_open (const char *name, grub_disk_t disk)
 {
-  grub_size_t i;
+  int i;
   grub_uint32_t secsize;
   char fdir[200];
   char *buf;
+  int num = -1;
+  struct virtdisk *vd;
 
-  for (i = 0; i < vdiskcnt; i++)
-    if (grub_strcmp (name, virtdisks[i].fullname) == 0)
+  /* For compatibility with pv-grub legacy menu.lst accept hdX as disk name */
+  if (name[0] == 'h' && name[1] == 'd' && name[2])
+    {
+      num = grub_strtoul (name + 2, 0, 10);
+      if (grub_errno)
+	{
+	  grub_errno = 0;
+	  num = -1;
+	}
+    }
+  for (i = 0, vd = compat_head; vd; vd = vd->compat_next, i++)
+    if (i == num || grub_strcmp (name, vd->fullname) == 0)
       break;
-  if (i == vdiskcnt)
+  if (!vd)
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a virtdisk");
-  disk->data = &virtdisks[i];
-  disk->id = i;
+  disk->data = vd;
+  disk->id = vd - virtdisks;
 
-  grub_snprintf (fdir, sizeof (fdir), "%s/sectors", virtdisks[i].backend_dir);
+  grub_snprintf (fdir, sizeof (fdir), "%s/sectors", vd->backend_dir);
   buf = grub_xenstore_get_file (fdir, NULL);
   if (!buf)
     return grub_errno;
@@ -87,8 +101,7 @@ grub_virtdisk_open (const char *name, grub_disk_t disk)
   if (grub_errno)
     return grub_errno;
 
-  grub_snprintf (fdir, sizeof (fdir), "%s/sector-size",
-		 virtdisks[i].backend_dir);
+  grub_snprintf (fdir, sizeof (fdir), "%s/sector-size", vd->backend_dir);
   buf = grub_xenstore_get_file (fdir, NULL);
   if (!buf)
     return grub_errno;
@@ -264,6 +277,7 @@ fill (const char *dir, void *data)
   grub_err_t err;
   void *buf;
   struct evtchn_alloc_unbound alloc_unbound;
+  struct virtdisk **prev = &compat_head, *vd = compat_head;
 
   /* Shouldn't happen unles some hotplug happened.  */
   if (vdiskcnt >= *ctr)
@@ -374,6 +388,19 @@ fill (const char *dir, void *data)
 
   virtdisks[vdiskcnt].frontend_dir = grub_strdup (fdir);
 
+  /* For compatibility with pv-grub maintain linked list sorted by handle
+     value in increasing order. This allows mapping of (hdX) disk names
+     from legacy menu.lst */
+  while (vd)
+    {
+      if (vd->handle > virtdisks[vdiskcnt].handle)
+	break;
+      prev = &vd->compat_next;
+      vd = vd->compat_next;
+    }
+  virtdisks[vdiskcnt].compat_next = vd;
+  *prev = &virtdisks[vdiskcnt];
+
   vdiskcnt++;
   return 0;
 
-- 
tg: (4afd010..) e/xen/hdX-compat (depends on: master)

Re: pvgrub2 and old grub menu.lst

[Michael Chang]

On Wed, Apr 02, 2014 at 08:13:14PM +0400, Andrey Borzenkov wrote:
> В Wed, 2 Apr 2014 14:29:50 +0400
> Andrey Borzenkov <arvidjaar@gmail.com> пишет:
> 
> > On Wed, Apr 2, 2014 at 12:43 PM, Michael Chang <mchang@suse.com> wrote:
> > > On Tue, Apr 01, 2014 at 08:06:44PM +0400, Andrey Borzenkov wrote:
> > >> В Tue, 1 Apr 2014 15:46:22 +0800
> > >> Michael Chang <mchang@suse.com> пишет:
> > >>
> > >> > While trying to boot xen pv guest with old grub config (menu.lst),
> > >> > the pvgrub2 loads it's menu successfully, but when you attemp to boot
> > >> > the kernel it failed with the following message.
> > >> >
> > >> >   error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
> > >> >   error: you need to load the kernel first.
> > >> >
> > >> >   Press any key to continue..."
> > >> >
> > >> > The kernel is there and boots fine with pygrub.
> > >> >
> > >> > After some investigation, it boils down to the menu.lst has explicitly
> > >> > set it's root device to (hd0,1) and therefore the root device is not
> > >> > recognized by pvgrub2.
> > >> >
> > >> > title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
> > >> >   root (hd0,1)
> > >> >   kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
> > >> > splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen
> > >> >
> > >> > I'd like to know whether old grub configuration is considered or planned
> > >> > to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
> > >> > valid xen disk name because it's quite common in some existing configs ?
> > >> >
> > >>
> > >> I believe this technically should work; but this needs someone who
> > >> knows how Xen disks is mapped to hd number in legacy grub.
> > >>
> > >> It would be something like (completely untested)
> > >
> > > Thank you for the patch. I applied it and tried to test it. The
> > > virtdisks[i].handle never matched with num as it's id of the disk (on
> > > my system it is 51712).
> > >
> > 
> > As I said, completely untested :)
> > 
> > > Please pardon my ignorant, can num be the index to the virtdisks array
> > > to get the mapped xen disk ?
> > >
> > 
> > pv-grub additionally sorts by handle:
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=stubdom/grub/mini-os.c;h=9d4bcc76d5f3e9907889948ee0dcea32e19c185b;hb=HEAD#l73
> > 
> > So to actually map pv-grub hdX we need to do the same.
> 
> Could you test patch below (compile-tested)?

This patch works very well for me. It can boot into system using legacy
menu.lst.

I also setup two disks for testing, and set root=hdX,Y to list the
file in repective disk correct. Then swapping the disk order in xen
config and again the result is correct.

FWIW here my acknowledge for my positive test result.

Tested-By: Michael Chang <mchang@suse.com>.

Feel free to let me know anything I can help in this topic, and really
appreciate for your work and time on this.

Thanks,
Michael

> 
> From: Andrey Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] accept also hdX as alias to native Xen disk name
> 
> To assign correct disk numbers, sort disks by increasing order of handle
> value.  This allows reusing legacy pv-grub menu.lst which is using hdX names.
> 
> Suggested-By: Michael Chang <mchang@suse.com>
> 
> ---
>  grub-core/disk/xen/xendisk.c | 45 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/grub-core/disk/xen/xendisk.c b/grub-core/disk/xen/xendisk.c
> index 2b11c2a..b18a923 100644
> --- a/grub-core/disk/xen/xendisk.c
> +++ b/grub-core/disk/xen/xendisk.c
> @@ -40,6 +40,7 @@ struct virtdisk
>    grub_xen_evtchn_t evtchn;
>    void *dma_page;
>    grub_xen_grant_t dma_grant;
> +  struct virtdisk *compat_next;
>  };
>  
>  #define xen_wmb() mb()
> @@ -47,6 +48,7 @@ struct virtdisk
>  
>  static struct virtdisk *virtdisks;
>  static grub_size_t vdiskcnt;
> +struct virtdisk *compat_head;
>  
>  static int
>  grub_virtdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
> @@ -66,20 +68,32 @@ grub_virtdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
>  static grub_err_t
>  grub_virtdisk_open (const char *name, grub_disk_t disk)
>  {
> -  grub_size_t i;
> +  int i;
>    grub_uint32_t secsize;
>    char fdir[200];
>    char *buf;
> +  int num = -1;
> +  struct virtdisk *vd;
>  
> -  for (i = 0; i < vdiskcnt; i++)
> -    if (grub_strcmp (name, virtdisks[i].fullname) == 0)
> +  /* For compatibility with pv-grub legacy menu.lst accept hdX as disk name */
> +  if (name[0] == 'h' && name[1] == 'd' && name[2])
> +    {
> +      num = grub_strtoul (name + 2, 0, 10);
> +      if (grub_errno)
> +	{
> +	  grub_errno = 0;
> +	  num = -1;
> +	}
> +    }
> +  for (i = 0, vd = compat_head; vd; vd = vd->compat_next, i++)
> +    if (i == num || grub_strcmp (name, vd->fullname) == 0)
>        break;
> -  if (i == vdiskcnt)
> +  if (!vd)
>      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a virtdisk");
> -  disk->data = &virtdisks[i];
> -  disk->id = i;
> +  disk->data = vd;
> +  disk->id = vd - virtdisks;
>  
> -  grub_snprintf (fdir, sizeof (fdir), "%s/sectors", virtdisks[i].backend_dir);
> +  grub_snprintf (fdir, sizeof (fdir), "%s/sectors", vd->backend_dir);
>    buf = grub_xenstore_get_file (fdir, NULL);
>    if (!buf)
>      return grub_errno;
> @@ -87,8 +101,7 @@ grub_virtdisk_open (const char *name, grub_disk_t disk)
>    if (grub_errno)
>      return grub_errno;
>  
> -  grub_snprintf (fdir, sizeof (fdir), "%s/sector-size",
> -		 virtdisks[i].backend_dir);
> +  grub_snprintf (fdir, sizeof (fdir), "%s/sector-size", vd->backend_dir);
>    buf = grub_xenstore_get_file (fdir, NULL);
>    if (!buf)
>      return grub_errno;
> @@ -264,6 +277,7 @@ fill (const char *dir, void *data)
>    grub_err_t err;
>    void *buf;
>    struct evtchn_alloc_unbound alloc_unbound;
> +  struct virtdisk **prev = &compat_head, *vd = compat_head;
>  
>    /* Shouldn't happen unles some hotplug happened.  */
>    if (vdiskcnt >= *ctr)
> @@ -374,6 +388,19 @@ fill (const char *dir, void *data)
>  
>    virtdisks[vdiskcnt].frontend_dir = grub_strdup (fdir);
>  
> +  /* For compatibility with pv-grub maintain linked list sorted by handle
> +     value in increasing order. This allows mapping of (hdX) disk names
> +     from legacy menu.lst */
> +  while (vd)
> +    {
> +      if (vd->handle > virtdisks[vdiskcnt].handle)
> +	break;
> +      prev = &vd->compat_next;
> +      vd = vd->compat_next;
> +    }
> +  virtdisks[vdiskcnt].compat_next = vd;
> +  *prev = &virtdisks[vdiskcnt];
> +
>    vdiskcnt++;
>    return 0;
>  
> -- 
> tg: (4afd010..) e/xen/hdX-compat (depends on: master)

Re: pvgrub2 and old grub menu.lst

[Andrey Borzenkov]

On Thu, Apr 3, 2014 at 10:50 AM, Michael Chang <mchang@suse.com> wrote:
>
> I also setup two disks for testing, and set root=hdX,Y to list the
> file in repective disk correct. Then swapping the disk order in xen
> config and again the result is correct.
>

Did you compare disk order in pv-grub and grub2?

Re: pvgrub2 and old grub menu.lst

[Michael Chang]

On Thu, Apr 03, 2014 at 12:25:19PM +0400, Andrey Borzenkov wrote:
> On Thu, Apr 3, 2014 at 10:50 AM, Michael Chang <mchang@suse.com> wrote:
> >
> > I also setup two disks for testing, and set root=hdX,Y to list the
> > file in repective disk correct. Then swapping the disk order in xen
> > config and again the result is correct.
> >
> 
> Did you compare disk order in pv-grub and grub2?

No.
If you think it necessay I can test. But from the user experience I
didn't feel anything wrong so far.

Thanks,
Michael

Re: pvgrub2 and old grub menu.lst

[Andrei Borzenkov]

В Thu, 3 Apr 2014 14:50:02 +0800
Michael Chang <mchang@suse.com> пишет:

> On Wed, Apr 02, 2014 at 08:13:14PM +0400, Andrey Borzenkov wrote:
> > В Wed, 2 Apr 2014 14:29:50 +0400
> > Andrey Borzenkov <arvidjaar@gmail.com> пишет:
> > 
> > > On Wed, Apr 2, 2014 at 12:43 PM, Michael Chang <mchang@suse.com> wrote:
> > > > On Tue, Apr 01, 2014 at 08:06:44PM +0400, Andrey Borzenkov wrote:
> > > >> В Tue, 1 Apr 2014 15:46:22 +0800
> > > >> Michael Chang <mchang@suse.com> пишет:
> > > >>
> > > >> > While trying to boot xen pv guest with old grub config (menu.lst),
> > > >> > the pvgrub2 loads it's menu successfully, but when you attemp to boot
> > > >> > the kernel it failed with the following message.
> > > >> >
> > > >> >   error: couldn't load file /boot/vmlinuz-3.0.76-0.11-xen.
> > > >> >   error: you need to load the kernel first.
> > > >> >
> > > >> >   Press any key to continue..."
> > > >> >
> > > >> > The kernel is there and boots fine with pygrub.
> > > >> >
> > > >> > After some investigation, it boils down to the menu.lst has explicitly
> > > >> > set it's root device to (hd0,1) and therefore the root device is not
> > > >> > recognized by pvgrub2.
> > > >> >
> > > >> > title Xen -- SUSE Linux Enterprise Server 11 SP3 - 3.0.76-0.11
> > > >> >   root (hd0,1)
> > > >> >   kernel /boot/vmlinuz-3.0.76-0.11-xen root=/dev/xvda2 resume=/dev/xvda1
> > > >> > splash=silent showopts initrd /boot/initrd-3.0.76-0.11.xen
> > > >> >
> > > >> > I'd like to know whether old grub configuration is considered or planned
> > > >> > to work with pvgrub2 ? And is it a viable suggestion to have (hdX,Y) a
> > > >> > valid xen disk name because it's quite common in some existing configs ?
> > > >> >
> > > >>
> > > >> I believe this technically should work; but this needs someone who
> > > >> knows how Xen disks is mapped to hd number in legacy grub.
> > > >>
> > > >> It would be something like (completely untested)
> > > >
> > > > Thank you for the patch. I applied it and tried to test it. The
> > > > virtdisks[i].handle never matched with num as it's id of the disk (on
> > > > my system it is 51712).
> > > >
> > > 
> > > As I said, completely untested :)
> > > 
> > > > Please pardon my ignorant, can num be the index to the virtdisks array
> > > > to get the mapped xen disk ?
> > > >
> > > 
> > > pv-grub additionally sorts by handle:
> > > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=stubdom/grub/mini-os.c;h=9d4bcc76d5f3e9907889948ee0dcea32e19c185b;hb=HEAD#l73
> > > 
> > > So to actually map pv-grub hdX we need to do the same.
> > 
> > Could you test patch below (compile-tested)?
> 
> This patch works very well for me. It can boot into system using legacy
> menu.lst.
> 
> I also setup two disks for testing, and set root=hdX,Y to list the
> file in repective disk correct. Then swapping the disk order in xen
> config and again the result is correct.
> 
> FWIW here my acknowledge for my positive test result.
> 
> Tested-By: Michael Chang <mchang@suse.com>.
> 
> Feel free to let me know anything I can help in this topic, and really
> appreciate for your work and time on this.
> 

Finally pushed.

> Thanks,
> Michael
> 
> > 
> > From: Andrey Borzenkov <arvidjaar@gmail.com>
> > Subject: [PATCH] accept also hdX as alias to native Xen disk name
> > 
> > To assign correct disk numbers, sort disks by increasing order of handle
> > value.  This allows reusing legacy pv-grub menu.lst which is using hdX names.
> > 
> > Suggested-By: Michael Chang <mchang@suse.com>
> > 
> > ---
> >  grub-core/disk/xen/xendisk.c | 45 +++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 36 insertions(+), 9 deletions(-)
> > 
> > diff --git a/grub-core/disk/xen/xendisk.c b/grub-core/disk/xen/xendisk.c
> > index 2b11c2a..b18a923 100644
> > --- a/grub-core/disk/xen/xendisk.c
> > +++ b/grub-core/disk/xen/xendisk.c
> > @@ -40,6 +40,7 @@ struct virtdisk
> >    grub_xen_evtchn_t evtchn;
> >    void *dma_page;
> >    grub_xen_grant_t dma_grant;
> > +  struct virtdisk *compat_next;
> >  };
> >  
> >  #define xen_wmb() mb()
> > @@ -47,6 +48,7 @@ struct virtdisk
> >  
> >  static struct virtdisk *virtdisks;
> >  static grub_size_t vdiskcnt;
> > +struct virtdisk *compat_head;
> >  
> >  static int
> >  grub_virtdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
> > @@ -66,20 +68,32 @@ grub_virtdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
> >  static grub_err_t
> >  grub_virtdisk_open (const char *name, grub_disk_t disk)
> >  {
> > -  grub_size_t i;
> > +  int i;
> >    grub_uint32_t secsize;
> >    char fdir[200];
> >    char *buf;
> > +  int num = -1;
> > +  struct virtdisk *vd;
> >  
> > -  for (i = 0; i < vdiskcnt; i++)
> > -    if (grub_strcmp (name, virtdisks[i].fullname) == 0)
> > +  /* For compatibility with pv-grub legacy menu.lst accept hdX as disk name */
> > +  if (name[0] == 'h' && name[1] == 'd' && name[2])
> > +    {
> > +      num = grub_strtoul (name + 2, 0, 10);
> > +      if (grub_errno)
> > +	{
> > +	  grub_errno = 0;
> > +	  num = -1;
> > +	}
> > +    }
> > +  for (i = 0, vd = compat_head; vd; vd = vd->compat_next, i++)
> > +    if (i == num || grub_strcmp (name, vd->fullname) == 0)
> >        break;
> > -  if (i == vdiskcnt)
> > +  if (!vd)
> >      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a virtdisk");
> > -  disk->data = &virtdisks[i];
> > -  disk->id = i;
> > +  disk->data = vd;
> > +  disk->id = vd - virtdisks;
> >  
> > -  grub_snprintf (fdir, sizeof (fdir), "%s/sectors", virtdisks[i].backend_dir);
> > +  grub_snprintf (fdir, sizeof (fdir), "%s/sectors", vd->backend_dir);
> >    buf = grub_xenstore_get_file (fdir, NULL);
> >    if (!buf)
> >      return grub_errno;
> > @@ -87,8 +101,7 @@ grub_virtdisk_open (const char *name, grub_disk_t disk)
> >    if (grub_errno)
> >      return grub_errno;
> >  
> > -  grub_snprintf (fdir, sizeof (fdir), "%s/sector-size",
> > -		 virtdisks[i].backend_dir);
> > +  grub_snprintf (fdir, sizeof (fdir), "%s/sector-size", vd->backend_dir);
> >    buf = grub_xenstore_get_file (fdir, NULL);
> >    if (!buf)
> >      return grub_errno;
> > @@ -264,6 +277,7 @@ fill (const char *dir, void *data)
> >    grub_err_t err;
> >    void *buf;
> >    struct evtchn_alloc_unbound alloc_unbound;
> > +  struct virtdisk **prev = &compat_head, *vd = compat_head;
> >  
> >    /* Shouldn't happen unles some hotplug happened.  */
> >    if (vdiskcnt >= *ctr)
> > @@ -374,6 +388,19 @@ fill (const char *dir, void *data)
> >  
> >    virtdisks[vdiskcnt].frontend_dir = grub_strdup (fdir);
> >  
> > +  /* For compatibility with pv-grub maintain linked list sorted by handle
> > +     value in increasing order. This allows mapping of (hdX) disk names
> > +     from legacy menu.lst */
> > +  while (vd)
> > +    {
> > +      if (vd->handle > virtdisks[vdiskcnt].handle)
> > +	break;
> > +      prev = &vd->compat_next;
> > +      vd = vd->compat_next;
> > +    }
> > +  virtdisks[vdiskcnt].compat_next = vd;
> > +  *prev = &virtdisks[vdiskcnt];
> > +
> >    vdiskcnt++;
> >    return 0;
> >  
> > -- 
> > tg: (4afd010..) e/xen/hdX-compat (depends on: master)