[Qemu-devel] [PATCH v2 0/4] blkdebug: Fix config with multiple states

[Qemu-devel] [PATCH v2 0/4] blkdebug: Fix config with multiple states

[Kevin Wolf]

Turns out that using more than one state doesn't really work well. I'm trying
to reproduce a bug for which I need states, so now is the time to fix it.

v2:
- Use Markus' qemu_opts_reset for freeing QemuOpts. Makes the code reusable and
  fixes a use-after-free bug.

Kevin Wolf (3):
  blkdebug: Fix set_state_opts definition
  blkdebug: Free QemuOpts after having read the config
  blkdebug: Initialize state as 1

Markus Armbruster (1):
  qemu-option: New qemu_opts_reset()

 block/blkdebug.c |    7 ++++++-
 qemu-option.c    |    9 +++++++++
 qemu-option.h    |    1 +
 3 files changed, 16 insertions(+), 1 deletions(-)

[Qemu-devel] [PATCH v2 1/4] qemu-option: New qemu_opts_reset()

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

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

diff --git a/qemu-option.c b/qemu-option.c
index 7f70d0f..30327d4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -719,6 +719,15 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exist
     return opts;
 }
 
+void qemu_opts_reset(QemuOptsList *list)
+{
+    QemuOpts *opts, *next_opts;
+
+    QTAILQ_FOREACH_SAFE(opts, &list->head, next, next_opts) {
+        qemu_opts_del(opts);
+    }
+}
+
 int qemu_opts_set(QemuOptsList *list, const char *id,
                   const char *name, const char *value)
 {
diff --git a/qemu-option.h b/qemu-option.h
index 4823219..9e2406c 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -115,6 +115,7 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists);
+void qemu_opts_reset(QemuOptsList *list);
 int qemu_opts_set(QemuOptsList *list, const char *id,
                   const char *name, const char *value);
 const char *qemu_opts_id(QemuOpts *opts);
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 2/4] blkdebug: Fix set_state_opts definition

[Kevin Wolf]

The list head was initialized to point to the wrong list, so all actions ended
up being handled as inject-error even if they were set-state in fact.

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

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 98fed94..4ec8ca6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -111,7 +111,7 @@ static QemuOptsList inject_error_opts = {
 
 static QemuOptsList set_state_opts = {
     .name = "set-state",
-    .head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
+    .head = QTAILQ_HEAD_INITIALIZER(set_state_opts.head),
     .desc = {
         {
             .name = "event",
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 3/4] blkdebug: Free QemuOpts after having read the config

[Kevin Wolf]

Forgetting to free them means that the next instance inherits all rules and
gets its own rules only additionally.

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

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 4ec8ca6..78cbff4 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -267,6 +267,8 @@ static int read_config(BDRVBlkdebugState *s, const char *filename)
 
     ret = 0;
 fail:
+    qemu_opts_reset(&inject_error_opts);
+    qemu_opts_reset(&set_state_opts);
     fclose(f);
     return ret;
 }
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 4/4] blkdebug: Initialize state as 1

[Kevin Wolf]

state = 0 in rules means that the rule is valid for any state. Therefore it's
impossible to have a rule that works only in the initial state. This changes
the initial state from 0 to 1 to make this possible.

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

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 78cbff4..2a63df9 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -301,6 +301,9 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
     }
     filename = c + 1;
 
+    /* Set initial state */
+    s->vars.state = 1;
+
     /* Open the backing file */
     ret = bdrv_file_open(&bs->file, filename, flags);
     if (ret < 0) {
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH v2 0/4] blkdebug: Fix config with multiple states

[Markus Armbruster]

Looks good to me.