[Qemu-devel] [PULL 1.3 0/5] SCSI updates for 2012-11-12

[Qemu-devel] [PULL 1.3 0/5] SCSI updates for 2012-11-12

[Paolo Bonzini]

Anthony,

The following changes since commit 3c5645fab3c4b65d0cffbe1aaafc787e4be63d0f:

  tcg: properly check that op's output needs to be synced to memory (2012-11-11 16:06:46 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git scsi-next

for you to fetch changes up to accfeb2dd32ece73350b06cee1b2403f47e86fe3:

  scsi-disk: flush cache after disabling it (2012-11-12 15:00:27 +0100)

----------------------------------------------------------------
Paolo Bonzini (5):
      scsi: do not return short responses for emulated commands
      virtio-scsi: factor checks for VIRTIO_SCSI_S_DRIVER_OK when reporting events
      scsi: remove superfluous call to scsi_device_set_ua
      megasas: do not include block_int.h
      scsi-disk: flush cache after disabling it

 hw/megasas.c     |  1 -
 hw/scsi-disk.c   | 44 +++++++++++++++++++++++++++-----------------
 hw/virtio-scsi.c |  8 +++++---
 3 file modificati, 32 inserzioni(+), 21 rimozioni(-)
-- 
1.7.12.1

[Qemu-devel] [PATCH 1/5] scsi: do not return short responses for emulated commands

[Paolo Bonzini]

The inquiry command, for the case of VPD=1, was returning short
responses; the number of returned bytes was just the number of bytes
in the request, without padding to the specified allocation length
with zero bytes.  This is usually harmless, but it is a violation
of the SCSI specification.

To fix this, always pad with zero bytes to r->cmd.xfer in
scsi_disk_emulate_command, and return at most r->buflen bytes
(the size of the buffer for command data) rather than at most
buflen bytes (the number of bytes that was filled in).

Before this patch, "strace sg_inq -p0x83 /dev/sda" would report a
non-zero resid value.  After this patch, it reports resid=0.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 34 ++++++++++++++++++----------------
 1 file modificato, 18 inserzioni(+), 16 rimozioni(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 1b0afa6..098558d 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -652,7 +652,6 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
     if (buflen > SCSI_MAX_INQUIRY_LEN) {
         buflen = SCSI_MAX_INQUIRY_LEN;
     }
-    memset(outbuf, 0, buflen);
 
     outbuf[0] = s->qdev.type & 0x1f;
     outbuf[1] = (s->features & (1 << SCSI_DISK_F_REMOVABLE)) ? 0x80 : 0;
@@ -1596,24 +1595,26 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         break;
     }
 
+    /*
+     * FIXME: we shouldn't return anything bigger than 4k, but the code
+     * requires the buffer to be as big as req->cmd.xfer in several
+     * places.  So, do not allow CDBs with a very large ALLOCATION
+     * LENGTH.  The real fix would be to modify scsi_read_data and
+     * dma_buf_read, so that they return data beyond the buflen
+     * as all zeros.
+     */
+    if (req->cmd.xfer > 65536) {
+        goto illegal_request;
+    }
+    r->buflen = MAX(4096, req->cmd.xfer);
+
     if (!r->iov.iov_base) {
-        /*
-         * FIXME: we shouldn't return anything bigger than 4k, but the code
-         * requires the buffer to be as big as req->cmd.xfer in several
-         * places.  So, do not allow CDBs with a very large ALLOCATION
-         * LENGTH.  The real fix would be to modify scsi_read_data and
-         * dma_buf_read, so that they return data beyond the buflen
-         * as all zeros.
-         */
-        if (req->cmd.xfer > 65536) {
-            goto illegal_request;
-        }
-        r->buflen = MAX(4096, req->cmd.xfer);
         r->iov.iov_base = qemu_blockalign(s->qdev.conf.bs, r->buflen);
     }
 
     buflen = req->cmd.xfer;
     outbuf = r->iov.iov_base;
+    memset(outbuf, 0, r->buflen);
     switch (req->cmd.buf[0]) {
     case TEST_UNIT_READY:
         assert(!s->tray_open && bdrv_is_inserted(s->qdev.conf.bs));
@@ -1694,12 +1695,14 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         outbuf[5] = 0;
         outbuf[6] = s->qdev.blocksize >> 8;
         outbuf[7] = 0;
-        buflen = 8;
         break;
     case REQUEST_SENSE:
         /* Just return "NO SENSE".  */
         buflen = scsi_build_sense(NULL, 0, outbuf, r->buflen,
                                   (req->cmd.buf[1] & 1) == 0);
+        if (buflen < 0) {
+            goto illegal_request;
+        }
         break;
     case MECHANISM_STATUS:
         buflen = scsi_emulate_mechanism_status(s, outbuf);
@@ -1770,7 +1773,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
             }
 
             /* Protection, exponent and lowest lba field left blank. */
-            buflen = req->cmd.xfer;
             break;
         }
         DPRINTF("Unsupported Service Action In\n");
@@ -1827,7 +1829,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         return 0;
     }
     assert(!r->req.aiocb);
-    r->iov.iov_len = MIN(buflen, req->cmd.xfer);
+    r->iov.iov_len = MIN(r->buflen, req->cmd.xfer);
     if (r->iov.iov_len == 0) {
         scsi_req_complete(&r->req, GOOD);
     }
-- 
1.7.12.1

[Qemu-devel] [PATCH 2/5] virtio-scsi: factor checks for VIRTIO_SCSI_S_DRIVER_OK when reporting events

[Paolo Bonzini]

Suggested by Laszlo Ersek.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c | 8 +++++---
 1 file modificato, 5 inserzioni(+), 3 rimozioni(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index b54c789..30d3f8a 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -596,6 +596,10 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
     VirtIOSCSIEvent *evt;
     int in_size;
 
+    if (!(s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        return;
+    }
+
     if (!req) {
         s->events_dropped = true;
         return;
@@ -648,7 +652,6 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
 
     if (((s->vdev.guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) &&
-        (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) &&
         dev->type != TYPE_ROM) {
         virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
                                sense.asc | (sense.ascq << 8));
@@ -659,8 +662,7 @@ static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
 {
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
 
-    if (((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) &&
-        (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+    if ((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
         virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
                                VIRTIO_SCSI_EVT_RESET_RESCAN);
     }
-- 
1.7.12.1

[Qemu-devel] [PATCH 3/5] scsi: remove superfluous call to scsi_device_set_ua

[Paolo Bonzini]

Suggested by Laszlo Ersek.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 1 -
 1 file modificato. 1 rimozione(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 098558d..d15f891 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1964,7 +1964,6 @@ static void scsi_disk_resize_cb(void *opaque)
      * direct-access devices.
      */
     if (s->qdev.type == TYPE_DISK) {
-        scsi_device_set_ua(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
         scsi_device_report_change(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
     }
 }
-- 
1.7.12.1

[Qemu-devel] [PATCH 4/5] megasas: do not include block_int.h

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/megasas.c | 1 -
 1 file modificato. 1 rimozione(-)

diff --git a/hw/megasas.c b/hw/megasas.c
index 7a2036e..b845ea7 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -25,7 +25,6 @@
 #include "iov.h"
 #include "scsi.h"
 #include "scsi-defs.h"
-#include "block_int.h"
 #include "trace.h"
 
 #include "mfi.h"
-- 
1.7.12.1

[Qemu-devel] [PATCH 5/5] scsi-disk: flush cache after disabling it

[Paolo Bonzini]

SBC says that "if an application client changes the WCE bit from one to
zero via a MODE SELECT command, then the device server shall write
any data in volatile cache to non-volatile medium before completing
the command".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 9 +++++++++
 1 file modificato, 9 inserzioni(+)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d15f891..49b5686 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1387,6 +1387,7 @@ invalid_param_len:
 
 static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
 {
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint8_t *p = inbuf;
     int cmd = r->req.cmd.buf[0];
     int len = r->req.cmd.xfer;
@@ -1423,6 +1424,14 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
             return;
         }
     }
+    if (!bdrv_enable_write_cache(s->qdev.conf.bs)) {
+        /* The request is used as the AIO opaque value, so add a ref.  */
+        scsi_req_ref(&r->req);
+        bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
+        r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
+        return;
+    }
+
     scsi_req_complete(&r->req, GOOD);
     return;
 
-- 
1.7.12.1

Re: [Qemu-devel] [PULL 1.3 0/5] SCSI updates for 2012-11-12

[Paolo Bonzini]

Il 12/11/2012 15:03, Paolo Bonzini ha scritto:
> Anthony,
> 
> The following changes since commit 3c5645fab3c4b65d0cffbe1aaafc787e4be63d0f:
> 
>   tcg: properly check that op's output needs to be synced to memory (2012-11-11 16:06:46 +0100)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git scsi-next
> 
> for you to fetch changes up to accfeb2dd32ece73350b06cee1b2403f47e86fe3:
> 
>   scsi-disk: flush cache after disabling it (2012-11-12 15:00:27 +0100)

Updated to commit 4003e24fce1df84a2e8c992376ed2c294816c33c to include
"megasas: Correct target/lun mapping".

Paolo

Re: [Qemu-devel] [PULL 1.3 0/5] SCSI updates for 2012-11-12

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Anthony,
>
> The following changes since commit 3c5645fab3c4b65d0cffbe1aaafc787e4be63d0f:
>
>   tcg: properly check that op's output needs to be synced to memory (2012-11-11 16:06:46 +0100)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git scsi-next

Pulled. Thanks.

Regards,

Anthony Liguori

>
> for you to fetch changes up to accfeb2dd32ece73350b06cee1b2403f47e86fe3:
>
>   scsi-disk: flush cache after disabling it (2012-11-12 15:00:27 +0100)
>
> ----------------------------------------------------------------
> Paolo Bonzini (5):
>       scsi: do not return short responses for emulated commands
>       virtio-scsi: factor checks for VIRTIO_SCSI_S_DRIVER_OK when reporting events
>       scsi: remove superfluous call to scsi_device_set_ua
>       megasas: do not include block_int.h
>       scsi-disk: flush cache after disabling it
>
>  hw/megasas.c     |  1 -
>  hw/scsi-disk.c   | 44 +++++++++++++++++++++++++++-----------------
>  hw/virtio-scsi.c |  8 +++++---
>  3 file modificati, 32 inserzioni(+), 21 rimozioni(-)
> -- 
> 1.7.12.1