5:32 p.m.
On 11/19/2010 09:18 AM, Adam Litke wrote:
Implement getBackingStore() for QED images. The header format is
defined in
the QED spec: http://wiki.qemu.org/Features/QED .
Signed-off-by: Adam Litke <agl(a)us.ibm.com>
Cc: Stefan Hajnoczi <stefan.hajnoczi(a)uk.ibm.com>
Cc: Anthony Liguori <aliguori(a)linux.vnet.ibm.com>
---
src/util/storage_file.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 77 insertions(+), 1 deletions(-)
Aha - I should have read this before commenting on patch 2.
@@ -105,6 +107,11 @@ static int vmdk4GetBackingStore(char **, int *,
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
#define QED_HDR_IMAGE_SIZE 40
+#define QED_HDR_FEATURES_OFFSET 16
+#define QED_HDR_BACKING_FILE_OFFSET 56
+#define QED_HDR_BACKING_FILE_SIZE 60
+#define QED_F_BACKING_FILE 0x01
+#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
Again, I'll break the offsets into pieces. See below.
+static unsigned long
+qedGetHeaderUL(const unsigned char *loc)
+{
+ return ( ((unsigned long)loc[3] << 24)
+ | ((unsigned long)loc[2] << 16)
+ | ((unsigned long)loc[1] << 8)
+ | ((unsigned long)loc[0] << 0));
+}
+
+static unsigned long long
+qedGetHeaderULL(const unsigned char *loc)
+{
+ return ( ((unsigned long)loc[7] << 56)
+ | ((unsigned long)loc[6] << 48)
+ | ((unsigned long)loc[5] << 40)
+ | ((unsigned long)loc[4] << 32)
+ | ((unsigned long)loc[3] << 24)
+ | ((unsigned long)loc[2] << 16)
+ | ((unsigned long)loc[1] << 8)
+ | ((unsigned long)loc[0] << 0));
+}
These two routines are independently useful for other little-endian
parsers in the same file. Should we (as a separate patch) rename them
and put them to wider use, as well as adding big-endian counterparts for
the remaining file formats to share? It would cut down on the number of
open-coded integer constructions elsewhere in the file.
+
+static int
+qedGetBackingStore(char **res,
+ int *format,
+ const unsigned char *buf,
+ size_t buf_size)
+{
+ unsigned long long flags;
+ unsigned long offset, size;
+
+ *res = NULL;
+ /* Check if this image has a backing file */
+ if (buf_size < QED_HDR_FEATURES_OFFSET+8)
+ return BACKING_STORE_INVALID;
+ flags = qedGetHeaderULL(buf + QED_HDR_FEATURES_OFFSET);
+ if (!(flags & QED_F_BACKING_FILE))
+ return BACKING_STORE_OK;
+
+ /* Parse the backing file */
+ if (buf_size < QED_HDR_BACKING_FILE_OFFSET+8)
+ return BACKING_STORE_INVALID;
+ offset = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_OFFSET);
+ if (offset > buf_size)
+ return BACKING_STORE_INVALID;
+ size = qedGetHeaderUL(buf + QED_HDR_BACKING_FILE_SIZE);
+ if (size == 0)
+ return BACKING_STORE_OK;
+ if (offset + size > buf_size || offset + size < offset)
+ return BACKING_STORE_INVALID;
+ if (size + 1 == 0)
+ return BACKING_STORE_INVALID;
This clause is redundant; you already rejected offset + size < offset,
and size + 1 == 0 implies size == -1, which would have failed that
earlier check.
ACK, with this squashed in. I've pushed your series.
gdiff --git i/src/util/storage_file.c w/src/util/storage_file.c
index d7b4073..aa117e7 100644
--- i/src/util/storage_file.c
+++ w/src/util/storage_file.c
@@ -107,10 +107,10 @@ qedGetBackingStore(char **, int *, const unsigned
char *, size_t);
#define QCOW2_HDR_EXTENSION_END 0
#define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
-#define QED_HDR_IMAGE_SIZE (4+4+4+4+8+8+8)
-#define QED_HDR_FEATURES_OFFSET 16
-#define QED_HDR_BACKING_FILE_OFFSET 56
-#define QED_HDR_BACKING_FILE_SIZE 60
+#define QED_HDR_FEATURES_OFFSET (4+4+4+4)
+#define QED_HDR_IMAGE_SIZE (QED_HDR_FEATURES_OFFSET+8+8+8)
+#define QED_HDR_BACKING_FILE_OFFSET (QED_HDR_IMAGE_SIZE+8+8)
+#define QED_HDR_BACKING_FILE_SIZE (QED_HDR_BACKING_FILE_OFFSET+4)
#define QED_F_BACKING_FILE 0x01
#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
@@ -475,8 +475,6 @@ qedGetBackingStore(char **res,
return BACKING_STORE_OK;
if (offset + size > buf_size || offset + size < offset)
return BACKING_STORE_INVALID;
- if (size + 1 == 0)
- return BACKING_STORE_INVALID;
if (VIR_ALLOC_N(*res, size + 1) < 0) {
virReportOOMError();
return BACKING_STORE_ERROR;
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org