From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40566) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnsij-0005Aw-TT for qemu-devel@nongnu.org; Mon, 01 Aug 2011 09:39:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qnsii-0003c3-Qa for qemu-devel@nongnu.org; Mon, 01 Aug 2011 09:39:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qnsii-0003bv-DQ for qemu-devel@nongnu.org; Mon, 01 Aug 2011 09:39:48 -0400 Message-ID: <4E36AD51.8020605@redhat.com> Date: Mon, 01 Aug 2011 15:42:41 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311914994-20482-1-git-send-email-devin122@gmail.com> <1311914994-20482-4-git-send-email-devin122@gmail.com> In-Reply-To: <1311914994-20482-4-git-send-email-devin122@gmail.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 03/24] block: add bdrv_open_conversion_target() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Devin Nakamura Cc: qemu-devel@nongnu.org Am 29.07.2011 06:49, schrieb Devin Nakamura: > Conflicts: > > block.h You can probably remove this note. ;-) > > Signed-off-by: Devin Nakamura > --- > block.c | 32 ++++++++++++++++++++++++++++++++ > block.h | 4 ++++ > 2 files changed, 36 insertions(+), 0 deletions(-) > > diff --git a/block.c b/block.c > index 4503b7b..9530577 100644 > --- a/block.c > +++ b/block.c > @@ -3038,6 +3038,38 @@ out: > return ret; > } > > +int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file, > + BlockConversionOptions *drv_options, > + QEMUOptionParameter *usr_options, > + const char *target_fmt) > +{ > + BlockDriver *drv; > + BlockDriverState *bss; > + > + drv = bdrv_find_format(target_fmt); > + if (!drv) { > + return -ENOENT; > + } > + > + if (!drv->bdrv_open_conversion_target) { > + return -ENOTSUP; > + } > + > + *bs = bdrv_new(""); > + bss = *bs; > + bss->file = file; > + bss->total_sectors = drv_options->image_size >> BDRV_SECTOR_BITS; > + bss->encrypted = 0; > + bss->valid_key = 0; > + bss->open_flags = 0; > + /* buffer_alignment defaulted to 512, drivers can change this value */ > + bss->buffer_alignment = 512; > + bss->opaque = qemu_mallocz(drv->instance_size); > + bss->drv = drv; > + return drv->bdrv_open_conversion_target(bss, drv_options, usr_options); > + > +} How big are the differences really to bdrv_open_common? Have you checked if you could reuse it? It looks to me as if you're just handling fewer cases and could achieve the same by passing the right flags to bdrv_open_common. The only real difference is drv->bdrv_open vs. drv->bdrv_open_conversion_target, which could probably be handled by another flag. The problem with keeping it separate is that it makes it easy to change bdrv_open without changing bdrv_open_conversion_target. Most people touching the code won't use in-place conversion very often, so they won't notice any breakage in their testing. Kevin