From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43005) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDNNS-0005V3-O0 for qemu-devel@nongnu.org; Tue, 11 Feb 2014 19:08:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WDNNM-00068P-Oi for qemu-devel@nongnu.org; Tue, 11 Feb 2014 19:08:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:12295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDNNM-00068F-Gg for qemu-devel@nongnu.org; Tue, 11 Feb 2014 19:08:28 -0500 Message-ID: <52FABC07.1040108@redhat.com> Date: Wed, 12 Feb 2014 01:10:47 +0100 From: Max Reitz MIME-Version: 1.0 References: <1391881159-13585-1-git-send-email-mreitz@redhat.com> <1391881159-13585-2-git-send-email-mreitz@redhat.com> <20140210124250.GC2832@dhcp-200-207.str.redhat.com> In-Reply-To: <20140210124250.GC2832@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/8] block: Change BDS parameter of bdrv_open() to ** List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Jeff Cody , qemu-devel@nongnu.org, Stefan Hajnoczi , =?ISO-8859-1?Q?Beno=EEt_Canet?= On 10.02.2014 13:42, Kevin Wolf wrote: > Am 08.02.2014 um 18:39 hat Max Reitz geschrieben: >> Make bdrv_open() take a pointer to a BDS pointer, similarly to >> bdrv_file_open(). If a pointer to a NULL pointer is given, bdrv_open() >> will create a new BDS with an empty name; if the BDS pointer is not >> NULL, that existing BDS will be reused (in the same way as bdrv_open() >> already did). >> >> Signed-off-by: Max Reitz >> --- >> block.c | 64 +++++++++++++++++++++++++++++++-------------------- >> block/blkdebug.c | 1 + >> block/blkverify.c | 2 ++ >> block/qcow2.c | 14 +++++++---- >> block/vmdk.c | 5 ++-- >> block/vvfat.c | 6 ++--- >> blockdev.c | 20 ++++++++-------- >> hw/block/xen_disk.c | 2 +- >> include/block/block.h | 2 +- >> qemu-img.c | 10 ++++---- >> qemu-io.c | 2 +- >> qemu-nbd.c | 2 +- >> 12 files changed, 72 insertions(+), 58 deletions(-) >> @@ -1160,6 +1158,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) >> * BlockdevRef. >> * >> * The BlockdevRef will be removed from the options QDict. >> + * >> + * As with bdrv_open(), if *pbs is NULL, a new BDS will be created with a >> + * pointer to it stored there. If it is not NULL, the referenced BDS will >> + * be reused. >> */ >> int bdrv_open_image(BlockDriverState **pbs, const char *filename, >> QDict *options, const char *bdref_key, int flags, > There are no callers that make use of *pbs != NULL. Are you planning to > add such users? Otherwise, we could just assert() it here instead of > documenting behaviour that is never used. No, currently, there aren't. Since we're planning to eventually adjust everything to give a pointer to a NULL pointer to these functions (i.e., nobody except bdrv_open() uses bdrv_new()), adding an assert() in order to prevent anyone from "exploiting" this in a way which would technically be fine but actually not what we want (i.e., reusing an already existing BDS), is fine with me. I'll add it and adjust the comment. Max