* [Qemu-devel] [PULL 00/11] Block patches
@ 2014-09-26 18:58 Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 01/11] blkdebug: show an error for invalid event names Kevin Wolf
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
The following changes since commit 4f2280b2190e39aa6761cc8188626ed9aad350c1:
Merge remote-tracking branch 'remotes/mcayland/tags/qemu-sparc-signed' into staging (2014-09-24 13:45:13 +0100)
are available in the git repository at:
git://repo.or.cz/qemu/kevin.git tags/for-upstream
for you to fetch changes up to c9d17ad0dd3f04cdef44d58db97ea9864fbcdee7:
qemu-iotests: Fail test if explicit test case number is unknown (2014-09-25 15:25:20 +0200)
----------------------------------------------------------------
Block patches
----------------------------------------------------------------
Fam Zheng (1):
qemu-iotests: Fail test if explicit test case number is unknown
Kevin Wolf (3):
block: Specify -drive legacy option aliases in array
block: Catch simultaneous usage of options and their aliases
block: Validate node-name
Markus Armbruster (4):
blockdev: Disentangle BlockDriverState and DriveInfo creation
block: Keep DriveInfo alive until BlockDriverState dies
qemu-nbd: Destroy the BlockDriverState properly
block: Improve message for device name clashing with node name
Stefan Hajnoczi (3):
blkdebug: show an error for invalid event names
docs: add blkdebug block driver documentation
vpc: fix beX_to_cpu() and cpu_to_beX() confusion
block.c | 21 ++++--
block/blkdebug.c | 22 ++++++-
block/vpc.c | 44 ++++++-------
blockdev.c | 103 ++++++++++++++++++-----------
docs/blkdebug.txt | 161 +++++++++++++++++++++++++++++++++++++++++++++
include/qemu/option.h | 1 +
include/sysemu/blockdev.h | 1 +
qemu-img.c | 6 +-
qemu-nbd.c | 2 +-
stubs/Makefile.objs | 1 +
stubs/blockdev.c | 12 ++++
tests/qemu-iotests/051 | 23 +++++++
tests/qemu-iotests/051.out | 45 +++++++++++++
tests/qemu-iotests/087.out | 2 +-
tests/qemu-iotests/common | 10 ++-
util/qemu-option.c | 4 +-
16 files changed, 382 insertions(+), 76 deletions(-)
create mode 100644 docs/blkdebug.txt
create mode 100644 stubs/blockdev.c
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 01/11] blkdebug: show an error for invalid event names
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 02/11] blockdev: Disentangle BlockDriverState and DriveInfo creation Kevin Wolf
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Stefan Hajnoczi <stefanha@redhat.com>
It is easy to typo a blkdebug configuration and waste a lot of time
figuring out why no rules are matching.
Push the Error** down into add_rule() so we can report an error when the
event name is invalid.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/blkdebug.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index ced0b60..f8fbb0f 100644
--- 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:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 02/11] blockdev: Disentangle BlockDriverState and DriveInfo creation
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 01/11] blkdebug: show an error for invalid event names Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 03/11] block: Keep DriveInfo alive until BlockDriverState dies Kevin Wolf
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Markus Armbruster <armbru@redhat.com>
blockdev_init() mixes up BlockDriverState and DriveInfo initialization
Finish the BlockDriverState job before starting to mess with
DriveInfo. Easier on the eyes.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b361fbb..5ec4635 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -301,6 +301,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 +457,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 +504,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 +514,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,9 +524,8 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
return dinfo;
err:
- bdrv_unref(dinfo->bdrv);
+ bdrv_unref(bs);
QTAILQ_REMOVE(&drives, dinfo, next);
-bdrv_new_err:
g_free(dinfo->id);
g_free(dinfo);
early_err:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 03/11] block: Keep DriveInfo alive until BlockDriverState dies
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 01/11] blkdebug: show an error for invalid event names Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 02/11] blockdev: Disentangle BlockDriverState and DriveInfo creation Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 04/11] qemu-nbd: Destroy the BlockDriverState properly Kevin Wolf
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Markus Armbruster <armbru@redhat.com>
If the BDS's refcnt > 0, drive_del() destroys the DriveInfo, but not
the BDS. This can happen in three places:
* Device model destruction during unplug: blockdev_auto_del()
* Xen IDE unplug: pci_piix3_xen_ide_unplug()
* drive_del command when no device model is attached: do_drive_del()
The other callers of drive_del are on error paths where refcnt == 1.
If the user somehow manages to plug in a device model using a BDS that
has gone through drive_del(), the legacy configuration passed in
DriveInfo doesn't reach the device model, and automatic deletion on
unplug doesn't work. Worse, some device models such as scsi-disk
crash when DriveInfo doesn't exist.
This is theoretical; I didn't research an actual reproducer. The problem
was introduced when we replaced DriveInfo reference counting by BDS
reference counting in commit a94a3fa..fa510eb.
Fix by keeping DriveInfo alive until its BDS dies.
This affects qemu_drive_opts: now you can't reuse the same ID for new
drive options until the BDS dies. Before, you could, but since the
code always attempts to create a BDS with the same ID next, the
enclosing operation "create a new drive" failed anyway. Different
error path, same result.
Unfortunately, the fix involves use of blockdev.c stuff from block.c,
which is a layering violation. Fortunately, my forthcoming
BlockBackend work will get rid of it again.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 2 ++
blockdev.c | 13 ++++++++-----
include/sysemu/blockdev.h | 1 +
stubs/Makefile.objs | 1 +
stubs/blockdev.c | 12 ++++++++++++
5 files changed, 24 insertions(+), 5 deletions(-)
create mode 100644 stubs/blockdev.c
diff --git a/block.c b/block.c
index a857913..0c4540c 100644
--- 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"
@@ -2110,6 +2111,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);
}
diff --git a/blockdev.c b/blockdev.c
index 5ec4635..450f95c 100644
--- 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);
@@ -525,9 +531,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
err:
bdrv_unref(bs);
- QTAILQ_REMOVE(&drives, dinfo, next);
- g_free(dinfo->id);
- g_free(dinfo);
early_err:
qemu_opts_del(opts);
err_no_opts:
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 23a5d10..abec381 100644
--- 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 */
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 5e347d0..c0b1f6a 100644
--- 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
diff --git a/stubs/blockdev.c b/stubs/blockdev.c
new file mode 100644
index 0000000..5d0a79c
--- /dev/null
+++ 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);
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 04/11] qemu-nbd: Destroy the BlockDriverState properly
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (2 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 03/11] block: Keep DriveInfo alive until BlockDriverState dies Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 05/11] block: Improve message for device name clashing with node name Kevin Wolf
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Markus Armbruster <armbru@redhat.com>
Match the bdrv_new() with a bdrv_unref(), just to be tidy.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-nbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index de9963f..fa60338 100644
--- 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);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 05/11] block: Improve message for device name clashing with node name
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (3 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 04/11] qemu-nbd: Destroy the BlockDriverState properly Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 06/11] block: Specify -drive legacy option aliases in array Kevin Wolf
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Markus Armbruster <armbru@redhat.com>
Suggested-by: Benoit Canet <benoit.canet@nodalink.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 3 ++-
tests/qemu-iotests/087.out | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 0c4540c..a2fa1e1 100644
--- a/block.c
+++ b/block.c
@@ -347,7 +347,8 @@ BlockDriverState *bdrv_new(const char *device_name, Error **errp)
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;
}
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 7fbee3f..75a54e0 100644
--- 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"}}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 06/11] block: Specify -drive legacy option aliases in array
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (4 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 05/11] block: Improve message for device name clashing with node name Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 07/11] block: Catch simultaneous usage of options and their aliases Kevin Wolf
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
Instead of a series of qemu_opt_rename() calls, use an array that
contains all of the renames and call qemu_opt_rename() in a loop. This
will keep the code readable even when we add an error return to
qemu_opt_rename().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 450f95c..88f7928 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -647,28 +647,37 @@ 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);
+ }
value = qemu_opt_get(all_opts, "cache");
if (value) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 07/11] block: Catch simultaneous usage of options and their aliases
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (5 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 06/11] block: Specify -drive legacy option aliases in array Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 08/11] docs: add blkdebug block driver documentation Kevin Wolf
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
While thinking about precedence of conflicting block device options from
different sources, I noticed that you can specify both an option and its
legacy alias at the same time (e.g. readonly=on,read-only=off). Rather
than specifying the order of precedence, we should simply forbid such
combinations.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 16 ++++++++++++++--
tests/qemu-iotests/051 | 23 +++++++++++++++++++++++
tests/qemu-iotests/051.out | 45 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 88f7928..ad43648 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -538,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);
}
@@ -676,7 +682,13 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
};
for (i = 0; i < ARRAY_SIZE(opt_renames); i++) {
- qemu_opt_rename(all_opts, opt_renames[i].from, opt_renames[i].to);
+ 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");
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index a41334e..11c858f 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -199,6 +199,29 @@ 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
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index a3f2820..2c7e808 100644
--- 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
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 08/11] docs: add blkdebug block driver documentation
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (6 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 07/11] block: Catch simultaneous usage of options and their aliases Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 09/11] vpc: fix beX_to_cpu() and cpu_to_beX() confusion Kevin Wolf
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Stefan Hajnoczi <stefanha@redhat.com>
The blkdebug block driver is undocumented. Documenting it is worthwhile
since it offers powerful error injection features that are used by
qemu-iotests test cases.
This document will make it easier for people to learn about and use
blkdebug.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/blkdebug.txt | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 161 insertions(+)
create mode 100644 docs/blkdebug.txt
diff --git a/docs/blkdebug.txt b/docs/blkdebug.txt
new file mode 100644
index 0000000..5dde072
--- /dev/null
+++ 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.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 09/11] vpc: fix beX_to_cpu() and cpu_to_beX() confusion
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (7 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 08/11] docs: add blkdebug block driver documentation Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 10/11] block: Validate node-name Kevin Wolf
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Stefan Hajnoczi <stefanha@redhat.com>
The beX_to_cpu() and cpu_to_beX() functions perform the same operation -
they do a byteswap if the host CPU endianness is little-endian or a
nothing otherwise.
The point of two names for the same operation is that it documents which
direction the data is being converted. This makes it clear whether the
data is suitable for CPU processing or in its external representation.
This patch fixes incorrect beX_to_cpu()/cpu_to_beX() usage.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vpc.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 4947369..e08144a 100644
--- 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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 10/11] block: Validate node-name
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (8 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 09/11] vpc: fix beX_to_cpu() and cpu_to_beX() confusion Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 11/11] qemu-iotests: Fail test if explicit test case number is unknown Kevin Wolf
2014-09-29 13:03 ` [Qemu-devel] [PULL 00/11] Block patches Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
The device_name of a BlockDriverState is currently checked because it is
always used as a QemuOpts ID and qemu_opts_create() checks whether such
IDs are wellformed.
node-name is supposed to share the same namespace, but it isn't checked
currently. This patch adds explicit checks both for device_name and
node-name so that the same rules will still apply even if QemuOpts won't
be used any more at some point.
qemu-img used to use names with spaces in them, which isn't allowed any
more. Replace them with underscores.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 16 +++++++++++++---
include/qemu/option.h | 1 +
qemu-img.c | 6 +++---
util/qemu-option.c | 4 ++--
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index a2fa1e1..c5a251c 100644
--- a/block.c
+++ b/block.c
@@ -335,12 +335,22 @@ 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);
@@ -863,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;
}
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 59bea75..945347c 100644
--- 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);
diff --git a/qemu-img.c b/qemu-img.c
index dbf0904..ea4bbae 100644
--- 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);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 6dc27ce..0cf9960 100644
--- 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");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PULL 11/11] qemu-iotests: Fail test if explicit test case number is unknown
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (9 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 10/11] block: Validate node-name Kevin Wolf
@ 2014-09-26 18:58 ` Kevin Wolf
2014-09-29 13:03 ` [Qemu-devel] [PULL 00/11] Block patches Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Kevin Wolf @ 2014-09-26 18:58 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf
From: Fam Zheng <famz@redhat.com>
When we expand a number range, we just print "$id - unknown test,
ignored", this is convenient if we want to run a range of tests.
When we designate a test case number explicitly, we shouldn't just
ignore it if the case script doesn't exist.
Print an error and fail the test.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/common | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 89c6dde..9e12bec 100644
--- 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
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PULL 00/11] Block patches
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
` (10 preceding siblings ...)
2014-09-26 18:58 ` [Qemu-devel] [PULL 11/11] qemu-iotests: Fail test if explicit test case number is unknown Kevin Wolf
@ 2014-09-29 13:03 ` Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2014-09-29 13:03 UTC (permalink / raw)
To: Kevin Wolf; +Cc: QEMU Developers
On 26 September 2014 19:58, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit 4f2280b2190e39aa6761cc8188626ed9aad350c1:
>
> Merge remote-tracking branch 'remotes/mcayland/tags/qemu-sparc-signed' into staging (2014-09-24 13:45:13 +0100)
>
> are available in the git repository at:
>
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to c9d17ad0dd3f04cdef44d58db97ea9864fbcdee7:
>
> qemu-iotests: Fail test if explicit test case number is unknown (2014-09-25 15:25:20 +0200)
>
> ----------------------------------------------------------------
> Block patches
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-09-29 13:03 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-26 18:58 [Qemu-devel] [PULL 00/11] Block patches Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 01/11] blkdebug: show an error for invalid event names Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 02/11] blockdev: Disentangle BlockDriverState and DriveInfo creation Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 03/11] block: Keep DriveInfo alive until BlockDriverState dies Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 04/11] qemu-nbd: Destroy the BlockDriverState properly Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 05/11] block: Improve message for device name clashing with node name Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 06/11] block: Specify -drive legacy option aliases in array Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 07/11] block: Catch simultaneous usage of options and their aliases Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 08/11] docs: add blkdebug block driver documentation Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 09/11] vpc: fix beX_to_cpu() and cpu_to_beX() confusion Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 10/11] block: Validate node-name Kevin Wolf
2014-09-26 18:58 ` [Qemu-devel] [PULL 11/11] qemu-iotests: Fail test if explicit test case number is unknown Kevin Wolf
2014-09-29 13:03 ` [Qemu-devel] [PULL 00/11] Block patches Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).