Re: [Qemu-devel] [PATCH v4 2/4] blkdebug: add "remove_break" command

[Max Reitz]

On 14.11.2013 10:10, Fam Zheng wrote:
> This adds "remove_break" command which is the reverse of blkdebug
> command "break": it removes all breakpoints with given tag and resumes
> all the requests.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block.c                   | 13 +++++++++++++
>  block/blkdebug.c          | 26 ++++++++++++++++++++++++++
>  include/block/block.h     |  1 +
>  include/block/block_int.h |  2 ++
>  qemu-io-cmds.c            | 22 ++++++++++++++++++++++
>  5 files changed, 64 insertions(+)
>
> diff --git a/block.c b/block.c
> index 58efb5b..6bcf945 100644
> --- a/block.c
> +++ b/block.c
> @@ -3412,6 +3412,19 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const 
> char *event,
>      return -ENOTSUP;
>  }
>  
> +int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)
> +{
> +    while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) {
> +        bs = bs->file;
> +    }
> +
> +    if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
> +        return bs->drv->bdrv_debug_remove_breakpoint(bs, tag);
> +    }
> +
> +    return -ENOTSUP;
> +}
> +
>  int bdrv_debug_resume(BlockDriverState *bs, const char *tag)
>  {
>      while (bs && bs->drv && !bs->drv->bdrv_debug_resume) {
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 16d2b91..23eceea 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -605,6 +605,30 @@ static int blkdebug_debug_resume(BlockDriverState *bs, 
> const char *tag)
>      return -ENOENT;
>  }
>  
> +static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
> +                                            const char *tag)
> +{
> +    BDRVBlkdebugState *s = bs->opaque;
> +    BlkdebugSuspendedReq *r;
> +    BlkdebugRule *rule, *next;
> +    int i;
> +
> +    for (i = 0; i < BLKDBG_EVENT_MAX; i++) {
> +        QLIST_FOREACH_SAFE(rule, &s->rules[i], next, next) {
> +            if (rule->action == ACTION_SUSPEND &&
> +                !strcmp(rule->options.suspend.tag, tag)) {
> +                remove_rule(rule);
> +            }
> +        }
> +    }
> +    QLIST_FOREACH(r, &s->suspended_reqs, next) {
> +        if (!strcmp(r->tag, tag)) {
> +            qemu_coroutine_enter(r->co, NULL);
> +            return 0;

As far as I can see, this does not resume all the request but only the
first one found, or does it?

Max

> +        }
> +    }
> +    return -ENOENT;
> +}
>  
>  static bool blkdebug_debug_is_suspended(BlockDriverState *bs, const char 
> *tag)
>  {
> @@ -639,6 +663,8 @@ static BlockDriver bdrv_blkdebug = {
>  
>      .bdrv_debug_event           = blkdebug_debug_event,
>      .bdrv_debug_breakpoint      = blkdebug_debug_breakpoint,
> +    .bdrv_debug_remove_breakpoint
> +                                = blkdebug_debug_remove_breakpoint,
>      .bdrv_debug_resume          = blkdebug_debug_resume,
>      .bdrv_debug_is_suspended    = blkdebug_debug_is_suspended,
>  };
> diff --git a/include/block/block.h b/include/block/block.h
> index 3560deb..6b17495 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -484,6 +484,7 @@ void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent 
> event);
>  
>  int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
>                             const char *tag);
> +int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag);
>  int bdrv_debug_resume(BlockDriverState *bs, const char *tag);
>  bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag);
>  
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 1666066..659dcf4 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -218,6 +218,8 @@ struct BlockDriver {
>      /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
>      int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event,
>          const char *tag);
> +    int (*bdrv_debug_remove_breakpoint)(BlockDriverState *bs,
> +        const char *tag);
>      int (*bdrv_debug_resume)(BlockDriverState *bs, const char *tag);
>      bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
>  
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 667f4e4..4cab57e 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1956,6 +1956,18 @@ static int break_f(BlockDriverState *bs, int argc, 
> char **argv)
>      return 0;
>  }
>  
> +static int remove_break_f(BlockDriverState *bs, int argc, char **argv)
> +{
> +    int ret;
> +
> +    ret = bdrv_debug_remove_breakpoint(bs, argv[1]);
> +    if (ret < 0) {
> +        printf("Could not remove breakpoint %s: %s\n", argv[1], 
> strerror(-ret));
> +    }
> +
> +    return 0;
> +}
> +
>  static const cmdinfo_t break_cmd = {
>         .name           = "break",
>         .argmin         = 2,
> @@ -1966,6 +1978,15 @@ static const cmdinfo_t break_cmd = {
>                           "request as tag",
>  };
>  
> +static const cmdinfo_t remove_break_cmd = {
> +       .name           = "remove_break",
> +       .argmin         = 1,
> +       .argmax         = 1,
> +       .cfunc          = remove_break_f,
> +       .args           = "tag",
> +       .oneline        = "remove a breakpoint by tag",
> +};
> +
>  static int resume_f(BlockDriverState *bs, int argc, char **argv)
>  {
>      int ret;
> @@ -2126,6 +2147,7 @@ static void __attribute((constructor)) 
> init_qemuio_commands(void)
>      qemuio_add_command(&alloc_cmd);
>      qemuio_add_command(&map_cmd);
>      qemuio_add_command(&break_cmd);
> +    qemuio_add_command(&remove_break_cmd);
>      qemuio_add_command(&resume_cmd);
>      qemuio_add_command(&wait_break_cmd);
>      qemuio_add_command(&abort_cmd);