All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrey Borzenkov <arvidjaar@gmail.com>
To: Andrey Borzenkov <arvidjaar@gmail.com>
Cc: The development of GNU GRUB <grub-devel@gnu.org>,
	Michael Chang <mchang@suse.com>
Subject: Re: pvgrub2 and old grub menu.lst
Date: Wed, 2 Apr 2014 20:13:14 +0400	[thread overview]
Message-ID: <20140402201314.11f396b2@opensuse.site> (raw)
In-Reply-To: <CAA91j0U3P+EO0ihHTWdMyO9=rG3AEgCBWDLs74V7-u0eFFsHag@mail.gmail.com>

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


  reply	other threads:[~2014-04-02 16:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-01  7:46 pvgrub2 and old grub menu.lst Michael Chang
2014-04-01 16:06 ` Andrey Borzenkov
2014-04-02  8:43   ` Michael Chang
2014-04-02 10:29     ` Andrey Borzenkov
2014-04-02 16:13       ` Andrey Borzenkov [this message]
2014-04-03  6:50         ` Michael Chang
2014-04-03  8:25           ` Andrey Borzenkov
2014-04-03 10:23             ` Michael Chang
2015-01-24  5:53           ` Andrei Borzenkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140402201314.11f396b2@opensuse.site \
    --to=arvidjaar@gmail.com \
    --cc=grub-devel@gnu.org \
    --cc=mchang@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.