Block patches

-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAABAgAGBQJUJbcwAAoJEH8JsnLIjy/WmckP/RBMK3gkr2aArp0k/sbdFcnW D7GusGkoj1jXRVFKuG3d+T/nozJTX0mkfnaRo8QCg/5VVJtqeHfsfY6JHgppQEai C5Z8gtzIFxsQNE0/8d2D6zJMnqsIeXAVtDONWgWcDOZstVXn+KEKOw5E3KCZeGxF yLA+h4h8sncT3ysPE0/jnQqeIwGJpLusFXmICiRY2lXRcCgY/qniymUq25FZO69m Isw8iVHrUr05u1kfthdQeSAmdPQqI5tK5siR4CERtO2XdlyiODmdo5DRZmATfGiL CGUIJd30mvrKpqRpIk8gbuGEtz92RuTnyoi9xkLcKi52eWyflGH4gjN3ojYe5Pro hRY06AcwI9lAVjCy7lKPO6dJn8RWOi/wkgfffK0CFujlo8r1BtwfCW0Y9koYuhdq mM/K0IEbjNgOvacHLIMOTQuKz3DLv7QB8wZQ+cTkdmvC6kNGnqD9yrA4cAQP4kjR wjm/WfDIRKPif6cMwgaOiQeNaL0VF2LIn4GRWgaiIqEYfcKzy65uBFoPqOlAoXr8 NtTzCIXDIBbTSzz3VLBKD22FOsP0pfPrMeXkf+GwJwStKMcp6PGY4iUd3+M3bfOP iUsxTJ4Uw5zMg0MgOaaWnUaDdbRtKGtH1y2UA+4X3gO+3W8Vz0e8B5cPMBqN/mpy lUS4mBA08mTvaLE520rp =ManP -----END PGP SIGNATURE----- Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging Block patches # gpg: Signature made Fri 26 Sep 2014 19:57:52 BST using RSA key ID C88F2FD6 # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" * remotes/kevin/tags/for-upstream: qemu-iotests: Fail test if explicit test case number is unknown block: Validate node-name vpc: fix beX_to_cpu() and cpu_to_beX() confusion docs: add blkdebug block driver documentation block: Catch simultaneous usage of options and their aliases block: Specify -drive legacy option aliases in array block: Improve message for device name clashing with node name qemu-nbd: Destroy the BlockDriverState properly block: Keep DriveInfo alive until BlockDriverState dies blockdev: Disentangle BlockDriverState and DriveInfo creation blkdebug: show an error for invalid event names Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2024-07-22 19:04:26 +00:00 · 2014-09-29 12:26:14 +01:00 · 2014-09-29 12:26:14 +01:00 · 0ebcc56453
parent e80084d352 c9d17ad0dd
commit 0ebcc56453
16 changed files with 382 additions and 76 deletions
--- a/block.c
+++ b/block.c
@ -29,6 +29,7 @@
 #include "qemu/module.h"
 #include "qapi/qmp/qjson.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/blockdev.h"    /* FIXME layering violation */
 #include "qemu/notify.h"
 #include "block/coroutine.h"
 #include "block/qapi.h"
@ -334,19 +335,30 @@ void bdrv_register(BlockDriver *bdrv)
    QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
 }

+static bool bdrv_is_valid_name(const char *name)
+{
+    return qemu_opts_id_wellformed(name);
+}
+
 /* create a new block device (by default it is empty) */
 BlockDriverState *bdrv_new(const char *device_name, Error **errp)
 {
    BlockDriverState *bs;
    int i;

+    if (*device_name && !bdrv_is_valid_name(device_name)) {
+        error_setg(errp, "Invalid device name");
+        return NULL;
+    }
+
    if (bdrv_find(device_name)) {
        error_setg(errp, "Device with id '%s' already exists",
                   device_name);
        return NULL;
    }
    if (bdrv_find_node(device_name)) {
-        error_setg(errp, "Device with node-name '%s' already exists",
+        error_setg(errp,
+                   "Device name '%s' conflicts with an existing node name",
                   device_name);
        return NULL;
    }
@ -861,9 +873,9 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
        return;
    }

-    /* empty string node name is invalid */
-    if (node_name[0] == '\0') {
-        error_setg(errp, "Empty node name");
+    /* Check for empty string or invalid characters */
+    if (!bdrv_is_valid_name(node_name)) {
+        error_setg(errp, "Invalid node name");
        return;
    }

@ -2110,6 +2122,7 @@ static void bdrv_delete(BlockDriverState *bs)
    /* remove from list, if necessary */
    bdrv_make_anon(bs);

+    drive_info_del(drive_get_by_blockdev(bs));
    g_free(bs);
 }

--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@ -214,6 +214,7 @@ static int get_event_by_name(const char *name, BlkDebugEvent *event)
 struct add_rule_data {
    BDRVBlkdebugState *s;
    int action;
+    Error **errp;
 };

 static int add_rule(QemuOpts *opts, void *opaque)
@ -226,7 +227,11 @@ static int add_rule(QemuOpts *opts, void *opaque)

    /* Find the right event for the rule */
    event_name = qemu_opt_get(opts, "event");
-    if (!event_name || get_event_by_name(event_name, &event) < 0) {
+    if (!event_name) {
+        error_setg(d->errp, "Missing event name for rule");
+        return -1;
+    } else if (get_event_by_name(event_name, &event) < 0) {
+        error_setg(d->errp, "Invalid event name \"%s\"", event_name);
        return -1;
    }

@ -312,10 +317,21 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,

    d.s = s;
    d.action = ACTION_INJECT_ERROR;
-    qemu_opts_foreach(&inject_error_opts, add_rule, &d, 0);
+    d.errp = &local_err;
+    qemu_opts_foreach(&inject_error_opts, add_rule, &d, 1);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }

    d.action = ACTION_SET_STATE;
-    qemu_opts_foreach(&set_state_opts, add_rule, &d, 0);
+    qemu_opts_foreach(&set_state_opts, add_rule, &d, 1);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }

    ret = 0;
 fail:
--- a/block/vpc.c
+++ b/block/vpc.c
@ -207,7 +207,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
            "incorrect.\n", bs->filename);

    /* Write 'checksum' back to footer, or else will leave it with zero. */
-    footer->checksum = be32_to_cpu(checksum);
+    footer->checksum = cpu_to_be32(checksum);

    // The visible size of a image in Virtual PC depends on the geometry
    // rather than on the size stored in the footer (the size in the footer
@ -472,7 +472,7 @@ static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)

    // Write BAT entry to disk
    bat_offset = s->bat_offset + (4 * index);
-    bat_value = be32_to_cpu(s->pagetable[index]);
+    bat_value = cpu_to_be32(s->pagetable[index]);
    ret = bdrv_pwrite_sync(bs->file, bat_offset, &bat_value, 4);
    if (ret < 0)
        goto fail;
@ -699,13 +699,13 @@ static int create_dynamic_disk(BlockDriverState *bs, uint8_t *buf,
     * Note: The spec is actually wrong here for data_offset, it says
     * 0xFFFFFFFF, but MS tools expect all 64 bits to be set.
     */
-    dyndisk_header->data_offset = be64_to_cpu(0xFFFFFFFFFFFFFFFFULL);
-    dyndisk_header->table_offset = be64_to_cpu(3 * 512);
-    dyndisk_header->version = be32_to_cpu(0x00010000);
-    dyndisk_header->block_size = be32_to_cpu(block_size);
-    dyndisk_header->max_table_entries = be32_to_cpu(num_bat_entries);
+    dyndisk_header->data_offset = cpu_to_be64(0xFFFFFFFFFFFFFFFFULL);
+    dyndisk_header->table_offset = cpu_to_be64(3 * 512);
+    dyndisk_header->version = cpu_to_be32(0x00010000);
+    dyndisk_header->block_size = cpu_to_be32(block_size);
+    dyndisk_header->max_table_entries = cpu_to_be32(num_bat_entries);

-    dyndisk_header->checksum = be32_to_cpu(vpc_checksum(buf, 1024));
+    dyndisk_header->checksum = cpu_to_be32(vpc_checksum(buf, 1024));

    // Write the header
    offset = 512;
@ -810,36 +810,36 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
    memcpy(footer->creator_app, "qemu", 4);
    memcpy(footer->creator_os, "Wi2k", 4);

-    footer->features = be32_to_cpu(0x02);
-    footer->version = be32_to_cpu(0x00010000);
+    footer->features = cpu_to_be32(0x02);
+    footer->version = cpu_to_be32(0x00010000);
    if (disk_type == VHD_DYNAMIC) {
-        footer->data_offset = be64_to_cpu(HEADER_SIZE);
+        footer->data_offset = cpu_to_be64(HEADER_SIZE);
    } else {
-        footer->data_offset = be64_to_cpu(0xFFFFFFFFFFFFFFFFULL);
+        footer->data_offset = cpu_to_be64(0xFFFFFFFFFFFFFFFFULL);
    }
-    footer->timestamp = be32_to_cpu(time(NULL) - VHD_TIMESTAMP_BASE);
+    footer->timestamp = cpu_to_be32(time(NULL) - VHD_TIMESTAMP_BASE);

    /* Version of Virtual PC 2007 */
-    footer->major = be16_to_cpu(0x0005);
-    footer->minor = be16_to_cpu(0x0003);
+    footer->major = cpu_to_be16(0x0005);
+    footer->minor = cpu_to_be16(0x0003);
    if (disk_type == VHD_DYNAMIC) {
-        footer->orig_size = be64_to_cpu(total_sectors * 512);
-        footer->size = be64_to_cpu(total_sectors * 512);
+        footer->orig_size = cpu_to_be64(total_sectors * 512);
+        footer->size = cpu_to_be64(total_sectors * 512);
    } else {
-        footer->orig_size = be64_to_cpu(total_size);
-        footer->size = be64_to_cpu(total_size);
+        footer->orig_size = cpu_to_be64(total_size);
+        footer->size = cpu_to_be64(total_size);
    }
-    footer->cyls = be16_to_cpu(cyls);
+    footer->cyls = cpu_to_be16(cyls);
    footer->heads = heads;
    footer->secs_per_cyl = secs_per_cyl;

-    footer->type = be32_to_cpu(disk_type);
+    footer->type = cpu_to_be32(disk_type);

 #if defined(CONFIG_UUID)
    uuid_generate(footer->uuid);
 #endif

-    footer->checksum = be32_to_cpu(vpc_checksum(buf, HEADER_SIZE));
+    footer->checksum = cpu_to_be32(vpc_checksum(buf, HEADER_SIZE));

    if (disk_type == VHD_DYNAMIC) {
        ret = create_dynamic_disk(bs, buf, total_sectors);
--- a/blockdev.c
+++ b/blockdev.c
@ -216,11 +216,17 @@ static void bdrv_format_print(void *opaque, const char *name)

 void drive_del(DriveInfo *dinfo)
 {
+    bdrv_unref(dinfo->bdrv);
+}
+
+void drive_info_del(DriveInfo *dinfo)
+{
+    if (!dinfo) {
+        return;
+    }
    if (dinfo->opts) {
        qemu_opts_del(dinfo->opts);
    }
-
-    bdrv_unref(dinfo->bdrv);
    g_free(dinfo->id);
    QTAILQ_REMOVE(&drives, dinfo, next);
    g_free(dinfo->serial);
@ -301,6 +307,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
    int ro = 0;
    int bdrv_flags = 0;
    int on_read_error, on_write_error;
+    BlockDriverState *bs;
    DriveInfo *dinfo;
    ThrottleConfig cfg;
    int snapshot = 0;
@ -456,26 +463,27 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
    }

    /* init */
-    dinfo = g_malloc0(sizeof(*dinfo));
-    dinfo->id = g_strdup(qemu_opts_id(opts));
-    dinfo->bdrv = bdrv_new(dinfo->id, &error);
-    if (error) {
-        error_propagate(errp, error);
-        goto bdrv_new_err;
+    bs = bdrv_new(qemu_opts_id(opts), errp);
+    if (!bs) {
+        goto early_err;
    }
-    dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
-    dinfo->bdrv->read_only = ro;
-    dinfo->bdrv->detect_zeroes = detect_zeroes;
-    QTAILQ_INSERT_TAIL(&drives, dinfo, next);
+    bs->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
+    bs->read_only = ro;
+    bs->detect_zeroes = detect_zeroes;

-    bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
+    bdrv_set_on_error(bs, on_read_error, on_write_error);

    /* disk I/O throttling */
    if (throttle_enabled(&cfg)) {
-        bdrv_io_limits_enable(dinfo->bdrv);
-        bdrv_set_io_limits(dinfo->bdrv, &cfg);
+        bdrv_io_limits_enable(bs);
+        bdrv_set_io_limits(bs, &cfg);
    }

+    dinfo = g_malloc0(sizeof(*dinfo));
+    dinfo->id = g_strdup(qemu_opts_id(opts));
+    dinfo->bdrv = bs;
+    QTAILQ_INSERT_TAIL(&drives, dinfo, next);
+
    if (!file || !*file) {
        if (has_driver_specific_opts) {
            file = NULL;
@ -502,7 +510,8 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
    bdrv_flags |= ro ? 0 : BDRV_O_RDWR;

    QINCREF(bs_opts);
-    ret = bdrv_open(&dinfo->bdrv, file, NULL, bs_opts, bdrv_flags, drv, &error);
+    ret = bdrv_open(&bs, file, NULL, bs_opts, bdrv_flags, drv, &error);
+    assert(bs == dinfo->bdrv);

    if (ret < 0) {
        error_setg(errp, "could not open disk image %s: %s",
@ -511,8 +520,9 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
        goto err;
    }

-    if (bdrv_key_required(dinfo->bdrv))
+    if (bdrv_key_required(bs)) {
        autostart = 0;
+    }

    QDECREF(bs_opts);
    qemu_opts_del(opts);
@ -520,11 +530,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
    return dinfo;

 err:
-    bdrv_unref(dinfo->bdrv);
-    QTAILQ_REMOVE(&drives, dinfo, next);
-bdrv_new_err:
-    g_free(dinfo->id);
-    g_free(dinfo);
+    bdrv_unref(bs);
 early_err:
    qemu_opts_del(opts);
 err_no_opts:
@ -532,12 +538,18 @@ err_no_opts:
    return NULL;
 }

-static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
+static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to,
+                            Error **errp)
 {
    const char *value;

    value = qemu_opt_get(opts, from);
    if (value) {
+        if (qemu_opt_find(opts, to)) {
+            error_setg(errp, "'%s' and its alias '%s' can't be used at the "
+                       "same time", to, from);
+            return;
+        }
        qemu_opt_set(opts, to, value);
        qemu_opt_unset(opts, from);
    }
@ -641,28 +653,43 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
    const char *serial;
    const char *filename;
    Error *local_err = NULL;
+    int i;

    /* Change legacy command line options into QMP ones */
-    qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
-    qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read");
-    qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write");
+    static const struct {
+        const char *from;
+        const char *to;
+    } opt_renames[] = {
+        { "iops",           "throttling.iops-total" },
+        { "iops_rd",        "throttling.iops-read" },
+        { "iops_wr",        "throttling.iops-write" },

-    qemu_opt_rename(all_opts, "bps", "throttling.bps-total");
-    qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read");
-    qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write");
+        { "bps",            "throttling.bps-total" },
+        { "bps_rd",         "throttling.bps-read" },
+        { "bps_wr",         "throttling.bps-write" },

-    qemu_opt_rename(all_opts, "iops_max", "throttling.iops-total-max");
-    qemu_opt_rename(all_opts, "iops_rd_max", "throttling.iops-read-max");
-    qemu_opt_rename(all_opts, "iops_wr_max", "throttling.iops-write-max");
+        { "iops_max",       "throttling.iops-total-max" },
+        { "iops_rd_max",    "throttling.iops-read-max" },
+        { "iops_wr_max",    "throttling.iops-write-max" },

-    qemu_opt_rename(all_opts, "bps_max", "throttling.bps-total-max");
-    qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max");
-    qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max");
+        { "bps_max",        "throttling.bps-total-max" },
+        { "bps_rd_max",     "throttling.bps-read-max" },
+        { "bps_wr_max",     "throttling.bps-write-max" },

-    qemu_opt_rename(all_opts,
-                    "iops_size", "throttling.iops-size");
+        { "iops_size",      "throttling.iops-size" },

-    qemu_opt_rename(all_opts, "readonly", "read-only");
+        { "readonly",       "read-only" },
+    };
+
+    for (i = 0; i < ARRAY_SIZE(opt_renames); i++) {
+        qemu_opt_rename(all_opts, opt_renames[i].from, opt_renames[i].to,
+                        &local_err);
+        if (local_err) {
+            error_report("%s", error_get_pretty(local_err));
+            error_free(local_err);
+            return NULL;
+        }
+    }

    value = qemu_opt_get(all_opts, "cache");
    if (value) {
--- a/docs/blkdebug.txt
+++ b/docs/blkdebug.txt
@ -0,0 +1,161 @@
+Block I/O error injection using blkdebug
+----------------------------------------
+Copyright (C) 2014 Red Hat Inc
+
+This work is licensed under the terms of the GNU GPL, version 2 or later.  See
+the COPYING file in the top-level directory.
+
+The blkdebug block driver is a rule-based error injection engine.  It can be
+used to exercise error code paths in block drivers including ENOSPC (out of
+space) and EIO.
+
+This document gives an overview of the features available in blkdebug.
+
+Background
+----------
+Block drivers have many error code paths that handle I/O errors.  Image formats
+are especially complex since metadata I/O errors during cluster allocation or
+while updating tables happen halfway through request processing and require
+discipline to keep image files consistent.
+
+Error injection allows test cases to trigger I/O errors at specific points.
+This way, all error paths can be tested to make sure they are correct.
+
+Rules
+-----
+The blkdebug block driver takes a list of "rules" that tell the error injection
+engine when to fail an I/O request.
+
+Each I/O request is evaluated against the rules.  If a rule matches the request
+then its "action" is executed.
+
+Rules can be placed in a configuration file; the configuration file
+follows the same .ini-like format used by QEMU's -readconfig option, and
+each section of the file represents a rule.
+
+The following configuration file defines a single rule:
+
+  $ cat blkdebug.conf
+  [inject-error]
+  event = "read_aio"
+  errno = "28"
+
+This rule fails all aio read requests with ENOSPC (28).  Note that the errno
+value depends on the host.  On Linux, see
+/usr/include/asm-generic/errno-base.h for errno values.
+
+Invoke QEMU as follows:
+
+  $ qemu-system-x86_64
+        -drive if=none,cache=none,file=blkdebug:blkdebug.conf:test.img,id=drive0 \
+        -device virtio-blk-pci,drive=drive0,id=virtio-blk-pci0
+
+Rules support the following attributes:
+
+  event - which type of operation to match (e.g. read_aio, write_aio,
+          flush_to_os, flush_to_disk).  See the "Events" section for
+          information on events.
+
+  state - (optional) the engine must be in this state number in order for this
+          rule to match.  See the "State transitions" section for information
+          on states.
+
+  errno - the numeric errno value to return when a request matches this rule.
+          The errno values depend on the host since the numeric values are not
+          standarized in the POSIX specification.
+
+  sector - (optional) a sector number that the request must overlap in order to
+           match this rule
+
+  once - (optional, default "off") only execute this action on the first
+         matching request
+
+  immediately - (optional, default "off") return a NULL BlockDriverAIOCB
+                pointer and fail without an errno instead.  This exercises the
+                code path where BlockDriverAIOCB fails and the caller's
+                BlockDriverCompletionFunc is not invoked.
+
+Events
+------
+Block drivers provide information about the type of I/O request they are about
+to make so rules can match specific types of requests.  For example, the qcow2
+block driver tells blkdebug when it accesses the L1 table so rules can match
+only L1 table accesses and not other metadata or guest data requests.
+
+The core events are:
+
+  read_aio - guest data read
+
+  write_aio - guest data write
+
+  flush_to_os - write out unwritten block driver state (e.g. cached metadata)
+
+  flush_to_disk - flush the host block device's disk cache
+
+See block/blkdebug.c:event_names[] for the full list of events.  You may need
+to grep block driver source code to understand the meaning of specific events.
+
+State transitions
+-----------------
+There are cases where more power is needed to match a particular I/O request in
+a longer sequence of requests.  For example:
+
+  write_aio
+  flush_to_disk
+  write_aio
+
+How do we match the 2nd write_aio but not the first?  This is where state
+transitions come in.
+
+The error injection engine has an integer called the "state" that always starts
+initialized to 1.  The state integer is internal to blkdebug and cannot be
+observed from outside but rules can interact with it for powerful matching
+behavior.
+
+Rules can be conditional on the current state and they can transition to a new
+state.
+
+When a rule's "state" attribute is non-zero then the current state must equal
+the attribute in order for the rule to match.
+
+For example, to match the 2nd write_aio:
+
+  [set-state]
+  event = "write_aio"
+  state = "1"
+  new_state = "2"
+
+  [inject-error]
+  event = "write_aio"
+  state = "2"
+  errno = "5"
+
+The first write_aio request matches the set-state rule and transitions from
+state 1 to state 2.  Once state 2 has been entered, the set-state rule no
+longer matches since it requires state 1.  But the inject-error rule now
+matches the next write_aio request and injects EIO (5).
+
+State transition rules support the following attributes:
+
+  event - which type of operation to match (e.g. read_aio, write_aio,
+          flush_to_os, flush_to_disk).  See the "Events" section for
+          information on events.
+
+  state - (optional) the engine must be in this state number in order for this
+          rule to match
+
+  new_state - transition to this state number
+
+Suspend and resume
+------------------
+Exercising code paths in block drivers may require specific ordering amongst
+concurrent requests.  The "breakpoint" feature allows requests to be halted on
+a blkdebug event and resumed later.  This makes it possible to achieve
+deterministic ordering when multiple requests are in flight.
+
+Breakpoints on blkdebug events are associated with a user-defined "tag" string.
+This tag serves as an identifier by which the request can be resumed at a later
+point.
+
+See the qemu-io(1) break, resume, remove_break, and wait_break commands for
+details.
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@ -103,6 +103,7 @@ typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaq
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
                     int abort_on_failure);

+int qemu_opts_id_wellformed(const char *id);
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
                           int fail_if_exists, Error **errp);
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@ -56,6 +56,7 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                    const char *optstr);
 DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
 void drive_del(DriveInfo *dinfo);
+void drive_info_del(DriveInfo *dinfo);

 /* device-hotplug */

--- a/qemu-img.c
+++ b/qemu-img.c
@ -1011,14 +1011,14 @@ static int img_compare(int argc, char **argv)
        goto out3;
    }

-    bs1 = bdrv_new_open("image 1", filename1, fmt1, flags, true, quiet);
+    bs1 = bdrv_new_open("image_1", filename1, fmt1, flags, true, quiet);
    if (!bs1) {
        error_report("Can't open file %s", filename1);
        ret = 2;
        goto out3;
    }

-    bs2 = bdrv_new_open("image 2", filename2, fmt2, flags, true, quiet);
+    bs2 = bdrv_new_open("image_2", filename2, fmt2, flags, true, quiet);
    if (!bs2) {
        error_report("Can't open file %s", filename2);
        ret = 2;
@ -1359,7 +1359,7 @@ static int img_convert(int argc, char **argv)

    total_sectors = 0;
    for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        char *id = bs_n > 1 ? g_strdup_printf("source %d", bs_i)
+        char *id = bs_n > 1 ? g_strdup_printf("source_%d", bs_i)
                            : g_strdup("source");
        bs[bs_i] = bdrv_new_open(id, argv[optind + bs_i], fmt, src_flags,
                                 true, quiet);
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@ -773,7 +773,7 @@ int main(int argc, char **argv)
        }
    } while (state != TERMINATED);

-    bdrv_close(bs);
+    bdrv_unref(bs);
    if (sockpath) {
        unlink(sockpath);
    }
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@ -1,5 +1,6 @@
 stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += bdrv-commit-all.o
+stub-obj-y += blockdev.o
 stub-obj-y += chr-baum-init.o
 stub-obj-y += chr-msmouse.o
 stub-obj-y += chr-testdev.o
--- a/stubs/blockdev.c
+++ b/stubs/blockdev.c
@ -0,0 +1,12 @@
+#include <assert.h>
+#include "sysemu/blockdev.h"
+
+DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
+{
+    return NULL;
+}
+
+void drive_info_del(DriveInfo *dinfo)
+{
+    assert(!dinfo);
+}
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@ -198,6 +198,29 @@ run_qemu -drive file.driver=nbd
 run_qemu -drive file.driver=raw
 run_qemu -drive foo=bar

+echo
+echo === Specifying both an option and its legacy alias ===
+echo
+
+run_qemu -drive file="$TEST_IMG",iops=1234,throttling.iops-total=5678
+run_qemu -drive file="$TEST_IMG",iops_rd=1234,throttling.iops-read=5678
+run_qemu -drive file="$TEST_IMG",iops_wr=1234,throttling.iops-write=5678
+
+run_qemu -drive file="$TEST_IMG",bps=1234,throttling.bps-total=5678
+run_qemu -drive file="$TEST_IMG",bps_rd=1234,throttling.bps-read=5678
+run_qemu -drive file="$TEST_IMG",bps_wr=1234,throttling.bps-write=5678
+
+run_qemu -drive file="$TEST_IMG",iops_max=1234,throttling.iops-total-max=5678
+run_qemu -drive file="$TEST_IMG",iops_rd_max=1234,throttling.iops-read-max=5678
+run_qemu -drive file="$TEST_IMG",iops_wr_max=1234,throttling.iops-write-max=5678
+
+run_qemu -drive file="$TEST_IMG",bps_max=1234,throttling.bps-total-max=5678
+run_qemu -drive file="$TEST_IMG",bps_rd_max=1234,throttling.bps-read-max=5678
+run_qemu -drive file="$TEST_IMG",bps_wr_max=1234,throttling.bps-write-max=5678
+
+run_qemu -drive file="$TEST_IMG",iops_size=1234,throttling.iops-size=5678
+run_qemu -drive file="$TEST_IMG",readonly=on,read-only=off
+
 echo
 echo === Parsing protocol from file name ===
 echo
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@ -274,6 +274,51 @@ Testing: -drive foo=bar
 QEMU_PROG: -drive foo=bar: could not open disk image ide0-hd0: Must specify either driver or file


+=== Specifying both an option and its legacy alias ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops=1234,throttling.iops-total=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops=1234,throttling.iops-total=5678: 'throttling.iops-total' and its alias 'iops' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_rd=1234,throttling.iops-read=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_rd=1234,throttling.iops-read=5678: 'throttling.iops-read' and its alias 'iops_rd' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_wr=1234,throttling.iops-write=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_wr=1234,throttling.iops-write=5678: 'throttling.iops-write' and its alias 'iops_wr' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1234,throttling.bps-total=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=1234,throttling.bps-total=5678: 'throttling.bps-total' and its alias 'bps' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd=1234,throttling.bps-read=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd=1234,throttling.bps-read=5678: 'throttling.bps-read' and its alias 'bps_rd' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_wr=1234,throttling.bps-write=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_wr=1234,throttling.bps-write=5678: 'throttling.bps-write' and its alias 'bps_wr' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_max=1234,throttling.iops-total-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_max=1234,throttling.iops-total-max=5678: 'throttling.iops-total-max' and its alias 'iops_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_rd_max=1234,throttling.iops-read-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_rd_max=1234,throttling.iops-read-max=5678: 'throttling.iops-read-max' and its alias 'iops_rd_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_wr_max=1234,throttling.iops-write-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_wr_max=1234,throttling.iops-write-max=5678: 'throttling.iops-write-max' and its alias 'iops_wr_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_max=1234,throttling.bps-total-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_max=1234,throttling.bps-total-max=5678: 'throttling.bps-total-max' and its alias 'bps_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd_max=1234,throttling.bps-read-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd_max=1234,throttling.bps-read-max=5678: 'throttling.bps-read-max' and its alias 'bps_rd_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_wr_max=1234,throttling.bps-write-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_wr_max=1234,throttling.bps-write-max=5678: 'throttling.bps-write-max' and its alias 'bps_wr_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_size=1234,throttling.iops-size=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_size=1234,throttling.iops-size=5678: 'throttling.iops-size' and its alias 'iops_size' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' and its alias 'readonly' can't be used at the same time
+
+
 === Parsing protocol from file name ===

 Testing: -hda foo:bar
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@ -20,7 +20,7 @@ QMP_VERSION
 {"return": {}}
 {"return": {}}
 {"error": {"class": "GenericError", "desc": "Device with id 'disk' already exists"}}
-{"error": {"class": "GenericError", "desc": "Device with node-name 'test-node' already exists"}}
+{"error": {"class": "GenericError", "desc": "Device name 'test-node' conflicts with an existing node name"}}
 main-loop: WARNING: I/O thread spun for 1000 iterations
 {"error": {"class": "GenericError", "desc": "could not open disk image disk2: node-name=disk is conflicting with a device id"}}
 {"error": {"class": "GenericError", "desc": "could not open disk image disk2: Duplicate node name"}}
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@ -376,10 +376,16 @@ BEGIN        { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
                    echo $id >>$tmp.list
                else
                    # oops
-                    echo "$id - unknown test, ignored"
+                    if [ "$start" == "$end" -a "$id" == "$end" ]
+                    then
+                        echo "$id - unknown test"
+                        exit 1
+                    else
+                        echo "$id - unknown test, ignored"
+                    fi
                fi
            fi
-        done
+        done || exit 1
    fi

 done
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@ -641,7 +641,7 @@ QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id)
    return NULL;
 }

-static int id_wellformed(const char *id)
+int qemu_opts_id_wellformed(const char *id)
 {
    int i;

@ -662,7 +662,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
    QemuOpts *opts = NULL;

    if (id) {
-        if (!id_wellformed(id)) {
+        if (!qemu_opts_id_wellformed(id)) {
            error_set(errp,QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
 #if 0 /* conversion from qerror_report() to error_set() broke this: */
            error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n");