[PATCH] Register drives directly in scsi_init_drive().

[Kevin O'Connor]

The scsi_init_drive() function has enough information to directly
register the drive there, so do the registration there.  This
simplifies the calling code.

Signed-off-by: Kevin O'Connor 
---

Paulo - this patch will conflict with your patch, but it seems like it
would also be a simplification for virtio-scsi.

---
 src/blockcmd.c |   25 +++++++++++++++----------
 src/blockcmd.h |    2 +-
 src/usb-msc.c  |   40 ++--------------------------------------
 3 files changed, 18 insertions(+), 49 deletions(-)

diff --git a/src/blockcmd.c b/src/blockcmd.c
index 3476cfc..dd35501 100644
--- a/src/blockcmd.c
+++ b/src/blockcmd.c
@@ -75,9 +75,9 @@ scsi_is_ready(struct disk_op_s *op)
     return 0;
 }

-// Validate drive and find block size and sector count.
+// Validate drive, find block size / sector count, and register drive.
 int
-scsi_init_drive(struct drive_s *drive, const char *s, int *pdt, char **desc)
+scsi_init_drive(struct drive_s *drive, const char *s, int prio)
 {
     if (!CONFIG_USB_MSC)
         return 0;
@@ -97,18 +97,19 @@ scsi_init_drive(struct drive_s *drive, const char *s, int *pdt, char **desc)
     nullTrailingSpace(product);
     strtcpy(rev, data.rev, sizeof(rev));
     nullTrailingSpace(rev);
-    *pdt = data.pdt & 0x1f;
+    int pdt = data.pdt & 0x1f;
     int removable = !!(data.removable & 0x80);
     dprintf(1, "%s vendor='%s' product='%s' rev='%s' type=%d removable=%d\n"
-            , s, vendor, product, rev, *pdt, removable);
+            , s, vendor, product, rev, pdt, removable);
     drive->removable = removable;

-    if (*pdt == SCSI_TYPE_CDROM) {
+    if (pdt == SCSI_TYPE_CDROM) {
         drive->blksize = CDROM_SECTOR_SIZE;
         drive->sectors = (u64)-1;

-        *desc = znprintf(MAXDESCSIZE, "DVD/CD [%s Drive %s %s %s]"
+        char *desc = znprintf(MAXDESCSIZE, "DVD/CD [%s Drive %s %s %s]"
                               , s, vendor, product, rev);
+        boot_add_cd(drive, desc, prio);
         return 0;
     }

@@ -127,6 +128,10 @@ scsi_init_drive(struct drive_s *drive, const char *s, int *pdt, char **desc)
     // We do not bother with READ CAPACITY(16) because BIOS does not support
     // 64-bit LBA anyway.
     drive->blksize = ntohl(capdata.blksize);
+    if (drive->blksize != DISK_SECTOR_SIZE) {
+        dprintf(1, "%s: unsupported block size %d\n", s, drive->blksize);
+        return -1;
+    }
     drive->sectors = (u64)ntohl(capdata.sectors) + 1;
     dprintf(1, "%s blksize=%d sectors=%d\n"
             , s, drive->blksize, (unsigned)drive->sectors);
@@ -143,13 +148,13 @@ scsi_init_drive(struct drive_s *drive, const char *s, int *pdt, char **desc)
             ((u32)drive->sectors % (geomdata.heads * cylinders) == 0)) {
             drive->pchs.cylinders = cylinders;
             drive->pchs.heads = geomdata.heads;
-            drive->pchs.spt = (u32)drive->sectors
-     / (geomdata.heads * cylinders);
+            drive->pchs.spt = (u32)drive->sectors / (geomdata.heads * cylinders);
         }
     }

-    *desc = znprintf(MAXDESCSIZE, "%s Drive %s %s %s"
-                      , s, vendor, product, rev);
+    char *desc = znprintf(MAXDESCSIZE, "%s Drive %s %s %s"
+                          , s, vendor, product, rev);
+    boot_add_hd(drive, desc, prio);
     return 0;
 }

diff --git a/src/blockcmd.h b/src/blockcmd.h
index bace649..ccfeeaf 100644
--- a/src/blockcmd.h
+++ b/src/blockcmd.h
@@ -110,6 +110,6 @@ int cdb_read(struct disk_op_s *op);
 int cdb_write(struct disk_op_s *op);

int scsi_is_ready(struct disk_op_s *op);
-int scsi_init_drive(struct drive_s *drive, const char *s, int *pdt, char **desc);
+int scsi_init_drive(struct drive_s *drive, const char *s, int prio);

#endif // blockcmd.h
diff --git a/src/usb-msc.c b/src/usb-msc.c
index 4a09972..ef9f440 100644
--- a/src/usb-msc.c
+++ b/src/usb-msc.c
@@ -122,33 +122,6 @@ fail:
  * Setup
  ****************************************************************/

-static int
-setup_drive_cdrom(struct drive_s *drive, char *desc)
-{
-    drive->sectors = (u64)-1;
-    struct usb_pipe *pipe = container_of(
-        drive, struct usbdrive_s, drive)->bulkout;
-    int prio = bootprio_find_usb(pipe->cntl->pci, pipe->path);
-    boot_add_cd(drive, desc, prio);
-    return 0;
-}
-
-static int
-setup_drive_hd(struct drive_s *drive, char *desc)
-{
-    if (drive->blksize != DISK_SECTOR_SIZE) {
-        dprintf(1, "Unsupported USB MSC block size %d\n", drive->blksize);
-        return -1;
-    }
-
-    // Register with bcv system.
-    struct usb_pipe *pipe = container_of(
-        drive, struct usbdrive_s, drive)->bulkout;
-    int prio = bootprio_find_usb(pipe->cntl->pci, pipe->path);
-    boot_add_hd(drive, desc, prio);
-    return 0;
-}
-
 // Configure a usb msc device.
 int
 usb_msc_init(struct usb_pipe *pipe
@@ -188,17 +161,8 @@ usb_msc_init(struct usb_pipe *pipe
     if (!udrive_g->bulkin || !udrive_g->bulkout)
         goto fail;

-    int ret, pdt;
-    char *desc = NULL;
-    ret = scsi_init_drive(&udrive_g->drive, "USB MSC", &pdt, &desc);
-    if (ret)
-        goto fail;
-
-    if (pdt == SCSI_TYPE_CDROM)
-        ret = setup_drive_cdrom(&udrive_g->drive, desc);
-    else
-        ret = setup_drive_hd(&udrive_g->drive, desc);
-
+    int prio = bootprio_find_usb(pipe->cntl->pci, pipe->path);
+    int ret = scsi_init_drive(&udrive_g->drive, "USB MSC", prio);
     if (ret)
         goto fail;

[Paolo Bonzini]

On 02/19/2012 05:34 PM, Kevin O'Connor wrote:
> The scsi_init_drive() function has enough information to directly
> register the drive there, so do the registration there.  This
> simplifies the calling code.
> Signed-off-by: Kevin O'Connor
>
> Paulo - this patch will conflict with your patch, but it seems like it
> would also be a simplification for virtio-scsi.

Yes, no problem from me.

Paolo