Restructure SCSI .id handling.

Reported and tested by: Aleš Nesrsta.

	* disk/ata.c (grub_atapi_close): Removed. All users updated.
	(grub_atapi_dev): Changed .name to "ata". New field .id.
	* disk/usbms.c (grub_usbms_close): Removed. All users updated.
	(grub_usbms_dev): New field .id.
	* disk/scsi.c (grub_scsi_iterate): Generate name.
	(grub_scsi_open): Parse name.
	* include/grub/scsi.h (grub_make_scsi_id): New function.
	(grub_scsi_dev): Change iterate and open to number instead of naming
	busses. All users updated.
	(grub_scsi): Remove name. Add .bus.

ChangeLog

@@ -1,3 +1,19 @@
+2010-07-10  Vladimir Serbinenko  <phcoder@gmail.com>
+
+	Restructure SCSI .id handling.
+	Reported and tested by: Aleš Nesrsta.
+
+	* disk/ata.c (grub_atapi_close): Removed. All users updated.
+	(grub_atapi_dev): Changed .name to "ata". New field .id.
+	* disk/usbms.c (grub_usbms_close): Removed. All users updated.
+	(grub_usbms_dev): New field .id.
+	* disk/scsi.c (grub_scsi_iterate): Generate name.
+	(grub_scsi_open): Parse name.
+	* include/grub/scsi.h (grub_make_scsi_id): New function.
+	(grub_scsi_dev): Change iterate and open to number instead of naming
+	busses. All users updated.
+	(grub_scsi): Remove name. Add .bus.
+
 2010-07-10  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	* commands/help.c (grub_cmd_help): Fix a typo.

disk/ata.c

@@ -768,14 +768,12 @@ static struct grub_disk_dev grub_atadisk_dev =
 /* ATAPI code.  */
 
 static int
-grub_atapi_iterate (int (*hook) (const char *name, int luns))
+grub_atapi_iterate (int (*hook) (int bus, int luns))
 {
   struct grub_ata_device *dev;
 
   for (dev = grub_ata_devices; dev; dev = dev->next)
     {
-      char devname[10];
-
       grub_err_t err;
 
       err = check_device (dev);
@@ -785,13 +783,10 @@ grub_atapi_iterate (int (*hook) (const char *name, int luns))
 	  continue;
 	}
 
-      grub_snprintf (devname, sizeof (devname),
-		     "ata%d", dev->port * 2 + dev->device);
-
       if (! dev->atapi)
 	continue;
 
-      if (hook (devname, 1))
+      if (hook (dev->port * 2 + dev->device, 1))
 	return 1;
     }
 
@@ -851,7 +846,7 @@ grub_atapi_write (struct grub_scsi *scsi __attribute__((unused)),
 }
 
 static grub_err_t
-grub_atapi_open (const char *name, struct grub_scsi *scsi)
+grub_atapi_open (int devnum, struct grub_scsi *scsi)
 {
   struct grub_ata_device *dev;
   struct grub_ata_device *devfnd = 0;
@@ -859,18 +854,14 @@ grub_atapi_open (const char *name, struct grub_scsi *scsi)
 
   for (dev = grub_ata_devices; dev; dev = dev->next)
     {
-      char devname[10];
+      if (dev->port * 2 + dev->device == devnum)
-      grub_snprintf (devname, sizeof (devname),
-		     "ata%d", dev->port * 2 + dev->device);
-
-      if (!grub_strcmp (devname, name))
 	{
 	  devfnd = dev;
 	  break;
 	}
     }
 
-  grub_dprintf ("ata", "opening ATAPI dev `%s'\n", name);
+  grub_dprintf ("ata", "opening ATAPI dev `ata%d'\n", devnum);
 
   if (! devfnd)
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "no such ATAPI device");
@@ -887,18 +878,13 @@ grub_atapi_open (const char *name, struct grub_scsi *scsi)
   return GRUB_ERR_NONE;
 }
 
-static void
-grub_atapi_close (struct grub_scsi *scsi)
-{
-  grub_free (scsi->name);
-}
 
 static struct grub_scsi_dev grub_atapi_dev =
   {
-    .name = "ATAPI",
+    .name = "ata",
+    .id = GRUB_SCSI_SUBSYSTEM_ATAPI,
     .iterate = grub_atapi_iterate,
     .open = grub_atapi_open,
-    .close = grub_atapi_close,
     .read = grub_atapi_read,
     .write = grub_atapi_write
   };

disk/scsi.c

@@ -318,15 +318,24 @@ grub_scsi_iterate (int (*hook) (const char *name))
 {
   grub_scsi_dev_t p;
 
-  auto int scsi_iterate (const char *name, int luns);
+  auto int scsi_iterate (int bus, int luns);
 
-  int scsi_iterate (const char *name, int luns)
+  int scsi_iterate (int bus, int luns)
     {
       int i;
 
       /* In case of a single LUN, just return `usbX'.  */
       if (luns == 1)
-	return hook (name);
+	{
+	  char *sname;
+	  int ret;
+	  sname = grub_xasprintf ("%s%d", p->name, bus);
+	  if (!sname)
+	    return 1;
+	  ret = hook (sname);
+	  grub_free (sname);
+	  return ret;
+	}
 
       /* In case of multiple LUNs, every LUN will get a prefix to
 	 distinguish it.  */
@@ -334,7 +343,7 @@ grub_scsi_iterate (int (*hook) (const char *name))
 	{
 	  char *sname;
 	  int ret;
-	  sname = grub_xasprintf ("%s%c", name, 'a' + i);
+	  sname = grub_xasprintf ("%s%d%c", p->name, bus, 'a' + i);
 	  if (!sname)
 	    return 1;
 	  ret = hook (sname);
@@ -358,37 +367,46 @@ grub_scsi_open (const char *name, grub_disk_t disk)
   grub_scsi_dev_t p;
   grub_scsi_t scsi;
   grub_err_t err;
-  int len;
+  int lun, bus;
-  int lun;
   grub_uint64_t maxtime;
+  const char *nameend;
+
+  nameend = name + grub_strlen (name) - 1;
+  /* Try to detect a LUN ('a'-'z'), otherwise just use the first
+     LUN.  */
+  if (nameend >= name && *nameend >= 'a' && *nameend <= 'z')
+    {
+      lun = *nameend - 'a';
+      nameend--;
+    }
+  else
+    lun = 0;
+
+  while (nameend >= name && grub_isdigit (*nameend))
+    nameend--;
+
+  if (!nameend[1] || !grub_isdigit (nameend[1]))
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a SCSI disk");
+
+  bus = grub_strtoul (nameend + 1, 0, 0);
 
   scsi = grub_malloc (sizeof (*scsi));
   if (! scsi)
     return grub_errno;
 
-  len = grub_strlen (name);
-  lun = name[len - 1] - 'a';
-
-  /* Try to detect a LUN ('a'-'z'), otherwise just use the first
-     LUN.  */
-  if (lun < 0 || lun > 26)
-    lun = 0;
-
   for (p = grub_scsi_dev_list; p; p = p->next)
     {
-      if (p->open (name, scsi))
+      if (grub_strncmp (p->name, name, nameend - name) != 0)
 	continue;
 
-      disk->id = (unsigned long) "scsi"; /* XXX */
+      if (p->open (bus, scsi))
+	continue;
+
+      disk->id = grub_make_scsi_id (scsi->dev->id, bus, lun);
       disk->data = scsi;
       scsi->dev = p;
       scsi->lun = lun;
-      scsi->name = grub_strdup (name);
+      scsi->bus = bus;
-      if (! scsi->name)
-	{
-	  grub_free (scsi);
-	  return grub_errno;
-	}
 
       grub_dprintf ("scsi", "dev opened\n");
 
@@ -476,7 +494,8 @@ grub_scsi_close (grub_disk_t disk)
   grub_scsi_t scsi;
 
   scsi = disk->data;
-  scsi->dev->close (scsi);
+  if (scsi->dev->close)
+    scsi->dev->close (scsi);
   grub_free (scsi);
 }
 

disk/usbms.c

@@ -222,22 +222,15 @@ grub_usbms_finddevs (void)
 
 
 static int
-grub_usbms_iterate (int (*hook) (const char *name, int luns))
+grub_usbms_iterate (int (*hook) (int bus, int luns))
 {
   grub_usbms_dev_t p;
   int cnt = 0;
 
   for (p = grub_usbms_dev_list; p; p = p->next)
     {
-      char *devname;
+      if (hook (cnt, p->luns))
-      devname = grub_xasprintf ("usb%d", cnt);
+	return 1;
-
-      if (hook (devname, p->luns))
-	{
-	  grub_free (devname);
-	  return 1;
-	}
-      grub_free (devname);
       cnt++;
     }
 
@@ -398,28 +391,18 @@ grub_usbms_write (struct grub_scsi *scsi, grub_size_t cmdsize, char *cmd,
 }
 
 static grub_err_t
-grub_usbms_open (const char *name, struct grub_scsi *scsi)
+grub_usbms_open (int devnum, struct grub_scsi *scsi)
 {
   grub_usbms_dev_t p;
-  int devnum;
   int i = 0;
 
-  if (grub_strncmp (name, "usb", 3))
-    return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
-		       "not a USB Mass Storage device");
-
-  devnum = grub_strtoul (name + 3, NULL, 10);
   for (p = grub_usbms_dev_list; p; p = p->next)
     {
       /* Check if this is the devnumth device.  */
       if (devnum == i)
 	{
 	  scsi->data = p;
-	  scsi->name = grub_strdup (name);
 	  scsi->luns = p->luns;
-	  if (! scsi->name)
-	    return grub_errno;
-
 	  return GRUB_ERR_NONE;
 	}
 
@@ -430,18 +413,12 @@ grub_usbms_open (const char *name, struct grub_scsi *scsi)
 		     "not a USB Mass Storage device");
 }
 
-static void
-grub_usbms_close (struct grub_scsi *scsi)
-{
-  grub_free (scsi->name);
-}
-
 static struct grub_scsi_dev grub_usbms_dev =
   {
     .name = "usb",
+    .id = GRUB_SCSI_SUBSYSTEM_USBMS,
     .iterate = grub_usbms_iterate,
     .open = grub_usbms_open,
-    .close = grub_usbms_close,
     .read = grub_usbms_read,
     .write = grub_usbms_write
   };

include/grub/scsi.h

@@ -26,16 +26,35 @@ void grub_scsi_dev_unregister (grub_scsi_dev_t dev);
 
 struct grub_scsi;
 
+enum
+  {
+    GRUB_SCSI_SUBSYSTEM_USBMS,
+    GRUB_SCSI_SUBSYSTEM_ATAPI
+  };
+
+#define GRUB_SCSI_ID_SUBSYSTEM_SHIFT 24
+#define GRUB_SCSI_ID_BUS_SHIFT 8
+#define GRUB_SCSI_ID_LUN_SHIFT 0
+
+static inline grub_uint32_t
+grub_make_scsi_id (int subsystem, int bus, int lun)
+{
+  return (subsystem << GRUB_SCSI_ID_SUBSYSTEM_SHIFT)
+    | (bus << GRUB_SCSI_ID_BUS_SHIFT) | (lun << GRUB_SCSI_ID_BUS_SHIFT);
+}
+
 struct grub_scsi_dev
 {
   /* The device name.  */
   const char *name;
 
+  grub_uint8_t id;
+
   /* Call HOOK with each device name, until HOOK returns non-zero.  */
-  int (*iterate) (int (*hook) (const char *name, int luns));
+  int (*iterate) (int (*hook) (int bus, int luns));
 
   /* Open the device named NAME, and set up SCSI.  */
-  grub_err_t (*open) (const char *name, struct grub_scsi *scsi);
+  grub_err_t (*open) (int bus, struct grub_scsi *scsi);
 
   /* Close the scsi device SCSI.  */
   void (*close) (struct grub_scsi *scsi);
@@ -56,15 +75,14 @@ struct grub_scsi_dev
 
 struct grub_scsi
 {
-  /* The scsi device name.  */
-  char *name;
-
   /* The underlying scsi device.  */
   grub_scsi_dev_t dev;
 
   /* Type of SCSI device.  XXX: Make enum.  */
   grub_uint8_t devtype;
 
+  int bus;
+
   /* Number of LUNs.  */
   int luns;