[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC] blkdebug: make the fault injection function
From: |
Hitoshi Mitake |
Subject: |
Re: [Qemu-devel] [PATCH RFC] blkdebug: make the fault injection functionality callable from QMP |
Date: |
Thu, 14 Aug 2014 16:13:00 +0900 |
User-agent: |
Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (Goj$(D+W(B) APEL/10.8 Emacs/23.4 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) |
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