Re: pvgrub2 and old grub menu.lst

[Andrei Borzenkov]

В Thu, 3 Apr 2014 14:50:02 +0800
Michael Chang <address@hidden> пишет:

> 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.
> 

Finally pushed.

> 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)