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

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

[Kevin Wolf]

The following changes since commit 45d1aa828f8c94b082a0aa2dbf76535594ee45df:

  Update OpenBIOS images to r1018 (2011-01-30 13:10:10 +0000)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git block

Blue Swirl (1):
      qcow2-refcount: remove write-only variables

Christoph Hellwig (3):
      block: add block_resize monitor command
      block: tell drivers about an image resize
      virtio-blk: tell the guest about size changes

Jes Sorensen (6):
      strtosz(): use unsigned char and switch to qemu_isspace()
      strtosz() use qemu_toupper() to simplify switch statement
      strtosz(): Fix name confusion in use of modf()
      strtosz(): Use suffix macros in switch() statement
      Add documentation for STRTOSZ_DEFSUFFIX_ macros
      Reorganize struct Qcow2Cache for better struct packing

Kevin Wolf (3):
      qemu-io: Fix discard command
      qcow2: Add bdrv_discard support
      raw-win32: Fix bdrv_flush return value

MORITA Kazutaka (1):
      sheepdog: support creating images on remote hosts

Markus Armbruster (11):
      scsi hotplug: Set DriveInfo member bus correctly
      blockdev: New drive_get_next(), replacing qdev_init_bdrv()
      blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h
      blockdev: Put BlockInterfaceType names and max_devs in tables
      blockdev: Fix regression in -drive if=scsi,index=N
      blockdev: Make drive_add() take explicit type, index parameters
      blockdev: Factor drive_index_to_{bus,unit}_id out of drive_init()
      blockdev: New drive_get_by_index()
      blockdev: Reject multiple definitions for the same drive
      blockdev: Replace drive_add()'s fmt, ... by optstr parameter
      blockdev: Fix drive_add for drives without media

Stefan Hajnoczi (3):
      virtio-pci: Disable virtio-ioeventfd when !CONFIG_IOTHREAD
      ahci: Fix cpu_physical_memory_unmap() argument ordering
      qed: Images with backing file do not require QED_F_NEED_CHECK

 block.c                |   12 ++-
 block.h                |    3 +-
 block/qcow2-cache.c    |    2 +-
 block/qcow2-cluster.c  |   82 +++++++++++++++++++++++
 block/qcow2-refcount.c |    5 +-
 block/qcow2.c          |    8 ++
 block/qcow2.h          |    2 +
 block/qed.c            |   24 +++++--
 block/raw-win32.c      |    2 +-
 block/sheepdog.c       |   17 ++++-
 block_int.h            |    5 +-
 blockdev.c             |  173 ++++++++++++++++++++++++++++++++---------------
 blockdev.h             |   19 ++++--
 cutils.c               |   28 ++++-----
 hmp-commands.hx        |   19 +++++
 hw/device-hotplug.c    |    5 +-
 hw/ide.h               |    2 +
 hw/ide/ahci.c          |   11 ++--
 hw/ide/core.c          |    6 ++-
 hw/pci-hotplug.c       |    1 +
 hw/pl181.c             |    7 +-
 hw/qdev.c              |   15 ----
 hw/qdev.h              |    2 -
 hw/scsi.h              |    3 +-
 hw/sd.c                |    7 ++-
 hw/ssi-sd.c            |    7 +-
 hw/usb-msd.c           |    3 +-
 hw/virtio-blk.c        |   10 +++
 kvm-all.c              |    8 ++-
 qemu-common.h          |   13 ++--
 qemu-io.c              |    2 +-
 qmp-commands.hx        |   28 ++++++++
 savevm.c               |    1 -
 vl.c                   |  104 +++++++++++++++++------------
 34 files changed, 447 insertions(+), 189 deletions(-)

[Qemu-devel] [PATCH 01/28] strtosz(): use unsigned char and switch to qemu_isspace()

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

isspace() behavior is undefined for signed char.

Bug pointed out by Eric Blake, thanks!

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

diff --git a/cutils.c b/cutils.c
index 4d2e27c..a067cf4 100644
--- a/cutils.c
+++ b/cutils.c
@@ -294,7 +294,8 @@ int fcntl_setfl(int fd, int flag)
 int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
     int64_t retval = -1;
-    char *endptr, c, d;
+    char *endptr;
+    unsigned char c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
 
@@ -314,7 +315,7 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
      */
     c = *endptr;
     d = c;
-    if (isspace(c) || c == '\0' || c == ',') {
+    if (qemu_isspace(c) || c == '\0' || c == ',') {
         c = 0;
         if (default_suffix) {
             d = default_suffix;
@@ -361,7 +362,7 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
      */
     if (c != 0) {
         endptr++;
-        if (!isspace(*endptr) && *endptr != ',' && *endptr != 0) {
+        if (!qemu_isspace(*endptr) && *endptr != ',' && *endptr != 0) {
             goto fail;
         }
     }
-- 
1.7.2.3

[Qemu-devel] [PATCH 02/28] strtosz() use qemu_toupper() to simplify switch statement

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 cutils.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/cutils.c b/cutils.c
index a067cf4..78d35e2 100644
--- a/cutils.c
+++ b/cutils.c
@@ -323,16 +323,14 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
             d = c;
         }
     }
-    switch (d) {
+    switch (qemu_toupper(d)) {
     case 'B':
-    case 'b':
         mul = 1;
         if (mul_required) {
             goto fail;
         }
         break;
     case 'K':
-    case 'k':
         mul = 1 << 10;
         break;
     case 0:
@@ -340,15 +338,12 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
             goto fail;
         }
     case 'M':
-    case 'm':
         mul = 1ULL << 20;
         break;
     case 'G':
-    case 'g':
         mul = 1ULL << 30;
         break;
     case 'T':
-    case 't':
         mul = 1ULL << 40;
         break;
     default:
-- 
1.7.2.3

[Qemu-devel] [PATCH 03/28] strtosz(): Fix name confusion in use of modf()

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 cutils.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cutils.c b/cutils.c
index 78d35e2..369a016 100644
--- a/cutils.c
+++ b/cutils.c
@@ -304,8 +304,8 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
     if (isnan(val) || endptr == nptr || errno != 0) {
         goto fail;
     }
-    integral = modf(val, &fraction);
-    if (integral != 0) {
+    fraction = modf(val, &integral);
+    if (fraction != 0) {
         mul_required = 1;
     }
     /*
-- 
1.7.2.3

[Qemu-devel] [PATCH 04/28] strtosz(): Use suffix macros in switch() statement

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 cutils.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cutils.c b/cutils.c
index 369a016..8d562b2 100644
--- a/cutils.c
+++ b/cutils.c
@@ -324,26 +324,26 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
         }
     }
     switch (qemu_toupper(d)) {
-    case 'B':
+    case STRTOSZ_DEFSUFFIX_B:
         mul = 1;
         if (mul_required) {
             goto fail;
         }
         break;
-    case 'K':
+    case STRTOSZ_DEFSUFFIX_KB:
         mul = 1 << 10;
         break;
     case 0:
         if (mul_required) {
             goto fail;
         }
-    case 'M':
+    case STRTOSZ_DEFSUFFIX_MB:
         mul = 1ULL << 20;
         break;
-    case 'G':
+    case STRTOSZ_DEFSUFFIX_GB:
         mul = 1ULL << 30;
         break;
-    case 'T':
+    case STRTOSZ_DEFSUFFIX_TB:
         mul = 1ULL << 40;
         break;
     default:
-- 
1.7.2.3

[Qemu-devel] [PATCH 05/28] block: add block_resize monitor command

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Add a monitor command that allows resizing of block devices while
qemu is running.  It uses the existing bdrv_truncate method already
used by qemu-img to do it's work.  Compared to qemu-img the size
parsing is very simplicistic, but I think having a properly numering
object is more useful for non-humand monitor users than having
the units and relative resize parsing.

For SCSI devices the new size can be updated in Linux guests by
doing the following shell command:

	echo > /sys/class/scsi_device/0:0:0:0/device/rescan

For ATA devices I don't know of a way to update the block device
size in Linux system, and for virtio-blk the next two patches
will provide an automatic update of the size when this command
is issued on the host.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c      |   30 ++++++++++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   19 +++++++++++++++++++
 qmp-commands.hx |   28 ++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f7f591f..e48d33d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -705,3 +705,33 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 
     return 0;
 }
+
+/*
+ * XXX: replace the QERR_UNDEFINED_ERROR errors with real values once the
+ * existing QERR_ macro mess is cleaned up.  A good example for better
+ * error reports can be found in the qemu-img resize code.
+ */
+int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    int64_t size = qdict_get_int(qdict, "size");
+    BlockDriverState *bs;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, device);
+        return -1;
+    }
+
+    if (size < 0) {
+        qerror_report(QERR_UNDEFINED_ERROR);
+        return -1;
+    }
+
+    if (bdrv_truncate(bs, size)) {
+        qerror_report(QERR_UNDEFINED_ERROR);
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/blockdev.h b/blockdev.h
index 4536b5c..b8a88bf 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -53,5 +53,6 @@ int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1cea572..8df4adf 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -53,6 +53,25 @@ Quit the emulator.
 ETEXI
 
     {
+        .name       = "block_resize",
+        .args_type  = "device:B,size:o",
+        .params     = "device size",
+        .help       = "resize a block image",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_block_resize,
+    },
+
+STEXI
+@item block_resize
+@findex block_resize
+Resize a block image while a guest is running.  Usually requires guest
+action to see the updated size.  Resize to a lower size is supported,
+but should be used with extreme caution.  Note that this command only
+resizes image files, it can not resize block devices like LVM volumes.
+ETEXI
+
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 56c4d8b..9f79f5f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -601,6 +601,34 @@ Example:
 -> { "execute": "netdev_del", "arguments": { "id": "netdev1" } }
 <- { "return": {} }
 
+
+EQMP
+
+    {
+        .name       = "block_resize",
+        .args_type  = "device:B,size:o",
+        .params     = "device size",
+        .help       = "resize a block image",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_block_resize,
+    },
+
+SQMP
+block_resize
+------------
+
+Resize a block image while a guest is running.
+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "size": new size
+
+Example:
+
+-> { "execute": "block_resize", "arguments": { "device": "scratch", "size": 1073741824 } }
+<- { "return": {} }
+
 EQMP
 
     {
-- 
1.7.2.3

[Qemu-devel] [PATCH 06/28] block: tell drivers about an image resize

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Extend the change_cb callback with a reason argument, and use it
to tell drivers about size changes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c       |   12 ++++++++----
 block.h       |    3 ++-
 block_int.h   |    5 ++++-
 hw/ide/core.c |    6 +++++-
 hw/sd.c       |    7 ++++++-
 5 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 7ad3ddf..998df1b 100644
--- a/block.c
+++ b/block.c
@@ -645,7 +645,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
         /* call the change callback */
         bs->media_changed = 1;
         if (bs->change_cb)
-            bs->change_cb(bs->change_opaque);
+            bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
     }
 
     return 0;
@@ -684,7 +684,7 @@ void bdrv_close(BlockDriverState *bs)
         /* call the change callback */
         bs->media_changed = 1;
         if (bs->change_cb)
-            bs->change_cb(bs->change_opaque);
+            bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
     }
 }
 
@@ -1135,6 +1135,9 @@ 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);
+        }
     }
     return ret;
 }
@@ -1366,7 +1369,8 @@ int bdrv_enable_write_cache(BlockDriverState *bs)
 
 /* XXX: no longer used */
 void bdrv_set_change_cb(BlockDriverState *bs,
-                        void (*change_cb)(void *opaque), void *opaque)
+                        void (*change_cb)(void *opaque, int reason),
+                        void *opaque)
 {
     bs->change_cb = change_cb;
     bs->change_opaque = opaque;
@@ -1411,7 +1415,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
         /* call the change callback now, we skipped it on open */
         bs->media_changed = 1;
         if (bs->change_cb)
-            bs->change_cb(bs->change_opaque);
+            bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
     }
     return ret;
 }
diff --git a/block.h b/block.h
index f923add..239f729 100644
--- a/block.h
+++ b/block.h
@@ -182,7 +182,8 @@ 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), void *opaque);
+                        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 12663e8..6ebdc3e 100644
--- a/block_int.h
+++ b/block_int.h
@@ -153,7 +153,7 @@ struct BlockDriverState {
     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);
+    void (*change_cb)(void *opaque, int reason);
     void *change_opaque;
 
     BlockDriver *drv; /* NULL means no media */
@@ -203,6 +203,9 @@ 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 e698c13..dd63664 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1584,11 +1584,15 @@ 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)
+static void cdrom_change_cb(void *opaque, int reason)
 {
     IDEState *s = opaque;
     uint64_t nb_sectors;
 
+    if (!(reason & CHANGE_MEDIA)) {
+        return;
+    }
+
     bdrv_get_geometry(s->bs, &nb_sectors);
     s->nb_sectors = nb_sectors;
 
diff --git a/hw/sd.c b/hw/sd.c
index 601545b..789ca84 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -422,9 +422,14 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
     sd->pwd_len = 0;
 }
 
-static void sd_cardchange(void *opaque)
+static void sd_cardchange(void *opaque, int reason)
 {
     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);
-- 
1.7.2.3

[Qemu-devel] [PATCH 07/28] virtio-blk: tell the guest about size changes

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

Raise a config change interrupt when the size changed.  This allows
virtio-blk guest drivers to read-read the information from the
config space once it got the config chaged interrupt.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/virtio-blk.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 9f2a9c0..ffac5a4 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -514,6 +514,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)
+{
+    VirtIOBlock *s = opaque;
+
+    if (reason & CHANGE_SIZE) {
+        virtio_notify_config(&s->vdev);
+    }
+}
+
 VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
 {
     VirtIOBlock *s;
@@ -556,6 +565,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);
     s->bs->buffer_alignment = conf->logical_block_size;
 
     add_boot_device_path(conf->bootindex, dev, "/disk@0,0");
-- 
1.7.2.3

[Qemu-devel] [PATCH 08/28] virtio-pci: Disable virtio-ioeventfd when !CONFIG_IOTHREAD

[Kevin Wolf]

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

It is not possible to use virtio-ioeventfd when building without an I/O
thread.  We rely on a signal to kick us out of vcpu execution.  Timers
and AIO use SIGALRM and SIGUSR2 respectively.  Unfortunately eventfd
does not support O_ASYNC (SIGIO) so eventfd cannot be used in a signal
driven manner.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 kvm-all.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 255b6fa..8f0e17c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -449,10 +449,14 @@ int kvm_check_extension(KVMState *s, unsigned int extension)
 
 static int kvm_check_many_ioeventfds(void)
 {
-    /* Older kernels have a 6 device limit on the KVM io bus.  Find out so we
+    /* Userspace can use ioeventfd for io notification.  This requires a host
+     * that supports eventfd(2) and an I/O thread; since eventfd does not
+     * support SIGIO it cannot interrupt the vcpu.
+     *
+     * Older kernels have a 6 device limit on the KVM io bus.  Find out so we
      * can avoid creating too many ioeventfds.
      */
-#ifdef CONFIG_EVENTFD
+#if defined(CONFIG_EVENTFD) && defined(CONFIG_IOTHREAD)
     int ioeventfds[7];
     int i, ret = 0;
     for (i = 0; i < ARRAY_SIZE(ioeventfds); i++) {
-- 
1.7.2.3

[Qemu-devel] [PATCH 09/28] Add documentation for STRTOSZ_DEFSUFFIX_ macros

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-common.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qemu-common.h b/qemu-common.h
index c351131..79e1149 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -153,6 +153,13 @@ int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
 
+/*
+ * strtosz() suffixes used to specify the default treatment of an
+ * argument passed to strtosz() without an explicit suffix.
+ * These should be defined using upper case characters in the range
+ * A-Z, as strtosz() will use qemu_toupper() on the given argument
+ * prior to comparison.
+ */
 #define STRTOSZ_DEFSUFFIX_TB	'T'
 #define STRTOSZ_DEFSUFFIX_GB	'G'
 #define STRTOSZ_DEFSUFFIX_MB	'M'
-- 
1.7.2.3

[Qemu-devel] [PATCH 10/28] ahci: Fix cpu_physical_memory_unmap() argument ordering

[Kevin Wolf]

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

The len and is_write arguments to cpu_physical_memory_unmap() were
swapped.  This patch changes calls to use the correct argument ordering.

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

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 968fdce..433171c 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -513,12 +513,12 @@ static void map_page(uint8_t **ptr, uint64_t addr, uint32_t wanted)
     target_phys_addr_t len = wanted;
 
     if (*ptr) {
-        cpu_physical_memory_unmap(*ptr, 1, len, len);
+        cpu_physical_memory_unmap(*ptr, len, 1, len);
     }
 
     *ptr = cpu_physical_memory_map(addr, &len, 1);
     if (len < wanted) {
-        cpu_physical_memory_unmap(*ptr, 1, len, len);
+        cpu_physical_memory_unmap(*ptr, len, 1, len);
         *ptr = NULL;
     }
 }
@@ -956,7 +956,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
     ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
 
     if (cmd_mapped) {
-        cpu_physical_memory_unmap(cmd_fis, 0, cmd_len, cmd_len);
+        cpu_physical_memory_unmap(cmd_fis, cmd_len, 0, cmd_len);
     }
 }
 
@@ -1002,7 +1002,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist)
     }
 
 out:
-    cpu_physical_memory_unmap(prdt, 0, prdt_len, prdt_len);
+    cpu_physical_memory_unmap(prdt, prdt_len, 0, prdt_len);
     return r;
 }
 
@@ -1228,7 +1228,7 @@ static int handle_cmd(AHCIState *s, int port, int slot)
     }
 
 out:
-    cpu_physical_memory_unmap(cmd_fis, 1, cmd_len, cmd_len);
+    cpu_physical_memory_unmap(cmd_fis, cmd_len, 1, cmd_len);
 
     if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) {
         /* async command, complete later */
-- 
1.7.2.3

[Qemu-devel] [PATCH 11/28] Reorganize struct Qcow2Cache for better struct packing

[Kevin Wolf]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move size after the two pointers in struct Qcow2Cache to get better
packing of struct elements on 64 bit architectures.

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

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 8f2955b..3824739 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -35,9 +35,9 @@ typedef struct Qcow2CachedTable {
 } Qcow2CachedTable;
 
 struct Qcow2Cache {
-    int                     size;
     Qcow2CachedTable*       entries;
     struct Qcow2Cache*      depends;
+    int                     size;
     bool                    depends_on_flush;
     bool                    writethrough;
 };
-- 
1.7.2.3

[Qemu-devel] [PATCH 12/28] sheepdog: support creating images on remote hosts

[Kevin Wolf]

From: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

This patch parses the input filename in sd_create(), and enables us
specifying a target server to create sheepdog images.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/sheepdog.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index e62820a..a54e0de 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1294,12 +1294,23 @@ static int do_sd_create(char *filename, int64_t vdi_size,
 static int sd_create(const char *filename, QEMUOptionParameter *options)
 {
     int ret;
-    uint32_t vid = 0;
+    uint32_t vid = 0, base_vid = 0;
     int64_t vdi_size = 0;
     char *backing_file = NULL;
+    BDRVSheepdogState s;
+    char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
+    uint32_t snapid;
 
     strstart(filename, "sheepdog:", (const char **)&filename);
 
+    memset(&s, 0, sizeof(s));
+    memset(vdi, 0, sizeof(vdi));
+    memset(tag, 0, sizeof(tag));
+    if (parse_vdiname(&s, filename, vdi, &snapid, tag) < 0) {
+        error_report("invalid filename\n");
+        return -EINVAL;
+    }
+
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
             vdi_size = options->value.n;
@@ -1338,11 +1349,11 @@ static int sd_create(const char *filename, QEMUOptionParameter *options)
             return -EINVAL;
         }
 
-        vid = s->inode.vdi_id;
+        base_vid = s->inode.vdi_id;
         bdrv_delete(bs);
     }
 
-    return do_sd_create((char *)filename, vdi_size, vid, NULL, 0, NULL, NULL);
+    return do_sd_create((char *)vdi, vdi_size, base_vid, &vid, 0, s.addr, s.port);
 }
 
 static void sd_close(BlockDriverState *bs)
-- 
1.7.2.3

[Qemu-devel] [PATCH 13/28] qemu-io: Fix discard command

[Kevin Wolf]

qemu-io passed bytes where it's supposed to pass sectors, so discard requests
were off.

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

diff --git a/qemu-io.c b/qemu-io.c
index 5b24c5e..4470e49 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1465,7 +1465,7 @@ discard_f(int argc, char **argv)
 	}
 
 	gettimeofday(&t1, NULL);
-	ret = bdrv_discard(bs, offset, count);
+	ret = bdrv_discard(bs, offset >> BDRV_SECTOR_BITS, count >> BDRV_SECTOR_BITS);
 	gettimeofday(&t2, NULL);
 
 	if (ret < 0) {
-- 
1.7.2.3

[Qemu-devel] [PATCH 14/28] qcow2: Add bdrv_discard support

[Kevin Wolf]

This adds a bdrv_discard function to qcow2 that frees the discarded clusters.
It does not yet pass the discard on to the underlying file system driver, but
the space can be reused by future writes to the image.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qcow2-cluster.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c         |    8 +++++
 block/qcow2.h         |    2 +
 3 files changed, 92 insertions(+), 0 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1c2003a..5fb8c66 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -888,3 +888,85 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
     }
     return 0;
 }
+
+/*
+ * This discards as many clusters of nb_clusters as possible at once (i.e.
+ * all clusters in the same L2 table) and returns the number of discarded
+ * clusters.
+ */
+static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
+    unsigned int nb_clusters)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint64_t l2_offset, *l2_table;
+    int l2_index;
+    int ret;
+    int i;
+
+    ret = get_cluster_table(bs, offset, &l2_table, &l2_offset, &l2_index);
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* Limit nb_clusters to one L2 table */
+    nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+
+    for (i = 0; i < nb_clusters; i++) {
+        uint64_t old_offset;
+
+        old_offset = be64_to_cpu(l2_table[l2_index + i]);
+        old_offset &= ~QCOW_OFLAG_COPIED;
+
+        if (old_offset == 0) {
+            continue;
+        }
+
+        /* First remove L2 entries */
+        qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+        l2_table[l2_index + i] = cpu_to_be64(0);
+
+        /* Then decrease the refcount */
+        qcow2_free_any_clusters(bs, old_offset, 1);
+    }
+
+    ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
+    if (ret < 0) {
+        return ret;
+    }
+
+    return nb_clusters;
+}
+
+int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
+    int nb_sectors)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint64_t end_offset;
+    unsigned int nb_clusters;
+    int ret;
+
+    end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
+
+    /* Round start up and end down */
+    offset = align_offset(offset, s->cluster_size);
+    end_offset &= ~(s->cluster_size - 1);
+
+    if (offset > end_offset) {
+        return 0;
+    }
+
+    nb_clusters = size_to_clusters(s, end_offset - offset);
+
+    /* Each L2 table is handled by its own loop iteration */
+    while (nb_clusters > 0) {
+        ret = discard_single_l2(bs, offset, nb_clusters);
+        if (ret < 0) {
+            return ret;
+        }
+
+        nb_clusters -= ret;
+        offset += (ret * s->cluster_size);
+    }
+
+    return 0;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 49bf7b9..dbe4fdd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1084,6 +1084,13 @@ static int qcow2_make_empty(BlockDriverState *bs)
     return 0;
 }
 
+static int qcow2_discard(BlockDriverState *bs, int64_t sector_num,
+    int nb_sectors)
+{
+    return qcow2_discard_clusters(bs, sector_num << BDRV_SECTOR_BITS,
+        nb_sectors);
+}
+
 static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
 {
     BDRVQcowState *s = bs->opaque;
@@ -1349,6 +1356,7 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_aio_writev    = qcow2_aio_writev,
     .bdrv_aio_flush     = qcow2_aio_flush,
 
+    .bdrv_discard           = qcow2_discard,
     .bdrv_truncate          = qcow2_truncate,
     .bdrv_write_compressed  = qcow2_write_compressed,
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 6d80120..a019831 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -209,6 +209,8 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
                                          int compressed_size);
 
 int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
+    int nb_sectors);
 
 /* qcow2-snapshot.c functions */
 int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
-- 
1.7.2.3

[Qemu-devel] [PATCH 15/28] qed: Images with backing file do not require QED_F_NEED_CHECK

[Kevin Wolf]

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

The consistency check on open is necessary in order to fix inconsistent
table offsets left as a result of a crash mid-operation.  Images with a
backing file actually flush before updating table offsets and are
therefore guaranteed to be consistent.  Do not mark these images dirty.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qed.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index a46f9ef..3273448 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -977,6 +977,19 @@ static void qed_aio_write_prefill(void *opaque, int ret)
 }
 
 /**
+ * Check if the QED_F_NEED_CHECK bit should be set during allocating write
+ */
+static bool qed_should_set_need_check(BDRVQEDState *s)
+{
+    /* The flush before L2 update path ensures consistency */
+    if (s->bs->backing_hd) {
+        return false;
+    }
+
+    return !(s->header.features & QED_F_NEED_CHECK);
+}
+
+/**
  * Write new data cluster
  *
  * @acb:        Write request
@@ -1001,15 +1014,12 @@ static void qed_aio_write_alloc(QEDAIOCB *acb, size_t len)
     acb->cur_cluster = qed_alloc_clusters(s, acb->cur_nclusters);
     qemu_iovec_copy(&acb->cur_qiov, acb->qiov, acb->qiov_offset, len);
 
-    /* Write new cluster if the image is already marked dirty */
-    if (s->header.features & QED_F_NEED_CHECK) {
+    if (qed_should_set_need_check(s)) {
+        s->header.features |= QED_F_NEED_CHECK;
+        qed_write_header(s, qed_aio_write_prefill, acb);
+    } else {
         qed_aio_write_prefill(acb, 0);
-        return;
     }
-
-    /* Mark the image dirty before writing the new cluster */
-    s->header.features |= QED_F_NEED_CHECK;
-    qed_write_header(s, qed_aio_write_prefill, acb);
 }
 
 /**
-- 
1.7.2.3

[Qemu-devel] [PATCH 16/28] raw-win32: Fix bdrv_flush return value

[Kevin Wolf]

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

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 06c9710..c204a80 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -153,7 +153,7 @@ static int raw_flush(BlockDriverState *bs)
     int ret;
 
     ret = FlushFileBuffers(s->hfile);
-    if (ret != 0) {
+    if (ret == 0) {
         return -EIO;
     }
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 17/28] scsi hotplug: Set DriveInfo member bus correctly

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

drive_init() picks the first free bus and unit number, unless the user
specifies them.

This isn't a good fit for the drive_add monitor command, because there
we specify the controller by PCI address instead of using bus number
set by drive_init().

scsi_hot_add() takes care to replace the unit number set by
drive_init() by the real one, but it neglects to replace the bus
number.  Thus, bus/unit in DriveInfo may be bogus.  Affects
drive_get() and drive_get_max_bus().  I'm not aware of anything bad
happening because of that; looks like by the time we're hot-plugging,
the two functions aren't used anymore.  Fix it anyway.

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

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 270a982..b6dcbda 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -90,6 +90,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
      * specified).
      */
     dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
+    dinfo->bus = scsibus->busnr;
     scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit, false);
     if (!scsidev) {
         return -1;
-- 
1.7.2.3

[Qemu-devel] [PATCH 18/28] qcow2-refcount: remove write-only variables

[Kevin Wolf]

From: Blue Swirl <blauwirbel@gmail.com>

Variables l2_modified and l2_size are not really used, remove them.
Spotted by GCC 4.6.0:
  CC    block/qcow2-refcount.o
/src/qemu/block/qcow2-refcount.c: In function 'qcow2_update_snapshot_refcount':
/src/qemu/block/qcow2-refcount.c:708:37: error: variable 'l2_modified' set but not used [-Werror=unused-but-set-variable]
/src/qemu/block/qcow2-refcount.c:708:9: error: variable 'l2_size' set but not used [-Werror=unused-but-set-variable]

CC: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index e37e226..915d85a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -705,7 +705,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
     BDRVQcowState *s = bs->opaque;
     uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2, l1_allocated;
     int64_t old_offset, old_l2_offset;
-    int l2_size, i, j, l1_modified, l2_modified, nb_csectors, refcount;
+    int i, j, l1_modified, nb_csectors, refcount;
     int ret;
 
     l2_table = NULL;
@@ -729,14 +729,12 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
         l1_allocated = 0;
     }
 
-    l2_size = s->l2_size * sizeof(uint64_t);
     l1_modified = 0;
     for(i = 0; i < l1_size; i++) {
         l2_offset = l1_table[i];
         if (l2_offset) {
             old_l2_offset = l2_offset;
             l2_offset &= ~QCOW_OFLAG_COPIED;
-            l2_modified = 0;
 
             ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
                 (void**) &l2_table);
@@ -788,7 +786,6 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                                 s->refcount_block_cache);
                         }
                         l2_table[j] = cpu_to_be64(offset);
-                        l2_modified = 1;
                         qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
                     }
                 }
-- 
1.7.2.3

[Qemu-devel] [PATCH 19/28] blockdev: New drive_get_next(), replacing qdev_init_bdrv()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

qdev_init_bdrv() doesn't belong into qdev.c; it's about drives, not
qdevs.  Rename to drive_get_next, move to blockdev.c, drop the bogus
DeviceState argument, and return DriveInfo instead of
BlockDriverState.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c  |   10 ++++++++++
 blockdev.h  |    1 +
 hw/pl181.c  |    7 ++++---
 hw/qdev.c   |   14 --------------
 hw/qdev.h   |    2 --
 hw/ssi-sd.c |    7 ++++---
 6 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e48d33d..699f312 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -93,6 +93,16 @@ int drive_get_max_bus(BlockInterfaceType type)
     return max_bus;
 }
 
+/* Get a block device.  This should only be used for single-drive devices
+   (e.g. SD/Floppy/MTD).  Multi-disk devices (scsi/ide) should use the
+   appropriate bus.  */
+DriveInfo *drive_get_next(BlockInterfaceType type)
+{
+    static int next_block_unit[IF_COUNT];
+
+    return drive_get(type, 0, next_block_unit[type]++);
+}
+
 DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
 {
     DriveInfo *dinfo;
diff --git a/blockdev.h b/blockdev.h
index b8a88bf..3ed6634 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -36,6 +36,7 @@ struct DriveInfo {
 
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 int drive_get_max_bus(BlockInterfaceType type);
+DriveInfo *drive_get_next(BlockInterfaceType type);
 void drive_uninit(DriveInfo *dinfo);
 DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
 
diff --git a/hw/pl181.c b/hw/pl181.c
index 3e5f92f..36d9d02 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -7,6 +7,7 @@
  * This code is licenced under the GPL.
  */
 
+#include "blockdev.h"
 #include "sysbus.h"
 #include "sd.h"
 
@@ -449,15 +450,15 @@ static int pl181_init(SysBusDevice *dev)
 {
     int iomemtype;
     pl181_state *s = FROM_SYSBUS(pl181_state, dev);
-    BlockDriverState *bd;
+    DriveInfo *dinfo;
 
     iomemtype = cpu_register_io_memory(pl181_readfn, pl181_writefn, s,
                                        DEVICE_NATIVE_ENDIAN);
     sysbus_init_mmio(dev, 0x1000, iomemtype);
     sysbus_init_irq(dev, &s->irq[0]);
     sysbus_init_irq(dev, &s->irq[1]);
-    bd = qdev_init_bdrv(&dev->qdev, IF_SD);
-    s->card = sd_init(bd, 0);
+    dinfo = drive_get_next(IF_SD);
+    s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
     qemu_register_reset(pl181_reset, s);
     pl181_reset(s);
     /* ??? Save/restore.  */
diff --git a/hw/qdev.c b/hw/qdev.c
index 5b8d374..0c94fb2 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -458,20 +458,6 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
     }
 }
 
-static int next_block_unit[IF_COUNT];
-
-/* Get a block device.  This should only be used for single-drive devices
-   (e.g. SD/Floppy/MTD).  Multi-disk devices (scsi/ide) should use the
-   appropriate bus.  */
-BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type)
-{
-    int unit = next_block_unit[type]++;
-    DriveInfo *dinfo;
-
-    dinfo = drive_get(type, 0, unit);
-    return dinfo ? dinfo->bdrv : NULL;
-}
-
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
diff --git a/hw/qdev.h b/hw/qdev.h
index e520aaa..9808f85 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -137,8 +137,6 @@ bool qdev_machine_modified(void);
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
 void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
 
-BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type);
-
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
 /*** Device API.  ***/
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index a1a63b2..fb4b649 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -7,6 +7,7 @@
  * This code is licenced under the GNU GPL v2.
  */
 
+#include "blockdev.h"
 #include "ssi.h"
 #include "sd.h"
 
@@ -231,11 +232,11 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
 static int ssi_sd_init(SSISlave *dev)
 {
     ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
-    BlockDriverState *bs;
+    DriveInfo *dinfo;
 
     s->mode = SSI_SD_CMD;
-    bs = qdev_init_bdrv(&dev->qdev, IF_SD);
-    s->sd = sd_init(bs, 1);
+    dinfo = drive_get_next(IF_SD);
+    s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, 1);
     register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
     return 0;
 }
-- 
1.7.2.3

[Qemu-devel] [PATCH 20/28] blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.h    |    6 ++++++
 qemu-common.h |    6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index 3ed6634..2cbbb87 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -18,6 +18,12 @@ void blockdev_auto_del(BlockDriverState *bs);
 
 #define BLOCK_SERIAL_STRLEN 20
 
+typedef enum {
+    IF_NONE,
+    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_COUNT
+} BlockInterfaceType;
+
 struct DriveInfo {
     BlockDriverState *bdrv;
     char *id;
diff --git a/qemu-common.h b/qemu-common.h
index 79e1149..c7ff280 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -274,12 +274,6 @@ typedef struct VirtIODevice VirtIODevice;
 
 typedef uint64_t pcibus_t;
 
-typedef enum {
-    IF_NONE,
-    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
-    IF_COUNT
-} BlockInterfaceType;
-
 void cpu_exec_init_all(unsigned long tb_size);
 
 /* CPU save/load.  */
-- 
1.7.2.3

[Qemu-devel] [PATCH 21/28] blockdev: Put BlockInterfaceType names and max_devs in tables

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Turns drive_init()'s lengthy conditional into a concise loop, and
makes the data available elsewhere.

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

diff --git a/blockdev.c b/blockdev.c
index 699f312..9c883ce 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -19,6 +19,23 @@
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
 
+static const char *const if_name[IF_COUNT] = {
+    [IF_NONE] = "none",
+    [IF_IDE] = "ide",
+    [IF_SCSI] = "scsi",
+    [IF_FLOPPY] = "floppy",
+    [IF_PFLASH] = "pflash",
+    [IF_MTD] = "mtd",
+    [IF_SD] = "sd",
+    [IF_VIRTIO] = "virtio",
+    [IF_XEN] = "xen",
+};
+
+static const int if_max_devs[IF_COUNT] = {
+    [IF_IDE] = MAX_IDE_DEVS,
+    [IF_SCSI] = MAX_SCSI_DEVS,
+};
+
 /*
  * We automatically delete the drive when a device using it gets
  * unplugged.  Questionable feature, but we can't just drop it.
@@ -173,11 +190,9 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 
     if (default_to_scsi) {
         type = IF_SCSI;
-        max_devs = MAX_SCSI_DEVS;
         pstrcpy(devname, sizeof(devname), "scsi");
     } else {
         type = IF_IDE;
-        max_devs = MAX_IDE_DEVS;
         pstrcpy(devname, sizeof(devname), "ide");
     }
     media = MEDIA_DISK;
@@ -199,38 +214,14 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 
     if ((buf = qemu_opt_get(opts, "if")) != NULL) {
         pstrcpy(devname, sizeof(devname), buf);
-        if (!strcmp(buf, "ide")) {
-	    type = IF_IDE;
-            max_devs = MAX_IDE_DEVS;
-        } else if (!strcmp(buf, "scsi")) {
-	    type = IF_SCSI;
-            max_devs = MAX_SCSI_DEVS;
-        } else if (!strcmp(buf, "floppy")) {
-	    type = IF_FLOPPY;
-            max_devs = 0;
-        } else if (!strcmp(buf, "pflash")) {
-	    type = IF_PFLASH;
-            max_devs = 0;
-	} else if (!strcmp(buf, "mtd")) {
-	    type = IF_MTD;
-            max_devs = 0;
-	} else if (!strcmp(buf, "sd")) {
-	    type = IF_SD;
-            max_devs = 0;
-        } else if (!strcmp(buf, "virtio")) {
-            type = IF_VIRTIO;
-            max_devs = 0;
-	} else if (!strcmp(buf, "xen")) {
-	    type = IF_XEN;
-            max_devs = 0;
-	} else if (!strcmp(buf, "none")) {
-	    type = IF_NONE;
-            max_devs = 0;
-	} else {
+        for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++)
+            ;
+        if (type == IF_COUNT) {
             error_report("unsupported bus type '%s'", buf);
             return NULL;
 	}
     }
+    max_devs = if_max_devs[type];
 
     if (cyls || heads || secs) {
         if (cyls < 1 || (type == IF_IDE && cyls > 16383)) {
-- 
1.7.2.3

[Qemu-devel] [PATCH 22/28] blockdev: Fix regression in -drive if=scsi, index=N

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Before commit 622b520f, index=12 meant bus=1,unit=5.

Since the commit, it means bus=0,unit=12.  The drive is created, but
not the guest device.  That's because the controllers we use with
if=scsi drives (lsi53c895a and esp) support only 7 units, and
scsi_bus_legacy_handle_cmdline() ignores drives with unit numbers
exceeding that limit.

Changing the mapping of index to bus, unit is a regression.  Breaking
-drive invocations that used to work just makes it worse.

Revert the part of commit 622b520f that causes this, and clean up
some.

Note that the fix only affects if=scsi.  You can still put more than 7
units on a SCSI bus with -device & friends.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c    |   18 ++++++++++++++++--
 blockdev.h    |    3 ---
 hw/ide.h      |    2 ++
 hw/ide/ahci.c |    1 -
 hw/qdev.c     |    1 -
 hw/scsi.h     |    3 ++-
 savevm.c      |    1 -
 7 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9c883ce..fba53f9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -32,8 +32,22 @@ static const char *const if_name[IF_COUNT] = {
 };
 
 static const int if_max_devs[IF_COUNT] = {
-    [IF_IDE] = MAX_IDE_DEVS,
-    [IF_SCSI] = MAX_SCSI_DEVS,
+    /*
+     * Do not change these numbers!  They govern how drive option
+     * index maps to unit and bus.  That mapping is ABI.
+     *
+     * All controllers used to imlement if=T drives need to support
+     * if_max_devs[T] units, for any T with if_max_devs[T] != 0.
+     * Otherwise, some index values map to "impossible" bus, unit
+     * values.
+     *
+     * For instance, if you change [IF_SCSI] to 255, -drive
+     * if=scsi,index=12 no longer means bus=1,unit=5, but
+     * bus=0,unit=12.  With an lsi53c895a controller (7 units max),
+     * the drive can't be set up.  Regression.
+     */
+    [IF_IDE] = 2,
+    [IF_SCSI] = 7,
 };
 
 /*
diff --git a/blockdev.h b/blockdev.h
index 2cbbb87..cf8fc01 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -37,9 +37,6 @@ struct DriveInfo {
     QTAILQ_ENTRY(DriveInfo) next;
 };
 
-#define MAX_IDE_DEVS	2
-#define MAX_SCSI_DEVS	255
-
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 int drive_get_max_bus(BlockInterfaceType type);
 DriveInfo *drive_get_next(BlockInterfaceType type);
diff --git a/hw/ide.h b/hw/ide.h
index 2b5ae7c..73fb550 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -4,6 +4,8 @@
 #include "isa.h"
 #include "pci.h"
 
+#define MAX_IDE_DEVS	2
+
 /* ide-isa.c */
 ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
                         DriveInfo *hd0, DriveInfo *hd1);
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 433171c..671b4df 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -70,7 +70,6 @@
 #include "monitor.h"
 #include "dma.h"
 #include "cpu-common.h"
-#include "blockdev.h"
 #include "internal.h"
 #include <hw/ide/pci.h>
 
diff --git a/hw/qdev.c b/hw/qdev.c
index 0c94fb2..c7fec44 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,7 +29,6 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
-#include "blockdev.h"
 
 static int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
diff --git a/hw/scsi.h b/hw/scsi.h
index 846fbba..d3b5d56 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,9 +3,10 @@
 
 #include "qdev.h"
 #include "block.h"
-#include "blockdev.h"
 #include "block_int.h"
 
+#define MAX_SCSI_DEVS	255
+
 #define SCSI_CMD_BUF_SIZE     16
 
 /* scsi-disk.c */
diff --git a/savevm.c b/savevm.c
index fcd8db4..4453217 100644
--- a/savevm.c
+++ b/savevm.c
@@ -78,7 +78,6 @@
 #include "sysemu.h"
 #include "qemu-timer.h"
 #include "qemu-char.h"
-#include "blockdev.h"
 #include "audio/audio.h"
 #include "migration.h"
 #include "qemu_socket.h"
-- 
1.7.2.3

[Qemu-devel] [PATCH 23/28] blockdev: Make drive_add() take explicit type, index parameters

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Before, type & index were hidden in printf-like fmt, ... parameters,
which get expanded into an option string.  Rather inconvenient for
uses later in this series.

New IF_DEFAULT to ask for the machine's default interface.  Before,
that was done by having no option "if" in the option string.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c          |   20 +++++++++++++++++---
 blockdev.h          |    8 +++++++-
 hw/device-hotplug.c |    2 +-
 vl.c                |   43 +++++++++++++++++++++++--------------------
 4 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fba53f9..6d828f2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -75,20 +75,34 @@ void blockdev_auto_del(BlockDriverState *bs)
     }
 }
 
-QemuOpts *drive_add(const char *file, const char *fmt, ...)
+QemuOpts *drive_def(const char *optstr)
+{
+    return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
+}
+
+QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
+                    const char *fmt, ...)
 {
     va_list ap;
     char optstr[1024];
     QemuOpts *opts;
+    char buf[32];
 
     va_start(ap, fmt);
     vsnprintf(optstr, sizeof(optstr), fmt, ap);
     va_end(ap);
 
-    opts = qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
+    opts = drive_def(optstr);
     if (!opts) {
         return NULL;
     }
+    if (type != IF_DEFAULT) {
+        qemu_opt_set(opts, "if", if_name[type]);
+    }
+    if (index >= 0) {
+        snprintf(buf, sizeof(buf), "%d", index);
+        qemu_opt_set(opts, "index", buf);
+    }
     if (file)
         qemu_opt_set(opts, "file", file);
     return opts;
@@ -473,7 +487,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
         if (devaddr)
             qemu_opt_set(opts, "addr", devaddr);
         break;
-    case IF_COUNT:
+    default:
         abort();
     }
     if (!file || !*file) {
diff --git a/blockdev.h b/blockdev.h
index cf8fc01..0c01e08 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -19,6 +19,7 @@ void blockdev_auto_del(BlockDriverState *bs);
 #define BLOCK_SERIAL_STRLEN 20
 
 typedef enum {
+    IF_DEFAULT = -1,            /* for use with drive_add() only */
     IF_NONE,
     IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
@@ -43,7 +44,12 @@ DriveInfo *drive_get_next(BlockInterfaceType type);
 void drive_uninit(DriveInfo *dinfo);
 DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
 
-QemuOpts *drive_add(const char *file, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+QemuOpts *drive_def(const char *optstr);
+QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
+                    const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
+    /* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
+     * "zero-length gnu_printf format string" warning we insist to
+     * enable */
 DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
 
 /* device-hotplug */
diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c
index 9704e2f..95a6372 100644
--- a/hw/device-hotplug.c
+++ b/hw/device-hotplug.c
@@ -33,7 +33,7 @@ DriveInfo *add_init_drive(const char *optstr)
     DriveInfo *dinfo;
     QemuOpts *opts;
 
-    opts = drive_add(NULL, "%s", optstr);
+    opts = drive_def(optstr);
     if (!opts)
         return NULL;
 
diff --git a/vl.c b/vl.c
index 14255c4..5399e33 100644
--- a/vl.c
+++ b/vl.c
@@ -621,12 +621,13 @@ static int bt_parse(const char *opt)
 /***********************************************************/
 /* QEMU Block devices */
 
-#define HD_ALIAS "index=%d,media=disk"
-#define CDROM_ALIAS "index=2,media=cdrom"
-#define FD_ALIAS "index=%d,if=floppy"
-#define PFLASH_ALIAS "if=pflash"
-#define MTD_ALIAS "if=mtd"
-#define SD_ALIAS "index=0,if=sd"
+/* Any % in the following strings must be escaped as %% */
+#define HD_OPTS "media=disk"
+#define CDROM_OPTS "media=cdrom"
+#define FD_OPTS ""
+#define PFLASH_OPTS ""
+#define MTD_OPTS ""
+#define SD_OPTS ""
 
 static int drive_init_func(QemuOpts *opts, void *opaque)
 {
@@ -1987,7 +1988,7 @@ int main(int argc, char **argv, char **envp)
         if (optind >= argc)
             break;
         if (argv[optind][0] != '-') {
-	    hda_opts = drive_add(argv[optind++], HD_ALIAS, 0);
+	    hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
         } else {
             const QEMUOption *popt;
 
@@ -2027,11 +2028,11 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_hda:
                 if (cyls == 0)
-                    hda_opts = drive_add(optarg, HD_ALIAS, 0);
+                    hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
                 else
-                    hda_opts = drive_add(optarg, HD_ALIAS
+                    hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
 			     ",cyls=%d,heads=%d,secs=%d%s",
-                             0, cyls, heads, secs,
+                             cyls, heads, secs,
                              translation == BIOS_ATA_TRANSLATION_LBA ?
                                  ",trans=lba" :
                              translation == BIOS_ATA_TRANSLATION_NONE ?
@@ -2040,10 +2041,11 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
-                drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
+                drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
+                          HD_OPTS);
                 break;
             case QEMU_OPTION_drive:
-                drive_add(NULL, "%s", optarg);
+                drive_def(optarg);
 	        break;
             case QEMU_OPTION_set:
                 if (qemu_set_option(optarg) != 0)
@@ -2054,13 +2056,13 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
 	        break;
             case QEMU_OPTION_mtdblock:
-                drive_add(optarg, MTD_ALIAS);
+                drive_add(IF_MTD, -1, optarg, MTD_OPTS);
                 break;
             case QEMU_OPTION_sd:
-                drive_add(optarg, SD_ALIAS);
+                drive_add(IF_SD, 0, optarg, SD_OPTS);
                 break;
             case QEMU_OPTION_pflash:
-                drive_add(optarg, PFLASH_ALIAS);
+                drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
                 break;
             case QEMU_OPTION_snapshot:
                 snapshot = 1;
@@ -2139,7 +2141,7 @@ int main(int argc, char **argv, char **envp)
                 kernel_cmdline = optarg;
                 break;
             case QEMU_OPTION_cdrom:
-                drive_add(optarg, CDROM_ALIAS);
+                drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
                 break;
             case QEMU_OPTION_boot:
                 {
@@ -2192,7 +2194,8 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_fda:
             case QEMU_OPTION_fdb:
-                drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
+                drive_add(IF_FLOPPY, popt->index - QEMU_OPTION_fda,
+                          optarg, FD_OPTS);
                 break;
             case QEMU_OPTION_no_fd_bootchk:
                 fd_bootchk = 0;
@@ -2892,17 +2895,17 @@ int main(int argc, char **argv, char **envp)
 
     if (default_cdrom) {
         /* we always create the cdrom drive, even if no disk is there */
-        drive_add(NULL, CDROM_ALIAS);
+        drive_add(IF_DEFAULT, 2, NULL, CDROM_OPTS);
     }
 
     if (default_floppy) {
         /* we always create at least one floppy */
-        drive_add(NULL, FD_ALIAS, 0);
+        drive_add(IF_FLOPPY, 0, NULL, FD_OPTS);
     }
 
     if (default_sdcard) {
         /* we always create one sd slot, even if no card is in it */
-        drive_add(NULL, SD_ALIAS);
+        drive_add(IF_SD, 0, NULL, SD_OPTS);
     }
 
     /* open the virtual block devices */
-- 
1.7.2.3

[Qemu-devel] [PATCH 24/28] blockdev: Factor drive_index_to_{bus, unit}_id out of drive_init()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

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

diff --git a/blockdev.c b/blockdev.c
index 6d828f2..a42c5e4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -75,6 +75,18 @@ void blockdev_auto_del(BlockDriverState *bs)
     }
 }
 
+static int drive_index_to_bus_id(BlockInterfaceType type, int index)
+{
+    int max_devs = if_max_devs[type];
+    return max_devs ? index / max_devs : 0;
+}
+
+static int drive_index_to_unit_id(BlockInterfaceType type, int index)
+{
+    int max_devs = if_max_devs[type];
+    return max_devs ? index % max_devs : index;
+}
+
 QemuOpts *drive_def(const char *optstr)
 {
     return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
@@ -382,14 +394,8 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
             error_report("index cannot be used with bus and unit");
             return NULL;
         }
-        if (max_devs == 0)
-        {
-            unit_id = index;
-            bus_id = 0;
-        } else {
-            unit_id = index % max_devs;
-            bus_id = index / max_devs;
-        }
+        bus_id = drive_index_to_bus_id(type, index);
+        unit_id = drive_index_to_unit_id(type, index);
     }
 
     /* if user doesn't specify a unit_id,
-- 
1.7.2.3

[Qemu-devel] [PATCH 25/28] blockdev: New drive_get_by_index()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

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

diff --git a/blockdev.c b/blockdev.c
index a42c5e4..01228f6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -136,6 +136,13 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
     return NULL;
 }
 
+DriveInfo *drive_get_by_index(BlockInterfaceType type, int index)
+{
+    return drive_get(type,
+                     drive_index_to_bus_id(type, index),
+                     drive_index_to_unit_id(type, index));
+}
+
 int drive_get_max_bus(BlockInterfaceType type)
 {
     int max_bus;
diff --git a/blockdev.h b/blockdev.h
index 0c01e08..18278cc 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -39,6 +39,7 @@ struct DriveInfo {
 };
 
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
+DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
 int drive_get_max_bus(BlockInterfaceType type);
 DriveInfo *drive_get_next(BlockInterfaceType type);
 void drive_uninit(DriveInfo *dinfo);
-- 
1.7.2.3

[Qemu-devel] [PATCH 26/28] blockdev: Reject multiple definitions for the same drive

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

We silently ignore multiple definitions for the same drive:

    $ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive if=ide,index=1,file=tmp.qcow2 -drive if=ide,index=1,file=nonexistant
    QEMU 0.13.50 monitor - type 'help' for more information
    (qemu) info block
    ide0-hd1: type=hd removable=0 file=tmp.qcow2 backing_file=tmp.img ro=0 drv=qcow2 encrypted=0

With if=none, this can become quite confusing:

    $ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive if=none,index=1,file=tmp.qcow2,id=eins -drive if=none,index=1,file=nonexistant,id=zwei -device ide-drive,drive=eins -device ide-drive,drive=zwei
    qemu-system-x86_64: -device ide-drive,drive=zwei: Property 'ide-drive.drive' can't find value 'zwei'

The second -device fails, because it refers to drive zwei, which got
silently ignored.

Make multiple drive definitions fail cleanly.

Unfortunately, there's code that relies on multiple drive definitions
being silently ignored: main() merrily adds default drives even when
the user already defined these drives.  Fix that up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |    5 +++--
 vl.c       |   45 ++++++++++++++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 01228f6..3f7b560 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -429,11 +429,12 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     }
 
     /*
-     * ignore multiple definitions
+     * catch multiple definitions
      */
 
     if (drive_get(type, bus_id, unit_id) != NULL) {
-        *fatal_error = 0;
+        error_report("drive with bus=%d, unit=%d (index=%d) exists",
+                     bus_id, unit_id, index);
         return NULL;
     }
 
diff --git a/vl.c b/vl.c
index 5399e33..99f5bfe 100644
--- a/vl.c
+++ b/vl.c
@@ -649,6 +649,29 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static void default_drive(int enable, int snapshot, int use_scsi,
+                          BlockInterfaceType type, int index,
+                          const char *optstr)
+{
+    QemuOpts *opts;
+
+    if (type == IF_DEFAULT) {
+        type = use_scsi ? IF_SCSI : IF_IDE;
+    }
+
+    if (!enable || drive_get_by_index(type, index)) {
+        return;
+    }
+
+    opts = drive_add(type, index, NULL, optstr);
+    if (snapshot) {
+        drive_enable_snapshot(opts, NULL);
+    }
+    if (drive_init_func(opts, &use_scsi)) {
+        exit(1);
+    }
+}
+
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
 {
     boot_set_handler = func;
@@ -2893,27 +2916,19 @@ int main(int argc, char **argv, char **envp)
 
     blk_mig_init();
 
-    if (default_cdrom) {
-        /* we always create the cdrom drive, even if no disk is there */
-        drive_add(IF_DEFAULT, 2, NULL, CDROM_OPTS);
-    }
-
-    if (default_floppy) {
-        /* we always create at least one floppy */
-        drive_add(IF_FLOPPY, 0, NULL, FD_OPTS);
-    }
-
-    if (default_sdcard) {
-        /* we always create one sd slot, even if no card is in it */
-        drive_add(IF_SD, 0, NULL, SD_OPTS);
-    }
-
     /* open the virtual block devices */
     if (snapshot)
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, &machine->use_scsi, 1) != 0)
         exit(1);
 
+    default_drive(default_cdrom, snapshot, machine->use_scsi,
+                  IF_DEFAULT, 2, CDROM_OPTS);
+    default_drive(default_floppy, snapshot, machine->use_scsi,
+                  IF_FLOPPY, 0, FD_OPTS);
+    default_drive(default_sdcard, snapshot, machine->use_scsi,
+                  IF_SD, 0, SD_OPTS);
+
     register_savevm_live(NULL, "ram", 0, 4, NULL, ram_save_live, NULL,
                          ram_load, NULL);
 
-- 
1.7.2.3

[Qemu-devel] [PATCH 27/28] blockdev: Replace drive_add()'s fmt, ... by optstr parameter

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Let the callers build the optstr.  Only one wants to.  All the others
become simpler, because they don't have to worry about escaping '%'.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |    8 +-------
 blockdev.h |    5 +----
 vl.c       |   23 +++++++++++++----------
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3f7b560..4b2145c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -93,17 +93,11 @@ QemuOpts *drive_def(const char *optstr)
 }
 
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
-                    const char *fmt, ...)
+                    const char *optstr)
 {
-    va_list ap;
-    char optstr[1024];
     QemuOpts *opts;
     char buf[32];
 
-    va_start(ap, fmt);
-    vsnprintf(optstr, sizeof(optstr), fmt, ap);
-    va_end(ap);
-
     opts = drive_def(optstr);
     if (!opts) {
         return NULL;
diff --git a/blockdev.h b/blockdev.h
index 18278cc..e5d8c56 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -47,10 +47,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
 
 QemuOpts *drive_def(const char *optstr);
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
-                    const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
-    /* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
-     * "zero-length gnu_printf format string" warning we insist to
-     * enable */
+                    const char *optstr);
 DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
 
 /* device-hotplug */
diff --git a/vl.c b/vl.c
index 99f5bfe..f86724f 100644
--- a/vl.c
+++ b/vl.c
@@ -621,7 +621,6 @@ static int bt_parse(const char *opt)
 /***********************************************************/
 /* QEMU Block devices */
 
-/* Any % in the following strings must be escaped as %% */
 #define HD_OPTS "media=disk"
 #define CDROM_OPTS "media=cdrom"
 #define FD_OPTS ""
@@ -2050,17 +2049,21 @@ int main(int argc, char **argv, char **envp)
                 initrd_filename = optarg;
                 break;
             case QEMU_OPTION_hda:
-                if (cyls == 0)
-                    hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
-                else
-                    hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
-			     ",cyls=%d,heads=%d,secs=%d%s",
-                             cyls, heads, secs,
-                             translation == BIOS_ATA_TRANSLATION_LBA ?
+                {
+                    char buf[256];
+                    if (cyls == 0)
+                        snprintf(buf, sizeof(buf), "%s", HD_OPTS);
+                    else
+                        snprintf(buf, sizeof(buf),
+                                 "%s,cyls=%d,heads=%d,secs=%d%s",
+                                 HD_OPTS , cyls, heads, secs,
+                                 translation == BIOS_ATA_TRANSLATION_LBA ?
                                  ",trans=lba" :
-                             translation == BIOS_ATA_TRANSLATION_NONE ?
+                                 translation == BIOS_ATA_TRANSLATION_NONE ?
                                  ",trans=none" : "");
-                 break;
+                    drive_add(IF_DEFAULT, 0, optarg, buf);
+                    break;
+                }
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
-- 
1.7.2.3

[Qemu-devel] [PATCH 28/28] blockdev: Fix drive_add for drives without media

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Watch this:

    (qemu) drive_add 0 if=none
    (qemu) info block
    none0: type=hd removable=0 [not inserted]
    (qemu) drive_del none0
    Segmentation fault (core dumped)

add_init_drive() is confused about drive_init()'s failure modes, and
cleans up when it shouldn't.  This leaves the DriveInfo with member
opts dangling.  drive_del attempts to free it, and dies.

drive_init() behaves as follows:

* If it created a drive with media, it returns its DriveInfo.

* If it created a drive without media, it clears *fatal_error and
  returns NULL.

* If it couldn't create a drive, it sets *fatal_error and returns
  NULL.

Of its three callers:

* drive_init_func() is correct.

* usb_msd_init() assumes drive_init() failed when it returns NULL.
  This is correct only because it always passes option "file", and
  "drive without media" can't happen then.

* add_init_drive() assumes drive_init() failed when it returns NULL.
  This is incorrect.

Clean up drive_init() to return NULL on failure and only on failure.
Drop its parameter fatal_error.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c          |    8 ++------
 blockdev.h          |    2 +-
 hw/device-hotplug.c |    3 +--
 hw/usb-msd.c        |    3 +--
 vl.c                |    9 ++-------
 5 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4b2145c..1c56da0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -203,7 +203,7 @@ static int parse_block_error_action(const char *buf, int is_read)
     }
 }
 
-DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
+DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 {
     const char *buf;
     const char *file = NULL;
@@ -225,8 +225,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
     int snapshot = 0;
     int ret;
 
-    *fatal_error = 1;
-
     translation = BIOS_ATA_TRANSLATION_AUTO;
 
     if (default_to_scsi) {
@@ -499,8 +497,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
         abort();
     }
     if (!file || !*file) {
-        *fatal_error = 0;
-        return NULL;
+        return dinfo;
     }
     if (snapshot) {
         /* always use cache=unsafe with snapshot */
@@ -529,7 +526,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
 
     if (bdrv_key_required(dinfo->bdrv))
         autostart = 0;
-    *fatal_error = 0;
     return dinfo;
 }
 
diff --git a/blockdev.h b/blockdev.h
index e5d8c56..84e462a 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -48,7 +48,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
 QemuOpts *drive_def(const char *optstr);
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                     const char *optstr);
-DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
+DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi);
 
 /* device-hotplug */
 
diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c
index 95a6372..8b2ed7a 100644
--- a/hw/device-hotplug.c
+++ b/hw/device-hotplug.c
@@ -29,7 +29,6 @@
 
 DriveInfo *add_init_drive(const char *optstr)
 {
-    int fatal_error;
     DriveInfo *dinfo;
     QemuOpts *opts;
 
@@ -37,7 +36,7 @@ DriveInfo *add_init_drive(const char *optstr)
     if (!opts)
         return NULL;
 
-    dinfo = drive_init(opts, current_machine->use_scsi, &fatal_error);
+    dinfo = drive_init(opts, current_machine->use_scsi);
     if (!dinfo) {
         qemu_opts_del(opts);
         return NULL;
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 11722c7..97d1e4a 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -542,7 +542,6 @@ static USBDevice *usb_msd_init(const char *filename)
     QemuOpts *opts;
     DriveInfo *dinfo;
     USBDevice *dev;
-    int fatal_error;
     const char *p1;
     char fmt[32];
 
@@ -572,7 +571,7 @@ static USBDevice *usb_msd_init(const char *filename)
     qemu_opt_set(opts, "if", "none");
 
     /* create host drive */
-    dinfo = drive_init(opts, 0, &fatal_error);
+    dinfo = drive_init(opts, 0);
     if (!dinfo) {
         qemu_opts_del(opts);
         return NULL;
diff --git a/vl.c b/vl.c
index f86724f..ce5708b 100644
--- a/vl.c
+++ b/vl.c
@@ -631,13 +631,8 @@ static int bt_parse(const char *opt)
 static int drive_init_func(QemuOpts *opts, void *opaque)
 {
     int *use_scsi = opaque;
-    int fatal_error = 0;
 
-    if (drive_init(opts, *use_scsi, &fatal_error) == NULL) {
-        if (fatal_error)
-            return 1;
-    }
-    return 0;
+    return drive_init(opts, *use_scsi) == NULL;
 }
 
 static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
@@ -666,7 +661,7 @@ static void default_drive(int enable, int snapshot, int use_scsi,
     if (snapshot) {
         drive_enable_snapshot(opts, NULL);
     }
-    if (drive_init_func(opts, &use_scsi)) {
+    if (!drive_init(opts, use_scsi)) {
         exit(1);
     }
 }
-- 
1.7.2.3

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

[Anthony Liguori]

On 01/31/2011 09:28 AM, Kevin Wolf wrote:
> The following changes since commit 45d1aa828f8c94b082a0aa2dbf76535594ee45df:
>
>    Update OpenBIOS images to r1018 (2011-01-30 13:10:10 +0000)
>
> are available in the git repository at:
>    git://repo.or.cz/qemu/kevin.git block
>    

Pulled.  Thanks.

Regards,

Anthony Liguori

> Blue Swirl (1):
>        qcow2-refcount: remove write-only variables
>
> Christoph Hellwig (3):
>        block: add block_resize monitor command
>        block: tell drivers about an image resize
>        virtio-blk: tell the guest about size changes
>
> Jes Sorensen (6):
>        strtosz(): use unsigned char and switch to qemu_isspace()
>        strtosz() use qemu_toupper() to simplify switch statement
>        strtosz(): Fix name confusion in use of modf()
>        strtosz(): Use suffix macros in switch() statement
>        Add documentation for STRTOSZ_DEFSUFFIX_ macros
>        Reorganize struct Qcow2Cache for better struct packing
>
> Kevin Wolf (3):
>        qemu-io: Fix discard command
>        qcow2: Add bdrv_discard support
>        raw-win32: Fix bdrv_flush return value
>
> MORITA Kazutaka (1):
>        sheepdog: support creating images on remote hosts
>
> Markus Armbruster (11):
>        scsi hotplug: Set DriveInfo member bus correctly
>        blockdev: New drive_get_next(), replacing qdev_init_bdrv()
>        blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h
>        blockdev: Put BlockInterfaceType names and max_devs in tables
>        blockdev: Fix regression in -drive if=scsi,index=N
>        blockdev: Make drive_add() take explicit type, index parameters
>        blockdev: Factor drive_index_to_{bus,unit}_id out of drive_init()
>        blockdev: New drive_get_by_index()
>        blockdev: Reject multiple definitions for the same drive
>        blockdev: Replace drive_add()'s fmt, ... by optstr parameter
>        blockdev: Fix drive_add for drives without media
>
> Stefan Hajnoczi (3):
>        virtio-pci: Disable virtio-ioeventfd when !CONFIG_IOTHREAD
>        ahci: Fix cpu_physical_memory_unmap() argument ordering
>        qed: Images with backing file do not require QED_F_NEED_CHECK
>
>   block.c                |   12 ++-
>   block.h                |    3 +-
>   block/qcow2-cache.c    |    2 +-
>   block/qcow2-cluster.c  |   82 +++++++++++++++++++++++
>   block/qcow2-refcount.c |    5 +-
>   block/qcow2.c          |    8 ++
>   block/qcow2.h          |    2 +
>   block/qed.c            |   24 +++++--
>   block/raw-win32.c      |    2 +-
>   block/sheepdog.c       |   17 ++++-
>   block_int.h            |    5 +-
>   blockdev.c             |  173 ++++++++++++++++++++++++++++++++---------------
>   blockdev.h             |   19 ++++--
>   cutils.c               |   28 ++++-----
>   hmp-commands.hx        |   19 +++++
>   hw/device-hotplug.c    |    5 +-
>   hw/ide.h               |    2 +
>   hw/ide/ahci.c          |   11 ++--
>   hw/ide/core.c          |    6 ++-
>   hw/pci-hotplug.c       |    1 +
>   hw/pl181.c             |    7 +-
>   hw/qdev.c              |   15 ----
>   hw/qdev.h              |    2 -
>   hw/scsi.h              |    3 +-
>   hw/sd.c                |    7 ++-
>   hw/ssi-sd.c            |    7 +-
>   hw/usb-msd.c           |    3 +-
>   hw/virtio-blk.c        |   10 +++
>   kvm-all.c              |    8 ++-
>   qemu-common.h          |   13 ++--
>   qemu-io.c              |    2 +-
>   qmp-commands.hx        |   28 ++++++++
>   savevm.c               |    1 -
>   vl.c                   |  104 +++++++++++++++++------------
>   34 files changed, 447 insertions(+), 189 deletions(-)
>