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

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

[Kevin Wolf]

The following changes since commit f69539b14bdba7a5cd22e1f4bed439b476b17286:

  apb_pci: convert PCI space to memory API (2011-09-04 09:28:04 +0000)

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

Fam Zheng (8):
      VMDK: enable twoGbMaxExtentFlat
      VMDK: add twoGbMaxExtentSparse support
      VMDK: separate vmdk_read_extent/vmdk_write_extent
      VMDK: Opening compressed extent.
      VMDK: read/write compressed extent
      VMDK: creating streamOptimized subformat
      VMDK: bugfix, open Haiku vmdk image
      VMDK: bugfix, opening vSphere 4 exported image

Frediano Ziglio (1):
      linux aio: some comments

Kevin Wolf (3):
      qcow2: Properly initialise QcowL2Meta
      qcow2: Fix error cases to run depedent requests
      async: Allow nested qemu_bh_poll calls

Markus Armbruster (14):
      block: Attach non-qdev devices as well
      block: Generalize change_cb() to BlockDevOps
      block: Split change_cb() into change_media_cb(), resize_cb()
      ide: Update command code definitions as per ACS-2 Table B.2
      ide: Clean up case label indentation in ide_exec_cmd()
      ide: Give vmstate structs internal linkage where possible
      block/raw: Fix to forward method bdrv_media_changed()
      block: Leave tracking media change to device models
      fdc: Make media change detection more robust
      block: Clean up bdrv_flush_all()
      savevm: Include writable devices with removable media
      xen: Clean up pci_piix3_xen_ide_unplug()'s test for "not a CD"
      spitz tosa: Simplify "drive is suitable for microdrive" test
      block: Declare qemu_blockalign() in block.h, not block_int.h

Paolo Bonzini (5):
      scsi: execute SYNCHRONIZE_CACHE asynchronously
      scsi: fix accounting of writes
      scsi: refine constants for READ CAPACITY 16
      scsi: fill in additional sense length correctly
      scsi: improve MODE SENSE emulation

 async.c              |   24 +++-
 block.c              |  104 ++++++++-------
 block.h              |   28 +++-
 block/qcow2.c        |   12 +-
 block/raw-posix.c    |    4 +
 block/raw.c          |    7 +
 block/vmdk.c         |  346 +++++++++++++++++++++++++++++++++++++++-----------
 block_int.h          |   14 +--
 blockdev.c           |    5 +-
 hw/fdc.c             |   46 ++++----
 hw/ide/core.c        |   35 +++---
 hw/ide/internal.h    |  171 +++++++++++++------------
 hw/ide/piix.c        |    7 +-
 hw/pflash_cfi01.c    |    1 +
 hw/pflash_cfi02.c    |    1 +
 hw/qdev-properties.c |    6 +-
 hw/scsi-bus.c        |    6 +-
 hw/scsi-defs.h       |    8 +-
 hw/scsi-disk.c       |  157 +++++++++++++----------
 hw/sd.c              |   14 +-
 hw/spitz.c           |   10 +-
 hw/tosa.c            |   10 +-
 hw/usb-msd.c         |    2 +-
 hw/virtio-blk.c      |   12 +-
 hw/xen_disk.c        |    1 +
 linux-aio.c          |    1 +
 savevm.c             |    4 +-
 27 files changed, 652 insertions(+), 384 deletions(-)

[Qemu-devel] [PATCH 01/31] linux aio: some comments

[Kevin Wolf]

From: Frediano Ziglio <freddy77@gmail.com>

Add some notes about Linux AIO explaining why we don't use AIO in
some situations.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw-posix.c |    4 ++++
 linux-aio.c       |    1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c5c9944..bcf50b2 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -236,6 +236,10 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
     }
 
 #ifdef CONFIG_LINUX_AIO
+    /*
+     * Currently Linux do AIO only for files opened with O_DIRECT
+     * specified so check NOCACHE flag too
+     */
     if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
                       (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
 
diff --git a/linux-aio.c b/linux-aio.c
index 5fd3932..5265a02 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -181,6 +181,7 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
     case QEMU_AIO_READ:
         io_prep_preadv(iocbs, fd, qiov->iov, qiov->niov, offset);
 	break;
+    /* Currently Linux kernel does not support other operations */
     default:
         fprintf(stderr, "%s: invalid AIO request type 0x%x.\n",
                         __func__, type);
-- 
1.7.6

[Qemu-devel] [PATCH 02/31] qcow2: Properly initialise QcowL2Meta

[Kevin Wolf]

Dependency list pointers filled with random garbage from the stack aren't a
good idea.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b725d68..f26f7b6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -526,13 +526,14 @@ static int qcow2_co_writev(BlockDriverState *bs,
     int n_end;
     int ret;
     int cur_nr_sectors; /* number of sectors in current iteration */
-    QCowL2Meta l2meta;
     uint64_t cluster_offset;
     QEMUIOVector hd_qiov;
     uint64_t bytes_done = 0;
     uint8_t *cluster_data = NULL;
+    QCowL2Meta l2meta = {
+        .nb_clusters = 0,
+    };
 
-    l2meta.nb_clusters = 0;
     qemu_co_queue_init(&l2meta.dependent_requests);
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
-- 
1.7.6

[Qemu-devel] [PATCH 03/31] qcow2: Fix error cases to run depedent requests

[Kevin Wolf]

Requests depending on a failed request would end up waiting forever. This fixes
the error path to continue dependent requests even when the request has failed.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index f26f7b6..8aed310 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -593,13 +593,12 @@ static int qcow2_co_writev(BlockDriverState *bs,
         }
 
         ret = qcow2_alloc_cluster_link_l2(bs, &l2meta);
-
-        run_dependent_requests(s, &l2meta);
-
         if (ret < 0) {
             goto fail;
         }
 
+        run_dependent_requests(s, &l2meta);
+
         remaining_sectors -= cur_nr_sectors;
         sector_num += cur_nr_sectors;
         bytes_done += cur_nr_sectors * 512;
@@ -607,6 +606,8 @@ static int qcow2_co_writev(BlockDriverState *bs,
     ret = 0;
 
 fail:
+    run_dependent_requests(s, &l2meta);
+
     qemu_co_mutex_unlock(&s->lock);
 
     qemu_iovec_destroy(&hd_qiov);
-- 
1.7.6

[Qemu-devel] [PATCH 04/31] async: Allow nested qemu_bh_poll calls

[Kevin Wolf]

qemu may segfault when a BH handler first deletes a BH and then (possibly
indirectly) calls a nested qemu_bh_poll(). This is because the inner instance
frees the BH and deletes it from the list that the outer one processes.

This patch deletes BHs only in the outermost qemu_bh_poll instance.

Commit 7887f620 already tried to achieve the same, but it assumed that the BH
handler would only delete its own BH. With a nested qemu_bh_poll(), this isn't
guaranteed, so that commit wasn't enough. Hope this one fixes it for real.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 async.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/async.c b/async.c
index 9d4e960..ca13962 100644
--- a/async.c
+++ b/async.c
@@ -55,6 +55,9 @@ int qemu_bh_poll(void)
 {
     QEMUBH *bh, **bhp, *next;
     int ret;
+    static int nesting = 0;
+
+    nesting++;
 
     ret = 0;
     for (bh = first_bh; bh; bh = next) {
@@ -68,15 +71,20 @@ int qemu_bh_poll(void)
         }
     }
 
+    nesting--;
+
     /* remove deleted bhs */
-    bhp = &first_bh;
-    while (*bhp) {
-        bh = *bhp;
-        if (bh->deleted) {
-            *bhp = bh->next;
-            g_free(bh);
-        } else
-            bhp = &bh->next;
+    if (!nesting) {
+        bhp = &first_bh;
+        while (*bhp) {
+            bh = *bhp;
+            if (bh->deleted) {
+                *bhp = bh->next;
+                g_free(bh);
+            } else {
+                bhp = &bh->next;
+            }
+        }
     }
 
     return ret;
-- 
1.7.6

[Qemu-devel] [PATCH 05/31] block: Attach non-qdev devices as well

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

For now, this just protects against programming errors like having the
same drive back multiple non-qdev devices, or untimely bdrv_delete().
Later commits will add other interesting uses.

While there, rename BlockDriverState member peer to dev, bdrv_attach()
to bdrv_attach_dev(), bdrv_detach() to bdrv_detach_dev(), and
bdrv_get_attached() to bdrv_get_attached_dev().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c              |   29 ++++++++++++++++++++---------
 block.h              |    7 ++++---
 block_int.h          |    3 ++-
 blockdev.c           |    5 ++---
 hw/ide/core.c        |    1 +
 hw/ide/piix.c        |    4 ++--
 hw/pflash_cfi01.c    |    1 +
 hw/pflash_cfi02.c    |    1 +
 hw/qdev-properties.c |    6 +++---
 hw/sd.c              |    1 +
 hw/usb-msd.c         |    2 +-
 hw/xen_disk.c        |    1 +
 12 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 43742b7..fb9341c 100644
--- a/block.c
+++ b/block.c
@@ -755,7 +755,7 @@ void bdrv_make_anon(BlockDriverState *bs)
 
 void bdrv_delete(BlockDriverState *bs)
 {
-    assert(!bs->peer);
+    assert(!bs->dev);
 
     /* remove from list, if necessary */
     bdrv_make_anon(bs);
@@ -769,26 +769,37 @@ void bdrv_delete(BlockDriverState *bs)
     g_free(bs);
 }
 
-int bdrv_attach(BlockDriverState *bs, DeviceState *qdev)
+int bdrv_attach_dev(BlockDriverState *bs, void *dev)
+/* TODO change to DeviceState *dev when all users are qdevified */
 {
-    if (bs->peer) {
+    if (bs->dev) {
         return -EBUSY;
     }
-    bs->peer = qdev;
+    bs->dev = dev;
     return 0;
 }
 
-void bdrv_detach(BlockDriverState *bs, DeviceState *qdev)
+/* TODO qdevified devices don't use this, remove when devices are qdevified */
+void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev)
 {
-    assert(bs->peer == qdev);
-    bs->peer = NULL;
+    if (bdrv_attach_dev(bs, dev) < 0) {
+        abort();
+    }
+}
+
+void bdrv_detach_dev(BlockDriverState *bs, void *dev)
+/* TODO change to DeviceState *dev when all users are qdevified */
+{
+    assert(bs->dev == dev);
+    bs->dev = NULL;
     bs->change_cb = NULL;
     bs->change_opaque = NULL;
 }
 
-DeviceState *bdrv_get_attached(BlockDriverState *bs)
+/* TODO change to return DeviceState * when all users are qdevified */
+void *bdrv_get_attached_dev(BlockDriverState *bs)
 {
-    return bs->peer;
+    return bs->dev;
 }
 
 /*
diff --git a/block.h b/block.h
index 3ac0b94..c8eea67 100644
--- a/block.h
+++ b/block.h
@@ -74,9 +74,10 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
               BlockDriver *drv);
 void bdrv_close(BlockDriverState *bs);
-int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
-void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
-DeviceState *bdrv_get_attached(BlockDriverState *bs);
+int bdrv_attach_dev(BlockDriverState *bs, void *dev);
+void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
+void bdrv_detach_dev(BlockDriverState *bs, void *dev);
+void *bdrv_get_attached_dev(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors);
 int bdrv_write(BlockDriverState *bs, int64_t sector_num,
diff --git a/block_int.h b/block_int.h
index 8a72b80..adc26f8 100644
--- a/block_int.h
+++ b/block_int.h
@@ -168,7 +168,8 @@ struct BlockDriverState {
     BlockDriver *drv; /* NULL means no media */
     void *opaque;
 
-    DeviceState *peer;
+    void *dev;                  /* attached device model, if any */
+    /* TODO change to DeviceState when all users are qdevified */
 
     char filename[1024];
     char backing_file[1024]; /* if non zero, the image is a diff of
diff --git a/blockdev.c b/blockdev.c
index 2602591..049dda5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -14,7 +14,6 @@
 #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);
@@ -738,12 +737,12 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
     bdrv_flush(bs);
     bdrv_close(bs);
 
-    /* if we have a device associated with this BlockDriverState (bs->peer)
+    /* if we have a device attached to this BlockDriverState
      * then we need to make the drive anonymous until the device
      * can be removed.  If this is a drive with no device backing
      * then we can just get rid of the block driver state right here.
      */
-    if (bs->peer) {
+    if (bdrv_get_attached_dev(bs)) {
         bdrv_make_anon(bs);
     } else {
         drive_uninit(drive_get_by_blockdev(bs));
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 40abc1e..428f033 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1890,6 +1890,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                 error_report("Can't set up IDE drive %s", dinfo->id);
                 exit(1);
             }
+            bdrv_attach_dev_nofail(dinfo->bdrv, &bus->ifs[i]);
         } else {
             ide_reset(&bus->ifs[i]);
         }
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 8525336..b9cdcd6 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -177,9 +177,9 @@ static int pci_piix3_xen_ide_unplug(DeviceState *dev)
     for (; i < 3; i++) {
         di = drive_get_by_index(IF_IDE, i);
         if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) {
-            DeviceState *ds = bdrv_get_attached(di->bdrv);
+            DeviceState *ds = bdrv_get_attached_dev(di->bdrv);
             if (ds) {
-                bdrv_detach(di->bdrv, ds);
+                bdrv_detach_dev(di->bdrv, ds);
             }
             bdrv_close(di->bdrv);
             pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c
index 90e1301..a2d9402 100644
--- a/hw/pflash_cfi01.c
+++ b/hw/pflash_cfi01.c
@@ -620,6 +620,7 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t base, ram_addr_t off,
             g_free(pfl);
             return NULL;
         }
+        bdrv_attach_dev_nofail(pfl->bs, pfl);
     }
 #if 0 /* XXX: there should be a bit to set up read-only,
        *      the same way the hardware does (with WP pin).
diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
index ac5115e..919cfc4 100644
--- a/hw/pflash_cfi02.c
+++ b/hw/pflash_cfi02.c
@@ -643,6 +643,7 @@ pflash_t *pflash_cfi02_register(target_phys_addr_t base, ram_addr_t off,
             g_free(pfl);
             return NULL;
         }
+        bdrv_attach_dev_nofail(pfl->bs, pfl);
     }
 #if 0 /* XXX: there should be a bit to set up read-only,
        *      the same way the hardware does (with WP pin).
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0c0c292..7ce95b6 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -312,7 +312,7 @@ static int parse_drive(DeviceState *dev, Property *prop, const char *str)
     bs = bdrv_find(str);
     if (bs == NULL)
         return -ENOENT;
-    if (bdrv_attach(bs, dev) < 0)
+    if (bdrv_attach_dev(bs, dev) < 0)
         return -EEXIST;
     *ptr = bs;
     return 0;
@@ -323,7 +323,7 @@ static void free_drive(DeviceState *dev, Property *prop)
     BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
 
     if (*ptr) {
-        bdrv_detach(*ptr, dev);
+        bdrv_detach_dev(*ptr, dev);
         blockdev_auto_del(*ptr);
     }
 }
@@ -678,7 +678,7 @@ int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *va
 {
     int res;
 
-    res = bdrv_attach(value, dev);
+    res = bdrv_attach_dev(value, dev);
     if (res < 0) {
         error_report("Can't attach drive %s to %s.%s: %s",
                      bdrv_get_device_name(value),
diff --git a/hw/sd.c b/hw/sd.c
index bb8c2ff..ab52634 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -449,6 +449,7 @@ SDState *sd_init(BlockDriverState *bs, int is_spi)
     sd->enable = 1;
     sd_reset(sd, bs);
     if (sd->bdrv) {
+        bdrv_attach_dev_nofail(sd->bdrv, sd);
         bdrv_set_change_cb(sd->bdrv, sd_cardchange, sd);
     }
     return sd;
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 4072efd..e92434c 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -518,7 +518,7 @@ static int usb_msd_initfn(USBDevice *dev)
      *
      * The hack is probably a bad idea.
      */
-    bdrv_detach(bs, &s->dev.qdev);
+    bdrv_detach_dev(bs, &s->dev.qdev);
     s->conf.bs = NULL;
 
     if (!s->serial) {
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index bd5c669..da531a6 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -697,6 +697,7 @@ static int blk_init(struct XenDevice *xendev)
         xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
         blkdev->bs = blkdev->dinfo->bdrv;
     }
+    bdrv_attach_dev_nofail(blkdev->bs, blkdev);
     blkdev->file_blk  = BLOCK_SIZE;
     blkdev->file_size = bdrv_getlength(blkdev->bs);
     if (blkdev->file_size < 0) {
-- 
1.7.6

[Qemu-devel] [PATCH 06/31] block: Generalize change_cb() to BlockDevOps

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

So we can more easily add device model callbacks.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c         |   43 +++++++++++++++++++++----------------------
 block.h         |    9 ++++++---
 block_int.h     |    5 ++---
 hw/ide/core.c   |    6 +++++-
 hw/sd.c         |    6 +++++-
 hw/virtio-blk.c |    6 +++++-
 6 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/block.c b/block.c
index fb9341c..00fe345 100644
--- a/block.c
+++ b/block.c
@@ -44,6 +44,7 @@
 #include <windows.h>
 #endif
 
+static void bdrv_dev_change_cb(BlockDriverState *bs, int reason);
 static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
@@ -688,10 +689,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
     }
 
     if (!bdrv_key_required(bs)) {
-        /* call the change callback */
         bs->media_changed = 1;
-        if (bs->change_cb)
-            bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
+        bdrv_dev_change_cb(bs, CHANGE_MEDIA);
     }
 
     return 0;
@@ -727,10 +726,8 @@ void bdrv_close(BlockDriverState *bs)
             bdrv_close(bs->file);
         }
 
-        /* call the change callback */
         bs->media_changed = 1;
-        if (bs->change_cb)
-            bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
+        bdrv_dev_change_cb(bs, CHANGE_MEDIA);
     }
 }
 
@@ -792,8 +789,8 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev)
 {
     assert(bs->dev == dev);
     bs->dev = NULL;
-    bs->change_cb = NULL;
-    bs->change_opaque = NULL;
+    bs->dev_ops = NULL;
+    bs->dev_opaque = NULL;
 }
 
 /* TODO change to return DeviceState * when all users are qdevified */
@@ -802,6 +799,20 @@ void *bdrv_get_attached_dev(BlockDriverState *bs)
     return bs->dev;
 }
 
+void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
+                      void *opaque)
+{
+    bs->dev_ops = ops;
+    bs->dev_opaque = opaque;
+}
+
+static void bdrv_dev_change_cb(BlockDriverState *bs, int reason)
+{
+    if (bs->dev_ops && bs->dev_ops->change_cb) {
+        bs->dev_ops->change_cb(bs->dev_opaque, reason);
+    }
+}
+
 /*
  * Run consistency checks on an image
  *
@@ -1272,9 +1283,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
     ret = drv->bdrv_truncate(bs, offset);
     if (ret == 0) {
         ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
-        if (bs->change_cb) {
-            bs->change_cb(bs->change_opaque, CHANGE_SIZE);
-        }
+        bdrv_dev_change_cb(bs, CHANGE_SIZE);
     }
     return ret;
 }
@@ -1612,15 +1621,6 @@ int bdrv_enable_write_cache(BlockDriverState *bs)
     return bs->enable_write_cache;
 }
 
-/* XXX: no longer used */
-void bdrv_set_change_cb(BlockDriverState *bs,
-                        void (*change_cb)(void *opaque, int reason),
-                        void *opaque)
-{
-    bs->change_cb = change_cb;
-    bs->change_opaque = opaque;
-}
-
 int bdrv_is_encrypted(BlockDriverState *bs)
 {
     if (bs->backing_hd && bs->backing_hd->encrypted)
@@ -1659,8 +1659,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
         bs->valid_key = 1;
         /* call the change callback now, we skipped it on open */
         bs->media_changed = 1;
-        if (bs->change_cb)
-            bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
+        bdrv_dev_change_cb(bs, CHANGE_MEDIA);
     }
     return ret;
 }
diff --git a/block.h b/block.h
index c8eea67..a72cfc9 100644
--- a/block.h
+++ b/block.h
@@ -28,6 +28,10 @@ typedef struct QEMUSnapshotInfo {
     uint64_t vm_clock_nsec; /* VM clock relative to boot */
 } QEMUSnapshotInfo;
 
+typedef struct BlockDevOps {
+    void (*change_cb)(void *opaque, int reason);
+} BlockDevOps;
+
 #define BDRV_O_RDWR        0x0002
 #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
 #define BDRV_O_NOCACHE     0x0020 /* do not use the host page cache */
@@ -78,6 +82,8 @@ int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
 void bdrv_detach_dev(BlockDriverState *bs, void *dev);
 void *bdrv_get_attached_dev(BlockDriverState *bs);
+void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
+                      void *opaque);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
               uint8_t *buf, int nb_sectors);
 int bdrv_write(BlockDriverState *bs, int64_t sector_num,
@@ -193,9 +199,6 @@ int bdrv_media_changed(BlockDriverState *bs);
 int bdrv_is_locked(BlockDriverState *bs);
 void bdrv_set_locked(BlockDriverState *bs, int locked);
 int bdrv_eject(BlockDriverState *bs, int eject_flag);
-void bdrv_set_change_cb(BlockDriverState *bs,
-                        void (*change_cb)(void *opaque, int reason),
-                        void *opaque);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
 BlockDriverState *bdrv_find(const char *name);
 BlockDriverState *bdrv_next(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index adc26f8..ab31dc9 100644
--- a/block_int.h
+++ b/block_int.h
@@ -161,15 +161,14 @@ struct BlockDriverState {
     int encrypted; /* if true, the media is encrypted */
     int valid_key; /* if true, a valid encryption key has been set */
     int sg;        /* if true, the device is a /dev/sg* */
-    /* event callback when inserting/removing */
-    void (*change_cb)(void *opaque, int reason);
-    void *change_opaque;
 
     BlockDriver *drv; /* NULL means no media */
     void *opaque;
 
     void *dev;                  /* attached device model, if any */
     /* TODO change to DeviceState when all users are qdevified */
+    const BlockDevOps *dev_ops;
+    void *dev_opaque;
 
     char filename[1024];
     char backing_file[1024]; /* if non zero, the image is a diff of
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 428f033..26a9c5b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1742,6 +1742,10 @@ void ide_bus_reset(IDEBus *bus)
     bus->dma->ops->reset(bus->dma);
 }
 
+static const BlockDevOps ide_cd_block_ops = {
+    .change_cb = cdrom_change_cb,
+};
+
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
                    const char *version, const char *serial)
 {
@@ -1776,7 +1780,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->smart_errors = 0;
     s->smart_selftest_count = 0;
     if (kind == IDE_CD) {
-        bdrv_set_change_cb(bs, cdrom_change_cb, s);
+        bdrv_set_dev_ops(bs, &ide_cd_block_ops, s);
         bs->buffer_alignment = 2048;
     } else {
         if (!bdrv_is_inserted(s->bs)) {
diff --git a/hw/sd.c b/hw/sd.c
index ab52634..1f00910 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -435,6 +435,10 @@ static void sd_cardchange(void *opaque, int reason)
     }
 }
 
+static const BlockDevOps sd_block_ops = {
+    .change_cb = sd_cardchange,
+};
+
 /* We do not model the chip select pin, so allow the board to select
    whether card should be in SSI or MMC/SD mode.  It is also up to the
    board to ensure that ssi transfers only occur when the chip select
@@ -450,7 +454,7 @@ SDState *sd_init(BlockDriverState *bs, int is_spi)
     sd_reset(sd, bs);
     if (sd->bdrv) {
         bdrv_attach_dev_nofail(sd->bdrv, sd);
-        bdrv_set_change_cb(sd->bdrv, sd_cardchange, sd);
+        bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
     }
     return sd;
 }
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 2a8ccd0..c10afa9 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -552,6 +552,10 @@ static void virtio_blk_change_cb(void *opaque, int reason)
     }
 }
 
+static const BlockDevOps virtio_block_ops = {
+    .change_cb = virtio_blk_change_cb,
+};
+
 VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
                               char **serial)
 {
@@ -598,7 +602,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
     register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_removable(s->bs, 0);
-    bdrv_set_change_cb(s->bs, virtio_blk_change_cb, s);
+    bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
     s->bs->buffer_alignment = conf->logical_block_size;
 
     add_boot_device_path(conf->bootindex, dev, "/disk@0,0");
-- 
1.7.6

[Qemu-devel] [PATCH 07/31] block: Split change_cb() into change_media_cb(), resize_cb()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Multiplexing callbacks complicates matters needlessly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c         |   23 +++++++++++++++--------
 block.h         |   12 +++++++++++-
 block_int.h     |    3 ---
 hw/ide/core.c   |    8 ++------
 hw/sd.c         |    8 ++------
 hw/virtio-blk.c |    8 +++-----
 6 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/block.c b/block.c
index 00fe345..0aba1bf 100644
--- a/block.c
+++ b/block.c
@@ -44,7 +44,7 @@
 #include <windows.h>
 #endif
 
-static void bdrv_dev_change_cb(BlockDriverState *bs, int reason);
+static void bdrv_dev_change_media_cb(BlockDriverState *bs);
 static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque);
@@ -690,7 +690,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
 
     if (!bdrv_key_required(bs)) {
         bs->media_changed = 1;
-        bdrv_dev_change_cb(bs, CHANGE_MEDIA);
+        bdrv_dev_change_media_cb(bs);
     }
 
     return 0;
@@ -727,7 +727,7 @@ void bdrv_close(BlockDriverState *bs)
         }
 
         bs->media_changed = 1;
-        bdrv_dev_change_cb(bs, CHANGE_MEDIA);
+        bdrv_dev_change_media_cb(bs);
     }
 }
 
@@ -806,10 +806,17 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
     bs->dev_opaque = opaque;
 }
 
-static void bdrv_dev_change_cb(BlockDriverState *bs, int reason)
+static void bdrv_dev_change_media_cb(BlockDriverState *bs)
 {
-    if (bs->dev_ops && bs->dev_ops->change_cb) {
-        bs->dev_ops->change_cb(bs->dev_opaque, reason);
+    if (bs->dev_ops && bs->dev_ops->change_media_cb) {
+        bs->dev_ops->change_media_cb(bs->dev_opaque);
+    }
+}
+
+static void bdrv_dev_resize_cb(BlockDriverState *bs)
+{
+    if (bs->dev_ops && bs->dev_ops->resize_cb) {
+        bs->dev_ops->resize_cb(bs->dev_opaque);
     }
 }
 
@@ -1283,7 +1290,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
     ret = drv->bdrv_truncate(bs, offset);
     if (ret == 0) {
         ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
-        bdrv_dev_change_cb(bs, CHANGE_SIZE);
+        bdrv_dev_resize_cb(bs);
     }
     return ret;
 }
@@ -1659,7 +1666,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
         bs->valid_key = 1;
         /* call the change callback now, we skipped it on open */
         bs->media_changed = 1;
-        bdrv_dev_change_cb(bs, CHANGE_MEDIA);
+        bdrv_dev_change_media_cb(bs);
     }
     return ret;
 }
diff --git a/block.h b/block.h
index a72cfc9..7e3f587 100644
--- a/block.h
+++ b/block.h
@@ -28,8 +28,18 @@ typedef struct QEMUSnapshotInfo {
     uint64_t vm_clock_nsec; /* VM clock relative to boot */
 } QEMUSnapshotInfo;
 
+/* Callbacks for block device models */
 typedef struct BlockDevOps {
-    void (*change_cb)(void *opaque, int reason);
+    /*
+     * Runs when virtual media changed (monitor commands eject, change)
+     * Beware: doesn't run when a host device's physical media
+     * changes.  Sure would be useful if it did.
+     */
+    void (*change_media_cb)(void *opaque);
+    /*
+     * Runs when the size changed (e.g. monitor command block_resize)
+     */
+    void (*resize_cb)(void *opaque);
 } BlockDevOps;
 
 #define BDRV_O_RDWR        0x0002
diff --git a/block_int.h b/block_int.h
index ab31dc9..19ae75e 100644
--- a/block_int.h
+++ b/block_int.h
@@ -211,9 +211,6 @@ struct BlockDriverState {
     void *private;
 };
 
-#define CHANGE_MEDIA    0x01
-#define CHANGE_SIZE     0x02
-
 struct BlockDriverAIOCB {
     AIOPool *pool;
     BlockDriverState *bs;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 26a9c5b..47a706e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -783,15 +783,11 @@ static void ide_cfata_metadata_write(IDEState *s)
 }
 
 /* called when the inserted state of the media has changed */
-static void cdrom_change_cb(void *opaque, int reason)
+static void ide_cd_change_cb(void *opaque)
 {
     IDEState *s = opaque;
     uint64_t nb_sectors;
 
-    if (!(reason & CHANGE_MEDIA)) {
-        return;
-    }
-
     bdrv_get_geometry(s->bs, &nb_sectors);
     s->nb_sectors = nb_sectors;
 
@@ -1743,7 +1739,7 @@ void ide_bus_reset(IDEBus *bus)
 }
 
 static const BlockDevOps ide_cd_block_ops = {
-    .change_cb = cdrom_change_cb,
+    .change_media_cb = ide_cd_change_cb,
 };
 
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
diff --git a/hw/sd.c b/hw/sd.c
index 1f00910..45e95f9 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -420,14 +420,10 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
     sd->pwd_len = 0;
 }
 
-static void sd_cardchange(void *opaque, int reason)
+static void sd_cardchange(void *opaque)
 {
     SDState *sd = opaque;
 
-    if (!(reason & CHANGE_MEDIA)) {
-        return;
-    }
-
     qemu_set_irq(sd->inserted_cb, bdrv_is_inserted(sd->bdrv));
     if (bdrv_is_inserted(sd->bdrv)) {
         sd_reset(sd, sd->bdrv);
@@ -436,7 +432,7 @@ static void sd_cardchange(void *opaque, int reason)
 }
 
 static const BlockDevOps sd_block_ops = {
-    .change_cb = sd_cardchange,
+    .change_media_cb = sd_cardchange,
 };
 
 /* We do not model the chip select pin, so allow the board to select
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index c10afa9..4df23f4 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -543,17 +543,15 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static void virtio_blk_change_cb(void *opaque, int reason)
+static void virtio_blk_resize(void *opaque)
 {
     VirtIOBlock *s = opaque;
 
-    if (reason & CHANGE_SIZE) {
-        virtio_notify_config(&s->vdev);
-    }
+    virtio_notify_config(&s->vdev);
 }
 
 static const BlockDevOps virtio_block_ops = {
-    .change_cb = virtio_blk_change_cb,
+    .resize_cb = virtio_blk_resize,
 };
 
 VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf,
-- 
1.7.6

[Qemu-devel] [PATCH 08/31] ide: Update command code definitions as per ACS-2 Table B.2

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Drop WIN_SRST, it has the same value as WIN_DEVICE_RESET.

Drop unused WIN_RESTORE, it has the same value as WIN_RECAL.

Drop codes that are not implemented and long obsolete: WIN_READ_LONG,
WIN_READ_LONG_ONCE, WIN_WRITE_LONG, WIN_WRITE_LONG_ONCE, WIN_FORMAT
(all obsolete since ATA4), WIN_ACKMEDIACHANGE, WIN_POSTBOOT,
WIN_PREBOOT (obsolete since ATA3), WIN_WRITE_SAME (obsolete since
ATA3, code reused for something else in ACS2), WIN_IDENTIFY_DMA
(obsolete since ATA4).

Drop codes that are not implemented and vendor-specific:
EXABYTE_ENABLE_NEST, DISABLE_SEAGATE.

Drop WIN_INIT, it isn't implemented, its value used to be reserved,
and is used for something else since ATA8.

CFA_IDLEIMMEDIATE isn't specific to CFATA.  ACS-2 shows it as a
defined command in ATA-1, -2 and -3.  Rename to WIN_IDLEIMMEDIATE2.

Mark vendor specific, retired, and obsolete codes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c     |    4 +-
 hw/ide/internal.h |  171 ++++++++++++++++++++++++++++-------------------------
 2 files changed, 92 insertions(+), 83 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 47a706e..d1cbfe7 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1129,7 +1129,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
     case WIN_STANDBYNOW1:
     case WIN_STANDBYNOW2:
     case WIN_IDLEIMMEDIATE:
-    case CFA_IDLEIMMEDIATE:
+    case WIN_IDLEIMMEDIATE2:
     case WIN_SETIDLE1:
     case WIN_SETIDLE2:
     case WIN_SLEEPNOW1:
@@ -1168,7 +1168,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
                           */
         ide_set_irq(s->bus);
         break;
-    case WIN_SRST:
+    case WIN_DEVICE_RESET:
         if (s->drive_kind != IDE_CD)
             goto abort_cmd;
         ide_set_signature(s);
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 7f5ef8d..1117852 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -55,111 +55,120 @@ typedef struct IDEDMAOps IDEDMAOps;
 #define IDE_CMD_RESET           0x04
 #define IDE_CMD_DISABLE_IRQ     0x02
 
-/* ATA/ATAPI Commands pre T13 Spec */
+/* ACS-2 T13/2015-D Table B.2 Command codes */
 #define WIN_NOP				0x00
-/*
- *	0x01->0x02 Reserved
- */
+/* reserved                             0x01..0x02 */
 #define CFA_REQ_EXT_ERROR_CODE		0x03 /* CFA Request Extended Error Code */
-/*
- *      0x04->0x05 Reserved
- */
+/* reserved                             0x04..0x05 */
 #define WIN_DSM                         0x06
-/*
- *      0x07 Reserved
- */
-#define WIN_SRST			0x08 /* ATAPI soft reset command */
+/* reserved                             0x07 */
 #define WIN_DEVICE_RESET		0x08
-/*
- *	0x09->0x0F Reserved
- */
-#define WIN_RECAL			0x10
-#define WIN_RESTORE			WIN_RECAL
-/*
- *	0x10->0x1F Reserved
- */
+/* reserved                             0x09..0x0a */
+/* REQUEST SENSE DATA EXT               0x0B */
+/* reserved                             0x0C..0x0F */
+#define WIN_RECAL                       0x10 /* obsolete since ATA4 */
+/* obsolete since ATA3, retired in ATA4 0x11..0x1F */
 #define WIN_READ			0x20 /* 28-Bit */
-#define WIN_READ_ONCE			0x21 /* 28-Bit without retries */
-#define WIN_READ_LONG			0x22 /* 28-Bit */
-#define WIN_READ_LONG_ONCE		0x23 /* 28-Bit without retries */
+#define WIN_READ_ONCE                   0x21 /* 28-Bit w/o retries, obsolete since ATA5 */
+/* obsolete since ATA4                  0x22..0x23 */
 #define WIN_READ_EXT			0x24 /* 48-Bit */
 #define WIN_READDMA_EXT			0x25 /* 48-Bit */
-#define WIN_READDMA_QUEUED_EXT		0x26 /* 48-Bit */
+#define WIN_READDMA_QUEUED_EXT          0x26 /* 48-Bit, obsolete since ACS2 */
 #define WIN_READ_NATIVE_MAX_EXT		0x27 /* 48-Bit */
-/*
- *	0x28
- */
+/* reserved                             0x28 */
 #define WIN_MULTREAD_EXT		0x29 /* 48-Bit */
-/*
- *	0x2A->0x2F Reserved
- */
+/* READ STREAM DMA EXT                  0x2A */
+/* READ STREAM EXT                      0x2B */
+/* reserved                             0x2C..0x2E */
+/* READ LOG EXT                         0x2F */
 #define WIN_WRITE			0x30 /* 28-Bit */
-#define WIN_WRITE_ONCE			0x31 /* 28-Bit without retries */
-#define WIN_WRITE_LONG			0x32 /* 28-Bit */
-#define WIN_WRITE_LONG_ONCE		0x33 /* 28-Bit without retries */
+#define WIN_WRITE_ONCE                  0x31 /* 28-Bit w/o retries, obsolete since ATA5 */
+/* obsolete since ATA4                  0x32..0x33 */
 #define WIN_WRITE_EXT			0x34 /* 48-Bit */
 #define WIN_WRITEDMA_EXT		0x35 /* 48-Bit */
 #define WIN_WRITEDMA_QUEUED_EXT		0x36 /* 48-Bit */
+#define WIN_SET_MAX_EXT                 0x37 /* 48-Bit, obsolete since ACS2 */
 #define WIN_SET_MAX_EXT			0x37 /* 48-Bit */
 #define CFA_WRITE_SECT_WO_ERASE		0x38 /* CFA Write Sectors without erase */
 #define WIN_MULTWRITE_EXT		0x39 /* 48-Bit */
-/*
- *	0x3A->0x3B Reserved
- */
-#define WIN_WRITE_VERIFY		0x3C /* 28-Bit */
-/*
- *	0x3D->0x3F Reserved
- */
+/* WRITE STREAM DMA EXT                 0x3A */
+/* WRITE STREAM EXT                     0x3B */
+#define WIN_WRITE_VERIFY                0x3C /* 28-Bit, obsolete since ATA4 */
+/* WRITE DMA FUA EXT                    0x3D */
+/* obsolete since ACS2                  0x3E */
+/* WRITE LOG EXT                        0x3F */
 #define WIN_VERIFY			0x40 /* 28-Bit - Read Verify Sectors */
-#define WIN_VERIFY_ONCE			0x41 /* 28-Bit - without retries */
+#define WIN_VERIFY_ONCE                 0x41 /* 28-Bit - w/o retries, obsolete since ATA5 */
 #define WIN_VERIFY_EXT			0x42 /* 48-Bit */
-/*
- *	0x43->0x4F Reserved
- */
-#define WIN_FORMAT			0x50
-/*
- *	0x51->0x5F Reserved
- */
-#define WIN_INIT			0x60
-/*
- *	0x61->0x5F Reserved
- */
-#define WIN_SEEK			0x70 /* 0x70-0x7F Reserved */
+/* reserved                             0x43..0x44 */
+/* WRITE UNCORRECTABLE EXT              0x45 */
+/* reserved                             0x46 */
+/* READ LOG DMA EXT                     0x47 */
+/* reserved                             0x48..0x4F */
+/* obsolete since ATA4                  0x50 */
+/* CONFIGURE STREAM                     0x51 */
+/* reserved                             0x52..0x56 */
+/* WRITE LOG DMA EXT                    0x57 */
+/* reserved                             0x58..0x5A */
+/* TRUSTED NON DATA                     0x5B */
+/* TRUSTED RECEIVE                      0x5C */
+/* TRUSTED RECEIVE DMA                  0x5D */
+/* TRUSTED SEND                         0x5E */
+/* TRUSTED SEND DMA                     0x5F */
+/* READ FPDMA QUEUED                    0x60 */
+/* WRITE FPDMA QUEUED                   0x61 */
+/* reserved                             0x62->0x6F */
+#define WIN_SEEK                        0x70 /* obsolete since ATA7 */
+/* reserved                             0x71-0x7F */
+/* vendor specific                      0x80-0x86 */
 #define CFA_TRANSLATE_SECTOR		0x87 /* CFA Translate Sector */
+/* vendor specific                      0x88-0x8F */
 #define WIN_DIAGNOSE			0x90
-#define WIN_SPECIFY			0x91 /* set drive geometry translation */
+#define WIN_SPECIFY                     0x91 /* set drive geometry translation, obsolete since ATA6 */
 #define WIN_DOWNLOAD_MICROCODE		0x92
-#define WIN_STANDBYNOW2			0x94
-#define CFA_IDLEIMMEDIATE		0x95 /* force drive to become "ready" */
-#define WIN_STANDBY2			0x96
-#define WIN_SETIDLE2			0x97
-#define WIN_CHECKPOWERMODE2		0x98
-#define WIN_SLEEPNOW2			0x99
-/*
- *	0x9A VENDOR
- */
+/* DOWNLOAD MICROCODE DMA               0x93 */
+#define WIN_STANDBYNOW2                 0x94 /* retired in ATA4 */
+#define WIN_IDLEIMMEDIATE2              0x95 /* force drive to become "ready", retired in ATA4 */
+#define WIN_STANDBY2                    0x96 /* retired in ATA4 */
+#define WIN_SETIDLE2                    0x97 /* retired in ATA4 */
+#define WIN_CHECKPOWERMODE2             0x98 /* retired in ATA4 */
+#define WIN_SLEEPNOW2                   0x99 /* retired in ATA4 */
+/* vendor specific                      0x9A */
+/* reserved                             0x9B..0x9F */
 #define WIN_PACKETCMD			0xA0 /* Send a packet command. */
 #define WIN_PIDENTIFY			0xA1 /* identify ATAPI device	*/
-#define WIN_QUEUED_SERVICE		0xA2
+#define WIN_QUEUED_SERVICE              0xA2 /* obsolete since ACS2 */
+/* reserved                             0xA3..0xAF */
 #define WIN_SMART			0xB0 /* self-monitoring and reporting */
+/* Device Configuration Overlay         0xB1 */
+/* reserved                             0xB2..0xB3 */
+/* Sanitize Device                      0xB4 */
+/* reserved                             0xB5 */
+/* NV Cache                             0xB6 */
+/* reserved for CFA                     0xB7..0xBB */
 #define CFA_ACCESS_METADATA_STORAGE	0xB8
+/* reserved                             0xBC..0xBF */
 #define CFA_ERASE_SECTORS       	0xC0 /* microdrives implement as NOP */
+/* vendor specific                      0xC1..0xC3 */
 #define WIN_MULTREAD			0xC4 /* read sectors using multiple mode*/
 #define WIN_MULTWRITE			0xC5 /* write sectors using multiple mode */
 #define WIN_SETMULT			0xC6 /* enable/disable multiple mode */
-#define WIN_READDMA_QUEUED		0xC7 /* read sectors using Queued DMA transfers */
+#define WIN_READDMA_QUEUED              0xC7 /* read sectors using Queued DMA transfers, obsolete since ACS2 */
 #define WIN_READDMA			0xC8 /* read sectors using DMA transfers */
-#define WIN_READDMA_ONCE		0xC9 /* 28-Bit - without retries */
+#define WIN_READDMA_ONCE                0xC9 /* 28-Bit - w/o retries, obsolete since ATA5 */
 #define WIN_WRITEDMA			0xCA /* write sectors using DMA transfers */
-#define WIN_WRITEDMA_ONCE		0xCB /* 28-Bit - without retries */
-#define WIN_WRITEDMA_QUEUED		0xCC /* write sectors using Queued DMA transfers */
+#define WIN_WRITEDMA_ONCE               0xCB /* 28-Bit - w/o retries, obsolete since ATA5 */
+#define WIN_WRITEDMA_QUEUED		0xCC /* write sectors using Queued DMA transfers, obsolete since ACS2 */
 #define CFA_WRITE_MULTI_WO_ERASE	0xCD /* CFA Write multiple without erase */
-#define WIN_GETMEDIASTATUS		0xDA
-#define WIN_ACKMEDIACHANGE		0xDB /* ATA-1, ATA-2 vendor */
-#define WIN_POSTBOOT			0xDC
-#define WIN_PREBOOT			0xDD
-#define WIN_DOORLOCK			0xDE /* lock door on removable drives */
-#define WIN_DOORUNLOCK			0xDF /* unlock door on removable drives */
+/* WRITE MULTIPLE FUA EXT               0xCE */
+/* reserved                             0xCF..0xDO */
+/* CHECK MEDIA CARD TYPE                0xD1 */
+/* reserved for media card pass through 0xD2..0xD4 */
+/* reserved                             0xD5..0xD9 */
+#define WIN_GETMEDIASTATUS              0xDA /* obsolete since ATA8 */
+/* obsolete since ATA3, retired in ATA4 0xDB..0xDD */
+#define WIN_DOORLOCK                    0xDE /* lock door on removable drives, obsolete since ATA8 */
+#define WIN_DOORUNLOCK                  0xDF /* unlock door on removable drives, obsolete since ATA8 */
 #define WIN_STANDBYNOW1			0xE0
 #define WIN_IDLEIMMEDIATE		0xE1 /* force drive to become "ready" */
 #define WIN_STANDBY             	0xE2 /* Set device in Standby Mode */
@@ -169,25 +178,25 @@ typedef struct IDEDMAOps IDEDMAOps;
 #define WIN_SLEEPNOW1			0xE6
 #define WIN_FLUSH_CACHE			0xE7
 #define WIN_WRITE_BUFFER		0xE8 /* force write only 1 sector */
-#define WIN_WRITE_SAME			0xE9 /* read ata-2 to use */
-	/* SET_FEATURES 0x22 or 0xDD */
+/* READ BUFFER DMA                      0xE9 */
 #define WIN_FLUSH_CACHE_EXT		0xEA /* 48-Bit */
+/* WRITE BUFFER DMA                     0xEB */
 #define WIN_IDENTIFY			0xEC /* ask drive to identify itself	*/
-#define WIN_MEDIAEJECT			0xED
-#define WIN_IDENTIFY_DMA		0xEE /* same as WIN_IDENTIFY, but DMA */
+#define WIN_MEDIAEJECT                  0xED /* obsolete since ATA8 */
+/* obsolete since ATA4                  0xEE */
 #define WIN_SETFEATURES			0xEF /* set special drive features */
-#define EXABYTE_ENABLE_NEST		0xF0
-#define IBM_SENSE_CONDITION		0xF0 /* measure disk temperature */
+#define IBM_SENSE_CONDITION             0xF0 /* measure disk temperature, vendor specific */
 #define WIN_SECURITY_SET_PASS		0xF1
 #define WIN_SECURITY_UNLOCK		0xF2
 #define WIN_SECURITY_ERASE_PREPARE	0xF3
 #define WIN_SECURITY_ERASE_UNIT		0xF4
 #define WIN_SECURITY_FREEZE_LOCK	0xF5
-#define CFA_WEAR_LEVEL			0xF5 /* microdrives implement as NOP */
+#define CFA_WEAR_LEVEL                  0xF5 /* microdrives implement as NOP; not specified in T13! */
 #define WIN_SECURITY_DISABLE		0xF6
+/* vendor specific                      0xF7 */
 #define WIN_READ_NATIVE_MAX		0xF8 /* return the native maximum address */
 #define WIN_SET_MAX			0xF9
-#define DISABLE_SEAGATE			0xFB
+/* vendor specific                      0xFA..0xFF */
 
 /* set to 1 set disable mult support */
 #define MAX_MULT_SECTORS 16
-- 
1.7.6

[Qemu-devel] [PATCH 09/31] ide: Clean up case label indentation in ide_exec_cmd()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d1cbfe7..ff59a45 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -979,7 +979,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
         break;
-	case WIN_READ_EXT:
+    case WIN_READ_EXT:
 	lba48 = 1;
     case WIN_READ:
     case WIN_READ_ONCE:
@@ -989,7 +989,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         s->req_nb_sectors = 1;
         ide_sector_read(s);
         break;
-	case WIN_WRITE_EXT:
+    case WIN_WRITE_EXT:
 	lba48 = 1;
     case WIN_WRITE:
     case WIN_WRITE_ONCE:
@@ -1002,7 +1002,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
         s->media_changed = 1;
         break;
-	case WIN_MULTREAD_EXT:
+    case WIN_MULTREAD_EXT:
 	lba48 = 1;
     case WIN_MULTREAD:
         if (!s->mult_sectors)
@@ -1027,7 +1027,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_write);
         s->media_changed = 1;
         break;
-	case WIN_READDMA_EXT:
+    case WIN_READDMA_EXT:
 	lba48 = 1;
     case WIN_READDMA:
     case WIN_READDMA_ONCE:
@@ -1036,7 +1036,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 	ide_cmd_lba48_transform(s, lba48);
         ide_sector_start_dma(s, IDE_DMA_READ);
         break;
-	case WIN_WRITEDMA_EXT:
+    case WIN_WRITEDMA_EXT:
 	lba48 = 1;
     case WIN_WRITEDMA:
     case WIN_WRITEDMA_ONCE:
@@ -1261,7 +1261,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
         ide_set_irq(s->bus);
         break;
 
-	case WIN_SMART:
+    case WIN_SMART:
 	if (s->drive_kind == IDE_CD)
 		goto abort_cmd;
 	if (s->hcyl != 0xc2 || s->lcyl != 0x4f)
-- 
1.7.6

[Qemu-devel] [PATCH 10/31] ide: Give vmstate structs internal linkage where possible

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index ff59a45..1806e00 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2010,7 +2010,7 @@ static bool ide_error_needed(void *opaque)
 }
 
 /* Fields for GET_EVENT_STATUS_NOTIFICATION ATAPI command */
-const VMStateDescription vmstate_ide_atapi_gesn_state = {
+static const VMStateDescription vmstate_ide_atapi_gesn_state = {
     .name ="ide_drive/atapi/gesn_state",
     .version_id = 1,
     .minimum_version_id = 1,
@@ -2022,7 +2022,7 @@ const VMStateDescription vmstate_ide_atapi_gesn_state = {
     }
 };
 
-const VMStateDescription vmstate_ide_drive_pio_state = {
+static const VMStateDescription vmstate_ide_drive_pio_state = {
     .name = "ide_drive/pio_state",
     .version_id = 1,
     .minimum_version_id = 1,
@@ -2084,7 +2084,7 @@ const VMStateDescription vmstate_ide_drive = {
     }
 };
 
-const VMStateDescription vmstate_ide_error_status = {
+static const VMStateDescription vmstate_ide_error_status = {
     .name ="ide_bus/error",
     .version_id = 1,
     .minimum_version_id = 1,
-- 
1.7.6

[Qemu-devel] [PATCH 11/31] block/raw: Fix to forward method bdrv_media_changed()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Block driver "raw" forwards most methods to the underlying block
driver.  However, it doesn't implement method bdrv_media_changed().
Makes bdrv_media_changed() always return -ENOTSUP.

I believe -fda /dev/fd0 gives you raw over host_floppy, and disk
change detection (fdc register 7 bit 7) is broken.  Testing my theory
requires a computer museum, though.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/block/raw.c b/block/raw.c
index 555db4f..f197479 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -75,6 +75,11 @@ static int raw_is_inserted(BlockDriverState *bs)
     return bdrv_is_inserted(bs->file);
 }
 
+static int raw_media_changed(BlockDriverState *bs)
+{
+    return bdrv_media_changed(bs->file);
+}
+
 static void raw_eject(BlockDriverState *bs, int eject_flag)
 {
     bdrv_eject(bs->file, eject_flag);
@@ -137,8 +142,10 @@ static BlockDriver bdrv_raw = {
     .bdrv_discard       = raw_discard,
 
     .bdrv_is_inserted   = raw_is_inserted,
+    .bdrv_media_changed = raw_media_changed,
     .bdrv_eject         = raw_eject,
     .bdrv_set_locked    = raw_set_locked,
+
     .bdrv_ioctl         = raw_ioctl,
     .bdrv_aio_ioctl     = raw_aio_ioctl,
 
-- 
1.7.6

Re: [Qemu-devel] [PATCH 11/31] block/raw: Fix to forward method bdrv_media_changed()

[Blue Swirl]

On Tue, Sep 6, 2011 at 3:39 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> Block driver "raw" forwards most methods to the underlying block
> driver.  However, it doesn't implement method bdrv_media_changed().
> Makes bdrv_media_changed() always return -ENOTSUP.
>
> I believe -fda /dev/fd0 gives you raw over host_floppy, and disk
> change detection (fdc register 7 bit 7) is broken.  Testing my theory
> requires a computer museum, though.

Or software to emulate ancient computers? Maybe such software could be
already available to you? ;-)

>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/raw.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/block/raw.c b/block/raw.c
> index 555db4f..f197479 100644
> --- a/block/raw.c
> +++ b/block/raw.c
> @@ -75,6 +75,11 @@ static int raw_is_inserted(BlockDriverState *bs)
>     return bdrv_is_inserted(bs->file);
>  }
>
> +static int raw_media_changed(BlockDriverState *bs)
> +{
> +    return bdrv_media_changed(bs->file);
> +}
> +
>  static void raw_eject(BlockDriverState *bs, int eject_flag)
>  {
>     bdrv_eject(bs->file, eject_flag);
> @@ -137,8 +142,10 @@ static BlockDriver bdrv_raw = {
>     .bdrv_discard       = raw_discard,
>
>     .bdrv_is_inserted   = raw_is_inserted,
> +    .bdrv_media_changed = raw_media_changed,
>     .bdrv_eject         = raw_eject,
>     .bdrv_set_locked    = raw_set_locked,
> +
>     .bdrv_ioctl         = raw_ioctl,
>     .bdrv_aio_ioctl     = raw_aio_ioctl,
>
> --
> 1.7.6
>
>
>

Re: [Qemu-devel] [PATCH 11/31] block/raw: Fix to forward method bdrv_media_changed()

[Kevin Wolf]

Am 07.09.2011 21:25, schrieb Blue Swirl:
> On Tue, Sep 6, 2011 at 3:39 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> Block driver "raw" forwards most methods to the underlying block
>> driver.  However, it doesn't implement method bdrv_media_changed().
>> Makes bdrv_media_changed() always return -ENOTSUP.
>>
>> I believe -fda /dev/fd0 gives you raw over host_floppy, and disk
>> change detection (fdc register 7 bit 7) is broken.  Testing my theory
>> requires a computer museum, though.
> 
> Or software to emulate ancient computers? Maybe such software could be
> already available to you? ;-)

In general, such software is buggy. ;-)

Kevin

[Qemu-devel] [PATCH 12/31] block: Leave tracking media change to device models

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

hw/fdc.c is the only one that cares.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c     |   20 ++++++--------------
 block_int.h |    1 -
 hw/fdc.c    |   40 ++++++++++++++++++----------------------
 3 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/block.c b/block.c
index 0aba1bf..2158f83 100644
--- a/block.c
+++ b/block.c
@@ -689,7 +689,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
     }
 
     if (!bdrv_key_required(bs)) {
-        bs->media_changed = 1;
         bdrv_dev_change_media_cb(bs);
     }
 
@@ -726,7 +725,6 @@ void bdrv_close(BlockDriverState *bs)
             bdrv_close(bs->file);
         }
 
-        bs->media_changed = 1;
         bdrv_dev_change_media_cb(bs);
     }
 }
@@ -1665,7 +1663,6 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
     } else if (!bs->valid_key) {
         bs->valid_key = 1;
         /* call the change callback now, we skipped it on open */
-        bs->media_changed = 1;
         bdrv_dev_change_media_cb(bs);
     }
     return ret;
@@ -3040,22 +3037,17 @@ int bdrv_is_inserted(BlockDriverState *bs)
 }
 
 /**
- * Return TRUE if the media changed since the last call to this
- * function. It is currently only used for floppy disks
+ * Return whether the media changed since the last call to this
+ * function, or -ENOTSUP if we don't know.  Most drivers don't know.
  */
 int bdrv_media_changed(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
-    int ret;
 
-    if (!drv || !drv->bdrv_media_changed)
-        ret = -ENOTSUP;
-    else
-        ret = drv->bdrv_media_changed(bs);
-    if (ret == -ENOTSUP)
-        ret = bs->media_changed;
-    bs->media_changed = 0;
-    return ret;
+    if (drv && drv->bdrv_media_changed) {
+        return drv->bdrv_media_changed(bs);
+    }
+    return -ENOTSUP;
 }
 
 /**
diff --git a/block_int.h b/block_int.h
index 19ae75e..539b4ed 100644
--- a/block_int.h
+++ b/block_int.h
@@ -175,7 +175,6 @@ struct BlockDriverState {
                                 this file image */
     char backing_format[16]; /* if non-zero and backing_file exists */
     int is_temporary;
-    int media_changed;
 
     BlockDriverState *backing_hd;
     BlockDriverState *file;
diff --git a/hw/fdc.c b/hw/fdc.c
index 580b657..4f4c621 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -535,30 +535,11 @@ static CPUWriteMemoryFunc * const fdctrl_mem_write_strict[3] = {
     NULL,
 };
 
-static void fdrive_media_changed_pre_save(void *opaque)
-{
-    FDrive *drive = opaque;
-
-    drive->media_changed = drive->bs->media_changed;
-}
-
-static int fdrive_media_changed_post_load(void *opaque, int version_id)
-{
-    FDrive *drive = opaque;
-
-    if (drive->bs != NULL) {
-        drive->bs->media_changed = drive->media_changed;
-    }
-
-    /* User ejected the floppy when drive->bs == NULL */
-    return 0;
-}
-
 static bool fdrive_media_changed_needed(void *opaque)
 {
     FDrive *drive = opaque;
 
-    return (drive->bs != NULL && drive->bs->media_changed != 1);
+    return (drive->bs != NULL && drive->media_changed != 1);
 }
 
 static const VMStateDescription vmstate_fdrive_media_changed = {
@@ -566,8 +547,6 @@ static const VMStateDescription vmstate_fdrive_media_changed = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
-    .pre_save = fdrive_media_changed_pre_save,
-    .post_load = fdrive_media_changed_post_load,
     .fields      = (VMStateField[]) {
         VMSTATE_UINT8(media_changed, FDrive),
         VMSTATE_END_OF_LIST()
@@ -920,6 +899,10 @@ static int fdctrl_media_changed(FDrive *drv)
     if (!drv->bs)
         return 0;
     ret = bdrv_media_changed(drv->bs);
+    if (ret < 0) {
+        ret = drv->media_changed;
+    }
+    drv->media_changed = 0;
     if (ret) {
         fd_revalidate(drv);
     }
@@ -1791,6 +1774,17 @@ static void fdctrl_result_timer(void *opaque)
     fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
 }
 
+static void fdctrl_change_cb(void *opaque)
+{
+    FDrive *drive = opaque;
+
+    drive->media_changed = 1;
+}
+
+static const BlockDevOps fdctrl_block_ops = {
+    .change_media_cb = fdctrl_change_cb,
+};
+
 /* Init functions */
 static int fdctrl_connect_drives(FDCtrl *fdctrl)
 {
@@ -1814,7 +1808,9 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
         fd_init(drive);
         fd_revalidate(drive);
         if (drive->bs) {
+            drive->media_changed = 1;
             bdrv_set_removable(drive->bs, 1);
+            bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive);
         }
     }
     return 0;
-- 
1.7.6

[Qemu-devel] [PATCH 13/31] fdc: Make media change detection more robust

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

fdctrl_change_cb() gets called on a virtual media change via monitor.
It would be nice if host device block drivers called it on physical
media change, but they don't.

bdrv_media_changed() lets you poll for media change, but it returns
"don't know" except with block driver "host_floppy".

FDrive member media_changed gets set on device initialization and by
fdctrl_change_cb(), and cleared by fdctrl_media_changed().  Thus, it's
set on first entry to fdctrl_media_changed() since device
initialization or virtual media change.

fdctrl_media_changed() ignores media_changed unless
bdrv_media_changed() returns "don't know".  If we change media via
monitor (setting media_changed), and the new media's block driver
returns 0, we lose.  Fortunately, "host_floppy" always returns 1 on
first call.  Brittle.  Clean it up not to rely on it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/fdc.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 4f4c621..1d44bbd 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -898,11 +898,15 @@ static int fdctrl_media_changed(FDrive *drv)
 
     if (!drv->bs)
         return 0;
-    ret = bdrv_media_changed(drv->bs);
-    if (ret < 0) {
-        ret = drv->media_changed;
+    if (drv->media_changed) {
+        drv->media_changed = 0;
+        ret = 1;
+    } else {
+        ret = bdrv_media_changed(drv->bs);
+        if (ret < 0) {
+            ret = 0;            /* we don't know, assume no */
+        }
     }
-    drv->media_changed = 0;
     if (ret) {
         fd_revalidate(drv);
     }
-- 
1.7.6

[Qemu-devel] [PATCH 14/31] block: Clean up bdrv_flush_all()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Change (!bdrv_is_removable(bs) || bdrv_is_inserted(bs)) to just
bdrv_is_inserted().  Rationale:

    The value of bdrv_is_removable(bs) matters only when
    bdrv_is_inserted(bs) is false.

    bdrv_is_inserted(bs) is true when bs is open (bs->drv != NULL) and
    not an empty host drive (CD-ROM or floppy).

    Therefore, bdrv_is_removable(bs) matters only when:

    1. bs is not open
       old: may call bdrv_flush(bs), which does nothing
       new: won't call

    2. bs is an empty host drive
       old: may call bdrv_flush(bs), which calls driver method
            raw_flush(), which calls fdatasync() or equivalent, which
            can't do anything useful while the drive is empty
       new: won't call

Result is bs->drv && !bdrv_is_read_only(bs) && bdrv_is_inserted(bs).
bdrv_is_inserted(bs) implies bs->drv.  Drop the redundant test.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 2158f83..487a6a9 100644
--- a/block.c
+++ b/block.c
@@ -1753,8 +1753,7 @@ void bdrv_flush_all(void)
     BlockDriverState *bs;
 
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
-        if (bs->drv && !bdrv_is_read_only(bs) &&
-            (!bdrv_is_removable(bs) || bdrv_is_inserted(bs))) {
+        if (!bdrv_is_read_only(bs) && bdrv_is_inserted(bs)) {
             bdrv_flush(bs);
         }
     }
-- 
1.7.6

[Qemu-devel] [PATCH 15/31] savevm: Include writable devices with removable media

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

savevm and loadvm silently ignore block devices with removable media,
such as floppies and SD cards.  Rolling back a VM to a previous
checkpoint will *not* roll back writes to block devices with removable
media.

Moreover, bdrv_is_removable() is a confused mess, and wrong in at
least one case: it considers "-drive if=xen,media=cdrom -M xenpv"
removable.  It'll be cleaned up later in this series.

Read-only block devices are also ignored, but that's okay.

Fix by ignoring only read-only block devices and empty block devices.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c  |    2 +-
 savevm.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 487a6a9..a8c789a 100644
--- a/block.c
+++ b/block.c
@@ -2097,7 +2097,7 @@ void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
 int bdrv_can_snapshot(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
-    if (!drv || bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
+    if (!drv || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
         return 0;
     }
 
diff --git a/savevm.c b/savevm.c
index b06308b..1feaa70 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1914,7 +1914,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     bs = NULL;
     while ((bs = bdrv_next(bs))) {
 
-        if (bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
+        if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
             continue;
         }
 
@@ -2034,7 +2034,7 @@ int load_vmstate(const char *name)
     bs = NULL;
     while ((bs = bdrv_next(bs))) {
 
-        if (bdrv_is_removable(bs) || bdrv_is_read_only(bs)) {
+        if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
             continue;
         }
 
-- 
1.7.6

[Qemu-devel] [PATCH 16/31] xen: Clean up pci_piix3_xen_ide_unplug()'s test for "not a CD"

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

pci_piix3_xen_ide_unplug() unplugs only disks, not CD-ROMs.  It peeks
into the DriveInfo's BlockDriverState to distinguish between the two.
Unclean; use DriveInfo member media_cd, like xen_config_dev_blk().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/piix.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index b9cdcd6..88d3181 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -27,7 +27,6 @@
 #include <hw/pci.h>
 #include <hw/isa.h>
 #include "block.h"
-#include "block_int.h"
 #include "sysemu.h"
 #include "dma.h"
 
@@ -176,7 +175,7 @@ static int pci_piix3_xen_ide_unplug(DeviceState *dev)
 
     for (; i < 3; i++) {
         di = drive_get_by_index(IF_IDE, i);
-        if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) {
+        if (di != NULL && !di->media_cd) {
             DeviceState *ds = bdrv_get_attached_dev(di->bdrv);
             if (ds) {
                 bdrv_detach_dev(di->bdrv, ds);
-- 
1.7.6

[Qemu-devel] [PATCH 17/31] spitz tosa: Simplify "drive is suitable for microdrive" test

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

We try the drive defined with -drive if=ide,index=0 (or equivalent
sugar).  We use it only if (dinfo && bdrv_is_inserted(dinfo->bdrv) &&
!bdrv_is_removable(dinfo->bdrv)).  This is a convoluted way to test
for "drive media can't be removed".

The only way to create such a drive with -drive if=ide is media=cdrom.
And that sets dinfo->media_cd, so just test that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/spitz.c |   10 +++-------
 hw/tosa.c  |   10 +++-------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/hw/spitz.c b/hw/spitz.c
index c05b5f7..0adae59 100644
--- a/hw/spitz.c
+++ b/hw/spitz.c
@@ -708,17 +708,13 @@ static void spitz_ssp_attach(PXA2xxState *cpu)
 static void spitz_microdrive_attach(PXA2xxState *cpu, int slot)
 {
     PCMCIACardState *md;
-    BlockDriverState *bs;
     DriveInfo *dinfo;
 
     dinfo = drive_get(IF_IDE, 0, 0);
-    if (!dinfo)
+    if (!dinfo || dinfo->media_cd)
         return;
-    bs = dinfo->bdrv;
-    if (bdrv_is_inserted(bs) && !bdrv_is_removable(bs)) {
-        md = dscm1xxxx_init(dinfo);
-        pxa2xx_pcmcia_attach(cpu->pcmcia[slot], md);
-    }
+    md = dscm1xxxx_init(dinfo);
+    pxa2xx_pcmcia_attach(cpu->pcmcia[slot], md);
 }
 
 /* Wm8750 and Max7310 on I2C */
diff --git a/hw/tosa.c b/hw/tosa.c
index a7967a2..7b407f4 100644
--- a/hw/tosa.c
+++ b/hw/tosa.c
@@ -51,17 +51,13 @@
 static void tosa_microdrive_attach(PXA2xxState *cpu)
 {
     PCMCIACardState *md;
-    BlockDriverState *bs;
     DriveInfo *dinfo;
 
     dinfo = drive_get(IF_IDE, 0, 0);
-    if (!dinfo)
+    if (!dinfo || dinfo->media_cd)
         return;
-    bs = dinfo->bdrv;
-    if (bdrv_is_inserted(bs) && !bdrv_is_removable(bs)) {
-        md = dscm1xxxx_init(dinfo);
-        pxa2xx_pcmcia_attach(cpu->pcmcia[0], md);
-    }
+    md = dscm1xxxx_init(dinfo);
+    pxa2xx_pcmcia_attach(cpu->pcmcia[0], md);
 }
 
 static void tosa_out_switch(void *opaque, int line, int level)
-- 
1.7.6

[Qemu-devel] [PATCH 18/31] block: Declare qemu_blockalign() in block.h, not block_int.h

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Device models should be able to use it without an unclean include of
block_int.h.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.h     |    2 ++
 block_int.h |    2 --
 hw/sd.c     |    1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/block.h b/block.h
index 7e3f587..8ec409f 100644
--- a/block.h
+++ b/block.h
@@ -258,6 +258,8 @@ int bdrv_img_create(const char *filename, const char *fmt,
                     const char *base_filename, const char *base_fmt,
                     char *options, uint64_t img_size, int flags);
 
+void *qemu_blockalign(BlockDriverState *bs, size_t size);
+
 #define BDRV_SECTORS_PER_DIRTY_CHUNK 2048
 
 void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable);
diff --git a/block_int.h b/block_int.h
index 539b4ed..5dc0074 100644
--- a/block_int.h
+++ b/block_int.h
@@ -224,8 +224,6 @@ void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs,
                    BlockDriverCompletionFunc *cb, void *opaque);
 void qemu_aio_release(void *p);
 
-void *qemu_blockalign(BlockDriverState *bs, size_t size);
-
 #ifdef _WIN32
 int is_windows_drive(const char *filename);
 #endif
diff --git a/hw/sd.c b/hw/sd.c
index 45e95f9..1af62b2 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -31,7 +31,6 @@
 
 #include "hw.h"
 #include "block.h"
-#include "block_int.h"
 #include "sd.h"
 
 //#define DEBUG_SD 1
-- 
1.7.6

[Qemu-devel] [PATCH 19/31] VMDK: enable twoGbMaxExtentFlat

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Enable the createType 'twoGbMaxExtentFlat'. The supporting code is
already in.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 5f673e9..1e9e8d0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -551,7 +551,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
     if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
         return -EINVAL;
     }
-    if (strcmp(ct, "monolithicFlat")) {
+    if (strcmp(ct, "monolithicFlat") &&
+        strcmp(ct, "twoGbMaxExtentFlat")) {
         fprintf(stderr,
                 "VMDK: Not supported image type \"%s\""".\n", ct);
         return -ENOTSUP;
@@ -672,6 +673,7 @@ static int get_cluster_offset(BlockDriverState *bs,
         return 0;
     }
 
+    offset -= (extent->end_sector - extent->sectors) * SECTOR_SIZE;
     l1_index = (offset >> 9) / extent->l1_entry_sectors;
     if (l1_index >= extent->l1_size) {
         return -1;
-- 
1.7.6

[Qemu-devel] [PATCH 20/31] VMDK: add twoGbMaxExtentSparse support

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Add twoGbMaxExtentSparse support. Introduce vmdk_free_last_extent.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |  133 ++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 83 insertions(+), 50 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 1e9e8d0..49473f6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -174,6 +174,17 @@ static void vmdk_free_extents(BlockDriverState *bs)
     g_free(s->extents);
 }
 
+static void vmdk_free_last_extent(BlockDriverState *bs)
+{
+    BDRVVmdkState *s = bs->opaque;
+
+    if (s->num_extents == 0) {
+        return;
+    }
+    s->num_extents--;
+    s->extents = g_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
+}
+
 static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
 {
     char desc[DESC_SIZE];
@@ -357,18 +368,18 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
     return ret;
 }
 
-static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+static int vmdk_open_vmdk3(BlockDriverState *bs,
+                           BlockDriverState *file,
+                           int flags)
 {
     int ret;
     uint32_t magic;
     VMDK3Header header;
-    BDRVVmdkState *s = bs->opaque;
     VmdkExtent *extent;
 
-    s->desc_offset = 0x200;
-    ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+    ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
     if (ret < 0) {
-        goto fail;
+        return ret;
     }
     extent = vmdk_add_extent(bs,
                              bs->file, false,
@@ -378,58 +389,45 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
                              le32_to_cpu(header.granularity));
     ret = vmdk_init_tables(bs, extent);
     if (ret) {
-        /* vmdk_init_tables cleans up on fail, so only free allocation of
-         * vmdk_add_extent here. */
-        goto fail;
+        /* free extent allocated by vmdk_add_extent */
+        vmdk_free_last_extent(bs);
     }
-    return 0;
- fail:
-    vmdk_free_extents(bs);
     return ret;
 }
 
-static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
+static int vmdk_open_vmdk4(BlockDriverState *bs,
+                           BlockDriverState *file,
+                           int flags)
 {
     int ret;
     uint32_t magic;
     uint32_t l1_size, l1_entry_sectors;
     VMDK4Header header;
-    BDRVVmdkState *s = bs->opaque;
     VmdkExtent *extent;
 
-    s->desc_offset = 0x200;
-    ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+    ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
     if (ret < 0) {
-        goto fail;
+        return ret;
     }
     l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
                         * le64_to_cpu(header.granularity);
+    if (l1_entry_sectors <= 0) {
+        return -EINVAL;
+    }
     l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
                 / l1_entry_sectors;
-    extent = vmdk_add_extent(bs, bs->file, false,
+    extent = vmdk_add_extent(bs, file, false,
                           le64_to_cpu(header.capacity),
                           le64_to_cpu(header.gd_offset) << 9,
                           le64_to_cpu(header.rgd_offset) << 9,
                           l1_size,
                           le32_to_cpu(header.num_gtes_per_gte),
                           le64_to_cpu(header.granularity));
-    if (extent->l1_entry_sectors <= 0) {
-        ret = -EINVAL;
-        goto fail;
-    }
-    /* try to open parent images, if exist */
-    ret = vmdk_parent_open(bs);
-    if (ret) {
-        goto fail;
-    }
-    s->parent_cid = vmdk_read_cid(bs, 1);
     ret = vmdk_init_tables(bs, extent);
     if (ret) {
-        goto fail;
+        /* free extent allocated by vmdk_add_extent */
+        vmdk_free_last_extent(bs);
     }
-    return 0;
- fail:
-    vmdk_free_extents(bs);
     return ret;
 }
 
@@ -460,6 +458,31 @@ static int vmdk_parse_description(const char *desc, const char *opt_name,
     return 0;
 }
 
+/* Open an extent file and append to bs array */
+static int vmdk_open_sparse(BlockDriverState *bs,
+                            BlockDriverState *file,
+                            int flags)
+{
+    uint32_t magic;
+
+    if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+        return -EIO;
+    }
+
+    magic = be32_to_cpu(magic);
+    switch (magic) {
+        case VMDK3_MAGIC:
+            return vmdk_open_vmdk3(bs, file, flags);
+            break;
+        case VMDK4_MAGIC:
+            return vmdk_open_vmdk4(bs, file, flags);
+            break;
+        default:
+            return -EINVAL;
+            break;
+    }
+}
+
 static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
         const char *desc_file_path)
 {
@@ -470,6 +493,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     const char *p = desc;
     int64_t sectors = 0;
     int64_t flat_offset;
+    char extent_path[PATH_MAX];
+    BlockDriverState *extent_file;
 
     while (*p) {
         /* parse extent line:
@@ -504,24 +529,29 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
             goto next_line;
         }
 
+        path_combine(extent_path, sizeof(extent_path),
+                desc_file_path, fname);
+        ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags);
+        if (ret) {
+            return ret;
+        }
+
         /* save to extents array */
         if (!strcmp(type, "FLAT")) {
             /* FLAT extent */
-            char extent_path[PATH_MAX];
-            BlockDriverState *extent_file;
             VmdkExtent *extent;
 
-            path_combine(extent_path, sizeof(extent_path),
-                    desc_file_path, fname);
-            ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags);
-            if (ret) {
-                return ret;
-            }
             extent = vmdk_add_extent(bs, extent_file, true, sectors,
                             0, 0, 0, 0, sectors);
             extent->flat_start_offset = flat_offset;
+        } else if (!strcmp(type, "SPARSE")) {
+            /* SPARSE extent */
+            ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
+            if (ret) {
+                bdrv_delete(extent_file);
+                return ret;
+            }
         } else {
-            /* SPARSE extent, not supported for now */
             fprintf(stderr,
                 "VMDK: Not supported extent type \"%s\""".\n", type);
             return -ENOTSUP;
@@ -552,6 +582,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
         return -EINVAL;
     }
     if (strcmp(ct, "monolithicFlat") &&
+        strcmp(ct, "twoGbMaxExtentSparse") &&
         strcmp(ct, "twoGbMaxExtentFlat")) {
         fprintf(stderr,
                 "VMDK: Not supported image type \"%s\""".\n", ct);
@@ -574,17 +605,19 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
 
 static int vmdk_open(BlockDriverState *bs, int flags)
 {
-    uint32_t magic;
-
-    if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
-        return -EIO;
-    }
+    int ret;
+    BDRVVmdkState *s = bs->opaque;
 
-    magic = be32_to_cpu(magic);
-    if (magic == VMDK3_MAGIC) {
-        return vmdk_open_vmdk3(bs, flags);
-    } else if (magic == VMDK4_MAGIC) {
-        return vmdk_open_vmdk4(bs, flags);
+    if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
+        s->desc_offset = 0x200;
+        /* try to open parent images, if exist */
+        ret = vmdk_parent_open(bs);
+        if (ret) {
+            vmdk_free_extents(bs);
+            return ret;
+        }
+        s->parent_cid = vmdk_read_cid(bs, 1);
+        return 0;
     } else {
         return vmdk_open_desc_file(bs, flags);
     }
-- 
1.7.6

[Qemu-devel] [PATCH 21/31] VMDK: separate vmdk_read_extent/vmdk_write_extent

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Factor out read/write extent code, since there will be more things to
take care of once reading/writing compressed clusters is introduced.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |   54 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 49473f6..106fdbd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -833,6 +833,43 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
     return ret;
 }
 
+static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+                            int64_t offset_in_cluster, const uint8_t *buf,
+                            int nb_sectors, int64_t sector_num)
+{
+    int ret;
+    const uint8_t *write_buf = buf;
+    int write_len = nb_sectors * 512;
+
+    ret = bdrv_pwrite(extent->file,
+                        cluster_offset + offset_in_cluster,
+                        write_buf,
+                        write_len);
+    if (ret != write_len) {
+        ret = ret < 0 ? ret : -EIO;
+        goto out;
+    }
+    ret = 0;
+ out:
+    return ret;
+}
+
+static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+                            int64_t offset_in_cluster, uint8_t *buf,
+                            int nb_sectors)
+{
+    int ret;
+
+    ret = bdrv_pread(extent->file,
+                      cluster_offset + offset_in_cluster,
+                      buf, nb_sectors * 512);
+    if (ret == nb_sectors * 512) {
+        return 0;
+    } else {
+        return -EIO;
+    }
+}
+
 static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
                     uint8_t *buf, int nb_sectors)
 {
@@ -869,10 +906,10 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
                 memset(buf, 0, 512 * n);
             }
         } else {
-            ret = bdrv_pread(extent->file,
-                            cluster_offset + index_in_cluster * 512,
-                            buf, n * 512);
-            if (ret < 0) {
+            ret = vmdk_read_extent(extent,
+                            cluster_offset, index_in_cluster * 512,
+                            buf, n);
+            if (ret) {
                 return ret;
             }
         }
@@ -921,11 +958,10 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
             n = nb_sectors;
         }
 
-        ret = bdrv_pwrite(extent->file,
-                        cluster_offset + index_in_cluster * 512,
-                        buf,
-                        n * 512);
-        if (ret < 0) {
+        ret = vmdk_write_extent(extent,
+                        cluster_offset, index_in_cluster * 512,
+                        buf, n, sector_num);
+        if (ret) {
             return ret;
         }
         if (m_data.valid) {
-- 
1.7.6

[Qemu-devel] [PATCH 22/31] VMDK: Opening compressed extent.

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Added flags field for compressed/streamOptimized extents, open and save
image configuration.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 106fdbd..8a96cfd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -26,9 +26,13 @@
 #include "qemu-common.h"
 #include "block_int.h"
 #include "module.h"
+#include "zlib.h"
 
 #define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
 #define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
+#define VMDK4_COMPRESSION_DEFLATE 1
+#define VMDK4_FLAG_COMPRESS (1 << 16)
+#define VMDK4_FLAG_MARKER (1 << 17)
 
 typedef struct {
     uint32_t version;
@@ -56,6 +60,7 @@ typedef struct {
     int64_t grain_offset;
     char filler[1];
     char check_bytes[4];
+    uint16_t compressAlgorithm;
 } QEMU_PACKED VMDK4Header;
 
 #define L2_CACHE_SIZE 16
@@ -63,6 +68,8 @@ typedef struct {
 typedef struct VmdkExtent {
     BlockDriverState *file;
     bool flat;
+    bool compressed;
+    bool has_marker;
     int64_t sectors;
     int64_t end_sector;
     int64_t flat_start_offset;
@@ -98,6 +105,12 @@ typedef struct VmdkMetaData {
     int valid;
 } VmdkMetaData;
 
+typedef struct VmdkGrainMarker {
+    uint64_t lba;
+    uint32_t size;
+    uint8_t  data[0];
+} VmdkGrainMarker;
+
 static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
     uint32_t magic;
@@ -423,6 +436,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
                           l1_size,
                           le32_to_cpu(header.num_gtes_per_gte),
                           le64_to_cpu(header.granularity));
+    extent->compressed =
+        le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE;
+    extent->has_marker = le32_to_cpu(header.flags) & VMDK4_FLAG_MARKER;
     ret = vmdk_init_tables(bs, extent);
     if (ret) {
         /* free extent allocated by vmdk_add_extent */
-- 
1.7.6

[Qemu-devel] [PATCH 23/31] VMDK: read/write compressed extent

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Add support for reading/writing compressed extent.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 100 insertions(+), 11 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 8a96cfd..36a761f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -775,10 +775,12 @@ static int get_cluster_offset(BlockDriverState *bs,
 
         /* Avoid the L2 tables update for the images that have snapshots. */
         *cluster_offset = bdrv_getlength(extent->file);
-        bdrv_truncate(
-            extent->file,
-            *cluster_offset + (extent->cluster_sectors << 9)
-        );
+        if (!extent->compressed) {
+            bdrv_truncate(
+                extent->file,
+                *cluster_offset + (extent->cluster_sectors << 9)
+            );
+        }
 
         *cluster_offset >>= 9;
         tmp = cpu_to_le32(*cluster_offset);
@@ -854,9 +856,28 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
                             int nb_sectors, int64_t sector_num)
 {
     int ret;
+    VmdkGrainMarker *data = NULL;
+    uLongf buf_len;
     const uint8_t *write_buf = buf;
     int write_len = nb_sectors * 512;
 
+    if (extent->compressed) {
+        if (!extent->has_marker) {
+            ret = -EINVAL;
+            goto out;
+        }
+        buf_len = (extent->cluster_sectors << 9) * 2;
+        data = g_malloc(buf_len + sizeof(VmdkGrainMarker));
+        if (compress(data->data, &buf_len, buf, nb_sectors << 9) != Z_OK ||
+                buf_len == 0) {
+            ret = -EINVAL;
+            goto out;
+        }
+        data->lba = sector_num;
+        data->size = buf_len;
+        write_buf = (uint8_t *)data;
+        write_len = buf_len + sizeof(VmdkGrainMarker);
+    }
     ret = bdrv_pwrite(extent->file,
                         cluster_offset + offset_in_cluster,
                         write_buf,
@@ -867,6 +888,7 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
     }
     ret = 0;
  out:
+    g_free(data);
     return ret;
 }
 
@@ -875,15 +897,65 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
                             int nb_sectors)
 {
     int ret;
+    int cluster_bytes, buf_bytes;
+    uint8_t *cluster_buf, *compressed_data;
+    uint8_t *uncomp_buf;
+    uint32_t data_len;
+    VmdkGrainMarker *marker;
+    uLongf buf_len;
 
+
+    if (!extent->compressed) {
+        ret = bdrv_pread(extent->file,
+                          cluster_offset + offset_in_cluster,
+                          buf, nb_sectors * 512);
+        if (ret == nb_sectors * 512) {
+            return 0;
+        } else {
+            return -EIO;
+        }
+    }
+    cluster_bytes = extent->cluster_sectors * 512;
+    /* Read two clusters in case GrainMarker + compressed data > one cluster */
+    buf_bytes = cluster_bytes * 2;
+    cluster_buf = g_malloc(buf_bytes);
+    uncomp_buf = g_malloc(cluster_bytes);
     ret = bdrv_pread(extent->file,
-                      cluster_offset + offset_in_cluster,
-                      buf, nb_sectors * 512);
-    if (ret == nb_sectors * 512) {
-        return 0;
-    } else {
-        return -EIO;
+                cluster_offset,
+                cluster_buf, buf_bytes);
+    if (ret < 0) {
+        goto out;
     }
+    compressed_data = cluster_buf;
+    buf_len = cluster_bytes;
+    data_len = cluster_bytes;
+    if (extent->has_marker) {
+        marker = (VmdkGrainMarker *)cluster_buf;
+        compressed_data = marker->data;
+        data_len = le32_to_cpu(marker->size);
+    }
+    if (!data_len || data_len > buf_bytes) {
+        ret = -EINVAL;
+        goto out;
+    }
+    ret = uncompress(uncomp_buf, &buf_len, compressed_data, data_len);
+    if (ret != Z_OK) {
+        ret = -EINVAL;
+        goto out;
+
+    }
+    if (offset_in_cluster < 0 ||
+            offset_in_cluster + nb_sectors * 512 > buf_len) {
+        ret = -EINVAL;
+        goto out;
+    }
+    memcpy(buf, uncomp_buf + offset_in_cluster, nb_sectors * 512);
+    ret = 0;
+
+ out:
+    g_free(uncomp_buf);
+    g_free(cluster_buf);
+    return ret;
 }
 
 static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
@@ -963,8 +1035,25 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
                                 bs,
                                 extent,
                                 &m_data,
-                                sector_num << 9, 1,
+                                sector_num << 9, !extent->compressed,
                                 &cluster_offset);
+        if (extent->compressed) {
+            if (ret == 0) {
+                /* Refuse write to allocated cluster for streamOptimized */
+                fprintf(stderr,
+                        "VMDK: can't write to allocated cluster"
+                        " for streamOptimized\n");
+                return -EIO;
+            } else {
+                /* allocate */
+                ret = get_cluster_offset(
+                                        bs,
+                                        extent,
+                                        &m_data,
+                                        sector_num << 9, 1,
+                                        &cluster_offset);
+            }
+        }
         if (ret) {
             return -EINVAL;
         }
-- 
1.7.6

[Qemu-devel] [PATCH 24/31] VMDK: creating streamOptimized subformat

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Creating streamOptimized subformat. Added subformat option
'streamOptimized', to create a image with compression enabled and each
cluster with a GrainMarker.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 36a761f..54f7441 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1090,7 +1090,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
 }
 
 
-static int vmdk_create_extent(const char *filename, int64_t filesize, bool flat)
+static int vmdk_create_extent(const char *filename, int64_t filesize,
+                              bool flat, bool compress)
 {
     int ret, i;
     int fd = 0;
@@ -1114,7 +1115,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, bool flat)
     magic = cpu_to_be32(VMDK4_MAGIC);
     memset(&header, 0, sizeof(header));
     header.version = 1;
-    header.flags = 3; /* ?? */
+    header.flags =
+        3 | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0);
+    header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
     header.capacity = filesize / 512;
     header.granularity = 128;
     header.num_gtes_per_gte = 512;
@@ -1144,6 +1147,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, bool flat)
     header.rgd_offset = cpu_to_le64(header.rgd_offset);
     header.gd_offset = cpu_to_le64(header.gd_offset);
     header.grain_offset = cpu_to_le64(header.grain_offset);
+    header.compressAlgorithm = cpu_to_le16(header.compressAlgorithm);
 
     header.check_bytes[0] = 0xa;
     header.check_bytes[1] = 0x20;
@@ -1285,7 +1289,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     const char *fmt = NULL;
     int flags = 0;
     int ret = 0;
-    bool flat, split;
+    bool flat, split, compress;
     char ext_desc_lines[BUF_SIZE] = "";
     char path[PATH_MAX], prefix[PATH_MAX], postfix[PATH_MAX];
     const int64_t split_size = 0x80000000;  /* VMDK has constant split size */
@@ -1334,7 +1338,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     } else if (strcmp(fmt, "monolithicFlat") &&
                strcmp(fmt, "monolithicSparse") &&
                strcmp(fmt, "twoGbMaxExtentSparse") &&
-               strcmp(fmt, "twoGbMaxExtentFlat")) {
+               strcmp(fmt, "twoGbMaxExtentFlat") &&
+               strcmp(fmt, "streamOptimized")) {
         fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt);
         return -EINVAL;
     }
@@ -1342,6 +1347,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
               strcmp(fmt, "twoGbMaxExtentSparse"));
     flat = !(strcmp(fmt, "monolithicFlat") &&
              strcmp(fmt, "twoGbMaxExtentFlat"));
+    compress = !strcmp(fmt, "streamOptimized");
     if (flat) {
         desc_extent_line = "RW %lld FLAT \"%s\" 0\n";
     } else {
@@ -1396,7 +1402,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
         snprintf(ext_filename, sizeof(ext_filename), "%s%s",
                 path, desc_filename);
 
-        if (vmdk_create_extent(ext_filename, size, flat)) {
+        if (vmdk_create_extent(ext_filename, size, flat, compress)) {
             return -EINVAL;
         }
         filesize -= size;
@@ -1510,7 +1516,7 @@ static QEMUOptionParameter vmdk_create_options[] = {
         .type = OPT_STRING,
         .help =
             "VMDK flat extent format, can be one of "
-            "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat} "
+            "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} "
     },
     { NULL }
 };
-- 
1.7.6

[Qemu-devel] [PATCH 25/31] VMDK: bugfix, open Haiku vmdk image

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

Haiku provides a specially formed vmdk image, which let qemu abort. It a
combination of sparse header and flat data (i.e. with not l1/l2 table at
all). The fix is turn to descriptor when sparse header is zero in field
'capacity'.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 54f7441..892b18e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -408,6 +408,9 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
     return ret;
 }
 
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
+                               int64_t desc_offset);
+
 static int vmdk_open_vmdk4(BlockDriverState *bs,
                            BlockDriverState *file,
                            int flags)
@@ -422,6 +425,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
     if (ret < 0) {
         return ret;
     }
+    if (header.capacity == 0 && header.desc_offset) {
+        return vmdk_open_desc_file(bs, flags, header.desc_offset << 9);
+    }
     l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
                         * le64_to_cpu(header.granularity);
     if (l1_entry_sectors <= 0) {
@@ -559,7 +565,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
 
             extent = vmdk_add_extent(bs, extent_file, true, sectors,
                             0, 0, 0, 0, sectors);
-            extent->flat_start_offset = flat_offset;
+            extent->flat_start_offset = flat_offset << 9;
         } else if (!strcmp(type, "SPARSE")) {
             /* SPARSE extent */
             ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
@@ -582,14 +588,15 @@ next_line:
     return 0;
 }
 
-static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
+                               int64_t desc_offset)
 {
     int ret;
     char buf[2048];
     char ct[128];
     BDRVVmdkState *s = bs->opaque;
 
-    ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
+    ret = bdrv_pread(bs->file, desc_offset, buf, sizeof(buf));
     if (ret < 0) {
         return ret;
     }
@@ -635,7 +642,7 @@ static int vmdk_open(BlockDriverState *bs, int flags)
         s->parent_cid = vmdk_read_cid(bs, 1);
         return 0;
     } else {
-        return vmdk_open_desc_file(bs, flags);
+        return vmdk_open_desc_file(bs, flags, 0);
     }
 }
 
-- 
1.7.6

[Qemu-devel] [PATCH 26/31] VMDK: bugfix, opening vSphere 4 exported image

[Kevin Wolf]

From: Fam Zheng <famcool@gmail.com>

The vSphere 4 exported image is streamOptimized extent, which is not
quite correctly handled. Ignore rdgOffset when RGD flag bit not set.

Signed-off-by: Fam Zheng <famcool@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 892b18e..6c8edfc 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -31,6 +31,7 @@
 #define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
 #define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
 #define VMDK4_COMPRESSION_DEFLATE 1
+#define VMDK4_FLAG_RGD (1 << 1)
 #define VMDK4_FLAG_COMPRESS (1 << 16)
 #define VMDK4_FLAG_MARKER (1 << 17)
 
@@ -55,8 +56,8 @@ typedef struct {
     int64_t desc_offset;
     int64_t desc_size;
     int32_t num_gtes_per_gte;
-    int64_t rgd_offset;
     int64_t gd_offset;
+    int64_t rgd_offset;
     int64_t grain_offset;
     char filler[1];
     char check_bytes[4];
@@ -420,6 +421,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
     uint32_t l1_size, l1_entry_sectors;
     VMDK4Header header;
     VmdkExtent *extent;
+    int64_t l1_backup_offset = 0;
 
     ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
     if (ret < 0) {
@@ -435,10 +437,13 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
     }
     l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
                 / l1_entry_sectors;
+    if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) {
+        l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9;
+    }
     extent = vmdk_add_extent(bs, file, false,
                           le64_to_cpu(header.capacity),
                           le64_to_cpu(header.gd_offset) << 9,
-                          le64_to_cpu(header.rgd_offset) << 9,
+                          l1_backup_offset,
                           l1_size,
                           le32_to_cpu(header.num_gtes_per_gte),
                           le64_to_cpu(header.granularity));
-- 
1.7.6

[Qemu-devel] [PATCH 27/31] scsi: execute SYNCHRONIZE_CACHE asynchronously

[Kevin Wolf]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-disk.c |   41 +++++++++++++++++++++++++----------------
 1 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 3cc830f..5c652a2 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -129,6 +129,24 @@ static void scsi_read_complete(void * opaque, int ret)
     scsi_req_data(&r->req, r->iov.iov_len);
 }
 
+static void scsi_flush_complete(void * opaque, int ret)
+{
+    SCSIDiskReq *r = (SCSIDiskReq *)opaque;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+    if (r->req.aiocb != NULL) {
+        r->req.aiocb = NULL;
+        bdrv_acct_done(s->bs, &r->acct);
+    }
+
+    if (ret < 0) {
+        if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_FLUSH)) {
+            return;
+        }
+    }
+
+    scsi_req_complete(&r->req, GOOD);
+}
 
 /* Read more data from scsi device into buffer.  */
 static void scsi_read_data(SCSIRequest *req)
@@ -792,7 +810,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     uint64_t nb_sectors;
     int buflen = 0;
-    int ret;
 
     switch (req->cmd.buf[0]) {
     case TEST_UNIT_READY:
@@ -864,20 +881,6 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         outbuf[7] = 0;
         buflen = 8;
         break;
-    case SYNCHRONIZE_CACHE:
-    {
-        BlockAcctCookie acct;
-
-        bdrv_acct_start(s->bs, &acct, 0, BDRV_ACCT_FLUSH);
-        ret = bdrv_flush(s->bs);
-        bdrv_acct_done(s->bs, &acct);
-        if (ret < 0) {
-            if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_FLUSH)) {
-                return -1;
-            }
-        }
-        break;
-    }
     case GET_CONFIGURATION:
         memset(outbuf, 0, 8);
         /* ??? This should probably return much more information.  For now
@@ -985,7 +988,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     case START_STOP:
     case ALLOW_MEDIUM_REMOVAL:
     case READ_CAPACITY_10:
-    case SYNCHRONIZE_CACHE:
     case READ_TOC:
     case GET_CONFIGURATION:
     case SERVICE_ACTION_IN:
@@ -997,6 +999,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
 
         r->iov.iov_len = rc;
         break;
+    case SYNCHRONIZE_CACHE:
+        bdrv_acct_start(s->bs, &r->acct, 0, BDRV_ACCT_FLUSH);
+        r->req.aiocb = bdrv_aio_flush(s->bs, scsi_flush_complete, r);
+        if (r->req.aiocb == NULL) {
+            scsi_flush_complete(r, -EIO);
+        }
+        return 0;
     case READ_6:
     case READ_10:
     case READ_12:
-- 
1.7.6

[Qemu-devel] [PATCH 28/31] scsi: fix accounting of writes

[Kevin Wolf]

From: Paolo Bonzini <pbonzini@redhat.com>

Writes go through scsi_write_complete at least twice, the first time
to get some data without having actually written anything.  Because
of this, the first time scsi_write_complete is called it will call
bdrv_acct_done and account a read incorrectly.  Fix this by looking
at the aiocb.  I am doing the same in scsi_read_complete for symmetry,
but it is only needed in the (bogus) case of bdrv_aio_readv returning
NULL.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-disk.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5c652a2..c23c2b8 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -111,9 +111,10 @@ static void scsi_read_complete(void * opaque, int ret)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     int n;
 
-    r->req.aiocb = NULL;
-
-    bdrv_acct_done(s->bs, &r->acct);
+    if (r->req.aiocb != NULL) {
+        r->req.aiocb = NULL;
+        bdrv_acct_done(s->bs, &r->acct);
+    }
 
     if (ret) {
         if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_READ)) {
@@ -235,9 +236,10 @@ static void scsi_write_complete(void * opaque, int ret)
     uint32_t len;
     uint32_t n;
 
-    r->req.aiocb = NULL;
-
-    bdrv_acct_done(s->bs, &r->acct);
+    if (r->req.aiocb != NULL) {
+        r->req.aiocb = NULL;
+        bdrv_acct_done(s->bs, &r->acct);
+    }
 
     if (ret) {
         if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_WRITE)) {
-- 
1.7.6

[Qemu-devel] [PATCH 29/31] scsi: refine constants for READ CAPACITY 16

[Kevin Wolf]

From: Paolo Bonzini <pbonzini@redhat.com>

Rename SERVICE_ACTION_IN to SERVICE_ACTION_IN_16 to distinguish
from the 12-byte CDB variant, and add a constant for the subcommand.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-bus.c  |    3 ++-
 hw/scsi-defs.h |    8 +++++++-
 hw/scsi-disk.c |    6 +++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 6f0d039..d6fd19a 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -977,7 +977,7 @@ static const char *scsi_command_name(uint8_t cmd)
         [ LOCATE_16                ] = "LOCATE_16",
         [ WRITE_SAME_16            ] = "WRITE_SAME_16",
         [ ERASE_16                 ] = "ERASE_16",
-        [ SERVICE_ACTION_IN        ] = "SERVICE_ACTION_IN",
+        [ SERVICE_ACTION_IN_16     ] = "SERVICE_ACTION_IN_16",
         [ WRITE_LONG_16            ] = "WRITE_LONG_16",
         [ REPORT_LUNS              ] = "REPORT_LUNS",
         [ BLANK                    ] = "BLANK",
@@ -987,6 +987,7 @@ static const char *scsi_command_name(uint8_t cmd)
         [ LOAD_UNLOAD              ] = "LOAD_UNLOAD",
         [ READ_12                  ] = "READ_12",
         [ WRITE_12                 ] = "WRITE_12",
+        [ SERVICE_ACTION_IN_12     ] = "SERVICE_ACTION_IN_12",
         [ WRITE_VERIFY_12          ] = "WRITE_VERIFY_12",
         [ VERIFY_12                ] = "VERIFY_12",
         [ SEARCH_HIGH_12           ] = "SEARCH_HIGH_12",
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index ea288fa..bfe9392 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -102,7 +102,7 @@
 #define LOCATE_16             0x92
 #define WRITE_SAME_16         0x93
 #define ERASE_16              0x93
-#define SERVICE_ACTION_IN     0x9e
+#define SERVICE_ACTION_IN_16  0x9e
 #define WRITE_LONG_16         0x9f
 #define REPORT_LUNS           0xa0
 #define BLANK                 0xa1
@@ -112,6 +112,7 @@
 #define LOAD_UNLOAD           0xa6
 #define READ_12               0xa8
 #define WRITE_12              0xaa
+#define SERVICE_ACTION_IN_12  0xab
 #define WRITE_VERIFY_12       0xae
 #define VERIFY_12             0xaf
 #define SEARCH_HIGH_12        0xb0
@@ -123,6 +124,11 @@
 #define SET_CD_SPEED          0xbb
 
 /*
+ * SERVICE ACTION IN subcodes
+ */
+#define SAI_READ_CAPACITY_16  0x10
+
+/*
  *  SAM Status codes
  */
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c23c2b8..ead4163 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -890,9 +890,9 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         outbuf[7] = 8; // CD-ROM
         buflen = 8;
         break;
-    case SERVICE_ACTION_IN:
+    case SERVICE_ACTION_IN_16:
         /* Service Action In subcommands. */
-        if ((req->cmd.buf[1] & 31) == 0x10) {
+        if ((req->cmd.buf[1] & 31) == SAI_READ_CAPACITY_16) {
             DPRINTF("SAI READ CAPACITY(16)\n");
             memset(outbuf, 0, req->cmd.xfer);
             bdrv_get_geometry(s->bs, &nb_sectors);
@@ -992,7 +992,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     case READ_CAPACITY_10:
     case READ_TOC:
     case GET_CONFIGURATION:
-    case SERVICE_ACTION_IN:
+    case SERVICE_ACTION_IN_16:
     case VERIFY_10:
         rc = scsi_disk_emulate_command(r, outbuf);
         if (rc < 0) {
-- 
1.7.6

[Qemu-devel] [PATCH 30/31] scsi: fill in additional sense length correctly

[Kevin Wolf]

From: Paolo Bonzini <pbonzini@redhat.com>

Even though we do not use them, we should include the last three
bytes of sense data in the additional sense length.

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

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index d6fd19a..160eaee 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -498,6 +498,7 @@ void scsi_req_build_sense(SCSIRequest *req, SCSISense sense)
     memset(req->sense, 0, 18);
     req->sense[0] = 0xf0;
     req->sense[2] = sense.key;
+    req->sense[7] = 10;
     req->sense[12] = sense.asc;
     req->sense[13] = sense.ascq;
     req->sense_len = 18;
@@ -883,7 +884,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
         /* Return fixed format sense buffer */
         buf[0] = 0xf0;
         buf[2] = sense.key;
-        buf[7] = 7;
+        buf[7] = 10;
         buf[12] = sense.asc;
         buf[13] = sense.ascq;
         return MIN(len, 18);
-- 
1.7.6

[Qemu-devel] [PATCH 31/31] scsi: improve MODE SENSE emulation

[Kevin Wolf]

From: Paolo Bonzini <pbonzini@redhat.com>

- do not return extra pages when requesting all pages (PAGE CODE = 0x3f)

- return correct sense code for PC = 3 (saved parameters not supported)

- do not return geometry pages for CD devices

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi-disk.c |   96 +++++++++++++++++++++++++++++++-------------------------
 1 files changed, 53 insertions(+), 43 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index ead4163..9724d0f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -546,12 +546,12 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
     return buflen;
 }
 
-static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p,
+static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
                            int page_control)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     BlockDriverState *bdrv = s->bs;
     int cylinders, heads, secs;
+    uint8_t *p = *p_outbuf;
 
     /*
      * If Changeable Values are requested, a mask denoting those mode parameters
@@ -561,10 +561,13 @@ static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p,
      */
     switch (page) {
     case 4: /* Rigid disk device geometry page. */
+        if (s->qdev.type == TYPE_ROM) {
+            return -1;
+        }
         p[0] = 4;
         p[1] = 0x16;
         if (page_control == 1) { /* Changeable Values */
-            return p[1] + 2;
+            break;
         }
         /* if a geometry hint is available, use it */
         bdrv_get_geometry_hint(bdrv, &cylinders, &heads, &secs);
@@ -590,13 +593,16 @@ static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p,
         /* Medium rotation rate [rpm], 5400 rpm */
         p[20] = (5400 >> 8) & 0xff;
         p[21] = 5400 & 0xff;
-        return p[1] + 2;
+        break;
 
     case 5: /* Flexible disk device geometry page. */
+        if (s->qdev.type == TYPE_ROM) {
+            return -1;
+        }
         p[0] = 5;
         p[1] = 0x1e;
         if (page_control == 1) { /* Changeable Values */
-            return p[1] + 2;
+            break;
         }
         /* Transfer rate [kbit/s], 5Mbit/s */
         p[2] = 5000 >> 8;
@@ -629,26 +635,27 @@ static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p,
         /* Medium rotation rate [rpm], 5400 rpm */
         p[28] = (5400 >> 8) & 0xff;
         p[29] = 5400 & 0xff;
-        return p[1] + 2;
+        break;
 
     case 8: /* Caching page.  */
         p[0] = 8;
         p[1] = 0x12;
         if (page_control == 1) { /* Changeable Values */
-            return p[1] + 2;
+            break;
         }
         if (bdrv_enable_write_cache(s->bs)) {
             p[2] = 4; /* WCE */
         }
-        return p[1] + 2;
+        break;
 
     case 0x2a: /* CD Capabilities and Mechanical Status page. */
-        if (s->qdev.type != TYPE_ROM)
-            return 0;
+        if (s->qdev.type != TYPE_ROM) {
+            return -1;
+        }
         p[0] = 0x2a;
         p[1] = 0x14;
         if (page_control == 1) { /* Changeable Values */
-            return p[1] + 2;
+            break;
         }
         p[2] = 3; // CD-R & CD-RW read
         p[3] = 0; // Writing not supported
@@ -673,27 +680,30 @@ static int mode_sense_page(SCSIRequest *req, int page, uint8_t *p,
         p[19] = (16 * 176) & 0xff;
         p[20] = (16 * 176) >> 8; // 16x write speed current
         p[21] = (16 * 176) & 0xff;
-        return p[1] + 2;
+        break;
 
     default:
-        return 0;
+        return -1;
     }
+
+    *p_outbuf += p[1] + 2;
+    return p[1] + 2;
 }
 
-static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)
+static int scsi_disk_emulate_mode_sense(SCSIDiskReq *r, uint8_t *outbuf)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint64_t nb_sectors;
-    int page, dbd, buflen, page_control;
+    int page, dbd, buflen, ret, page_control;
     uint8_t *p;
     uint8_t dev_specific_param;
 
-    dbd = req->cmd.buf[1]  & 0x8;
-    page = req->cmd.buf[2] & 0x3f;
-    page_control = (req->cmd.buf[2] & 0xc0) >> 6;
+    dbd = r->req.cmd.buf[1]  & 0x8;
+    page = r->req.cmd.buf[2] & 0x3f;
+    page_control = (r->req.cmd.buf[2] & 0xc0) >> 6;
     DPRINTF("Mode Sense(%d) (page %d, xfer %zd, page_control %d)\n",
-        (req->cmd.buf[0] == MODE_SENSE) ? 6 : 10, page, req->cmd.xfer, page_control);
-    memset(outbuf, 0, req->cmd.xfer);
+        (r->req.cmd.buf[0] == MODE_SENSE) ? 6 : 10, page, r->req.cmd.xfer, page_control);
+    memset(outbuf, 0, r->req.cmd.xfer);
     p = outbuf;
 
     if (bdrv_is_read_only(s->bs)) {
@@ -702,7 +712,7 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)
         dev_specific_param = 0x00;
     }
 
-    if (req->cmd.buf[0] == MODE_SENSE) {
+    if (r->req.cmd.buf[0] == MODE_SENSE) {
         p[1] = 0; /* Default media type.  */
         p[2] = dev_specific_param;
         p[3] = 0; /* Block descriptor length.  */
@@ -716,7 +726,7 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)
 
     bdrv_get_geometry(s->bs, &nb_sectors);
     if (!dbd && nb_sectors) {
-        if (req->cmd.buf[0] == MODE_SENSE) {
+        if (r->req.cmd.buf[0] == MODE_SENSE) {
             outbuf[3] = 8; /* Block descriptor length  */
         } else { /* MODE_SENSE_10 */
             outbuf[7] = 8; /* Block descriptor length  */
@@ -735,23 +745,21 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)
         p += 8;
     }
 
-    if (page_control == 3) { /* Saved Values */
-        return -1; /* ILLEGAL_REQUEST */
+    if (page_control == 3) {
+        /* Saved Values */
+        scsi_check_condition(r, SENSE_CODE(SAVING_PARAMS_NOT_SUPPORTED));
+        return -1;
     }
 
-    switch (page) {
-    case 0x04:
-    case 0x05:
-    case 0x08:
-    case 0x2a:
-        p += mode_sense_page(req, page, p, page_control);
-        break;
-    case 0x3f:
-        p += mode_sense_page(req, 0x08, p, page_control);
-        p += mode_sense_page(req, 0x2a, p, page_control);
-        break;
-    default:
-        return -1; /* ILLEGAL_REQUEST */
+    if (page == 0x3f) {
+        for (page = 0; page <= 0x3e; page++) {
+            mode_sense_page(s, page, &p, page_control);
+        }
+    } else {
+        ret = mode_sense_page(s, page, &p, page_control);
+        if (ret == -1) {
+            return -1;
+        }
     }
 
     buflen = p - outbuf;
@@ -760,14 +768,14 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)
      * following data that is available to be transferred. The mode data
      * length does not include itself.
      */
-    if (req->cmd.buf[0] == MODE_SENSE) {
+    if (r->req.cmd.buf[0] == MODE_SENSE) {
         outbuf[0] = buflen - 1;
     } else { /* MODE_SENSE_10 */
         outbuf[0] = ((buflen - 2) >> 8) & 0xff;
         outbuf[1] = (buflen - 2) & 0xff;
     }
-    if (buflen > req->cmd.xfer)
-        buflen = req->cmd.xfer;
+    if (buflen > r->req.cmd.xfer)
+        buflen = r->req.cmd.xfer;
     return buflen;
 }
 
@@ -825,7 +833,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         break;
     case MODE_SENSE:
     case MODE_SENSE_10:
-        buflen = scsi_disk_emulate_mode_sense(req, outbuf);
+        buflen = scsi_disk_emulate_mode_sense(r, outbuf);
         if (buflen < 0)
             goto illegal_request;
         break;
@@ -946,7 +954,9 @@ not_ready:
     return -1;
 
 illegal_request:
-    scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+    if (r->req.status == -1) {
+        scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+    }
     return -1;
 }
 
-- 
1.7.6

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

[Anthony Liguori]

On 09/06/2011 10:39 AM, Kevin Wolf wrote:
> The following changes since commit f69539b14bdba7a5cd22e1f4bed439b476b17286:
>
>    apb_pci: convert PCI space to memory API (2011-09-04 09:28:04 +0000)
>
> are available in the git repository at:
>    git://repo.or.cz/qemu/kevin.git for-anthony


Pulled.  Thanks.

Regards,

Anthony Liguori

>
> Fam Zheng (8):
>        VMDK: enable twoGbMaxExtentFlat
>        VMDK: add twoGbMaxExtentSparse support
>        VMDK: separate vmdk_read_extent/vmdk_write_extent
>        VMDK: Opening compressed extent.
>        VMDK: read/write compressed extent
>        VMDK: creating streamOptimized subformat
>        VMDK: bugfix, open Haiku vmdk image
>        VMDK: bugfix, opening vSphere 4 exported image
>
> Frediano Ziglio (1):
>        linux aio: some comments
>
> Kevin Wolf (3):
>        qcow2: Properly initialise QcowL2Meta
>        qcow2: Fix error cases to run depedent requests
>        async: Allow nested qemu_bh_poll calls
>
> Markus Armbruster (14):
>        block: Attach non-qdev devices as well
>        block: Generalize change_cb() to BlockDevOps
>        block: Split change_cb() into change_media_cb(), resize_cb()
>        ide: Update command code definitions as per ACS-2 Table B.2
>        ide: Clean up case label indentation in ide_exec_cmd()
>        ide: Give vmstate structs internal linkage where possible
>        block/raw: Fix to forward method bdrv_media_changed()
>        block: Leave tracking media change to device models
>        fdc: Make media change detection more robust
>        block: Clean up bdrv_flush_all()
>        savevm: Include writable devices with removable media
>        xen: Clean up pci_piix3_xen_ide_unplug()'s test for "not a CD"
>        spitz tosa: Simplify "drive is suitable for microdrive" test
>        block: Declare qemu_blockalign() in block.h, not block_int.h
>
> Paolo Bonzini (5):
>        scsi: execute SYNCHRONIZE_CACHE asynchronously
>        scsi: fix accounting of writes
>        scsi: refine constants for READ CAPACITY 16
>        scsi: fill in additional sense length correctly
>        scsi: improve MODE SENSE emulation
>
>   async.c              |   24 +++-
>   block.c              |  104 ++++++++-------
>   block.h              |   28 +++-
>   block/qcow2.c        |   12 +-
>   block/raw-posix.c    |    4 +
>   block/raw.c          |    7 +
>   block/vmdk.c         |  346 +++++++++++++++++++++++++++++++++++++++-----------
>   block_int.h          |   14 +--
>   blockdev.c           |    5 +-
>   hw/fdc.c             |   46 ++++----
>   hw/ide/core.c        |   35 +++---
>   hw/ide/internal.h    |  171 +++++++++++++------------
>   hw/ide/piix.c        |    7 +-
>   hw/pflash_cfi01.c    |    1 +
>   hw/pflash_cfi02.c    |    1 +
>   hw/qdev-properties.c |    6 +-
>   hw/scsi-bus.c        |    6 +-
>   hw/scsi-defs.h       |    8 +-
>   hw/scsi-disk.c       |  157 +++++++++++++----------
>   hw/sd.c              |   14 +-
>   hw/spitz.c           |   10 +-
>   hw/tosa.c            |   10 +-
>   hw/usb-msd.c         |    2 +-
>   hw/virtio-blk.c      |   12 +-
>   hw/xen_disk.c        |    1 +
>   linux-aio.c          |    1 +
>   savevm.c             |    4 +-
>   27 files changed, 652 insertions(+), 384 deletions(-)
>
>

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

[Peter Maydell]

On 30 April 2014 19:23, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit e2da502c003b9a91b4aea7684959192bd07c1f1d:
>
>   Merge remote-tracking branch 'remotes/otubo/seccomp' into staging (2014-04-28 14:14:35 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to b7079df4100069959f4e9d90d5cb5ba7d4ebbf1a:
>
>   curl: Fix hang reading from slow connections (2014-04-30 16:34:21 +0200)
>
> ----------------------------------------------------------------
> Block patches

Applied, thanks.

-- PMM