* [Qemu-devel] [PATCH 1/3] nbd: Fix filename generation
2014-09-24 19:38 [Qemu-devel] [PATCH 0/3] block: Fix filename generation for blkdebug and nbd Max Reitz
@ 2014-09-24 19:38 ` Max Reitz
2014-09-24 20:09 ` Paolo Bonzini
2014-09-24 19:38 ` [Qemu-devel] [PATCH 2/3] blkdebug: Simplify and improve " Max Reitz
2014-09-24 19:38 ` [Qemu-devel] [PATCH 3/3] iotests: Plain blkdebug " Max Reitz
2 siblings, 1 reply; 5+ messages in thread
From: Max Reitz @ 2014-09-24 19:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Export names may be used with nbd+unix, too, fix nbd_refresh_filename()
accordingly. Also, for nbd+tcp, the documented path schema is
"nbd://host[:port]/export", so use it. Furthermore, as can be seen from
that schema, the port is optional.
That makes six single cases for how the filename can be formatted; it is
not easy to generalize these cases without the resulting statement being
completely unreadable, thus there is simply one snprintf() per case.
Finally, taking the options from BDRVNBDState::socket_opts is wrong,
because those will not contain the export name. Just use
BlockDriverState::options instead.
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/nbd.c | 44 +++++++++++++++++++++++++++++---------------
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 89775e1..04cc845 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -342,30 +342,44 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
static void nbd_refresh_filename(BlockDriverState *bs)
{
- BDRVNBDState *s = bs->opaque;
QDict *opts = qdict_new();
- const char *path = qemu_opt_get(s->socket_opts, "path");
- const char *host = qemu_opt_get(s->socket_opts, "host");
- const char *port = qemu_opt_get(s->socket_opts, "port");
- const char *export = qemu_opt_get(s->socket_opts, "export");
+ const char *path = qdict_get_try_str(bs->options, "path");
+ const char *host = qdict_get_try_str(bs->options, "host");
+ const char *port = qdict_get_try_str(bs->options, "port");
+ const char *export = qdict_get_try_str(bs->options, "export");
qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
- if (path) {
+ if (path && export) {
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix:%s", path);
- qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
- } else if (export) {
+ "nbd+unix:///%s?socket=%s", export, path);
+ } else if (path && !export) {
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd:%s:%s/%s", host, port, export);
- qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
- qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
- qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
- } else {
+ "nbd+unix://?socket=%s", path);
+ } else if (!path && export && port) {
+ snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s:%s/%s", host, port, export);
+ } else if (!path && export && !port) {
+ snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s/%s", host, export);
+ } else if (!path && !export && port) {
+ snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s:%s", host, port);
+ } else if (!path && !export && !port) {
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd:%s:%s", host, port);
+ "nbd://%s", host);
+ }
+
+ if (path) {
+ qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
+ } else if (port) {
qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
+ } else {
+ qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
+ }
+ if (export) {
+ qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
}
bs->full_open_options = opts;
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 2/3] blkdebug: Simplify and improve filename generation
2014-09-24 19:38 [Qemu-devel] [PATCH 0/3] block: Fix filename generation for blkdebug and nbd Max Reitz
2014-09-24 19:38 ` [Qemu-devel] [PATCH 1/3] nbd: Fix filename generation Max Reitz
@ 2014-09-24 19:38 ` Max Reitz
2014-09-24 19:38 ` [Qemu-devel] [PATCH 3/3] iotests: Plain blkdebug " Max Reitz
2 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2014-09-24 19:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Instead of actually recreating the options from scratch, just reuse the
options given for creating the BDS, which are the configuration file
name and additional options. In case there are no additional options we
can thus create a plain filename.
This obviously results in a different output for qemu-iotest 099 which
exactly tests this filename generation. Fix it up as well.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/blkdebug.c | 98 +++++++++++++---------------------------------
tests/qemu-iotests/099.out | 4 +-
2 files changed, 29 insertions(+), 73 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index f8fbb0f..cbe7715 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -719,93 +719,49 @@ static int64_t blkdebug_getlength(BlockDriverState *bs)
static void blkdebug_refresh_filename(BlockDriverState *bs)
{
- BDRVBlkdebugState *s = bs->opaque;
- struct BlkdebugRule *rule;
QDict *opts;
- QList *inject_error_list = NULL, *set_state_list = NULL;
- QList *suspend_list = NULL;
- int event;
+ const QDictEntry *e;
+ bool force_json = false;
+
+ for (e = qdict_first(bs->options); e; e = qdict_next(bs->options, e)) {
+ if (strcmp(qdict_entry_key(e), "config") &&
+ strcmp(qdict_entry_key(e), "x-image") &&
+ strcmp(qdict_entry_key(e), "image") &&
+ strncmp(qdict_entry_key(e), "image.", strlen("image.")))
+ {
+ force_json = true;
+ break;
+ }
+ }
- if (!bs->file->full_open_options) {
+ if (force_json && !bs->file->full_open_options) {
/* The config file cannot be recreated, so creating a plain filename
* is impossible */
return;
}
+ if (!force_json && bs->file->exact_filename[0]) {
+ snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "blkdebug:%s:%s", qdict_get_try_str(bs->options, "config"),
+ bs->file->exact_filename);
+ }
+
opts = qdict_new();
qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("blkdebug")));
QINCREF(bs->file->full_open_options);
qdict_put_obj(opts, "image", QOBJECT(bs->file->full_open_options));
- for (event = 0; event < BLKDBG_EVENT_MAX; event++) {
- QLIST_FOREACH(rule, &s->rules[event], next) {
- if (rule->action == ACTION_INJECT_ERROR) {
- QDict *inject_error = qdict_new();
-
- qdict_put_obj(inject_error, "event", QOBJECT(qstring_from_str(
- BlkdebugEvent_lookup[rule->event])));
- qdict_put_obj(inject_error, "state",
- QOBJECT(qint_from_int(rule->state)));
- qdict_put_obj(inject_error, "errno", QOBJECT(qint_from_int(
- rule->options.inject.error)));
- qdict_put_obj(inject_error, "sector", QOBJECT(qint_from_int(
- rule->options.inject.sector)));
- qdict_put_obj(inject_error, "once", QOBJECT(qbool_from_int(
- rule->options.inject.once)));
- qdict_put_obj(inject_error, "immediately",
- QOBJECT(qbool_from_int(
- rule->options.inject.immediately)));
-
- if (!inject_error_list) {
- inject_error_list = qlist_new();
- }
-
- qlist_append_obj(inject_error_list, QOBJECT(inject_error));
- } else if (rule->action == ACTION_SET_STATE) {
- QDict *set_state = qdict_new();
-
- qdict_put_obj(set_state, "event", QOBJECT(qstring_from_str(
- BlkdebugEvent_lookup[rule->event])));
- qdict_put_obj(set_state, "state",
- QOBJECT(qint_from_int(rule->state)));
- qdict_put_obj(set_state, "new_state", QOBJECT(qint_from_int(
- rule->options.set_state.new_state)));
-
- if (!set_state_list) {
- set_state_list = qlist_new();
- }
-
- qlist_append_obj(set_state_list, QOBJECT(set_state));
- } else if (rule->action == ACTION_SUSPEND) {
- QDict *suspend = qdict_new();
-
- qdict_put_obj(suspend, "event", QOBJECT(qstring_from_str(
- BlkdebugEvent_lookup[rule->event])));
- qdict_put_obj(suspend, "state",
- QOBJECT(qint_from_int(rule->state)));
- qdict_put_obj(suspend, "tag", QOBJECT(qstring_from_str(
- rule->options.suspend.tag)));
-
- if (!suspend_list) {
- suspend_list = qlist_new();
- }
-
- qlist_append_obj(suspend_list, QOBJECT(suspend));
- }
+ for (e = qdict_first(bs->options); e; e = qdict_next(bs->options, e)) {
+ if (strcmp(qdict_entry_key(e), "x-image") &&
+ strcmp(qdict_entry_key(e), "image") &&
+ strncmp(qdict_entry_key(e), "image.", strlen("image.")))
+ {
+ qobject_incref(qdict_entry_value(e));
+ qdict_put_obj(opts, qdict_entry_key(e), qdict_entry_value(e));
}
}
- if (inject_error_list) {
- qdict_put_obj(opts, "inject-error", QOBJECT(inject_error_list));
- }
- if (set_state_list) {
- qdict_put_obj(opts, "set-state", QOBJECT(set_state_list));
- }
- if (suspend_list) {
- qdict_put_obj(opts, "suspend", QOBJECT(suspend_list));
- }
-
bs->full_open_options = opts;
}
diff --git a/tests/qemu-iotests/099.out b/tests/qemu-iotests/099.out
index 55be4d4..50cc527 100644
--- a/tests/qemu-iotests/099.out
+++ b/tests/qemu-iotests/099.out
@@ -12,9 +12,9 @@ blkverify:TEST_DIR/t.IMGFMT.compare:TEST_DIR/t.IMGFMT
=== Testing JSON filename for blkdebug ===
-json:{"driver": "IMGFMT", "file": {"inject-error": [{"immediately": false, "once": false, "state": 0, "sector": -1, "event": "l1_update", "errno": 5}], "image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug"}}
+json:{"driver": "IMGFMT", "file": {"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "l1_update"}}
=== Testing indirectly enforced JSON filename ===
-json:{"driver": "raw", "file": {"test": {"driver": "IMGFMT", "file": {"inject-error": [{"immediately": false, "once": false, "state": 0, "sector": -1, "event": "l1_update", "errno": 5}], "image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug"}}, "driver": "blkverify", "raw": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.compare"}}}
+json:{"driver": "raw", "file": {"test": {"driver": "IMGFMT", "file": {"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "l1_update"}}, "driver": "blkverify", "raw": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.compare"}}}
*** done
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 3/3] iotests: Plain blkdebug filename generation
2014-09-24 19:38 [Qemu-devel] [PATCH 0/3] block: Fix filename generation for blkdebug and nbd Max Reitz
2014-09-24 19:38 ` [Qemu-devel] [PATCH 1/3] nbd: Fix filename generation Max Reitz
2014-09-24 19:38 ` [Qemu-devel] [PATCH 2/3] blkdebug: Simplify and improve " Max Reitz
@ 2014-09-24 19:38 ` Max Reitz
2 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2014-09-24 19:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Add a test whether blkdebug is able to generate a plain filename if
given a configuration file and a file to be tested only.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/099 | 9 ++++++++-
tests/qemu-iotests/099.out | 4 ++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
index ffc7ea7..74aa6d9 100755
--- a/tests/qemu-iotests/099
+++ b/tests/qemu-iotests/099
@@ -107,8 +107,15 @@ echo
# generate a JSON object here as well
test_qemu "file.driver=blkverify,file.raw.filename=$TEST_IMG.compare,file.test.file.driver=blkdebug,file.test.file.image.filename=$TEST_IMG,file.test.file.inject-error.0.event=l1_update"
+echo
+echo '=== Testing plain filename for blkdebug ==='
+echo
+
+touch "$TEST_DIR/blkdebug.conf"
+test_qemu "file.driver=blkdebug,file.config=$TEST_DIR/blkdebug.conf,file.image.filename=$TEST_IMG"
+
-rm -f "$TEST_IMG.compare"
+rm -f "$TEST_IMG.compare" "$TEST_DIR/blkdebug.conf"
# success, all done
echo "*** done"
diff --git a/tests/qemu-iotests/099.out b/tests/qemu-iotests/099.out
index 50cc527..3eebace 100644
--- a/tests/qemu-iotests/099.out
+++ b/tests/qemu-iotests/099.out
@@ -17,4 +17,8 @@ json:{"driver": "IMGFMT", "file": {"image": {"driver": "file", "filename": "TEST
=== Testing indirectly enforced JSON filename ===
json:{"driver": "raw", "file": {"test": {"driver": "IMGFMT", "file": {"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "l1_update"}}, "driver": "blkverify", "raw": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.compare"}}}
+
+=== Testing plain filename for blkdebug ===
+
+blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/t.IMGFMT
*** done
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread