[Qemu-devel] [PATCH 0/6] blkdebug changes extracted from 1.2 streaming patches

[Qemu-devel] [PATCH 0/6] blkdebug changes extracted from 1.2 streaming patches

[Paolo Bonzini]

Hi,

these patches are prerequisites for testing streaming error handling.
They should be useful on their own, so I am sending them early to keep
the queue small.

Paolo Bonzini (6):
  blkdebug: remove sync i/o events
  blkdebug: tiny cleanup
  blkdebug: pass getlength to underlying file
  blkdebug: store list of active rules
  blkdebug: optionally tie errors to a specific sector
  raw: hook into blkdebug

 block.h          |    2 -
 block/blkdebug.c |  107 +++++++++++++++++++++++++++++++-----------------------
 block/qed.c      |    2 +-
 block/raw.c      |    2 +
 4 files changed, 64 insertions(+), 49 deletions(-)

-- 
1.7.10.1

[Qemu-devel] [PATCH 1/6] blkdebug: remove sync i/o events

[Paolo Bonzini]

These are unused, except (by mistake more or less) in QED.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.h          |    2 --
 block/blkdebug.c |    2 --
 block/qed.c      |    2 +-
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/block.h b/block.h
index bff5a9b..80c1768 100644
--- a/block.h
+++ b/block.h
@@ -391,9 +391,7 @@ typedef enum {
     BLKDBG_L2_ALLOC_COW_READ,
     BLKDBG_L2_ALLOC_WRITE,
 
-    BLKDBG_READ,
     BLKDBG_READ_AIO,
-    BLKDBG_READ_BACKING,
     BLKDBG_READ_BACKING_AIO,
     BLKDBG_READ_COMPRESSED,
 
diff --git a/block/blkdebug.c b/block/blkdebug.c
index e56e37d..1eff940 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -147,9 +147,7 @@ static const char *event_names[BLKDBG_EVENT_MAX] = {
     [BLKDBG_L2_ALLOC_COW_READ]              = "l2_alloc.cow_read",
     [BLKDBG_L2_ALLOC_WRITE]                 = "l2_alloc.write",
 
-    [BLKDBG_READ]                           = "read",
     [BLKDBG_READ_AIO]                       = "read_aio",
-    [BLKDBG_READ_BACKING]                   = "read_backing",
     [BLKDBG_READ_BACKING_AIO]               = "read_backing_aio",
     [BLKDBG_READ_COMPRESSED]                = "read_compressed",
 
diff --git a/block/qed.c b/block/qed.c
index ab59724..dd2832a 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -748,7 +748,7 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
     /* If the read straddles the end of the backing file, shorten it */
     size = MIN((uint64_t)backing_length - pos, qiov->size);
 
-    BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING);
+    BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO);
     bdrv_aio_readv(s->bs->backing_hd, pos / BDRV_SECTOR_SIZE,
                    qiov, size / BDRV_SECTOR_SIZE, cb, opaque);
 }
-- 
1.7.10.1

[Qemu-devel] [PATCH 2/6] blkdebug: tiny cleanup

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 1eff940..1f79ef2 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -361,9 +361,7 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
         return inject_error(bs, cb, opaque);
     }
 
-    BlockDriverAIOCB *acb =
-        bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
-    return acb;
+    return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
 }
 
 static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
@@ -376,9 +374,7 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
         return inject_error(bs, cb, opaque);
     }
 
-    BlockDriverAIOCB *acb =
-        bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
-    return acb;
+    return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
 }
 
 static void blkdebug_close(BlockDriverState *bs)
-- 
1.7.10.1

[Qemu-devel] [PATCH 3/6] blkdebug: pass getlength to underlying file

[Paolo Bonzini]

This is required when using blkdebug with raw format.  Unlike qcow2/QED,
raw asks blkdebug for the length of the file, it doesn't get it from
a header.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 1f79ef2..b084a23 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -429,6 +429,11 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkDebugEvent event)
     }
 }
 
+static int64_t blkdebug_getlength(BlockDriverState *bs)
+{
+    return bdrv_getlength(bs->file);
+}
+
 static BlockDriver bdrv_blkdebug = {
     .format_name        = "blkdebug",
     .protocol_name      = "blkdebug",
@@ -437,6 +442,7 @@ static BlockDriver bdrv_blkdebug = {
 
     .bdrv_file_open     = blkdebug_open,
     .bdrv_close         = blkdebug_close,
+    .bdrv_getlength     = blkdebug_getlength,
 
     .bdrv_aio_readv     = blkdebug_aio_readv,
     .bdrv_aio_writev    = blkdebug_aio_writev,
-- 
1.7.10.1

[Qemu-devel] [PATCH 4/6] blkdebug: store list of active rules

[Paolo Bonzini]

This prepares for the next patch, where some active rules may actually
not trigger depending on input to readv/writev.  Store the active rules
in a SIMPLEQ (so that it can be emptied easily with QSIMPLEQ_INIT), and
fetch the errno/once/immediately arguments from there.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c |   69 ++++++++++++++++++++++++------------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index b084a23..d12ebbf 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -26,24 +26,10 @@
 #include "block_int.h"
 #include "module.h"
 
-typedef struct BlkdebugVars {
-    int state;
-
-    /* If inject_errno != 0, an error is injected for requests */
-    int inject_errno;
-
-    /* Decides if all future requests fail (false) or only the next one and
-     * after the next request inject_errno is reset to 0 (true) */
-    bool inject_once;
-
-    /* Decides if aio_readv/writev fails right away (true) or returns an error
-     * return value only in the callback (false) */
-    bool inject_immediately;
-} BlkdebugVars;
-
 typedef struct BDRVBlkdebugState {
-    BlkdebugVars vars;
-    QLIST_HEAD(list, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
+    int state;
+    QLIST_HEAD(, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
+    QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
 } BDRVBlkdebugState;
 
 typedef struct BlkdebugAIOCB {
@@ -79,6 +65,7 @@ typedef struct BlkdebugRule {
         } set_state;
     } options;
     QLIST_ENTRY(BlkdebugRule) next;
+    QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
 } BlkdebugRule;
 
 static QemuOptsList inject_error_opts = {
@@ -300,7 +287,7 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
     filename = c + 1;
 
     /* Set initial state */
-    s->vars.state = 1;
+    s->state = 1;
 
     /* Open the backing file */
     ret = bdrv_file_open(&bs->file, filename, flags);
@@ -326,18 +313,18 @@ static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
 }
 
 static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
-    BlockDriverCompletionFunc *cb, void *opaque)
+    BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    int error = s->vars.inject_errno;
+    int error = rule->options.inject.error;
     struct BlkdebugAIOCB *acb;
     QEMUBH *bh;
 
-    if (s->vars.inject_once) {
-        s->vars.inject_errno = 0;
+    if (rule->options.inject.once) {
+        QSIMPLEQ_INIT(&s->active_rules);
     }
 
-    if (s->vars.inject_immediately) {
+    if (rule->options.inject.immediately) {
         return NULL;
     }
 
@@ -356,9 +343,10 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
     BlockDriverCompletionFunc *cb, void *opaque)
 {
     BDRVBlkdebugState *s = bs->opaque;
+    BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules);
 
-    if (s->vars.inject_errno) {
-        return inject_error(bs, cb, opaque);
+    if (rule && rule->options.inject.error) {
+        return inject_error(bs, cb, opaque, rule);
     }
 
     return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
@@ -369,9 +357,10 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
     BlockDriverCompletionFunc *cb, void *opaque)
 {
     BDRVBlkdebugState *s = bs->opaque;
+    BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules);
 
-    if (s->vars.inject_errno) {
-        return inject_error(bs, cb, opaque);
+    if (rule && rule->options.inject.error) {
+        return inject_error(bs, cb, opaque, rule);
     }
 
     return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
@@ -391,41 +380,45 @@ static void blkdebug_close(BlockDriverState *bs)
     }
 }
 
-static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
-    BlkdebugVars *old_vars)
+static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
+    int old_state, bool injected)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugVars *vars = &s->vars;
 
     /* Only process rules for the current state */
-    if (rule->state && rule->state != old_vars->state) {
-        return;
+    if (rule->state && rule->state != old_state) {
+        return injected;
     }
 
     /* Take the action */
     switch (rule->action) {
     case ACTION_INJECT_ERROR:
-        vars->inject_errno       = rule->options.inject.error;
-        vars->inject_once        = rule->options.inject.once;
-        vars->inject_immediately = rule->options.inject.immediately;
+        if (!injected) {
+            QSIMPLEQ_INIT(&s->active_rules);
+            injected = true;
+        }
+        QSIMPLEQ_INSERT_HEAD(&s->active_rules, rule, active_next);
         break;
 
     case ACTION_SET_STATE:
-        vars->state              = rule->options.set_state.new_state;
+        s->state = rule->options.set_state.new_state;
         break;
     }
+    return injected;
 }
 
 static void blkdebug_debug_event(BlockDriverState *bs, BlkDebugEvent event)
 {
     BDRVBlkdebugState *s = bs->opaque;
     struct BlkdebugRule *rule;
-    BlkdebugVars old_vars = s->vars;
+    int old_state = s->state;
+    bool injected;
 
     assert((int)event >= 0 && event < BLKDBG_EVENT_MAX);
 
+    injected = false;
     QLIST_FOREACH(rule, &s->rules[event], next) {
-        process_rule(bs, rule, &old_vars);
+        injected = process_rule(bs, rule, old_state, injected);
     }
 }
 
-- 
1.7.10.1

[Qemu-devel] [PATCH 5/6] blkdebug: optionally tie errors to a specific sector

[Paolo Bonzini]

This makes blkdebug scripts more powerful, and independent of the
exact sequence of operations performed by streaming.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c |   26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index d12ebbf..59dcea0 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -59,6 +59,7 @@ typedef struct BlkdebugRule {
             int error;
             int immediately;
             int once;
+            int64_t sector;
         } inject;
         struct {
             int new_state;
@@ -85,6 +86,10 @@ static QemuOptsList inject_error_opts = {
             .type = QEMU_OPT_NUMBER,
         },
         {
+            .name = "sector",
+            .type = QEMU_OPT_NUMBER,
+        },
+        {
             .name = "once",
             .type = QEMU_OPT_BOOL,
         },
@@ -213,6 +218,7 @@ static int add_rule(QemuOpts *opts, void *opaque)
         rule->options.inject.once  = qemu_opt_get_bool(opts, "once", 0);
         rule->options.inject.immediately =
             qemu_opt_get_bool(opts, "immediately", 0);
+        rule->options.inject.sector = qemu_opt_get_number(opts, "sector", -1);
         break;
 
     case ACTION_SET_STATE:
@@ -343,7 +349,15 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
     BlockDriverCompletionFunc *cb, void *opaque)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules);
+    BlkdebugRule *rule = NULL;
+
+    QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
+        if (rule->options.inject.sector == -1 ||
+            (rule->options.inject.sector >= sector_num &&
+             rule->options.inject.sector < sector_num + nb_sectors)) {
+            break;
+        }
+    }
 
     if (rule && rule->options.inject.error) {
         return inject_error(bs, cb, opaque, rule);
@@ -357,7 +371,15 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
     BlockDriverCompletionFunc *cb, void *opaque)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules);
+    BlkdebugRule *rule = NULL;
+
+    QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
+        if (rule->options.inject.sector == -1 ||
+            (rule->options.inject.sector >= sector_num &&
+             rule->options.inject.sector < sector_num + nb_sectors)) {
+            break;
+        }
+    }
 
     if (rule && rule->options.inject.error) {
         return inject_error(bs, cb, opaque, rule);
-- 
1.7.10.1

[Qemu-devel] [PATCH 6/6] raw: hook into blkdebug

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/raw.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/raw.c b/block/raw.c
index 09d9b48..ff34ea4 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -12,12 +12,14 @@ static int raw_open(BlockDriverState *bs, int flags)
 static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
                                      int nb_sectors, QEMUIOVector *qiov)
 {
+    BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
     return bdrv_co_readv(bs->file, sector_num, nb_sectors, qiov);
 }
 
 static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
                                       int nb_sectors, QEMUIOVector *qiov)
 {
+    BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
     return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
 }
 
-- 
1.7.10.1

Re: [Qemu-devel] [PATCH 1/6] blkdebug: remove sync i/o events

[Stefan Hajnoczi]

On Wed, Jun 6, 2012 at 7:10 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> These are unused, except (by mistake more or less) in QED.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block.h          |    2 --
>  block/blkdebug.c |    2 --
>  block/qed.c      |    2 +-
>  3 files changed, 1 insertion(+), 5 deletions(-)

Acked-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH 5/6] blkdebug: optionally tie errors to a specific sector

[Stefan Hajnoczi]

On Wed, Jun 6, 2012 at 7:10 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This makes blkdebug scripts more powerful, and independent of the
> exact sequence of operations performed by streaming.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/blkdebug.c |   26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)

Seems useful.

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH 0/6] blkdebug changes extracted from 1.2 streaming patches

[Paolo Bonzini]

Il 06/06/2012 08:10, Paolo Bonzini ha scritto:
> Hi,
> 
> these patches are prerequisites for testing streaming error handling.
> They should be useful on their own, so I am sending them early to keep
> the queue small.
> 
> Paolo Bonzini (6):
>   blkdebug: remove sync i/o events
>   blkdebug: tiny cleanup
>   blkdebug: pass getlength to underlying file
>   blkdebug: store list of active rules
>   blkdebug: optionally tie errors to a specific sector
>   raw: hook into blkdebug
> 
>  block.h          |    2 -
>  block/blkdebug.c |  107 +++++++++++++++++++++++++++++++-----------------------
>  block/qed.c      |    2 +-
>  block/raw.c      |    2 +
>  4 files changed, 64 insertions(+), 49 deletions(-)
> 

Ping?

Paolo

Re: [Qemu-devel] [PATCH 4/6] blkdebug: store list of active rules

[Kevin Wolf]

Am 06.06.2012 08:10, schrieb Paolo Bonzini:
> This prepares for the next patch, where some active rules may actually
> not trigger depending on input to readv/writev.  Store the active rules
> in a SIMPLEQ (so that it can be emptied easily with QSIMPLEQ_INIT), and
> fetch the errno/once/immediately arguments from there.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/blkdebug.c |   69 ++++++++++++++++++++++++------------------------------
>  1 file changed, 31 insertions(+), 38 deletions(-)
> 
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index b084a23..d12ebbf 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -26,24 +26,10 @@
>  #include "block_int.h"
>  #include "module.h"
>  
> -typedef struct BlkdebugVars {
> -    int state;
> -
> -    /* If inject_errno != 0, an error is injected for requests */
> -    int inject_errno;
> -
> -    /* Decides if all future requests fail (false) or only the next one and
> -     * after the next request inject_errno is reset to 0 (true) */
> -    bool inject_once;
> -
> -    /* Decides if aio_readv/writev fails right away (true) or returns an error
> -     * return value only in the callback (false) */
> -    bool inject_immediately;
> -} BlkdebugVars;
> -
>  typedef struct BDRVBlkdebugState {
> -    BlkdebugVars vars;
> -    QLIST_HEAD(list, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
> +    int state;
> +    QLIST_HEAD(, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
> +    QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
>  } BDRVBlkdebugState;
>  
>  typedef struct BlkdebugAIOCB {
> @@ -79,6 +65,7 @@ typedef struct BlkdebugRule {
>          } set_state;
>      } options;
>      QLIST_ENTRY(BlkdebugRule) next;
> +    QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
>  } BlkdebugRule;
>  
>  static QemuOptsList inject_error_opts = {
> @@ -300,7 +287,7 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
>      filename = c + 1;
>  
>      /* Set initial state */
> -    s->vars.state = 1;
> +    s->state = 1;
>  
>      /* Open the backing file */
>      ret = bdrv_file_open(&bs->file, filename, flags);
> @@ -326,18 +313,18 @@ static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
>  }
>  
>  static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
> -    BlockDriverCompletionFunc *cb, void *opaque)
> +    BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule)
>  {
>      BDRVBlkdebugState *s = bs->opaque;
> -    int error = s->vars.inject_errno;
> +    int error = rule->options.inject.error;
>      struct BlkdebugAIOCB *acb;
>      QEMUBH *bh;
>  
> -    if (s->vars.inject_once) {
> -        s->vars.inject_errno = 0;
> +    if (rule->options.inject.once) {
> +        QSIMPLEQ_INIT(&s->active_rules);
>      }
>  
> -    if (s->vars.inject_immediately) {
> +    if (rule->options.inject.immediately) {
>          return NULL;
>      }
>  
> @@ -356,9 +343,10 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
>      BlockDriverCompletionFunc *cb, void *opaque)
>  {
>      BDRVBlkdebugState *s = bs->opaque;
> +    BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules);
>  
> -    if (s->vars.inject_errno) {
> -        return inject_error(bs, cb, opaque);
> +    if (rule && rule->options.inject.error) {
> +        return inject_error(bs, cb, opaque, rule);
>      }
>  
>      return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
> @@ -369,9 +357,10 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
>      BlockDriverCompletionFunc *cb, void *opaque)
>  {
>      BDRVBlkdebugState *s = bs->opaque;
> +    BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules);
>  
> -    if (s->vars.inject_errno) {
> -        return inject_error(bs, cb, opaque);
> +    if (rule && rule->options.inject.error) {
> +        return inject_error(bs, cb, opaque, rule);
>      }
>  
>      return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
> @@ -391,41 +380,45 @@ static void blkdebug_close(BlockDriverState *bs)
>      }
>  }
>  
> -static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
> -    BlkdebugVars *old_vars)
> +static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
> +    int old_state, bool injected)
>  {
>      BDRVBlkdebugState *s = bs->opaque;
> -    BlkdebugVars *vars = &s->vars;
>  
>      /* Only process rules for the current state */
> -    if (rule->state && rule->state != old_vars->state) {
> -        return;
> +    if (rule->state && rule->state != old_state) {
> +        return injected;
>      }
>  
>      /* Take the action */
>      switch (rule->action) {
>      case ACTION_INJECT_ERROR:
> -        vars->inject_errno       = rule->options.inject.error;
> -        vars->inject_once        = rule->options.inject.once;
> -        vars->inject_immediately = rule->options.inject.immediately;
> +        if (!injected) {
> +            QSIMPLEQ_INIT(&s->active_rules);
> +            injected = true;
> +        }

Does this mean that the next event will invalidate the active rules?
Currently it only does so if a rule for the new event exists. Not sure
if it makes a difference in practice, probably not, but worth mentioning
the commit log.

Kevin

Re: [Qemu-devel] [PATCH 4/6] blkdebug: store list of active rules

[Paolo Bonzini]

Il 03/07/2012 17:40, Kevin Wolf ha scritto:
>> >      case ACTION_INJECT_ERROR:
>> > -        vars->inject_errno       = rule->options.inject.error;
>> > -        vars->inject_once        = rule->options.inject.once;
>> > -        vars->inject_immediately = rule->options.inject.immediately;
>> > +        if (!injected) {
>> > +            QSIMPLEQ_INIT(&s->active_rules);
>> > +            injected = true;
>> > +        }
> Does this mean that the next event will invalidate the active rules?
> Currently it only does so if a rule for the new event exists.

No, the point of having the injected bool is exactly to keep the
currently active rules unless a rule exists for the new event.  We
cannot just do the QSIMPLEQ_INIT in blkdebug_debug_event, that would be
the behavior you describe.

Paolo

Re: [Qemu-devel] [PATCH 4/6] blkdebug: store list of active rules

[Kevin Wolf]

Am 03.07.2012 18:03, schrieb Paolo Bonzini:
> Il 03/07/2012 17:40, Kevin Wolf ha scritto:
>>>>      case ACTION_INJECT_ERROR:
>>>> -        vars->inject_errno       = rule->options.inject.error;
>>>> -        vars->inject_once        = rule->options.inject.once;
>>>> -        vars->inject_immediately = rule->options.inject.immediately;
>>>> +        if (!injected) {
>>>> +            QSIMPLEQ_INIT(&s->active_rules);
>>>> +            injected = true;
>>>> +        }
>> Does this mean that the next event will invalidate the active rules?
>> Currently it only does so if a rule for the new event exists.
> 
> No, the point of having the injected bool is exactly to keep the
> currently active rules unless a rule exists for the new event.  We
> cannot just do the QSIMPLEQ_INIT in blkdebug_debug_event, that would be
> the behavior you describe.

Ah, yes. Maybe I should learn reading sometime.

Kevin

Re: [Qemu-devel] [PATCH 0/6] blkdebug changes extracted from 1.2 streaming patches

[Kevin Wolf]

Am 06.06.2012 08:10, schrieb Paolo Bonzini:
> Hi,
> 
> these patches are prerequisites for testing streaming error handling.
> They should be useful on their own, so I am sending them early to keep
> the queue small.
> 
> Paolo Bonzini (6):
>   blkdebug: remove sync i/o events
>   blkdebug: tiny cleanup
>   blkdebug: pass getlength to underlying file
>   blkdebug: store list of active rules
>   blkdebug: optionally tie errors to a specific sector
>   raw: hook into blkdebug
> 
>  block.h          |    2 -
>  block/blkdebug.c |  107 +++++++++++++++++++++++++++++++-----------------------
>  block/qed.c      |    2 +-
>  block/raw.c      |    2 +
>  4 files changed, 64 insertions(+), 49 deletions(-)

Thanks, applied all to the block branch.

Kevin