Skip to contentSkip to conversation

blkdebug: Fix config with multiple states

PATCH In review Kevin Wolf 2 participants 6 messages Jul 1, 2010 Block layer

lore
Share & tools
lore

Kevin Wolf

author
#0 lore raw

v2 cover blkdebug: Fix config with multiple states

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(-)
author
#1 lore raw

PATCH v2 1/4 qemu-option: New qemu_opts_reset()

From: Markus Armbruster <[email protected]>

Signed-off-by Markus Armbruster <[email protected]>
Signed-off-by Kevin Wolf <[email protected]>
---
 qemu-option.c |    9 +++++++++
 qemu-option.h |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)
qemu-option.c +9 -0
index 7f70d0f..30327d4 100644
@@ -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)
 {
qemu-option.h +1 -0
index 4823219..9e2406c 100644
@@ -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);
Signature · 1 line
1.6.6.1
author
#2 lore raw

PATCH v2 2/4 blkdebug: Fix set_state_opts definition

In reply to 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 <[email protected]>
---
 block/blkdebug.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
block/blkdebug.c +1 -1
index 98fed94..4ec8ca6 100644
@@ -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",
Signature · 1 line
1.6.6.1
author
#3 lore raw

PATCH v2 3/4 blkdebug: Free QemuOpts after having read the config

In reply to 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 <[email protected]>
---
 block/blkdebug.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
block/blkdebug.c +2 -0
index 4ec8ca6..78cbff4 100644
@@ -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;
 }
Signature · 1 line
1.6.6.1
author
#4 lore raw

PATCH v2 4/4 blkdebug: Initialize state as 1

In reply to 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 <[email protected]>
---
 block/blkdebug.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
block/blkdebug.c +3 -0
index 78cbff4..2a63df9 100644
@@ -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) {
Signature · 1 line
1.6.6.1