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 <address@hidden> пишет:
> 
> > On Wed, Apr 2, 2014 at 12:43 PM, Michael Chang <address@hidden> wrote:
> > > On Tue, Apr 01, 2014 at 08:06:44PM +0400, Andrey Borzenkov wrote:
> > >> В Tue, 1 Apr 2014 15:46:22 +0800
> > >> Michael Chang <address@hidden> пишет:
> > >>
> > >> > 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 <address@hidden>.

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 <address@hidden>
> 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 <address@hidden>
> 
> ---
>  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)