From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LdjkQ-0001Mq-7U for qemu-devel@nongnu.org; Sun, 01 Mar 2009 06:22:18 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LdjkO-0001LD-1O for qemu-devel@nongnu.org; Sun, 01 Mar 2009 06:22:16 -0500 Received: from [199.232.76.173] (port=35814 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LdjkN-0001Ks-IW for qemu-devel@nongnu.org; Sun, 01 Mar 2009 06:22:15 -0500 Received: from mx2.redhat.com ([66.187.237.31]:42049) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LdjkN-0007AB-4e for qemu-devel@nongnu.org; Sun, 01 Mar 2009 06:22:15 -0500 Message-ID: <49AA6FDF.20603@redhat.com> Date: Sun, 01 Mar 2009 13:22:07 +0200 From: Uri Lublin MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] Introducing qcow2 extensions + keep backing file format (v4) References: <1234979501-9454-1-git-send-email-uril@redhat.com> <1234979501-9454-2-git-send-email-uril@redhat.com> <49A6E409.4030607@us.ibm.com> In-Reply-To: <49A6E409.4030607@us.ibm.com> Content-Type: multipart/mixed; boundary="------------080300070306050808010202" Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------080300070306050808010202 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Anthony Liguori wrote: > Uri Lublin wrote: >> Qcow2 extensions are build of magic (id) len (in bytes) and data. >> They reside between the end of the header and the filename. >> >> We can keep the backing file format in a such a qcow2 extension, to >> 1. Provide a way to know the backing file format without probing >> it (setting the format at creation time). >> 2. Enable using qcow2 format over host block devices. >> (only if the user specifically asks for it, by providing the format >> at creation time). >> >> I've added bdrv_create2 and drv->bdrv_create2 (implemented only >> by block-qcow2 currently) to pass the backing-format to create. >> >> Based on a work done by Shahar Frank. >> >> Also fixes a security flaw found by Daniel P. Berrange on [1] >> which summarizes: "Autoprobing: just say no." >> >> [1] http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html >> >> Signed-off-by: Uri Lublin >> > > This made it through my regression testing but... > >> int bdrv_create(BlockDriver *drv, >> const char *filename, int64_t size_in_sectors, >> const char *backing_file, int flags) >> @@ -348,6 +362,9 @@ int bdrv_open2(BlockDriverState *bs, const char >> *filename, int flags, >> >> /* if there is a backing file, use it */ >> bs1 = bdrv_new(""); >> +/* if (drv) */ >> +/* pstrcpy(bs1->backing_format, >> sizeof(bs->backing_format), */ >> +/* drv->format_name); */ >> > > I don't know if I missed this before or it was added since v3 but I'm > curious why this is here and why it's commented out. > It's an old code, which should have been deleted. The purpose was to make sure the temporary-snapshot-file backing-file-format is known. This is now done with bdrv_create2, that I've added. You can delete it yourself or apply the attached patch (or squash patches) Alternatively I can send it to you again as a single patch (v5) Also I've noticed that a small fix is needed for BDRV_O_SNAPSHOT. e.g we should use bdrv_open2 (so we would not probe the image if the format is known), so I'm attaching a patch to fix that too. Again if you want I can resend it as a single patch (squashed with v4) or you can apply(squash) this patch yourself on top of my previous patches. Thanks, Uri. --------------080300070306050808010202 Content-Type: text/x-patch; name="0001-block.c-remove-dead-commented-out-code.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="0001-block.c-remove-dead-commented-out-code.patch" >>From 5003cc362793c10f276ab186026b3f25b6b3ac9e Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Sun, 1 Mar 2009 12:43:45 +0200 Subject: [PATCH] block.c: remove dead commented out code Signed-off-by: Uri Lublin --- block.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 31d6d5b..1626e0c 100644 --- a/block.c +++ b/block.c @@ -362,9 +362,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, /* if there is a backing file, use it */ bs1 = bdrv_new(""); -/* if (drv) */ -/* pstrcpy(bs1->backing_format, sizeof(bs->backing_format), */ -/* drv->format_name); */ if (!bs1) { return -ENOMEM; } -- 1.6.0.6 --------------080300070306050808010202 Content-Type: text/x-patch; name="0002-block.c-If-image-format-is-known-use-it-for-BDRV_O.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0002-block.c-If-image-format-is-known-use-it-for-BDRV_O.patc"; filename*1="h" >>From bc79eae6c3226d21bd3fcbc370a86d7b24657aad Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Sun, 1 Mar 2009 12:44:38 +0200 Subject: [PATCH] block.c: If image format is known, use it for BDRV_O_SNAPSHOT flag as well Otherwise we'd 1. Unnecessarily probe the image to find out its format 2 Get it wrong for host-devices and non-raw images, which may affect created image size. Also, force the temporary snapshot image to be of format bdrv_qcow2 Signed-off-by: Uri Lublin --- block.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/block.c b/block.c index 1626e0c..0939cad 100644 --- a/block.c +++ b/block.c @@ -365,7 +365,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, if (!bs1) { return -ENOMEM; } - if (bdrv_open(bs1, filename, 0) < 0) { + if (bdrv_open2(bs1, filename, 0, drv) < 0) { bdrv_delete(bs1); return -1; } @@ -392,6 +392,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, return -1; } filename = tmp_filename; + drv = &bdrv_qcow2; bs->is_temporary = 1; } -- 1.6.0.6 --------------080300070306050808010202--