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