From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38815) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJqF9-00058d-LF for qemu-devel@nongnu.org; Fri, 06 Feb 2015 16:15:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJqF4-0006pH-Nt for qemu-devel@nongnu.org; Fri, 06 Feb 2015 16:15:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJqF4-0006pB-Ge for qemu-devel@nongnu.org; Fri, 06 Feb 2015 16:15:10 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t16LFAem000524 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 6 Feb 2015 16:15:10 -0500 Message-ID: <54D52EDB.2030708@redhat.com> Date: Fri, 06 Feb 2015 16:15:07 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423246984-26207-1-git-send-email-jsnow@redhat.com> In-Reply-To: <1423246984-26207-1-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] blkdebug: fix "once" rule List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com On 2015-02-06 at 13:23, John Snow wrote: > Background: > The blkdebug scripts are currently engineered so that when a debug > event occurs, a prefilter browses a master list of parsed rules for a > certain event and adds them to an "active list" of rules to be used for > the forthcoming action, provided the events and state numbers match. > > Then, once the request is received, the last active rule is used to > inject an error if certain parameters match. > > This active list is cleared every time the prefilter injects a new > rule for the first time during a debug event. > > The "once" rule currently causes the error injection, if it is > triggered, to only clear the active list. This is insufficient for > preventing future injections of the same rule. > > Remedy: > This patch /deletes/ the rule from the list that the prefilter > browses, so it is gone for good. In V2, we remove only the rule of > interest from the active list instead of allowing the "once" rule to > clear the entire list of active rules. > > Impact: > This affects iotests 026. Several ENOSPC tests that used "once" can > be seen to have output that shows multiple failure messages. After > this patch, the error messages tend to be smaller and less severe, but > the injection can still be seen to be working. I have patched the > expected output to expect the smaller error messages. > > V2: > - Remove only the offending "once" rule from the active list instead > of clearing the entire active list. Could you move this version information under the three dashes so it won't be included in the commit message? > Signed-off-by: John Snow > --- Here, that is. > block/blkdebug.c | 9 +++++---- > tests/qemu-iotests/026.out | 24 ++++-------------------- > 2 files changed, 9 insertions(+), 24 deletions(-) > > diff --git a/block/blkdebug.c b/block/blkdebug.c > index 9ce35cd..185695b 100644 > --- a/block/blkdebug.c > +++ b/block/blkdebug.c > @@ -473,14 +473,15 @@ static BlockAIOCB *inject_error(BlockDriverState *bs, > struct BlkdebugAIOCB *acb; > QEMUBH *bh; > > - if (rule->options.inject.once) { > - QSIMPLEQ_INIT(&s->active_rules); > - } > - > if (rule->options.inject.immediately) { > return NULL; > } > > + if (rule->options.inject.once) { > + QSIMPLEQ_REMOVE(&s->active_rules, rule, BlkdebugRule, active_next); > + remove_rule(rule); > + } > + > acb = qemu_aio_get(&blkdebug_aiocb_info, bs, cb, opaque); > acb->ret = -error; > The new code itself looks good, but why did you move it after the immediately block? Because now, if "immediately" is set, "once" will be ignored: $ ./qemu-img create -f qcow2 test.qcow2 64M; ./qemu-io -c 'aio_write 0 64k' "json:{'driver':'qcow2','file':{'driver':'blkdebug','image':{'driver':'file','filename':'test.qcow2'},'inject-error':[{'event':'write_aio','once':true,'immediately':true}]}}" Formatting 'test.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off aio_write failed: Input/output error Failed to flush the L2 table cache: Input/output error Failed to flush the refcount block cache: Input/output error (I know, I know, I should have noticed in v1...) Max