From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LZ5Sd-00072J-JZ for qemu-devel@nongnu.org; Mon, 16 Feb 2009 10:32:43 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LZ5Sa-00070l-H6 for qemu-devel@nongnu.org; Mon, 16 Feb 2009 10:32:43 -0500 Received: from [199.232.76.173] (port=47446 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LZ5Sa-00070f-Ax for qemu-devel@nongnu.org; Mon, 16 Feb 2009 10:32:40 -0500 Received: from e6.ny.us.ibm.com ([32.97.182.146]:56792) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LZ5SZ-0004x6-W3 for qemu-devel@nongnu.org; Mon, 16 Feb 2009 10:32:40 -0500 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e6.ny.us.ibm.com (8.13.1/8.13.1) with ESMTP id n1GFXNQm013548 for ; Mon, 16 Feb 2009 10:33:23 -0500 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id n1GFWcIA193346 for ; Mon, 16 Feb 2009 10:32:38 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n1GFVUF0016845 for ; Mon, 16 Feb 2009 10:31:30 -0500 Message-ID: <499986FB.8090601@us.ibm.com> Date: Mon, 16 Feb 2009 09:32:11 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1234234344-16325-1-git-send-email-uril@redhat.com> <1234234344-16325-2-git-send-email-uril@redhat.com> In-Reply-To: <1234234344-16325-2-git-send-email-uril@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RESEND][PATCH 1/2] Introducing qcow2 extensions + keep backing file format Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Uri Lublin Cc: qemu-devel@nongnu.org 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 > --- > block-qcow2.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > block.c | 29 +++++++++++++- > block.h | 4 ++ > block_int.h | 6 +++ > 4 files changed, 149 insertions(+), 5 deletions(-) > > diff --git a/block-qcow2.c b/block-qcow2.c > index 8a5b621..d2263d5 100644 > --- a/block-qcow2.c > +++ b/block-qcow2.c > @@ -45,6 +45,7 @@ > > //#define DEBUG_ALLOC > //#define DEBUG_ALLOC2 > +//#define DEBUG_EXT > > #define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb) > #define QCOW_VERSION 2 > @@ -77,6 +78,21 @@ typedef struct QCowHeader { > uint64_t snapshots_offset; > } QCowHeader; > > + > +typedef struct { > + /* > + * Alternatives: > + * should I make them uint64_t ? > + * should I add version ? > + * should I use a single magic and add type field ? > + * should I keep number-of-extenstions ? > + */ > What you have is fine. You should remove these comments though. > + uint32_t magic; > + uint32_t len; > +} QCowExtension; > +#define QCOW_EXT_MAGIC_END 0 > +#define QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA > + > typedef struct __attribute__((packed)) QCowSnapshotHeader { > /* header is 8 byte aligned */ > uint64_t l1_table_offset; > @@ -189,6 +205,66 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename) > return 0; > } > > + > +/* read qcow2 extension and fill bs > + * start reading from start_offset > + * finish reading upon magic of value 0 or when end_offset reached > + * unknown magic is skipped (future extension this version knows nothing about) > + * return 0 upon success, non-0 otherwise > + */ > So, if this falls after the header but before the backing name, and you need to find a '0, 0' combination to terminate the list, are we guaranteed that all old qcow2 files are going to be parsed correctly? Can't a backing file name start immediately after the header? Regards, Anthony Liguori