Patchwork [6/8] scsi: Split qdev "scsi-disk" into "scsi-hd" and "scsi-cd"

login
register
mail settings
Submitter Markus Armbruster
Date July 6, 2010, 12:37 p.m.
Message ID <1278419869-26126-7-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/58021/
State New
Headers show

Comments

Markus Armbruster - July 6, 2010, 12:37 p.m.
Disk vs. CD needs to be in qdev, because it belongs to the drive's
guest part.

Keep scsi-disk for backward compatibility.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-disk.c |  117 +++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 91 insertions(+), 26 deletions(-)
Christoph Hellwig - July 7, 2010, 1:37 a.m.
On Tue, Jul 06, 2010 at 02:37:47PM +0200, Markus Armbruster wrote:
> Disk vs. CD needs to be in qdev, because it belongs to the drive's
> guest part.

Looks good, but the scsi-hd name feelds kinda awkward.  This is one
case we're I'm really wondering if the compatiblity is worth it or
if we should just keep using scsi-disk for the real disk.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Kevin Wolf - July 7, 2010, 7:38 a.m.
Am 07.07.2010 03:37, schrieb Christoph Hellwig:
> On Tue, Jul 06, 2010 at 02:37:47PM +0200, Markus Armbruster wrote:
>> Disk vs. CD needs to be in qdev, because it belongs to the drive's
>> guest part.
> 
> Looks good, but the scsi-hd name feelds kinda awkward.  This is one
> case we're I'm really wondering if the compatiblity is worth it or
> if we should just keep using scsi-disk for the real disk.

In any case the name should be consistent with ide-hd. And I'm not sure
if it's really helpful to have ide-drive and ide-disk.

Kevin
Markus Armbruster - July 7, 2010, 9:33 a.m.
Kevin Wolf <kwolf@redhat.com> writes:

> Am 07.07.2010 03:37, schrieb Christoph Hellwig:
>> On Tue, Jul 06, 2010 at 02:37:47PM +0200, Markus Armbruster wrote:
>>> Disk vs. CD needs to be in qdev, because it belongs to the drive's
>>> guest part.
>> 
>> Looks good, but the scsi-hd name feelds kinda awkward.  This is one
>> case we're I'm really wondering if the compatiblity is worth it or
>> if we should just keep using scsi-disk for the real disk.
>
> In any case the name should be consistent with ide-hd. And I'm not sure
> if it's really helpful to have ide-drive and ide-disk.

{ide,scsi}-{hd,cd} is the best consistent set of names I could find
within the backward compatibility straightjacket.

By the way, we could use a way to mark qdevs and properties deprecated.

Patch

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f43f2d0..ebefcba 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -67,6 +67,7 @@  struct SCSIDiskState
     QEMUBH *bh;
     char *version;
     char *serial;
+    int is_cd;
 };
 
 static SCSIDiskReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
@@ -339,7 +340,7 @@  static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
             return -1;
         }
 
-        if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+        if (s->is_cd) {
             outbuf[buflen++] = 5;
         } else {
             outbuf[buflen++] = 0;
@@ -452,7 +453,7 @@  static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
         return buflen;
     }
 
-    if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+    if (s->is_cd) {
         outbuf[0] = 5;
         outbuf[1] = 0x80;
         memcpy(&outbuf[16], "QEMU CD-ROM     ", 16);
@@ -566,7 +567,7 @@  static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p)
         return 20;
 
     case 0x2a: /* CD Capabilities and Mechanical Status page. */
-        if (bdrv_get_type_hint(bdrv) != BDRV_TYPE_CDROM)
+        if (!s->is_cd)
             return 0;
         p[0] = 0x2a;
         p[1] = 0x14;
@@ -755,7 +756,7 @@  static int scsi_disk_emulate_command(SCSIRequest *req, uint8_t *outbuf)
             goto illegal_request;
         break;
     case START_STOP:
-        if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM && (req->cmd.buf[4] & 2)) {
+        if (s->is_cd && (req->cmd.buf[4] & 2)) {
             /* load/eject medium */
             bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
         }
@@ -1046,10 +1047,9 @@  static void scsi_destroy(SCSIDevice *dev)
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
-static int scsi_disk_initfn(SCSIDevice *dev)
+static int scsi_initfn(SCSIDevice *dev, int is_cd)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-    int is_cd;
     DriveInfo *dinfo;
 
     if (!s->qdev.conf.bs) {
@@ -1057,7 +1057,7 @@  static int scsi_disk_initfn(SCSIDevice *dev)
         return -1;
     }
     s->bs = s->qdev.conf.bs;
-    is_cd = bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM;
+    s->is_cd = is_cd;
 
     if (!is_cd && !bdrv_is_inserted(s->bs)) {
         error_report("Device needs media, but drive is empty");
@@ -1097,28 +1097,93 @@  static int scsi_disk_initfn(SCSIDevice *dev)
     return 0;
 }
 
-static SCSIDeviceInfo scsi_disk_info = {
-    .qdev.name    = "scsi-disk",
-    .qdev.desc    = "virtual scsi disk or cdrom",
-    .qdev.size    = sizeof(SCSIDiskState),
-    .qdev.reset   = scsi_disk_reset,
-    .init         = scsi_disk_initfn,
-    .destroy      = scsi_destroy,
-    .send_command = scsi_send_command,
-    .read_data    = scsi_read_data,
-    .write_data   = scsi_write_data,
-    .cancel_io    = scsi_cancel_io,
-    .get_buf      = scsi_get_buf,
-    .qdev.props   = (Property[]) {
-        DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
-        DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
-        DEFINE_PROP_STRING("serial",  SCSIDiskState, serial),
-        DEFINE_PROP_END_OF_LIST(),
-    },
+static int scsi_hd_initfn(SCSIDevice *dev)
+{
+    return scsi_initfn(dev, 0);
+}
+
+static int scsi_cd_initfn(SCSIDevice *dev)
+{
+    return scsi_initfn(dev, 1);
+}
+
+static int scsi_disk_initfn(SCSIDevice *dev)
+{
+    int is_cd;
+
+    if (!dev->conf.bs) {
+        is_cd = 0;              /* will die in scsi_initfn() */
+    } else {
+        is_cd = bdrv_get_type_hint(dev->conf.bs) == BDRV_TYPE_CDROM;
+    }
+
+    return scsi_initfn(dev, is_cd);
+}
+
+static SCSIDeviceInfo scsi_disk_info[] = {
+    {
+        .qdev.name    = "scsi-hd",
+        .qdev.desc    = "virtual scsi disk",
+        .qdev.size    = sizeof(SCSIDiskState),
+        .qdev.reset   = scsi_disk_reset,
+        .init         = scsi_hd_initfn,
+        .destroy      = scsi_destroy,
+        .send_command = scsi_send_command,
+        .read_data    = scsi_read_data,
+        .write_data   = scsi_write_data,
+        .cancel_io    = scsi_cancel_io,
+        .get_buf      = scsi_get_buf,
+        .qdev.props   = (Property[]) {
+            DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
+            DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
+            DEFINE_PROP_STRING("serial",  SCSIDiskState, serial),
+            DEFINE_PROP_END_OF_LIST(),
+        }
+    },{
+        .qdev.name    = "scsi-cd",
+        .qdev.desc    = "virtual scsi cdrom",
+        .qdev.size    = sizeof(SCSIDiskState),
+        .qdev.reset   = scsi_disk_reset,
+        .init         = scsi_cd_initfn,
+        .destroy      = scsi_destroy,
+        .send_command = scsi_send_command,
+        .read_data    = scsi_read_data,
+        .write_data   = scsi_write_data,
+        .cancel_io    = scsi_cancel_io,
+        .get_buf      = scsi_get_buf,
+        .qdev.props   = (Property[]) {
+            DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
+            DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
+            DEFINE_PROP_STRING("serial",  SCSIDiskState, serial),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+    },{
+        .qdev.name    = "scsi-disk", /* legacy -device scsi-disk */
+        .qdev.desc    = "virtual scsi disk or cdrom (legacy)",
+        .qdev.size    = sizeof(SCSIDiskState),
+        .qdev.reset   = scsi_disk_reset,
+        .init         = scsi_disk_initfn,
+        .destroy      = scsi_destroy,
+        .send_command = scsi_send_command,
+        .read_data    = scsi_read_data,
+        .write_data   = scsi_write_data,
+        .cancel_io    = scsi_cancel_io,
+        .get_buf      = scsi_get_buf,
+        .qdev.props   = (Property[]) {
+            DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
+            DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
+            DEFINE_PROP_STRING("serial",  SCSIDiskState, serial),
+            DEFINE_PROP_END_OF_LIST(),
+        }
+    }
 };
 
 static void scsi_disk_register_devices(void)
 {
-    scsi_qdev_register(&scsi_disk_info);
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(scsi_disk_info); i++) {
+        scsi_qdev_register(&scsi_disk_info[i]);
+    }
 }
 device_init(scsi_disk_register_devices)