From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sm5Dt-0002QH-RF for qemu-devel@nongnu.org; Tue, 03 Jul 2012 11:41:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sm5Dl-00026G-TG for qemu-devel@nongnu.org; Tue, 03 Jul 2012 11:41:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sm5Dl-000261-L4 for qemu-devel@nongnu.org; Tue, 03 Jul 2012 11:40:57 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q63FeuJ2012258 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 3 Jul 2012 11:40:56 -0400 Message-ID: <4FF31286.50000@redhat.com> Date: Tue, 03 Jul 2012 17:40:54 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1338963044-21445-1-git-send-email-pbonzini@redhat.com> <1338963044-21445-5-git-send-email-pbonzini@redhat.com> In-Reply-To: <1338963044-21445-5-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/6] blkdebug: store list of active rules List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org Am 06.06.2012 08:10, schrieb Paolo Bonzini: > This prepares for the next patch, where some active rules may actually > not trigger depending on input to readv/writev. Store the active rules > in a SIMPLEQ (so that it can be emptied easily with QSIMPLEQ_INIT), and > fetch the errno/once/immediately arguments from there. > > Signed-off-by: Paolo Bonzini > --- > block/blkdebug.c | 69 ++++++++++++++++++++++++------------------------------ > 1 file changed, 31 insertions(+), 38 deletions(-) > > diff --git a/block/blkdebug.c b/block/blkdebug.c > index b084a23..d12ebbf 100644 > --- a/block/blkdebug.c > +++ b/block/blkdebug.c > @@ -26,24 +26,10 @@ > #include "block_int.h" > #include "module.h" > > -typedef struct BlkdebugVars { > - int state; > - > - /* If inject_errno != 0, an error is injected for requests */ > - int inject_errno; > - > - /* Decides if all future requests fail (false) or only the next one and > - * after the next request inject_errno is reset to 0 (true) */ > - bool inject_once; > - > - /* Decides if aio_readv/writev fails right away (true) or returns an error > - * return value only in the callback (false) */ > - bool inject_immediately; > -} BlkdebugVars; > - > typedef struct BDRVBlkdebugState { > - BlkdebugVars vars; > - QLIST_HEAD(list, BlkdebugRule) rules[BLKDBG_EVENT_MAX]; > + int state; > + QLIST_HEAD(, BlkdebugRule) rules[BLKDBG_EVENT_MAX]; > + QSIMPLEQ_HEAD(, BlkdebugRule) active_rules; > } BDRVBlkdebugState; > > typedef struct BlkdebugAIOCB { > @@ -79,6 +65,7 @@ typedef struct BlkdebugRule { > } set_state; > } options; > QLIST_ENTRY(BlkdebugRule) next; > + QSIMPLEQ_ENTRY(BlkdebugRule) active_next; > } BlkdebugRule; > > static QemuOptsList inject_error_opts = { > @@ -300,7 +287,7 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) > filename = c + 1; > > /* Set initial state */ > - s->vars.state = 1; > + s->state = 1; > > /* Open the backing file */ > ret = bdrv_file_open(&bs->file, filename, flags); > @@ -326,18 +313,18 @@ static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb) > } > > static BlockDriverAIOCB *inject_error(BlockDriverState *bs, > - BlockDriverCompletionFunc *cb, void *opaque) > + BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule) > { > BDRVBlkdebugState *s = bs->opaque; > - int error = s->vars.inject_errno; > + int error = rule->options.inject.error; > struct BlkdebugAIOCB *acb; > QEMUBH *bh; > > - if (s->vars.inject_once) { > - s->vars.inject_errno = 0; > + if (rule->options.inject.once) { > + QSIMPLEQ_INIT(&s->active_rules); > } > > - if (s->vars.inject_immediately) { > + if (rule->options.inject.immediately) { > return NULL; > } > > @@ -356,9 +343,10 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs, > BlockDriverCompletionFunc *cb, void *opaque) > { > BDRVBlkdebugState *s = bs->opaque; > + BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules); > > - if (s->vars.inject_errno) { > - return inject_error(bs, cb, opaque); > + if (rule && rule->options.inject.error) { > + return inject_error(bs, cb, opaque, rule); > } > > return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque); > @@ -369,9 +357,10 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, > BlockDriverCompletionFunc *cb, void *opaque) > { > BDRVBlkdebugState *s = bs->opaque; > + BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules); > > - if (s->vars.inject_errno) { > - return inject_error(bs, cb, opaque); > + if (rule && rule->options.inject.error) { > + return inject_error(bs, cb, opaque, rule); > } > > return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque); > @@ -391,41 +380,45 @@ static void blkdebug_close(BlockDriverState *bs) > } > } > > -static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, > - BlkdebugVars *old_vars) > +static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, > + int old_state, bool injected) > { > BDRVBlkdebugState *s = bs->opaque; > - BlkdebugVars *vars = &s->vars; > > /* Only process rules for the current state */ > - if (rule->state && rule->state != old_vars->state) { > - return; > + if (rule->state && rule->state != old_state) { > + return injected; > } > > /* Take the action */ > switch (rule->action) { > case ACTION_INJECT_ERROR: > - vars->inject_errno = rule->options.inject.error; > - vars->inject_once = rule->options.inject.once; > - vars->inject_immediately = rule->options.inject.immediately; > + if (!injected) { > + QSIMPLEQ_INIT(&s->active_rules); > + injected = true; > + } Does this mean that the next event will invalidate the active rules? Currently it only does so if a rule for the new event exists. Not sure if it makes a difference in practice, probably not, but worth mentioning the commit log. Kevin