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
