From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Nw9ui-0003gA-SN
	for qemu-devel@nongnu.org; Mon, 29 Mar 2010 04:01:37 -0400
Received: from [140.186.70.92] (port=41048 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Nw9ue-0003eI-3U
	for qemu-devel@nongnu.org; Mon, 29 Mar 2010 04:01:34 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <kwolf@redhat.com>) id 1Nw9uc-0005Zp-Lh
	for qemu-devel@nongnu.org; Mon, 29 Mar 2010 04:01:31 -0400
Received: from mx1.redhat.com ([209.132.183.28]:42628)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <kwolf@redhat.com>) id 1Nw9uc-0005ZD-Ea
	for qemu-devel@nongnu.org; Mon, 29 Mar 2010 04:01:30 -0400
Message-ID: <4BB05E2E.5000108@redhat.com>
Date: Mon, 29 Mar 2010 10:00:46 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [RFC][PATCH 6/7] blkdebug: Add events and rules
References: <1268672915-12233-1-git-send-email-kwolf@redhat.com>
	<1268672915-12233-7-git-send-email-kwolf@redhat.com>
	<20100328131210.GA13453@lst.de>
In-Reply-To: <20100328131210.GA13453@lst.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Christoph Hellwig <hch@lst.de>
Cc: qemu-devel@nongnu.org

Am 28.03.2010 15:12, schrieb Christoph Hellwig:
> On Mon, Mar 15, 2010 at 06:08:34PM +0100, Kevin Wolf wrote:
>> +    fprintf(stderr, "bdrv_debug_event: %d\n", event);
> 
> Is this supposed to be in the final version or a leftover debugging aid?

It's not there in the final version (which I have already sent out, btw,
so you have reviewed an old version)

>> +#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
> 
> Why not call bdrv_debug_event directly?

Originally I had intended to add a flag to ./configure to enable
blkdebug and #ifdef this out if it's not compiled in. Maybe we still
want to add that, but then it's not really much that you save.

>> +    config = strdup(filename);
>> +    config[c - filename] = '\0';
>> +    ret = read_config(s, config);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    filename = c + 1;
>> +
>> +    /* Open the backing file */
>> +    ret = bdrv_file_open(&s->hd, filename, flags);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    return 0;
> 
> Don't we need to free config somewhere?

Oops, this one is still there in the final version. I'll send another
version of the series.

Kevin