From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LZ60r-0005h7-UT for qemu-devel@nongnu.org; Mon, 16 Feb 2009 11:08:05 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LZ60r-0005gS-2E for qemu-devel@nongnu.org; Mon, 16 Feb 2009 11:08:05 -0500 Received: from [199.232.76.173] (port=59870 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LZ60q-0005g5-OU for qemu-devel@nongnu.org; Mon, 16 Feb 2009 11:08:04 -0500 Received: from mx2.redhat.com ([66.187.237.31]:52940) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LZ60q-0008GV-4l for qemu-devel@nongnu.org; Mon, 16 Feb 2009 11:08:04 -0500 Message-ID: <49998F3B.1030600@redhat.com> Date: Mon, 16 Feb 2009 18:07:23 +0200 From: Uri Lublin MIME-Version: 1.0 References: <1234234344-16325-1-git-send-email-uril@redhat.com> <1234234344-16325-2-git-send-email-uril@redhat.com> <499986FB.8090601@us.ibm.com> In-Reply-To: <499986FB.8090601@us.ibm.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: Anthony Liguori Cc: qemu-devel@nongnu.org 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 >> --- >> 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. O.K. I'll remove those comments. > >> + 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? We are searching till one of the following conditions reached: 1. magic of value 0 or 2. end_offset reached (which is really the offset of the backing file name). If you find it confusing, I can probably drop the first condition (which was added as a sanity check). Also note that I'm ignoring unknown extensions (assumed to be written in the future), so magic of 0 will be skipped anyway. > Can't a backing file name start immediately after the header? It can. In that case the search will be over immediately, since start_offset == end_offset (end-of-qcow2-header == offset-of-backing-file-name). Thanks, Uri.