Re: [Qemu-devel] [PATCH RFC] blkdebug: make the fault injection functionality callable from QMP

[Hitoshi Mitake]

At Wed, 13 Aug 2014 15:33:38 +0100,
Stefan Hajnoczi wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> On Wed, Aug 13, 2014 at 01:48:41PM +0900, Hitoshi Mitake wrote:
> > @@ -485,6 +493,10 @@ static BlockDriverAIOCB 
> > *blkdebug_aio_readv(BlockDriverState *bs,
> >      BDRVBlkdebugState *s = bs->opaque;
> >      BlkdebugRule *rule = NULL;
> >  
> > +    if (s->qmp_fault_state != BLOCK_FAULT_STATE_NONE) {
> > +        return inject_error(bs, cb, opaque, rule);
> > +    }
> > +
> >      QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
> >          if (rule->options.inject.sector == -1 ||
> >              (rule->options.inject.sector >= sector_num &&

Hi Stefan, thanks a lot for your review.

> 
> A QMP command is a good idea but the hardcoded s->qmp_fault_state error
> injection is much less powerful than the rules that blkdebug already
> supports.
> 
> Can you make the QMP command take a list of rules instead of setting
> s->qmp_fault_state?
> 
> For example:
> 
>   # Clear all rules
>   blkdebug-set-rules drive0 []
> 
>   # Return EIO on disk flush after L1 update
>   blkdebug-set-rules drive0 [
>     {"type": "set-state",
>      "event": "BLKDBG_L1_UPDATE",
>      "state": 0,
>      "new-state" 1}
>     {"type": "inject-error",
>      "state": 1,
>      "event": "BLKDBG_FLUSH_TO_DISK",
>      "errno": "EIO",
>      "once": true},
>   ]
> 
> In other words, instead of adding a new (more limited) mechanism for
> triggering error injection, please make the QMP command install a list
> of blkdebug rules.

It seems a better idea than current implementation. It will be more
powerful and easy to maintain. I'll implement the above scheme in v2.

Thanks,
Hitoshi