[Qemu-devel] [PATCH 0/4] scsi fixes

[Qemu-devel] [PATCH 0/4] scsi fixes

[Hannes Reinecke]

Hi all,

these are some fixes I found during debugging my megasas HBA emulation.
This time I've sent them as a separate patchset for inclusion.
All of them have been acked, so please apply.

Hannes Reinecke (4):
  iov: Update parameter usage in iov_(to|from)_buf()
  scsi: Add 'hba_private' to SCSIRequest
  scsi-disk: Fixup debugging statement
  scsi-disk: Mask out serial number EVPD

 hw/esp.c               |    2 +-
 hw/lsi53c895a.c        |   22 +++++++-------------
 hw/scsi-bus.c          |    9 +++++--
 hw/scsi-disk.c         |   21 ++++++++++++++-----
 hw/scsi-generic.c      |    5 ++-
 hw/scsi.h              |   10 ++++++--
 hw/spapr_vscsi.c       |   29 ++++++++-------------------
 hw/usb-msd.c           |    9 +-------
 hw/virtio-net.c        |    2 +-
 hw/virtio-serial-bus.c |    2 +-
 iov.c                  |   49 ++++++++++++++++++++++++++---------------------
 iov.h                  |   10 ++++----
 12 files changed, 84 insertions(+), 86 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [PATCH 1/4] iov: Update parameter usage in iov_(to|from)_buf()

[Hannes Reinecke]

iov_to_buf() has an 'offset' parameter, iov_from_buf() hasn't.
This patch adds the missing parameter to iov_from_buf().
It also renames the 'offset' parameter to 'iov_off' to
emphasize it's the offset into the iovec and not the buffer.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Alexander Graf <agraf@suse.de>
---
 hw/virtio-net.c        |    2 +-
 hw/virtio-serial-bus.c |    2 +-
 iov.c                  |   49 ++++++++++++++++++++++++++---------------------
 iov.h                  |   10 ++++----
 4 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..a32cc01 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -657,7 +657,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_
 
         /* copy in packet.  ugh */
         len = iov_from_buf(sg, elem.in_num,
-                           buf + offset, size - offset);
+                           buf + offset, 0, size - offset);
         total += len;
         offset += len;
         /* If buffers can't be merged, at this point we
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7f6db7b..53c58d0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -103,7 +103,7 @@ static size_t write_to_port(VirtIOSerialPort *port,
         }
 
         len = iov_from_buf(elem.in_sg, elem.in_num,
-                           buf + offset, size - offset);
+                           buf + offset, 0, size - offset);
         offset += len;
 
         virtqueue_push(vq, &elem, len);
diff --git a/iov.c b/iov.c
index 588cd04..1e02791 100644
--- a/iov.c
+++ b/iov.c
@@ -14,56 +14,61 @@
 
 #include "iov.h"
 
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-                    const void *buf, size_t size)
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+                    const void *buf, size_t iov_off, size_t size)
 {
-    size_t offset;
+    size_t iovec_off, buf_off;
     unsigned int i;
 
-    offset = 0;
-    for (i = 0; offset < size && i < iovcnt; i++) {
-        size_t len;
+    iovec_off = 0;
+    buf_off = 0;
+    for (i = 0; i < iov_cnt && size; i++) {
+        if (iov_off < (iovec_off + iov[i].iov_len)) {
+            size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size);
 
-        len = MIN(iov[i].iov_len, size - offset);
+            memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off, len);
 
-        memcpy(iov[i].iov_base, buf + offset, len);
-        offset += len;
+            buf_off += len;
+            iov_off += len;
+            size -= len;
+        }
+        iovec_off += iov[i].iov_len;
     }
-    return offset;
+    return buf_off;
 }
 
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
-                  void *buf, size_t offset, size_t size)
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+                  void *buf, size_t iov_off, size_t size)
 {
     uint8_t *ptr;
-    size_t iov_off, buf_off;
+    size_t iovec_off, buf_off;
     unsigned int i;
 
     ptr = buf;
-    iov_off = 0;
+    iovec_off = 0;
     buf_off = 0;
-    for (i = 0; i < iovcnt && size; i++) {
-        if (offset < (iov_off + iov[i].iov_len)) {
-            size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
+    for (i = 0; i < iov_cnt && size; i++) {
+        if (iov_off < (iovec_off + iov[i].iov_len)) {
+            size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size);
 
-            memcpy(ptr + buf_off, iov[i].iov_base + (offset - iov_off), len);
+            memcpy(ptr + buf_off, iov[i].iov_base + (iov_off - iovec_off), len);
 
             buf_off += len;
-            offset += len;
+            iov_off += len;
             size -= len;
         }
-        iov_off += iov[i].iov_len;
+        iovec_off += iov[i].iov_len;
     }
     return buf_off;
 }
 
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt)
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt)
 {
     size_t len;
     unsigned int i;
 
     len = 0;
-    for (i = 0; i < iovcnt; i++) {
+    for (i = 0; i < iov_cnt; i++) {
         len += iov[i].iov_len;
     }
     return len;
diff --git a/iov.h b/iov.h
index 60a8547..110f67a 100644
--- a/iov.h
+++ b/iov.h
@@ -12,8 +12,8 @@
 
 #include "qemu-common.h"
 
-size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
-                    const void *buf, size_t size);
-size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
-                  void *buf, size_t offset, size_t size);
-size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+                    const void *buf, size_t iov_off, size_t size);
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
+                  void *buf, size_t iov_off, size_t size);
+size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt);
-- 
1.7.3.4

[Qemu-devel] [PATCH 2/4] scsi: Add 'hba_private' to SCSIRequest

[Hannes Reinecke]

'tag' is just an abstraction to identify the command
from the driver. So we should make that explicit by
replacing 'tag' with a driver-defined pointer 'hba_private'.
This saves the lookup for driver handling several commands
in parallel.
'tag' is still being kept for tracing purposes.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/esp.c          |    2 +-
 hw/lsi53c895a.c   |   22 ++++++++--------------
 hw/scsi-bus.c     |    9 ++++++---
 hw/scsi-disk.c    |    4 ++--
 hw/scsi-generic.c |    5 +++--
 hw/scsi.h         |   10 +++++++---
 hw/spapr_vscsi.c  |   29 +++++++++--------------------
 hw/usb-msd.c      |    9 +--------
 8 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 8e95672..69209bd 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
 
     DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
     lun = busid & 7;
-    s->current_req = scsi_req_new(s->current_dev, 0, lun);
+    s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
     datalen = scsi_req_enqueue(s->current_req, buf);
     s->ti_size = datalen;
     if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 940b43a..69eec1d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
 static void lsi_request_cancelled(SCSIRequest *req)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
-    lsi_request *p;
+    lsi_request *p = req->hba_private;
 
     if (s->current && req == s->current->req) {
         scsi_req_unref(req);
@@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
         return;
     }
 
-    p = lsi_find_by_tag(s, req->tag);
     if (p) {
         QTAILQ_REMOVE(&s->queue, p, next);
         scsi_req_unref(req);
@@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
 
 /* Record that data is available for a queued command.  Returns zero if
    the device was reselected, nonzero if the IO is deferred.  */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
+static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
 {
-    lsi_request *p;
-
-    p = lsi_find_by_tag(s, tag);
-    if (!p) {
-        BADF("IO with unknown tag %d\n", tag);
-        return 1;
-    }
+    lsi_request *p = req->hba_private;
 
     if (p->pending) {
-        BADF("Multiple IO pending for tag %d\n", tag);
+        BADF("Multiple IO pending for request %p\n", p);
     }
     p->pending = len;
     /* Reselect if waiting for it, or if reselection triggers an IRQ
@@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
-    if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
+    if (s->waiting == 1 || !s->current || req->hba_private != s->current ||
         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
-        if (lsi_queue_tag(s, req->tag, len)) {
+        if (lsi_queue_req(s, req, len)) {
             return;
         }
     }
@@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
     assert(s->current == NULL);
     s->current = qemu_mallocz(sizeof(lsi_request));
     s->current->tag = s->select_tag;
-    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
+    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
+                                   s->current);
 
     n = scsi_req_enqueue(s->current->req, buf);
     if (n) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ad6a730..8b1a412 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
     return res;
 }
 
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+                            uint32_t lun, void *hba_private)
 {
     SCSIRequest *req;
 
@@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     req->dev = d;
     req->tag = tag;
     req->lun = lun;
+    req->hba_private = hba_private;
     req->status = -1;
     trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
     return req;
 }
 
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+                          void *hba_private)
 {
-    return d->info->alloc_req(d, tag, lun);
+    return d->info->alloc_req(d, tag, lun, hba_private);
 }
 
 uint8_t *scsi_req_get_buf(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a8c7372..c2a99fe 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
 static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
 
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
-        uint32_t lun)
+                                     uint32_t lun, void *hba_private)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
     SCSIRequest *req;
     SCSIDiskReq *r;
 
-    req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun);
+    req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun, hba_private);
     r = DO_UPCAST(SCSIDiskReq, req, req);
     r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
     return req;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 8e59c7e..90345a7 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -96,11 +96,12 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
     return size;
 }
 
-static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
+static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
+                                     void *hba_private)
 {
     SCSIRequest *req;
 
-    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
+    req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun, hba_private);
     return req;
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index c1dca35..6b15bbc 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -43,6 +43,7 @@ struct SCSIRequest {
     } cmd;
     BlockDriverAIOCB  *aiocb;
     bool enqueued;
+    void *hba_private;
     QTAILQ_ENTRY(SCSIRequest) next;
 };
 
@@ -67,7 +68,8 @@ struct SCSIDeviceInfo {
     DeviceInfo qdev;
     scsi_qdev_initfn init;
     void (*destroy)(SCSIDevice *s);
-    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
+    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
+                              void *hba_private);
     void (*free_req)(SCSIRequest *req);
     int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
     void (*read_data)(SCSIRequest *req);
@@ -138,8 +140,10 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
 int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
 int scsi_sense_valid(SCSISense sense);
 
-SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
-SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
+SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
+                            uint32_t lun, void *hba_private);
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
+                          void *hba_private);
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 1c901ef..9e1cb2e 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -121,7 +121,7 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
     return NULL;
 }
 
-static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
+static void vscsi_put_req(vscsi_req *req)
 {
     if (req->sreq != NULL) {
         scsi_req_unref(req->sreq);
@@ -130,15 +130,6 @@ static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
     req->active = 0;
 }
 
-static vscsi_req *vscsi_find_req(VSCSIState *s, SCSIRequest *req)
-{
-    uint32_t tag = req->tag;
-    if (tag >= VSCSI_REQ_LIMIT || !s->reqs[tag].active) {
-        return NULL;
-    }
-    return &s->reqs[tag];
-}
-
 static void vscsi_decode_id_lun(uint64_t srp_lun, int *id, int *lun)
 {
     /* XXX Figure that one out properly ! This is crackpot */
@@ -454,7 +445,7 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     if (n) {
         req->senselen = n;
         vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-        vscsi_put_req(s, req);
+        vscsi_put_req(req);
         return;
     }
 
@@ -483,7 +474,7 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, sreq);
+    vscsi_req *req = sreq->hba_private;
     uint8_t *buf;
     int rc = 0;
 
@@ -530,8 +521,7 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
 static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status)
 {
-    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, sreq);
+    vscsi_req *req = sreq->hba_private;
     int32_t res_in = 0, res_out = 0;
 
     dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x status=0x%x, req=%p\n",
@@ -563,15 +553,14 @@ static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status)
         }
     }
     vscsi_send_rsp(s, req, 0, res_in, res_out);
-    vscsi_put_req(s, req);
+    vscsi_put_req(req);
 }
 
 static void vscsi_request_cancelled(SCSIRequest *sreq)
 {
-    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, sreq);
+    vscsi_req *req = sreq->hba_private;
 
-    vscsi_put_req(s, req);
+    vscsi_put_req(req);
 }
 
 static void vscsi_process_login(VSCSIState *s, vscsi_req *req)
@@ -659,7 +648,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
     }
 
     req->lun = lun;
-    req->sreq = scsi_req_new(sdev, req->qtag, lun);
+    req->sreq = scsi_req_new(sdev, req->qtag, lun, req);
     n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
 
     dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
@@ -858,7 +847,7 @@ static void vscsi_got_payload(VSCSIState *s, vscsi_crq *crq)
     }
 
     if (done) {
-        vscsi_put_req(s, req);
+        vscsi_put_req(req);
     }
 }
 
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 86582cc..bfea096 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -216,10 +216,6 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
 
-    if (req->tag != s->tag) {
-        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
-    }
-
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = len;
     s->scsi_buf = scsi_req_get_buf(req);
@@ -241,9 +237,6 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
 
-    if (req->tag != s->tag) {
-        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
-    }
     DPRINTF("Command complete %d\n", status);
     s->residue = s->data_len;
     s->result = status != 0;
@@ -387,7 +380,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
                     s->tag, cbw.flags, cbw.cmd_len, s->data_len);
             s->residue = 0;
             s->scsi_len = 0;
-            s->req = scsi_req_new(s->scsi_dev, s->tag, 0);
+            s->req = scsi_req_new(s->scsi_dev, s->tag, 0, NULL);
             scsi_req_enqueue(s->req, cbw.cmd);
             /* ??? Should check that USB and SCSI data transfer
                directions match.  */
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/4] scsi-disk: Fixup debugging statement

[Hannes Reinecke]

A debugging statement wasn't converted to the new interface.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c2a99fe..5804662 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1007,7 +1007,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
 
     command = buf[0];
     outbuf = (uint8_t *)r->iov.iov_base;
-    DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
+    DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", req->lun, req->tag, buf[0]);
 
     if (scsi_req_parse(&r->req, buf) != 0) {
         BADF("Unsupported command length, command %x\n", command);
-- 
1.7.3.4

[Qemu-devel] [PATCH 4/4] scsi-disk: Mask out serial number EVPD

[Hannes Reinecke]

If the serial number is not set we should mask it out in the
list of supported VPD pages and mark it as not supported.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5804662..05d14ab 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -398,7 +398,8 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
                     "buffer size %zd\n", req->cmd.xfer);
             pages = buflen++;
             outbuf[buflen++] = 0x00; // list of supported pages (this page)
-            outbuf[buflen++] = 0x80; // unit serial number
+            if (s->serial)
+                outbuf[buflen++] = 0x80; // unit serial number
             outbuf[buflen++] = 0x83; // device identification
             if (s->drive_kind == SCSI_HD) {
                 outbuf[buflen++] = 0xb0; // block limits
@@ -409,8 +410,14 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
         }
         case 0x80: /* Device serial number, optional */
         {
-            int l = strlen(s->serial);
+            int l;
 
+            if (!s->serial) {
+                DPRINTF("Inquiry (EVPD[Serial number] not supported\n");
+                return -1;
+            }
+
+            l = strlen(s->serial);
             if (l > req->cmd.xfer)
                 l = req->cmd.xfer;
             if (l > 20)
@@ -1203,7 +1210,9 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
     if (!s->serial) {
         /* try to fall back to value set with legacy -drive serial=... */
         dinfo = drive_get_by_blockdev(s->bs);
-        s->serial = qemu_strdup(*dinfo->serial ? dinfo->serial : "0");
+        if (*dinfo->serial) {
+            s->serial = qemu_strdup(dinfo->serial);
+        }
     }
 
     if (!s->version) {
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Stefan Hajnoczi]

On Mon, Jul 11, 2011 at 2:02 PM, Hannes Reinecke <hare@suse.de> wrote:
> Hi all,
>
> these are some fixes I found during debugging my megasas HBA emulation.
> This time I've sent them as a separate patchset for inclusion.
> All of them have been acked, so please apply.

Are SCSI patches going through Kevin's tree?

If not, perhaps Paolo or I should keep a tree and start doing some
sanity testing on the subsystem in the future.

Stefan

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Kevin Wolf]

Am 11.07.2011 15:34, schrieb Stefan Hajnoczi:
> On Mon, Jul 11, 2011 at 2:02 PM, Hannes Reinecke <hare@suse.de> wrote:
>> Hi all,
>>
>> these are some fixes I found during debugging my megasas HBA emulation.
>> This time I've sent them as a separate patchset for inclusion.
>> All of them have been acked, so please apply.
> 
> Are SCSI patches going through Kevin's tree?
> 
> If not, perhaps Paolo or I should keep a tree and start doing some
> sanity testing on the subsystem in the future.

As long as we don't have a SCSI maintainer, I'm going to pick them up
for the block tree when they have receive some review.

Doesn't mean that nobody should be doing sanity testing, of course. If
anyone wants to take care of picking up and reviewing all SCSI patches,
I'm also happy to pull from a separate tree.

Kevin

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Hannes Reinecke]

On 07/11/2011 03:42 PM, Kevin Wolf wrote:
> Am 11.07.2011 15:34, schrieb Stefan Hajnoczi:
>> On Mon, Jul 11, 2011 at 2:02 PM, Hannes Reinecke<hare@suse.de>  wrote:
>>> Hi all,
>>>
>>> these are some fixes I found during debugging my megasas HBA emulation.
>>> This time I've sent them as a separate patchset for inclusion.
>>> All of them have been acked, so please apply.
>>
>> Are SCSI patches going through Kevin's tree?
>>
>> If not, perhaps Paolo or I should keep a tree and start doing some
>> sanity testing on the subsystem in the future.
>
> As long as we don't have a SCSI maintainer, I'm going to pick them up
> for the block tree when they have receive some review.
>
> Doesn't mean that nobody should be doing sanity testing, of course. If
> anyone wants to take care of picking up and reviewing all SCSI patches,
> I'm also happy to pull from a separate tree.
>
Patches have already been reviewed and tested, in conjunction with 
my megasas HBA emulation patchset.
This is just a repost as a separate patchset to get them in.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Kevin Wolf]

Am 11.07.2011 15:02, schrieb Hannes Reinecke:
> Hi all,
> 
> these are some fixes I found during debugging my megasas HBA emulation.
> This time I've sent them as a separate patchset for inclusion.
> All of them have been acked, so please apply.
> 
> Hannes Reinecke (4):
>   iov: Update parameter usage in iov_(to|from)_buf()
>   scsi: Add 'hba_private' to SCSIRequest
>   scsi-disk: Fixup debugging statement
>   scsi-disk: Mask out serial number EVPD

Thanks, applied all to the block branch.

Kevin

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Hannes Reinecke]

On 07/12/2011 03:37 PM, Kevin Wolf wrote:
> Am 11.07.2011 15:02, schrieb Hannes Reinecke:
>> Hi all,
>>
>> these are some fixes I found during debugging my megasas HBA emulation.
>> This time I've sent them as a separate patchset for inclusion.
>> All of them have been acked, so please apply.
>>
>> Hannes Reinecke (4):
>>    iov: Update parameter usage in iov_(to|from)_buf()
>>    scsi: Add 'hba_private' to SCSIRequest
>>    scsi-disk: Fixup debugging statement
>>    scsi-disk: Mask out serial number EVPD
>
> Thanks, applied all to the block branch.
>
Any chance to have them pulled into the main tree?
My megasas emulation relies on them, and it feels a bit
stupid to send a patch relying on some fixes not in mainline.
At the same time it's really stupid to resend the entire
patchset again ...

Thanks.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Kevin Wolf]

Am 19.07.2011 08:31, schrieb Hannes Reinecke:
> On 07/12/2011 03:37 PM, Kevin Wolf wrote:
>> Am 11.07.2011 15:02, schrieb Hannes Reinecke:
>>> Hi all,
>>>
>>> these are some fixes I found during debugging my megasas HBA emulation.
>>> This time I've sent them as a separate patchset for inclusion.
>>> All of them have been acked, so please apply.
>>>
>>> Hannes Reinecke (4):
>>>    iov: Update parameter usage in iov_(to|from)_buf()
>>>    scsi: Add 'hba_private' to SCSIRequest
>>>    scsi-disk: Fixup debugging statement
>>>    scsi-disk: Mask out serial number EVPD
>>
>> Thanks, applied all to the block branch.
>>
> Any chance to have them pulled into the main tree?
> My megasas emulation relies on them, and it feels a bit
> stupid to send a patch relying on some fixes not in mainline.
> At the same time it's really stupid to resend the entire
> patchset again ...

I'm hoping to send a pull request today, now that the VMDK patches look
good finally.

Anyway, I don't think that not having the patches in master yet should
stop you from going forward with the next patches. They will go through
the block tree anyway, so basing them on that tree is fine (and you
wouldn't be the first one to do that). Just state in PATCH 0/n for the
reviewers that it depends on the other patches.

Kevin

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Hannes Reinecke]

On 07/19/2011 09:39 AM, Kevin Wolf wrote:
> Am 19.07.2011 08:31, schrieb Hannes Reinecke:
>> On 07/12/2011 03:37 PM, Kevin Wolf wrote:
>>> Am 11.07.2011 15:02, schrieb Hannes Reinecke:
>>>> Hi all,
>>>>
>>>> these are some fixes I found during debugging my megasas HBA emulation.
>>>> This time I've sent them as a separate patchset for inclusion.
>>>> All of them have been acked, so please apply.
>>>>
>>>> Hannes Reinecke (4):
>>>>     iov: Update parameter usage in iov_(to|from)_buf()
>>>>     scsi: Add 'hba_private' to SCSIRequest
>>>>     scsi-disk: Fixup debugging statement
>>>>     scsi-disk: Mask out serial number EVPD
>>>
>>> Thanks, applied all to the block branch.
>>>
>> Any chance to have them pulled into the main tree?
>> My megasas emulation relies on them, and it feels a bit
>> stupid to send a patch relying on some fixes not in mainline.
>> At the same time it's really stupid to resend the entire
>> patchset again ...
>
> I'm hoping to send a pull request today, now that the VMDK patches look
> good finally.
>
> Anyway, I don't think that not having the patches in master yet should
> stop you from going forward with the next patches. They will go through
> the block tree anyway, so basing them on that tree is fine (and you
> wouldn't be the first one to do that). Just state in PATCH 0/n for the
> reviewers that it depends on the other patches.
>
Well, the remaining patch is 'just' the megasas emulation itself.
And I want to make reviewing that as easy as possible, so that it's 
not again being held off by complains about missing patches.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/4] scsi fixes

[Alexander Graf]

On 19.07.2011, at 10:10, Hannes Reinecke wrote:

> On 07/19/2011 09:39 AM, Kevin Wolf wrote:
>> Am 19.07.2011 08:31, schrieb Hannes Reinecke:
>>> On 07/12/2011 03:37 PM, Kevin Wolf wrote:
>>>> Am 11.07.2011 15:02, schrieb Hannes Reinecke:
>>>>> Hi all,
>>>>> 
>>>>> these are some fixes I found during debugging my megasas HBA emulation.
>>>>> This time I've sent them as a separate patchset for inclusion.
>>>>> All of them have been acked, so please apply.
>>>>> 
>>>>> Hannes Reinecke (4):
>>>>>    iov: Update parameter usage in iov_(to|from)_buf()
>>>>>    scsi: Add 'hba_private' to SCSIRequest
>>>>>    scsi-disk: Fixup debugging statement
>>>>>    scsi-disk: Mask out serial number EVPD
>>>> 
>>>> Thanks, applied all to the block branch.
>>>> 
>>> Any chance to have them pulled into the main tree?
>>> My megasas emulation relies on them, and it feels a bit
>>> stupid to send a patch relying on some fixes not in mainline.
>>> At the same time it's really stupid to resend the entire
>>> patchset again ...
>> 
>> I'm hoping to send a pull request today, now that the VMDK patches look
>> good finally.
>> 
>> Anyway, I don't think that not having the patches in master yet should
>> stop you from going forward with the next patches. They will go through
>> the block tree anyway, so basing them on that tree is fine (and you
>> wouldn't be the first one to do that). Just state in PATCH 0/n for the
>> reviewers that it depends on the other patches.
>> 
> Well, the remaining patch is 'just' the megasas emulation itself.
> And I want to make reviewing that as easy as possible, so that it's not again being held off by complains about missing patches.

Yes, no worries. Just state in the patch description that it's based on the block branch and actually do base it on it - the endianness specific ld./st. patches are upstream, so everything you need should be in that branch.


Alex