[Qemu-devel] [PULL 0/5] SCSI patches 2012-03-19

[Qemu-devel] [PULL 0/5] SCSI patches 2012-03-19

[Paolo Bonzini]

Anthony,

the following changes since commit 5bd33de6635577744b3c10dd3913bfe8c5ffaf40:

  tcg: fix sparc host for AREG0 free operation (2012-03-18 19:15:32 +0000)

are available in the git repository at:
  git://github.com/bonzini/qemu.git scsi-next

Paolo Bonzini (5):
      get rid of CONFIG_VIRTIO_SCSI
      scsi-cd: check ready condition before processing several commands
      scsi: copy serial number into VPD page 0x83
      virtio-scsi: call unregister_savevm properly
      scsi: add get_dev_path

 Makefile.target                   |    2 +-
 default-configs/pci.mak           |    1 -
 default-configs/s390x-softmmu.mak |    1 -
 hw/scsi-bus.c                     |   18 +++++++++++++
 hw/scsi-disk.c                    |   49 +++++++++++++++++++++++++------------
 hw/virtio-scsi.c                  |    2 +
 6 files changed, 54 insertions(+), 19 deletions(-)
-- 
1.7.7.6

[Qemu-devel] [PATCH 1/5] get rid of CONFIG_VIRTIO_SCSI

[Paolo Bonzini]

This is useless because we do not have a header file for devices.
Setting it to off will cause a link error in virtio-pci.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.target                   |    2 +-
 default-configs/pci.mak           |    1 -
 default-configs/s390x-softmmu.mak |    1 -
 3 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 37fb7ed..63cf769 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -209,7 +209,7 @@ obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o ioport.o
 # need to fix this properly
 obj-$(CONFIG_NO_PCI) += pci-stub.o
 obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
-obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi.o
+obj-$(CONFIG_VIRTIO) += virtio-scsi.o
 obj-y += vhost_net.o
 obj-$(CONFIG_VHOST_NET) += vhost.o
 obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 21e4ccf..9d3e1db 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -1,6 +1,5 @@
 CONFIG_PCI=y
 CONFIG_VIRTIO_PCI=y
-CONFIG_VIRTIO_SCSI=y
 CONFIG_VIRTIO=y
 CONFIG_USB_UHCI=y
 CONFIG_USB_OHCI=y
diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
index e588803..3005729 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -1,2 +1 @@
 CONFIG_VIRTIO=y
-CONFIG_VIRTIO_SCSI=y
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/5] scsi-cd: check ready condition before processing several commands

[Paolo Bonzini]

This commit is more or less obvious.  What it caused is less obvious:
SCSI CD drives failed to eject under Linux, though for example the
"change" command worked okay.  This happens because of the autoclose
option in the Linux CD-ROM driver.

The actual chain of events is quite complex and somehow involves
udev helpers; the actual command that matters is READ TOC, though
honestly it's not really clear to me how because it should always be
invoked after autoclose, not before.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   42 +++++++++++++++++++++++++++++-------------
 1 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index add399e..7846f45 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1152,9 +1152,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
     outbuf = r->iov.iov_base;
     switch (req->cmd.buf[0]) {
     case TEST_UNIT_READY:
-        if (s->tray_open || !bdrv_is_inserted(s->qdev.conf.bs)) {
-            goto not_ready;
-        }
+        assert(!s->tray_open && bdrv_is_inserted(s->qdev.conf.bs));
         break;
     case INQUIRY:
         buflen = scsi_disk_emulate_inquiry(req, outbuf);
@@ -1209,7 +1207,8 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
         memset(outbuf, 0, 8);
         bdrv_get_geometry(s->qdev.conf.bs, &nb_sectors);
         if (!nb_sectors) {
-            goto not_ready;
+            scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
+            return -1;
         }
         if ((req->cmd.buf[8] & 1) == 0 && req->cmd.lba) {
             goto illegal_request;
@@ -1269,7 +1268,8 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
             memset(outbuf, 0, req->cmd.xfer);
             bdrv_get_geometry(s->qdev.conf.bs, &nb_sectors);
             if (!nb_sectors) {
-                goto not_ready;
+                scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
+                return -1;
             }
             if ((req->cmd.buf[14] & 1) == 0 && req->cmd.lba) {
                 goto illegal_request;
@@ -1314,14 +1314,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
     buflen = MIN(buflen, req->cmd.xfer);
     return buflen;
 
-not_ready:
-    if (s->tray_open || !bdrv_is_inserted(s->qdev.conf.bs)) {
-        scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
-    } else {
-        scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
-    }
-    return -1;
-
 illegal_request:
     if (r->req.status == -1) {
         scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
@@ -1356,6 +1348,30 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
 #endif
 
     switch (command) {
+    case INQUIRY:
+    case MODE_SENSE:
+    case MODE_SENSE_10:
+    case RESERVE:
+    case RESERVE_10:
+    case RELEASE:
+    case RELEASE_10:
+    case START_STOP:
+    case ALLOW_MEDIUM_REMOVAL:
+    case GET_CONFIGURATION:
+    case GET_EVENT_STATUS_NOTIFICATION:
+    case MECHANISM_STATUS:
+    case REQUEST_SENSE:
+        break;
+
+    default:
+        if (s->tray_open || !bdrv_is_inserted(s->qdev.conf.bs)) {
+            scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
+            return 0;
+        }
+        break;
+    }
+
+    switch (command) {
     case TEST_UNIT_READY:
     case INQUIRY:
     case MODE_SENSE:
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/5] scsi: copy serial number into VPD page 0x83

[Paolo Bonzini]

Currently QEMU passes the qdev device id to the guest in an ASCII-string
designator in page 0x83.  While this is fine, it does not match what
real hardware does; usually the ASCII-string designator there hosts
another copy of the serial number (there can be other designators,
for example with a world-wide name).  Do the same for QEMU SCSI
disks.

ATAPI does not support VPD pages, so it does not matter there.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 7846f45..9949786 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -471,8 +471,9 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 
         case 0x83: /* Device identification page, mandatory */
         {
-            int max_len = 255 - 8;
-            int id_len = strlen(bdrv_get_device_name(s->qdev.conf.bs));
+            const char *str = s->serial ?: bdrv_get_device_name(s->qdev.conf.bs);
+            int max_len = s->serial ? 20 : 255 - 8;
+            int id_len = strlen(str);
 
             if (id_len > max_len) {
                 id_len = max_len;
@@ -486,7 +487,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
             outbuf[buflen++] = 0;   // reserved
             outbuf[buflen++] = id_len; // length of data following
 
-            memcpy(outbuf+buflen, bdrv_get_device_name(s->qdev.conf.bs), id_len);
+            memcpy(outbuf+buflen, str, id_len);
             buflen += id_len;
             break;
         }
-- 
1.7.7.6

[Qemu-devel] [PATCH 4/5] virtio-scsi: call unregister_savevm properly

[Paolo Bonzini]

This fixes a use-after-free when migrating after hot-unplug.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index e607edc..45d54fa 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -613,5 +613,7 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
 
 void virtio_scsi_exit(VirtIODevice *vdev)
 {
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    unregister_savevm(s->qdev, "virtio-scsi", s);
     virtio_cleanup(vdev);
 }
-- 
1.7.7.6

[Qemu-devel] [PATCH 5/5] scsi: add get_dev_path

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 2cb5a18..8e76c5d 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -7,6 +7,7 @@
 #include "trace.h"
 #include "dma.h"
 
+static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
 static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
@@ -14,6 +15,7 @@ static void scsi_req_dequeue(SCSIRequest *req);
 static struct BusInfo scsi_bus_info = {
     .name  = "SCSI",
     .size  = sizeof(SCSIBus),
+    .get_dev_path = scsibus_get_dev_path,
     .get_fw_dev_path = scsibus_get_fw_dev_path,
     .props = (Property[]) {
         DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
@@ -1423,6 +1425,22 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
     sdev->unit_attention = sense;
 }
 
+static char *scsibus_get_dev_path(DeviceState *dev)
+{
+    SCSIDevice *d = DO_UPCAST(SCSIDevice, qdev, dev);
+    DeviceState *hba = dev->parent_bus->parent;
+    char *id = NULL;
+
+    if (hba && hba->parent_bus && hba->parent_bus->info->get_dev_path) {
+        id = hba->parent_bus->info->get_dev_path(hba);
+    }
+    if (id) {
+        return g_strdup_printf("%s/%d:%d:%d", id, d->channel, d->id, d->lun);
+    } else {
+        return g_strdup_printf("%d:%d:%d", d->channel, d->id, d->lun);
+    }
+}
+
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
 {
     SCSIDevice *d = SCSI_DEVICE(dev);
-- 
1.7.7.6

Re: [Qemu-devel] [PULL 0/5] SCSI patches 2012-03-19

[Anthony Liguori]

On 03/19/2012 11:19 AM, Paolo Bonzini wrote:
> Anthony,
>
> the following changes since commit 5bd33de6635577744b3c10dd3913bfe8c5ffaf40:
>
>    tcg: fix sparc host for AREG0 free operation (2012-03-18 19:15:32 +0000)

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> are available in the git repository at:
>    git://github.com/bonzini/qemu.git scsi-next
>
> Paolo Bonzini (5):
>        get rid of CONFIG_VIRTIO_SCSI
>        scsi-cd: check ready condition before processing several commands
>        scsi: copy serial number into VPD page 0x83
>        virtio-scsi: call unregister_savevm properly
>        scsi: add get_dev_path
>
>   Makefile.target                   |    2 +-
>   default-configs/pci.mak           |    1 -
>   default-configs/s390x-softmmu.mak |    1 -
>   hw/scsi-bus.c                     |   18 +++++++++++++
>   hw/scsi-disk.c                    |   49 +++++++++++++++++++++++++------------
>   hw/virtio-scsi.c                  |    2 +
>   6 files changed, 54 insertions(+), 19 deletions(-)