[Qemu-devel] [PULL 00/16] Block patches

[Qemu-devel] [PULL 00/16] Block patches

[Kevin Wolf]

The following changes since commit f711df67d611e4762966a249742a5f7499e19f99:

  microblaze: target-ify target_ucontext (2010-11-23 10:04:30 +0100)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Avi Kivity (1):
      ide: convert bmdma address ioport to ioport_register()

Christoph Hellwig (1):
      raw-posix: raw_pwrite comment fixup

Hannes Reinecke (5):
      scsi: Increase the number of possible devices
      scsi: Return SAM status codes
      scsi: INQUIRY VPD fixes
      scsi: Move sense handling into the driver
      scsi-disk: Remove duplicate cdb parsing

Kevin Wolf (5):
      block: Remove unused s->hd in various drivers
      ide: Factor ide_dma_set_inactive out
      ide: Set bus master inactive on error
      ide: Ignore double DMA transfer starts/stops
      ide: Reset current_addr after stopping DMA

Marcelo Tosatti (1):
      block migration: do not submit multiple AIOs for same sector (v2)

Ryan Harper (1):
      Implement drive_del to decouple block removal from device removal

Stefan Hajnoczi (1):
      scsi-disk: Move active request asserts

Stefano Stabellini (1):
      qemu and qemu-xen: support empty write barriers in xen_disk

 block-migration.c |   14 ++----
 block/qcow.c      |    1 -
 block/qcow2.h     |    1 -
 block/raw-posix.c |    2 +-
 block/vdi.c       |    1 -
 block/vmdk.c      |    1 -
 block/vpc.c       |    2 -
 blockdev.c        |   39 +++++++++++++++
 blockdev.h        |    3 +-
 hmp-commands.hx   |   18 +++++++
 hw/ide/cmd646.c   |    8 +--
 hw/ide/core.c     |   31 ++++++------
 hw/ide/internal.h |    2 +
 hw/ide/pci.c      |  131 +++++++++++++++++++-------------------------------
 hw/ide/pci.h      |    7 +--
 hw/ide/piix.c     |    8 +--
 hw/ide/via.c      |    8 +--
 hw/scsi-bus.c     |   12 +----
 hw/scsi-defs.h    |   20 ++++----
 hw/scsi-disk.c    |  137 +++++++++++++++++++++++++---------------------------
 hw/scsi-generic.c |   10 ++--
 hw/scsi.h         |   11 +----
 hw/xen_disk.c     |   12 ++++-
 23 files changed, 234 insertions(+), 245 deletions(-)

[Qemu-devel] [PATCH 01/16] scsi-disk: Move active request asserts

[Kevin Wolf]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

SCSI read/write requests should not be re-issued before the current
fragment of I/O completes.  There are asserts in scsi-disk.c that guard
this constraint but they trigger on SPARC Linux 2.4.  It turns out that
the asserts are too early in the code path and don't allow for read
requests to terminate.

Only the read assert needs to be moved but move the write assert too for
consistency.

Reported-by: Nigel Horne <njh@bandsman.co.uk>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-disk.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index dc71957..7d85899 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -170,6 +170,9 @@ static void scsi_read_request(SCSIDiskReq *r)
         return;
     }
 
+    /* No data transfer may already be in progress */
+    assert(r->req.aiocb == NULL);
+
     n = r->sector_count;
     if (n > SCSI_DMA_BUF_SIZE / 512)
         n = SCSI_DMA_BUF_SIZE / 512;
@@ -197,9 +200,6 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
         return;
     }
 
-    /* No data transfer may already be in progress */
-    assert(r->req.aiocb == NULL);
-
     scsi_read_request(r);
 }
 
@@ -269,6 +269,9 @@ static void scsi_write_request(SCSIDiskReq *r)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint32_t n;
 
+    /* No data transfer may already be in progress */
+    assert(r->req.aiocb == NULL);
+
     n = r->iov.iov_len / 512;
     if (n) {
         qemu_iovec_init_external(&r->qiov, &r->iov, 1);
@@ -298,9 +301,6 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag)
         return 1;
     }
 
-    /* No data transfer may already be in progress */
-    assert(r->req.aiocb == NULL);
-
     scsi_write_request(r);
 
     return 0;
-- 
1.7.2.3

[Qemu-devel] [PATCH 02/16] Implement drive_del to decouple block removal from device removal

[Kevin Wolf]

From: Ryan Harper <ryanh@us.ibm.com>

Currently device hotplug removal code is tied to device removal via
ACPI.  All pci devices that are removable via device_del() require the
guest to respond to the request.  In some cases the guest may not
respond leaving the device still accessible to the guest.  The management
layer doesn't currently have a reliable way to revoke access to host
resource in the presence of an uncooperative guest.

This patch implements a new monitor command, drive_del, which
provides an explicit command to revoke access to a host block device.

drive_del first quiesces the block device (qemu_aio_flush;
bdrv_flush() and bdrv_close()).  This prevents further IO from being
submitted against the host device.  Finally, drive_del cleans up
pointers between the drive object (host resource) and the device
object (guest resource).

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c      |   39 +++++++++++++++++++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   18 ++++++++++++++++++
 3 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6cb179a..f6ac439 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -14,6 +14,8 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 #include "sysemu.h"
+#include "hw/qdev.h"
+#include "block_int.h"
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
 
@@ -597,3 +599,40 @@ int do_change_block(Monitor *mon, const char *device,
     }
     return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
 }
+
+int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    BlockDriverState *bs;
+    BlockDriverState **ptr;
+    Property *prop;
+
+    bs = bdrv_find(id);
+    if (!bs) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, id);
+        return -1;
+    }
+
+    /* quiesce block driver; prevent further io */
+    qemu_aio_flush();
+    bdrv_flush(bs);
+    bdrv_close(bs);
+
+    /* clean up guest state from pointing to host resource by
+     * finding and removing DeviceState "drive" property */
+    for (prop = bs->peer->info->props; prop && prop->name; prop++) {
+        if (prop->info->type == PROP_TYPE_DRIVE) {
+            ptr = qdev_get_prop_ptr(bs->peer, prop);
+            if ((*ptr) == bs) {
+                bdrv_detach(bs, bs->peer);
+                *ptr = NULL;
+                break;
+            }
+        }
+    }
+
+    /* clean up host side */
+    drive_uninit(drive_get_by_blockdev(bs));
+
+    return 0;
+}
diff --git a/blockdev.h b/blockdev.h
index 653affc..2a0559e 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -51,5 +51,6 @@ int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt);
+int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e5585ba..23024ba 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -68,6 +68,24 @@ Eject a removable medium (use -f to force it).
 ETEXI
 
     {
+        .name       = "drive_del",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "remove host block device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_drive_del,
+    },
+
+STEXI
+@item drive_del @var{device}
+@findex drive_del
+Remove host block device.  The result is that guest generated IO is no longer
+submitted against the host device underlying the disk.  Once a drive has
+been deleted, the QEMU Block layer returns -EIO which results in IO
+errors in the guest for applications that are reading/writing to the device.
+ETEXI
+
+    {
         .name       = "change",
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
-- 
1.7.2.3

[Qemu-devel] [PATCH 03/16] block migration: do not submit multiple AIOs for same sector (v2)

[Kevin Wolf]

From: Marcelo Tosatti <mtosatti@redhat.com>

An old version of this patch was applied to master, so this contains the
differences between v1 and v2.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block-migration.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 3e66f49..1475325 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -146,8 +146,7 @@ static int bmds_aio_inflight(BlkMigDevState *bmds, int64_t sector)
 {
     int64_t chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK;
 
-    if (bmds->aio_bitmap &&
-        (sector << BDRV_SECTOR_BITS) < bdrv_getlength(bmds->bs)) {
+    if ((sector << BDRV_SECTOR_BITS) < bdrv_getlength(bmds->bs)) {
         return !!(bmds->aio_bitmap[chunk / (sizeof(unsigned long) * 8)] &
             (1UL << (chunk % (sizeof(unsigned long) * 8))));
     } else {
@@ -169,13 +168,9 @@ static void bmds_set_aio_inflight(BlkMigDevState *bmds, int64_t sector_num,
         bit = start % (sizeof(unsigned long) * 8);
         val = bmds->aio_bitmap[idx];
         if (set) {
-            if (!(val & (1UL << bit))) {
-                val |= 1UL << bit;
-            }
+            val |= 1UL << bit;
         } else {
-            if (val & (1UL << bit)) {
-                val &= ~(1UL << bit);
-            }
+            val &= ~(1UL << bit);
         }
         bmds->aio_bitmap[idx] = val;
     }
@@ -385,8 +380,9 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
     int nr_sectors;
 
     for (sector = bmds->cur_dirty; sector < bmds->total_sectors;) {
-        if (bmds_aio_inflight(bmds, sector))
+        if (bmds_aio_inflight(bmds, sector)) {
             qemu_aio_flush();
+        }
         if (bdrv_get_dirty(bmds->bs, sector)) {
 
             if (total_sectors - sector < BDRV_SECTORS_PER_DIRTY_CHUNK) {
-- 
1.7.2.3

[Qemu-devel] [PATCH 04/16] ide: convert bmdma address ioport to ioport_register()

[Kevin Wolf]

From: Avi Kivity <avi@redhat.com>

cmd646, via compile tested, pci lightly boot tested.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/cmd646.c   |    8 +----
 hw/ide/internal.h |    2 +
 hw/ide/pci.c      |   71 +++++++++++++---------------------------------------
 hw/ide/pci.h      |    7 +----
 hw/ide/piix.c     |    8 +----
 hw/ide/via.c      |    8 +----
 6 files changed, 27 insertions(+), 77 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index ff80dd5..dfe6091 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -179,12 +179,8 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,
             register_ioport_read(addr, 4, 1, bmdma_readb_1, d);
         }
 
-        register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm);
-        register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);
-        register_ioport_write(addr + 4, 4, 2, bmdma_addr_writew, bm);
-        register_ioport_read(addr + 4, 4, 2, bmdma_addr_readw, bm);
-        register_ioport_write(addr + 4, 4, 4, bmdma_addr_writel, bm);
-        register_ioport_read(addr + 4, 4, 4, bmdma_addr_readl, bm);
+        iorange_init(&bm->addr_ioport, &bmdma_addr_ioport_ops, addr + 4, 4);
+        ioport_register(&bm->addr_ioport);
         addr += 8;
     }
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index d652e06..85f4a16 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -8,6 +8,7 @@
  */
 #include <hw/ide.h>
 #include "block_int.h"
+#include "iorange.h"
 
 /* debug IDE devices */
 //#define DEBUG_IDE
@@ -496,6 +497,7 @@ struct BMDMAState {
     QEMUIOVector qiov;
     int64_t sector_num;
     uint32_t nsector;
+    IORange addr_ioport;
     QEMUBH *bh;
 };
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index ec90f26..3722b77 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -73,72 +73,37 @@ void bmdma_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-uint32_t bmdma_addr_readb(void *opaque, uint32_t addr)
+static void bmdma_addr_read(IORange *ioport, uint64_t addr,
+                            unsigned width, uint64_t *data)
 {
-    BMDMAState *bm = opaque;
-    uint32_t val;
-    val = (bm->addr >> ((addr & 3) * 8)) & 0xff;
-#ifdef DEBUG_IDE
-    printf("%s: 0x%08x\n", __func__, val);
-#endif
-    return val;
-}
+    BMDMAState *bm = container_of(ioport, BMDMAState, addr_ioport);
+    uint32_t mask = (1ULL << (width * 8)) - 1;
 
-void bmdma_addr_writeb(void *opaque, uint32_t addr, uint32_t val)
-{
-    BMDMAState *bm = opaque;
-    int shift = (addr & 3) * 8;
+    *data = (bm->addr >> (addr * 8)) & mask;
 #ifdef DEBUG_IDE
-    printf("%s: 0x%08x\n", __func__, val);
+    printf("%s: 0x%08x\n", __func__, (unsigned)*data);
 #endif
-    bm->addr &= ~(0xFF << shift);
-    bm->addr |= ((val & 0xFF) << shift) & ~3;
-    bm->cur_addr = bm->addr;
 }
 
-uint32_t bmdma_addr_readw(void *opaque, uint32_t addr)
+static void bmdma_addr_write(IORange *ioport, uint64_t addr,
+                             unsigned width, uint64_t data)
 {
-    BMDMAState *bm = opaque;
-    uint32_t val;
-    val = (bm->addr >> ((addr & 3) * 8)) & 0xffff;
-#ifdef DEBUG_IDE
-    printf("%s: 0x%08x\n", __func__, val);
-#endif
-    return val;
-}
+    BMDMAState *bm = container_of(ioport, BMDMAState, addr_ioport);
+    int shift = addr * 8;
+    uint32_t mask = (1ULL << (width * 8)) - 1;
 
-void bmdma_addr_writew(void *opaque, uint32_t addr, uint32_t val)
-{
-    BMDMAState *bm = opaque;
-    int shift = (addr & 3) * 8;
 #ifdef DEBUG_IDE
-    printf("%s: 0x%08x\n", __func__, val);
+    printf("%s: 0x%08x\n", __func__, (unsigned)data);
 #endif
-    bm->addr &= ~(0xFFFF << shift);
-    bm->addr |= ((val & 0xFFFF) << shift) & ~3;
+    bm->addr &= ~(mask << shift);
+    bm->addr |= ((data & mask) << shift) & ~3;
     bm->cur_addr = bm->addr;
 }
 
-uint32_t bmdma_addr_readl(void *opaque, uint32_t addr)
-{
-    BMDMAState *bm = opaque;
-    uint32_t val;
-    val = bm->addr;
-#ifdef DEBUG_IDE
-    printf("%s: 0x%08x\n", __func__, val);
-#endif
-    return val;
-}
-
-void bmdma_addr_writel(void *opaque, uint32_t addr, uint32_t val)
-{
-    BMDMAState *bm = opaque;
-#ifdef DEBUG_IDE
-    printf("%s: 0x%08x\n", __func__, val);
-#endif
-    bm->addr = val & ~3;
-    bm->cur_addr = bm->addr;
-}
+const IORangeOps bmdma_addr_ioport_ops = {
+    .read = bmdma_addr_read,
+    .write = bmdma_addr_write,
+};
 
 static bool ide_bmdma_current_needed(void *opaque)
 {
diff --git a/hw/ide/pci.h b/hw/ide/pci.h
index d46a95e..b81b26c 100644
--- a/hw/ide/pci.h
+++ b/hw/ide/pci.h
@@ -11,12 +11,7 @@ typedef struct PCIIDEState {
 } PCIIDEState;
 
 void bmdma_cmd_writeb(void *opaque, uint32_t addr, uint32_t val);
-uint32_t bmdma_addr_readb(void *opaque, uint32_t addr);
-void bmdma_addr_writeb(void *opaque, uint32_t addr, uint32_t val);
-uint32_t bmdma_addr_readw(void *opaque, uint32_t addr);
-void bmdma_addr_writew(void *opaque, uint32_t addr, uint32_t val);
-uint32_t bmdma_addr_readl(void *opaque, uint32_t addr);
-void bmdma_addr_writel(void *opaque, uint32_t addr, uint32_t val);
+extern const IORangeOps bmdma_addr_ioport_ops;
 void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table);
 
 extern const VMStateDescription vmstate_ide_pci;
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 07483e8..e02b89a 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -85,12 +85,8 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,
         register_ioport_write(addr + 1, 3, 1, bmdma_writeb, bm);
         register_ioport_read(addr, 4, 1, bmdma_readb, bm);
 
-        register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm);
-        register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);
-        register_ioport_write(addr + 4, 4, 2, bmdma_addr_writew, bm);
-        register_ioport_read(addr + 4, 4, 2, bmdma_addr_readw, bm);
-        register_ioport_write(addr + 4, 4, 4, bmdma_addr_writel, bm);
-        register_ioport_read(addr + 4, 4, 4, bmdma_addr_readl, bm);
+        iorange_init(&bm->addr_ioport, &bmdma_addr_ioport_ops, addr + 4, 4);
+        ioport_register(&bm->addr_ioport);
         addr += 8;
     }
 }
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b2c7cad..3e41d00 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -87,12 +87,8 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,
         register_ioport_write(addr + 1, 3, 1, bmdma_writeb, bm);
         register_ioport_read(addr, 4, 1, bmdma_readb, bm);
 
-        register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm);
-        register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);
-        register_ioport_write(addr + 4, 4, 2, bmdma_addr_writew, bm);
-        register_ioport_read(addr + 4, 4, 2, bmdma_addr_readw, bm);
-        register_ioport_write(addr + 4, 4, 4, bmdma_addr_writel, bm);
-        register_ioport_read(addr + 4, 4, 4, bmdma_addr_readl, bm);
+        iorange_init(&bm->addr_ioport, &bmdma_addr_ioport_ops, addr + 4, 4);
+        ioport_register(&bm->addr_ioport);
         addr += 8;
     }
 }
-- 
1.7.2.3

[Qemu-devel] [PATCH 05/16] qemu and qemu-xen: support empty write barriers in xen_disk

[Kevin Wolf]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

This patch can be applied to both qemu-xen and qemu and adds support
for empty write barriers to xen_disk.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/xen_disk.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 134ac33..85a1c85 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -181,6 +181,10 @@ static int ioreq_parse(struct ioreq *ioreq)
 	ioreq->prot = PROT_WRITE; /* to memory */
 	break;
     case BLKIF_OP_WRITE_BARRIER:
+        if (!ioreq->req.nr_segments) {
+            ioreq->presync = 1;
+            return 0;
+        }
 	if (!syncwrite)
 	    ioreq->presync = ioreq->postsync = 1;
 	/* fall through */
@@ -305,7 +309,7 @@ static int ioreq_runio_qemu_sync(struct ioreq *ioreq)
     int i, rc, len = 0;
     off_t pos;
 
-    if (ioreq_map(ioreq) == -1)
+    if (ioreq->req.nr_segments && ioreq_map(ioreq) == -1)
 	goto err;
     if (ioreq->presync)
 	bdrv_flush(blkdev->bs);
@@ -329,6 +333,8 @@ static int ioreq_runio_qemu_sync(struct ioreq *ioreq)
 	break;
     case BLKIF_OP_WRITE:
     case BLKIF_OP_WRITE_BARRIER:
+        if (!ioreq->req.nr_segments)
+            break;
 	pos = ioreq->start;
 	for (i = 0; i < ioreq->v.niov; i++) {
 	    rc = bdrv_write(blkdev->bs, pos / BLOCK_SIZE,
@@ -386,7 +392,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    if (ioreq_map(ioreq) == -1)
+    if (ioreq->req.nr_segments && ioreq_map(ioreq) == -1)
 	goto err;
 
     ioreq->aio_inflight++;
@@ -403,6 +409,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     case BLKIF_OP_WRITE:
     case BLKIF_OP_WRITE_BARRIER:
         ioreq->aio_inflight++;
+        if (!ioreq->req.nr_segments)
+            break;
         bdrv_aio_writev(blkdev->bs, ioreq->start / BLOCK_SIZE,
                         &ioreq->v, ioreq->v.size / BLOCK_SIZE,
                         qemu_aio_complete, ioreq);
-- 
1.7.2.3

[Qemu-devel] [PATCH 06/16] block: Remove unused s->hd in various drivers

[Kevin Wolf]

All drivers use bs->file instead of s->hd for quite a while now, so it's time
to remove s->hd.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qcow.c  |    1 -
 block/qcow2.h |    1 -
 block/vdi.c   |    1 -
 block/vmdk.c  |    1 -
 block/vpc.c   |    2 --
 5 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 9cd547d..f67d3d3 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -54,7 +54,6 @@ typedef struct QCowHeader {
 #define L2_CACHE_SIZE 16
 
 typedef struct BDRVQcowState {
-    BlockDriverState *hd;
     int cluster_bits;
     int cluster_size;
     int cluster_sectors;
diff --git a/block/qcow2.h b/block/qcow2.h
index 2d22e5e..5217bea 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -79,7 +79,6 @@ typedef struct QCowSnapshot {
 } QCowSnapshot;
 
 typedef struct BDRVQcowState {
-    BlockDriverState *hd;
     int cluster_bits;
     int cluster_size;
     int cluster_sectors;
diff --git a/block/vdi.c b/block/vdi.c
index 3b51e53..ab8f70f 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -186,7 +186,6 @@ typedef struct {
 } VdiHeader;
 
 typedef struct {
-    BlockDriverState *hd;
     /* The block map entries are little endian (even in memory). */
     uint32_t *bmap;
     /* Size of block (bytes). */
diff --git a/block/vmdk.c b/block/vmdk.c
index 872aeba..8fc9d67 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -61,7 +61,6 @@ typedef struct {
 #define L2_CACHE_SIZE 16
 
 typedef struct BDRVVmdkState {
-    BlockDriverState *hd;
     int64_t l1_table_offset;
     int64_t l1_backup_table_offset;
     uint32_t *l1_table;
diff --git a/block/vpc.c b/block/vpc.c
index 416f489..21e2a68 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -110,8 +110,6 @@ struct vhd_dyndisk_header {
 };
 
 typedef struct BDRVVPCState {
-    BlockDriverState *hd;
-
     uint8_t footer_buf[HEADER_SIZE];
     uint64_t free_data_block_offset;
     int max_table_entries;
-- 
1.7.2.3

[Qemu-devel] [PATCH 07/16] scsi: Increase the number of possible devices

[Kevin Wolf]

From: Hannes Reinecke <hare@suse.de>

The SCSI parallel interface has a limit of 8 devices, but
not the SCSI stack in general. So we should be removing the
hard-coded limit and use MAX_SCSI_DEVS instead.
And we only need to scan those devices which are allocated
by the bus.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.h    |    2 +-
 hw/scsi-bus.c |    2 +-
 hw/scsi.h     |    3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index 2a0559e..4cb8ca9 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -32,7 +32,7 @@ struct DriveInfo {
 };
 
 #define MAX_IDE_DEVS	2
-#define MAX_SCSI_DEVS	7
+#define MAX_SCSI_DEVS	255
 
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 int drive_get_max_bus(BlockInterfaceType type);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 5a3fd4b..74a08b7 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -108,7 +108,7 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
     int res = 0, unit;
 
     loc_push_none(&loc);
-    for (unit = 0; unit < MAX_SCSI_DEVS; unit++) {
+    for (unit = 0; unit < bus->ndev; unit++) {
         dinfo = drive_get(IF_SCSI, bus->busnr, unit);
         if (dinfo == NULL) {
             continue;
diff --git a/hw/scsi.h b/hw/scsi.h
index cb06d6d..9c798ae 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,6 +3,7 @@
 
 #include "qdev.h"
 #include "block.h"
+#include "blockdev.h"
 #include "block_int.h"
 
 #define SCSI_CMD_BUF_SIZE     16
@@ -86,7 +87,7 @@ struct SCSIBus {
     int tcq, ndev;
     scsi_completionfn complete;
 
-    SCSIDevice *devs[8];
+    SCSIDevice *devs[MAX_SCSI_DEVS];
 };
 
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
-- 
1.7.2.3

[Qemu-devel] [PATCH 08/16] scsi: Return SAM status codes

[Kevin Wolf]

From: Hannes Reinecke <hare@suse.de>

Traditionally, the linux stack is using SCSI status codes
which are shifted by one as compared to those defined in SAM.
A SCSI emulation should naturally return the SAM defined codes,
not the linux ones.
So to avoid any confusion this patch modifies the existing
definitions to match those found in SAM and removes any
(now obsolete) byte-shift from the returned status codes.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-defs.h    |   20 +++++++++++---------
 hw/scsi-generic.c |   10 +++++-----
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index a4a3518..1473ecb 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -111,18 +111,20 @@
 #define BLANK 0xa1
 
 /*
- *  Status codes
+ *  SAM Status codes
  */
 
 #define GOOD                 0x00
-#define CHECK_CONDITION      0x01
-#define CONDITION_GOOD       0x02
-#define BUSY                 0x04
-#define INTERMEDIATE_GOOD    0x08
-#define INTERMEDIATE_C_GOOD  0x0a
-#define RESERVATION_CONFLICT 0x0c
-#define COMMAND_TERMINATED   0x11
-#define QUEUE_FULL           0x14
+#define CHECK_CONDITION      0x02
+#define CONDITION_GOOD       0x04
+#define BUSY                 0x08
+#define INTERMEDIATE_GOOD    0x10
+#define INTERMEDIATE_C_GOOD  0x14
+#define RESERVATION_CONFLICT 0x18
+#define COMMAND_TERMINATED   0x22
+#define TASK_SET_FULL        0x28
+#define ACA_ACTIVE           0x30
+#define TASK_ABORTED         0x40
 
 #define STATUS_MASK          0x3e
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 7212091..9be1cca 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -96,17 +96,17 @@ static void scsi_command_complete(void *opaque, int ret)
         s->senselen = r->io_header.sb_len_wr;
 
     if (ret != 0)
-        r->req.status = BUSY << 1;
+        r->req.status = BUSY;
     else {
         if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
-            r->req.status = BUSY << 1;
+            r->req.status = BUSY;
             BADF("Driver Timeout\n");
         } else if (r->io_header.status)
             r->req.status = r->io_header.status;
         else if (s->driver_status & SG_ERR_DRIVER_SENSE)
-            r->req.status = CHECK_CONDITION << 1;
+            r->req.status = CHECK_CONDITION;
         else
-            r->req.status = GOOD << 1;
+            r->req.status = GOOD;
     }
     DPRINTF("Command complete 0x%p tag=0x%x status=%d\n",
             r, r->req.tag, r->req.status);
@@ -333,7 +333,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         s->senselen = 7;
         s->driver_status = SG_ERR_DRIVER_SENSE;
         bus = scsi_bus_from_device(d);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION << 1);
+        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
         return 0;
     }
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 09/16] scsi: INQUIRY VPD fixes

[Kevin Wolf]

From: Hannes Reinecke <hare@suse.de>

We should announce and support the block device characterics page
only on block devices, not on CDROMs. And the VPD page 0x83 has
an off-by-one error.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-disk.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 7d85899..c41dcfc 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -398,15 +398,20 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 
         switch (page_code) {
         case 0x00: /* Supported page codes, mandatory */
+        {
+            int pages;
             DPRINTF("Inquiry EVPD[Supported pages] "
                     "buffer size %zd\n", req->cmd.xfer);
-            outbuf[buflen++] = 4;    // number of pages
+            pages = buflen++;
             outbuf[buflen++] = 0x00; // list of supported pages (this page)
             outbuf[buflen++] = 0x80; // unit serial number
             outbuf[buflen++] = 0x83; // device identification
-            outbuf[buflen++] = 0xb0; // block device characteristics
+            if (bdrv_get_type_hint(s->bs) != BDRV_TYPE_CDROM) {
+                outbuf[buflen++] = 0xb0; // block device characteristics
+            }
+            outbuf[pages] = buflen - pages - 1; // number of pages
             break;
-
+        }
         case 0x80: /* Device serial number, optional */
         {
             int l = strlen(s->serial);
@@ -434,7 +439,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
             DPRINTF("Inquiry EVPD[Device identification] "
                     "buffer size %zd\n", req->cmd.xfer);
 
-            outbuf[buflen++] = 3 + id_len;
+            outbuf[buflen++] = 4 + id_len;
             outbuf[buflen++] = 0x2; // ASCII
             outbuf[buflen++] = 0;   // not officially assigned
             outbuf[buflen++] = 0;   // reserved
@@ -451,6 +456,11 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
             unsigned int opt_io_size =
                     s->qdev.conf.opt_io_size / s->qdev.blocksize;
 
+            if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+                DPRINTF("Inquiry (EVPD[%02X] not supported for CDROM\n",
+                        page_code);
+                return -1;
+            }
             /* required VPD size with unmap support */
             outbuf[3] = buflen = 0x3c;
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 10/16] scsi: Move sense handling into the driver

[Kevin Wolf]

From: Hannes Reinecke <hare@suse.de>

The current sense handling in scsi-bus is only used by the
scsi-disk driver; the scsi-generic driver is using its own.
So we should move the current sense handling into the
scsi-disk driver.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-bus.c  |   10 ----------
 hw/scsi-disk.c |   33 +++++++++++++++++++++++++--------
 hw/scsi.h      |    8 --------
 3 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 74a08b7..93f0e9a 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -123,16 +123,6 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
     return res;
 }
 
-void scsi_dev_clear_sense(SCSIDevice *dev)
-{
-    memset(&dev->sense, 0, sizeof(dev->sense));
-}
-
-void scsi_dev_set_sense(SCSIDevice *dev, uint8_t key)
-{
-    dev->sense.key = key;
-}
-
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun)
 {
     SCSIRequest *req;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c41dcfc..d692fb0 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -49,6 +49,10 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct SCSIDiskState SCSIDiskState;
 
+typedef struct SCSISense {
+    uint8_t key;
+} SCSISense;
+
 typedef struct SCSIDiskReq {
     SCSIRequest req;
     /* ??? We should probably keep track of whether the data transfer is
@@ -72,6 +76,7 @@ struct SCSIDiskState
     QEMUBH *bh;
     char *version;
     char *serial;
+    SCSISense sense;
 };
 
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -100,10 +105,22 @@ static SCSIDiskReq *scsi_find_request(SCSIDiskState *s, uint32_t tag)
     return DO_UPCAST(SCSIDiskReq, req, scsi_req_find(&s->qdev, tag));
 }
 
-static void scsi_req_set_status(SCSIRequest *req, int status, int sense_code)
+static void scsi_disk_clear_sense(SCSIDiskState *s)
 {
-    req->status = status;
-    scsi_dev_set_sense(req->dev, sense_code);
+    memset(&s->sense, 0, sizeof(s->sense));
+}
+
+static void scsi_disk_set_sense(SCSIDiskState *s, uint8_t key)
+{
+    s->sense.key = key;
+}
+
+static void scsi_req_set_status(SCSIDiskReq *r, int status, int sense_code)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+    r->req.status = status;
+    scsi_disk_set_sense(s, sense_code);
 }
 
 /* Helper function for command completion.  */
@@ -111,7 +128,7 @@ static void scsi_command_complete(SCSIDiskReq *r, int status, int sense)
 {
     DPRINTF("Command complete tag=0x%x status=%d sense=%d\n",
             r->req.tag, status, sense);
-    scsi_req_set_status(&r->req, status, sense);
+    scsi_req_set_status(r, status, sense);
     scsi_req_complete(&r->req);
     scsi_remove_request(r);
 }
@@ -822,7 +839,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
             goto illegal_request;
         memset(outbuf, 0, 4);
         buflen = 4;
-        if (req->dev->sense.key == NOT_READY && req->cmd.xfer >= 18) {
+        if (s->sense.key == NOT_READY && req->cmd.xfer >= 18) {
             memset(outbuf, 0, 18);
             buflen = 18;
             outbuf[7] = 10;
@@ -832,8 +849,8 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         }
         outbuf[0] = 0xf0;
         outbuf[1] = 0;
-        outbuf[2] = req->dev->sense.key;
-        scsi_dev_clear_sense(req->dev);
+        outbuf[2] = s->sense.key;
+        scsi_disk_clear_sense(s);
         break;
     case INQUIRY:
         buflen = scsi_disk_emulate_inquiry(req, outbuf);
@@ -966,7 +983,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
     default:
         goto illegal_request;
     }
-    scsi_req_set_status(req, GOOD, NO_SENSE);
+    scsi_req_set_status(r, GOOD, NO_SENSE);
     return buflen;
 
 not_ready:
diff --git a/hw/scsi.h b/hw/scsi.h
index 9c798ae..bf02adf 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -26,10 +26,6 @@ enum SCSIXferMode {
     SCSI_XFER_TO_DEV,    /*  WRITE, MODE_SELECT, ...         */
 };
 
-typedef struct SCSISense {
-    uint8_t key;
-} SCSISense;
-
 typedef struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
@@ -57,7 +53,6 @@ struct SCSIDevice
     QTAILQ_HEAD(, SCSIRequest) requests;
     int blocksize;
     int type;
-    struct SCSISense sense;
 };
 
 /* cdrom.c */
@@ -102,9 +97,6 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, int unit);
 int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 
-void scsi_dev_clear_sense(SCSIDevice *dev);
-void scsi_dev_set_sense(SCSIDevice *dev, uint8_t key);
-
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
 SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag);
 void scsi_req_free(SCSIRequest *req);
-- 
1.7.2.3

[Qemu-devel] [PATCH 11/16] scsi-disk: Remove duplicate cdb parsing

[Kevin Wolf]

From: Hannes Reinecke <hare@suse.de>

We parse the CDB twice, which is completely unnecessary.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-disk.c |   74 ++++++++++++++++----------------------------------------
 1 files changed, 21 insertions(+), 53 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d692fb0..6e49404 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1004,9 +1004,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
                                  uint8_t *buf, int lun)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-    uint64_t lba;
     uint32_t len;
-    int cmdlen;
     int is_write;
     uint8_t command;
     uint8_t *outbuf;
@@ -1025,55 +1023,21 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     outbuf = (uint8_t *)r->iov.iov_base;
     is_write = 0;
     DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
-    switch (command >> 5) {
-    case 0:
-        lba = (uint64_t) buf[3] | ((uint64_t) buf[2] << 8) |
-              (((uint64_t) buf[1] & 0x1f) << 16);
-        len = buf[4];
-        cmdlen = 6;
-        break;
-    case 1:
-    case 2:
-        lba = (uint64_t) buf[5] | ((uint64_t) buf[4] << 8) |
-              ((uint64_t) buf[3] << 16) | ((uint64_t) buf[2] << 24);
-        len = buf[8] | (buf[7] << 8);
-        cmdlen = 10;
-        break;
-    case 4:
-        lba = (uint64_t) buf[9] | ((uint64_t) buf[8] << 8) |
-              ((uint64_t) buf[7] << 16) | ((uint64_t) buf[6] << 24) |
-              ((uint64_t) buf[5] << 32) | ((uint64_t) buf[4] << 40) |
-              ((uint64_t) buf[3] << 48) | ((uint64_t) buf[2] << 56);
-        len = buf[13] | (buf[12] << 8) | (buf[11] << 16) | (buf[10] << 24);
-        cmdlen = 16;
-        break;
-    case 5:
-        lba = (uint64_t) buf[5] | ((uint64_t) buf[4] << 8) |
-              ((uint64_t) buf[3] << 16) | ((uint64_t) buf[2] << 24);
-        len = buf[9] | (buf[8] << 8) | (buf[7] << 16) | (buf[6] << 24);
-        cmdlen = 12;
-        break;
-    default:
+
+    if (scsi_req_parse(&r->req, buf) != 0) {
         BADF("Unsupported command length, command %x\n", command);
         goto fail;
     }
 #ifdef DEBUG_SCSI
     {
         int i;
-        for (i = 1; i < cmdlen; i++) {
+        for (i = 1; i < r->req.cmd.len; i++) {
             printf(" 0x%02x", buf[i]);
         }
         printf("\n");
     }
 #endif
 
-    if (scsi_req_parse(&r->req, buf) != 0) {
-        BADF("Unsupported command length, command %x\n", command);
-        goto fail;
-    }
-    assert(r->req.cmd.len == cmdlen);
-    assert(r->req.cmd.lba == lba);
-
     if (lun || buf[1] >> 5) {
         /* Only LUN 0 supported.  */
         DPRINTF("Unimplemented LUN %d\n", lun ? lun : buf[1] >> 5);
@@ -1111,10 +1075,11 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case READ_10:
     case READ_12:
     case READ_16:
-        DPRINTF("Read (sector %" PRId64 ", count %d)\n", lba, len);
-        if (lba > s->max_lba)
+        len = r->req.cmd.xfer / d->blocksize;
+        DPRINTF("Read (sector %" PRId64 ", count %d)\n", r->req.cmd.lba, len);
+        if (r->req.cmd.lba > s->max_lba)
             goto illegal_lba;
-        r->sector = lba * s->cluster_size;
+        r->sector = r->req.cmd.lba * s->cluster_size;
         r->sector_count = len * s->cluster_size;
         break;
     case WRITE_6:
@@ -1124,42 +1089,45 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case WRITE_VERIFY:
     case WRITE_VERIFY_12:
     case WRITE_VERIFY_16:
+        len = r->req.cmd.xfer / d->blocksize;
         DPRINTF("Write %s(sector %" PRId64 ", count %d)\n",
-                (command & 0xe) == 0xe ? "And Verify " : "", lba, len);
-        if (lba > s->max_lba)
+                (command & 0xe) == 0xe ? "And Verify " : "",
+                r->req.cmd.lba, len);
+        if (r->req.cmd.lba > s->max_lba)
             goto illegal_lba;
-        r->sector = lba * s->cluster_size;
+        r->sector = r->req.cmd.lba * s->cluster_size;
         r->sector_count = len * s->cluster_size;
         is_write = 1;
         break;
     case MODE_SELECT:
-        DPRINTF("Mode Select(6) (len %d)\n", len);
+        DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
         /* We don't support mode parameter changes.
            Allow the mode parameter header + block descriptors only. */
-        if (len > 12) {
+        if (r->req.cmd.xfer > 12) {
             goto fail;
         }
         break;
     case MODE_SELECT_10:
-        DPRINTF("Mode Select(10) (len %d)\n", len);
+        DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer);
         /* We don't support mode parameter changes.
            Allow the mode parameter header + block descriptors only. */
-        if (len > 16) {
+        if (r->req.cmd.xfer > 16) {
             goto fail;
         }
         break;
     case SEEK_6:
     case SEEK_10:
-        DPRINTF("Seek(%d) (sector %" PRId64 ")\n", command == SEEK_6 ? 6 : 10, lba);
-        if (lba > s->max_lba) {
+        DPRINTF("Seek(%d) (sector %" PRId64 ")\n", command == SEEK_6 ? 6 : 10,
+                r->req.cmd.lba);
+        if (r->req.cmd.lba > s->max_lba) {
             goto illegal_lba;
         }
         break;
     default:
-	DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
+        DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
     fail:
         scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
-	return 0;
+        return 0;
     illegal_lba:
         scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
         return 0;
-- 
1.7.2.3

[Qemu-devel] [PATCH 12/16] raw-posix: raw_pwrite comment fixup

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw-posix.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index d0960b8..9286fb8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -463,7 +463,7 @@ static int raw_pwrite(BlockDriverState *bs, int64_t offset,
                 count -= ret;
                 sum += ret;
             }
-            /* here, count < 512 because (count & ~sector_mask) == 0 */
+            /* here, count < sector_size because (count & ~sector_mask) == 0 */
             if (count) {
                 ret = raw_pread_aligned(bs, offset, s->aligned_buf,
                                      bs->buffer_alignment);
-- 
1.7.2.3

[Qemu-devel] [PATCH 13/16] ide: Factor ide_dma_set_inactive out

[Kevin Wolf]

Several places that stop a DMA transfer duplicate this code. Factor it out into
a common function.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/ide/core.c |   29 +++++++++++++----------------
 1 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 484e0ca..7136ade 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -473,6 +473,14 @@ static void dma_buf_commit(IDEState *s, int is_write)
     qemu_sglist_destroy(&s->sg);
 }
 
+static void ide_dma_set_inactive(BMDMAState *bm)
+{
+    bm->status &= ~BM_STATUS_DMAING;
+    bm->dma_cb = NULL;
+    bm->unit = -1;
+    bm->aiocb = NULL;
+}
+
 void ide_dma_error(IDEState *s)
 {
     ide_transfer_stop(s);
@@ -587,11 +595,8 @@ static void ide_read_dma_cb(void *opaque, int ret)
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
     eot:
-        bm->status &= ~BM_STATUS_DMAING;
         bm->status |= BM_STATUS_INT;
-        bm->dma_cb = NULL;
-        bm->unit = -1;
-        bm->aiocb = NULL;
+        ide_dma_set_inactive(bm);
         return;
     }
 
@@ -733,11 +738,8 @@ static void ide_write_dma_cb(void *opaque, int ret)
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
     eot:
-        bm->status &= ~BM_STATUS_DMAING;
         bm->status |= BM_STATUS_INT;
-        bm->dma_cb = NULL;
-        bm->unit = -1;
-        bm->aiocb = NULL;
+        ide_dma_set_inactive(bm);
         return;
     }
 
@@ -1061,11 +1063,8 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
         s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
         ide_set_irq(s->bus);
     eot:
-        bm->status &= ~BM_STATUS_DMAING;
         bm->status |= BM_STATUS_INT;
-        bm->dma_cb = NULL;
-        bm->unit = -1;
-        bm->aiocb = NULL;
+        ide_dma_set_inactive(bm);
         return;
     }
 
@@ -2954,12 +2953,10 @@ void ide_dma_cancel(BMDMAState *bm)
             printf("aio_cancel\n");
 #endif
             bdrv_aio_cancel(bm->aiocb);
-            bm->aiocb = NULL;
         }
-        bm->status &= ~BM_STATUS_DMAING;
+
         /* cancel DMA request */
-        bm->unit = -1;
-        bm->dma_cb = NULL;
+        ide_dma_set_inactive(bm);
     }
 }
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 14/16] ide: Set bus master inactive on error

[Kevin Wolf]

BMIDEA in the status register must be cleared on error. This makes FreeBSD
respond (more) correctly to I/O errors.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/ide/core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7136ade..430350f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -486,6 +486,8 @@ void ide_dma_error(IDEState *s)
     ide_transfer_stop(s);
     s->error = ABRT_ERR;
     s->status = READY_STAT | ERR_STAT;
+    ide_dma_set_inactive(s->bus->bmdma);
+    s->bus->bmdma->status |= BM_STATUS_INT;
     ide_set_irq(s->bus);
 }
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 15/16] ide: Ignore double DMA transfer starts/stops

[Kevin Wolf]

You can only start a DMA transfer if it's not running yet, and you can only
cancel it if it's running.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/ide/pci.c |   60 ++++++++++++++++++++++++++++++---------------------------
 1 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 3722b77..404f045 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -39,38 +39,42 @@ void bmdma_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
 #ifdef DEBUG_IDE
     printf("%s: 0x%08x\n", __func__, val);
 #endif
-    if (!(val & BM_CMD_START)) {
-        /*
-         * We can't cancel Scatter Gather DMA in the middle of the
-         * operation or a partial (not full) DMA transfer would reach
-         * the storage so we wait for completion instead (we beahve
-         * like if the DMA was completed by the time the guest trying
-         * to cancel dma with bmdma_cmd_writeb with BM_CMD_START not
-         * set).
-         *
-         * In the future we'll be able to safely cancel the I/O if the
-         * whole DMA operation will be submitted to disk with a single
-         * aio operation with preadv/pwritev.
-         */
-        if (bm->aiocb) {
-            qemu_aio_flush();
+
+    /* Ignore writes to SSBM if it keeps the old value */
+    if ((val & BM_CMD_START) != (bm->cmd & BM_CMD_START)) {
+        if (!(val & BM_CMD_START)) {
+            /*
+             * We can't cancel Scatter Gather DMA in the middle of the
+             * operation or a partial (not full) DMA transfer would reach
+             * the storage so we wait for completion instead (we beahve
+             * like if the DMA was completed by the time the guest trying
+             * to cancel dma with bmdma_cmd_writeb with BM_CMD_START not
+             * set).
+             *
+             * In the future we'll be able to safely cancel the I/O if the
+             * whole DMA operation will be submitted to disk with a single
+             * aio operation with preadv/pwritev.
+             */
+            if (bm->aiocb) {
+                qemu_aio_flush();
 #ifdef DEBUG_IDE
-            if (bm->aiocb)
-                printf("ide_dma_cancel: aiocb still pending");
-            if (bm->status & BM_STATUS_DMAING)
-                printf("ide_dma_cancel: BM_STATUS_DMAING still pending");
+                if (bm->aiocb)
+                    printf("ide_dma_cancel: aiocb still pending");
+                if (bm->status & BM_STATUS_DMAING)
+                    printf("ide_dma_cancel: BM_STATUS_DMAING still pending");
 #endif
+            }
+        } else {
+            if (!(bm->status & BM_STATUS_DMAING)) {
+                bm->status |= BM_STATUS_DMAING;
+                /* start dma transfer if possible */
+                if (bm->dma_cb)
+                    bm->dma_cb(bm, 0);
+            }
         }
-        bm->cmd = val & 0x09;
-    } else {
-        if (!(bm->status & BM_STATUS_DMAING)) {
-            bm->status |= BM_STATUS_DMAING;
-            /* start dma transfer if possible */
-            if (bm->dma_cb)
-                bm->dma_cb(bm, 0);
-        }
-        bm->cmd = val & 0x09;
     }
+
+    bm->cmd = val & 0x09;
 }
 
 static void bmdma_addr_read(IORange *ioport, uint64_t addr,
-- 
1.7.2.3

[Qemu-devel] [PATCH 16/16] ide: Reset current_addr after stopping DMA

[Kevin Wolf]

Whenever SSBM is reset in the command register all state information is lost.
Restarting DMA means that current_addr must be reset to the base address of the
PRD table. The OS is not required to change the base address register before
starting a DMA operation, it can reuse the value it wrote for an earlier
request.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/ide/pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 404f045..ad406ee 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -65,6 +65,7 @@ void bmdma_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
 #endif
             }
         } else {
+            bm->cur_addr = bm->addr;
             if (!(bm->status & BM_STATUS_DMAING)) {
                 bm->status |= BM_STATUS_DMAING;
                 /* start dma transfer if possible */
@@ -101,7 +102,6 @@ static void bmdma_addr_write(IORange *ioport, uint64_t addr,
 #endif
     bm->addr &= ~(mask << shift);
     bm->addr |= ((data & mask) << shift) & ~3;
-    bm->cur_addr = bm->addr;
 }
 
 const IORangeOps bmdma_addr_ioport_ops = {
-- 
1.7.2.3

Re: [Qemu-devel] [PULL 00/16] Block patches

[Anthony Liguori]

On 11/30/2010 11:58 AM, Kevin Wolf wrote:
> The following changes since commit f711df67d611e4762966a249742a5f7499e19f99:
>
>    microblaze: target-ify target_ucontext (2010-11-23 10:04:30 +0100)
>
> are available in the git repository at:
>    git://repo.or.cz/qemu/kevin.git for-anthony
>    

I see:

commit 393f398b69f9baadc3f29d822a0b5b74ca63b919
Author: Richard Henderson <rth@twiddle.net>
Date:   Mon Nov 22 14:57:58 2010 -0800

     tcg-ia64: Fix warning in qemu_ld.

     The usermode version of qemu_ld doesn't used mem_index,
     leading to set-but-not-used warnings.

     Signed-off-by: Richard Henderson <rth@twiddle.net>
     Signed-off-by: Edgar E. Iglesias <edgar@axis.com>

As the head on that branch which doesn't seem right.

Regards,

Anthony Liguori

> Avi Kivity (1):
>        ide: convert bmdma address ioport to ioport_register()
>
> Christoph Hellwig (1):
>        raw-posix: raw_pwrite comment fixup
>
> Hannes Reinecke (5):
>        scsi: Increase the number of possible devices
>        scsi: Return SAM status codes
>        scsi: INQUIRY VPD fixes
>        scsi: Move sense handling into the driver
>        scsi-disk: Remove duplicate cdb parsing
>
> Kevin Wolf (5):
>        block: Remove unused s->hd in various drivers
>        ide: Factor ide_dma_set_inactive out
>        ide: Set bus master inactive on error
>        ide: Ignore double DMA transfer starts/stops
>        ide: Reset current_addr after stopping DMA
>
> Marcelo Tosatti (1):
>        block migration: do not submit multiple AIOs for same sector (v2)
>
> Ryan Harper (1):
>        Implement drive_del to decouple block removal from device removal
>
> Stefan Hajnoczi (1):
>        scsi-disk: Move active request asserts
>
> Stefano Stabellini (1):
>        qemu and qemu-xen: support empty write barriers in xen_disk
>
>   block-migration.c |   14 ++----
>   block/qcow.c      |    1 -
>   block/qcow2.h     |    1 -
>   block/raw-posix.c |    2 +-
>   block/vdi.c       |    1 -
>   block/vmdk.c      |    1 -
>   block/vpc.c       |    2 -
>   blockdev.c        |   39 +++++++++++++++
>   blockdev.h        |    3 +-
>   hmp-commands.hx   |   18 +++++++
>   hw/ide/cmd646.c   |    8 +--
>   hw/ide/core.c     |   31 ++++++------
>   hw/ide/internal.h |    2 +
>   hw/ide/pci.c      |  131 +++++++++++++++++++-------------------------------
>   hw/ide/pci.h      |    7 +--
>   hw/ide/piix.c     |    8 +--
>   hw/ide/via.c      |    8 +--
>   hw/scsi-bus.c     |   12 +----
>   hw/scsi-defs.h    |   20 ++++----
>   hw/scsi-disk.c    |  137 +++++++++++++++++++++++++---------------------------
>   hw/scsi-generic.c |   10 ++--
>   hw/scsi.h         |   11 +----
>   hw/xen_disk.c     |   12 ++++-
>   23 files changed, 234 insertions(+), 245 deletions(-)
>
>
>    

Re: [Qemu-devel] [PULL 00/16] Block patches

[Kevin Wolf]

Am 06.12.2010 14:32, schrieb Anthony Liguori:
> On 11/30/2010 11:58 AM, Kevin Wolf wrote:
>> The following changes since commit f711df67d611e4762966a249742a5f7499e19f99:
>>
>>    microblaze: target-ify target_ucontext (2010-11-23 10:04:30 +0100)
>>
>> are available in the git repository at:
>>    git://repo.or.cz/qemu/kevin.git for-anthony
>>    
> 
> I see:
> 
> commit 393f398b69f9baadc3f29d822a0b5b74ca63b919
> Author: Richard Henderson <rth@twiddle.net>
> Date:   Mon Nov 22 14:57:58 2010 -0800
> 
>      tcg-ia64: Fix warning in qemu_ld.
> 
>      The usermode version of qemu_ld doesn't used mem_index,
>      leading to set-but-not-used warnings.
> 
>      Signed-off-by: Richard Henderson <rth@twiddle.net>
>      Signed-off-by: Edgar E. Iglesias <edgar@axis.com>
> 
> As the head on that branch which doesn't seem right.

You have pulled this already last week, so after a rebase on my side it
just points to some random commit in master. :-)

Kevin

Re: [Qemu-devel] [PULL 00/16] Block patches

[Anthony Liguori]

On 12/06/2010 07:41 AM, Kevin Wolf wrote:
>
>> I see:
>>
>> commit 393f398b69f9baadc3f29d822a0b5b74ca63b919
>> Author: Richard Henderson<rth@twiddle.net>
>> Date:   Mon Nov 22 14:57:58 2010 -0800
>>
>>       tcg-ia64: Fix warning in qemu_ld.
>>
>>       The usermode version of qemu_ld doesn't used mem_index,
>>       leading to set-but-not-used warnings.
>>
>>       Signed-off-by: Richard Henderson<rth@twiddle.net>
>>       Signed-off-by: Edgar E. Iglesias<edgar@axis.com>
>>
>> As the head on that branch which doesn't seem right.
>>      
> You have pulled this already last week, so after a rebase on my side it
> just points to some random commit in master. :-)
>    

Yeah, I still have origin pointing to Savannah and it confused me.  
Sorry for the noise.

Regards,

Anthony Liguori

> Kevin
>