Skip to contentSkip to conversation

blkdebug: Fix config with multiple states

PATCH Superseded Kevin Wolf 2 participants 7 messages Jun 30, 2010 – Jul 1, 2010 Block layer

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Kevin Wolf

author
#0 lore raw

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.

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

 block/blkdebug.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)
author
#1 lore raw

PATCH 1/3 blkdebug: Fix set_state_opts definition

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
#2 lore raw

PATCH 2/3 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 |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
block/blkdebug.c +7 -0
index 4ec8ca6..b084782 100644
@@ -242,6 +242,11 @@ static int add_rule(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static int free_opts(QemuOpts *opts, void *opaque) {
+    qemu_opts_del(opts);
+    return 0;
+}
+
 static int read_config(BDRVBlkdebugState *s, const char *filename)
 {
     FILE *f;

@@ -267,6 +272,8 @@ static int read_config(BDRVBlkdebugState *s, const char *filename)
 
     ret = 0;
 fail:
+    qemu_opts_foreach(&inject_error_opts, free_opts, NULL, 0);
+    qemu_opts_foreach(&set_state_opts, free_opts, NULL, 0);
     fclose(f);
     return ret;
 }
Signature · 1 line
1.6.6.1
#3 lore raw

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

Quoted 2 lines · Kevin Wolf

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

I also found a use for freeing a complete QemuOptsList, here's my solution. The code that needs it isn't ready, yet. If you'd like to use it, I can push it to my repo.

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);
author
#4 lore raw

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

Am 01.07.2010 08:47, schrieb Markus Armbruster:

Quoted 8 lines
Quoted 2 lines · Kevin Wolf

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

I also found a use for freeing a complete QemuOptsList, here's my solution. The code that needs it isn't ready, yet. If you'd like to use it, I can push it to my repo.

If you have a use for it, it's the better solution. I'll rebase on top of this as soon as you have sent it as a proper patch.

Kevin

author
#5 lore raw

PATCH 3/3 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 b084782..99340db 100644
@@ -306,6 +306,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
#6 lore raw
In reply to Kevin Wolf
Quoted 2 lines · 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.

I'm not familiar with blkdebug, but these look like obvious bug fixes.