qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [RESEND][PATCH 1/2] Introducing qcow2 extensions + keep


From: Anthony Liguori
Subject: [Qemu-devel] Re: [RESEND][PATCH 1/2] Introducing qcow2 extensions + keep backing file format
Date: Mon, 16 Feb 2009 09:32:11 -0600
User-agent: Thunderbird 2.0.0.19 (X11/20090105)

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 <address@hidden>
---
 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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]