3:49 p.m.
This is basically v2 for:
https://www.redhat.com/archives/libvir-list/2013-February/msg00151.html
Very small series, so there's not much more to say ;)
Martin Kletzander (2):
Support shifted magic in storage files
Add basic support for VDI images
src/util/virstoragefile.c | 70 ++++++++++++++++++++++++++++-------------------
src/util/virstoragefile.h | 3 +-
2 files changed, 44 insertions(+), 29 deletions(-)
--
1.8.1.2
3:49 p.m.
New subject: [libvirt] [PATCH v2 1/2] Support shifted magic in storage files
Some files have the magic shifted to some offset other than 0, so we
have to support that. I also cleaned up some lines to be more
readable and added missing magic for iso file format.
---
src/util/virstoragefile.c | 55 ++++++++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cdac5b1..4f31572 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1,7 +1,7 @@
/*
* virstoragefile.c: file utility functions for FS storage backend
*
- * Copyright (C) 2007-2012 Red Hat, Inc.
+ * Copyright (C) 2007-2013 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -66,6 +66,7 @@ enum {
/* Either 'magic' or 'extension' *must* be provided */
struct FileTypeInfo {
+ int magicOffset; /* Byte offset of the magic */
const char *magic; /* Optional string of file magic
* to check at head of file */
const char *extension; /* Optional file extension to check */
@@ -127,71 +128,75 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t);
static struct FileTypeInfo const fileTypeInfo[] = {
- [VIR_STORAGE_FILE_NONE] = { NULL, NULL, LV_LITTLE_ENDIAN,
+ [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
- [VIR_STORAGE_FILE_RAW] = { NULL, NULL, LV_LITTLE_ENDIAN,
+ [VIR_STORAGE_FILE_RAW] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
- [VIR_STORAGE_FILE_DIR] = { NULL, NULL, LV_LITTLE_ENDIAN,
+ [VIR_STORAGE_FILE_DIR] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
[VIR_STORAGE_FILE_BOCHS] = {
- /*"Bochs Virtual HD Image", */ /* Untested */ NULL,
- NULL,
+ /*"Bochs Virtual HD Image", */ /* Untested */
+ 0, NULL, NULL,
LV_LITTLE_ENDIAN, 64, 0x20000,
32+16+16+4+4+4+4+4, 8, 1, -1, NULL
},
[VIR_STORAGE_FILE_CLOOP] = {
- /*"#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t
iso9660 /dev/cloop $1\n", */ /* Untested */ NULL,
- NULL,
+ /* #!/bin/sh
+ #V2.0 Format
+ modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1
+ */ /* Untested */
+ 0, NULL, NULL,
LV_LITTLE_ENDIAN, -1, 0,
-1, 0, 0, -1, NULL
},
[VIR_STORAGE_FILE_COW] = {
- "OOOM", NULL,
+ 0, "OOOM", NULL,
LV_BIG_ENDIAN, 4, 2,
4+4+1024+4, 8, 1, -1, cowGetBackingStore
},
[VIR_STORAGE_FILE_DMG] = {
- NULL, /* XXX QEMU says there's no magic for dmg, but we should check... */
- ".dmg",
+ /* XXX QEMU says there's no magic for dmg,
+ * /usr/share/misc/magic says there is double magic both has
+ * to be true) and has it disabled as well */
+ 0, NULL, ".dmg",
0, -1, 0,
-1, 0, 0, -1, NULL
},
[VIR_STORAGE_FILE_ISO] = {
- NULL, /* XXX there's probably some magic for iso we can validate too... */
- ".iso",
+ 32769, "CD001", ".iso",
0, -1, 0,
-1, 0, 0, -1, NULL
},
[VIR_STORAGE_FILE_QCOW] = {
- "QFI", NULL,
+ 0, "QFI", NULL,
LV_BIG_ENDIAN, 4, 1,
QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW1_HDR_CRYPT, qcow1GetBackingStore,
},
[VIR_STORAGE_FILE_QCOW2] = {
- "QFI", NULL,
+ 0, "QFI", NULL,
LV_BIG_ENDIAN, 4, 2,
QCOWX_HDR_IMAGE_SIZE, 8, 1, QCOW2_HDR_CRYPT, qcow2GetBackingStore,
},
[VIR_STORAGE_FILE_QED] = {
/* http://wiki.qemu.org/Features/QED */
- "QED", NULL,
+ 0, "QED", NULL,
LV_LITTLE_ENDIAN, -2, -1,
QED_HDR_IMAGE_SIZE, 8, 1, -1, qedGetBackingStore,
},
[VIR_STORAGE_FILE_VMDK] = {
- "KDMV", NULL,
+ 0, "KDMV", NULL,
LV_LITTLE_ENDIAN, 4, 1,
4+4+4, 8, 512, -1, vmdk4GetBackingStore
},
[VIR_STORAGE_FILE_VPC] = {
- "conectix", NULL,
+ 0, "conectix", NULL,
LV_BIG_ENDIAN, 12, 0x10000,
8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, -1, NULL
},
/* Not direct file formats, but used for various drivers */
- [VIR_STORAGE_FILE_FAT] = { NULL, NULL, LV_LITTLE_ENDIAN,
+ [VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
- [VIR_STORAGE_FILE_VHD] = { NULL, NULL, LV_LITTLE_ENDIAN,
+ [VIR_STORAGE_FILE_VHD] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
};
verify(ARRAY_CARDINALITY(fileTypeInfo) == VIR_STORAGE_FILE_LAST);
@@ -571,16 +576,18 @@ virStorageFileMatchesMagic(int format,
size_t buflen)
{
int mlen;
+ int magicOffset = fileTypeInfo[format].magicOffset;
+ const char *magic = fileTypeInfo[format].magic;
- if (fileTypeInfo[format].magic == NULL)
+ if (magic == NULL)
return false;
/* Validate magic data */
- mlen = strlen(fileTypeInfo[format].magic);
- if (mlen > buflen)
+ mlen = strlen(magic);
+ if (magicOffset + mlen > buflen)
return false;
- if (memcmp(buf, fileTypeInfo[format].magic, mlen) != 0)
+ if (memcmp(buf + magicOffset, magic, mlen) != 0)
return false;
return true;
--
1.8.1.2
4:01 p.m.
New subject: [libvirt] [PATCH v2 1/2] Support shifted magic in storage files
On 02/04/2013 02:49 PM, Martin Kletzander wrote:
Some files have the magic shifted to some offset other than 0, so we
have to support that. I also cleaned up some lines to be more
readable and added missing magic for iso file format.
---
src/util/virstoragefile.c | 55 ++++++++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 24 deletions(-)
[VIR_STORAGE_FILE_DMG] = {
- NULL, /* XXX QEMU says there's no magic for dmg, but we should check... */
- ".dmg",
+ /* XXX QEMU says there's no magic for dmg,
+ * /usr/share/misc/magic says there is double magic both has
+ * to be true) and has it disabled as well */
This comment now has closing ')' with no open '('. Did you mean
something more like:
/* XXX QEMU says there's no magic for dmg,
* /usr/share/misc/magic lists double magic (both offsets would
* have to match) but then disables that check. */
ACK with the comment touched up.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:04 p.m.
New subject: [libvirt] [PATCH v2 1/2] Support shifted magic in storage files
On 02/04/2013 02:49 PM, Martin Kletzander wrote:
Some files have the magic shifted to some offset other than 0, so we
have to support that. I also cleaned up some lines to be more
readable and added missing magic for iso file format.
---
src/util/virstoragefile.c | 55 ++++++++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 24 deletions(-)
[VIR_STORAGE_FILE_ISO] = {
- NULL, /* XXX there's probably some magic for iso we can validate too... */
- ".iso",
+ 32769, "CD001", ".iso",
Oh, I didn't notice that this wouldn't work until I saw patch 2; you
need to hoist a hunk of that patch into this one.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:49 p.m.
New subject: [libvirt] [PATCH v2 2/2] Add basic support for VDI images
QEMU is fully capable of handling VDI images and we just refuse to
work with them. As qemu-img knows and supports this, there should be
no problem with this addition.
This is of course, just basic functionality, without searching for any
backing files, etc.
---
src/util/virstoragefile.c | 15 +++++++++++----
src/util/virstoragefile.h | 3 ++-
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4f31572..2700a89 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -51,7 +51,7 @@ VIR_ENUM_IMPL(virStorageFileFormat,
"raw", "dir", "bochs",
"cloop", "cow", "dmg", "iso",
"qcow", "qcow2", "qed", "vmdk",
"vpc",
- "fat", "vhd")
+ "fat", "vhd", "vdi")
enum lv_endian {
LV_LITTLE_ENDIAN = 1, /* 1234 */
@@ -122,9 +122,10 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t);
#define QED_F_BACKING_FILE 0x01
#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
-/* VMDK needs at least this to find backing store,
+/* VMDK needs at least 20*512 B to find backing store,
+ * ISO has 5 Byte magic on offset 32769,
* other formats need less */
-#define STORAGE_MAX_HEAD (20*512)
+#define STORAGE_MAX_HEAD 32769+5
static struct FileTypeInfo const fileTypeInfo[] = {
@@ -164,7 +165,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
},
[VIR_STORAGE_FILE_ISO] = {
32769, "CD001", ".iso",
- 0, -1, 0,
+ LV_LITTLE_ENDIAN, -2, 0,
-1, 0, 0, -1, NULL
},
[VIR_STORAGE_FILE_QCOW] = {
@@ -193,6 +194,12 @@ static struct FileTypeInfo const fileTypeInfo[] = {
LV_BIG_ENDIAN, 12, 0x10000,
8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, -1, NULL
},
+ /* TODO: add getBackingStore function */
+ [VIR_STORAGE_FILE_VDI] = {
+ 64, "\x7f\x10\xda\xbe", ".vdi",
+ LV_LITTLE_ENDIAN, 68, 0x00010001,
+ 68 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL},
+
/* Not direct file formats, but used for various drivers */
[VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 618b028..83dc3f2 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -1,7 +1,7 @@
/*
* virstoragefile.h: file utility functions for FS storage backend
*
- * Copyright (C) 2007-2009, 2012 Red Hat, Inc.
+ * Copyright (C) 2007-2009, 2012, 2013 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -44,6 +44,7 @@ enum virStorageFileFormat {
VIR_STORAGE_FILE_VPC,
VIR_STORAGE_FILE_FAT,
VIR_STORAGE_FILE_VHD,
+ VIR_STORAGE_FILE_VDI,
VIR_STORAGE_FILE_LAST,
};
--
1.8.1.2
4:06 p.m.
New subject: [libvirt] [PATCH v2 2/2] Add basic support for VDI images
On 02/04/2013 02:49 PM, Martin Kletzander wrote:
QEMU is fully capable of handling VDI images and we just refuse to
work with them. As qemu-img knows and supports this, there should be
no problem with this addition.
This is of course, just basic functionality, without searching for any
backing files, etc.
---
src/util/virstoragefile.c | 15 +++++++++++----
src/util/virstoragefile.h | 3 ++-
2 files changed, 13 insertions(+), 5 deletions(-)
@@ -122,9 +122,10 @@ qedGetBackingStore(char **, int *, const unsigned char *, size_t);
#define QED_F_BACKING_FILE 0x01
#define QED_F_BACKING_FORMAT_NO_PROBE 0x04
-/* VMDK needs at least this to find backing store,
+/* VMDK needs at least 20*512 B to find backing store,
+ * ISO has 5 Byte magic on offset 32769,
* other formats need less */
-#define STORAGE_MAX_HEAD (20*512)
+#define STORAGE_MAX_HEAD 32769+5
This hunk belongs in patch 1/2.
static struct FileTypeInfo const fileTypeInfo[] = {
@@ -164,7 +165,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
},
[VIR_STORAGE_FILE_ISO] = {
32769, "CD001", ".iso",
- 0, -1, 0,
+ LV_LITTLE_ENDIAN, -2, 0,
-1, 0, 0, -1, NULL
And this one, too.
},
+ /* TODO: add getBackingStore function */
+ [VIR_STORAGE_FILE_VDI] = {
+ 64, "\x7f\x10\xda\xbe", ".vdi",
+ LV_LITTLE_ENDIAN, 68, 0x00010001,
+ 68 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL},
+
Looks okay.
/* Not direct file formats, but used for various drivers */
[VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, 0, 0, 0, 0, 0, NULL },
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 618b028..83dc3f2 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -1,7 +1,7 @@
/*
* virstoragefile.h: file utility functions for FS storage backend
*
- * Copyright (C) 2007-2009, 2012 Red Hat, Inc.
+ * Copyright (C) 2007-2009, 2012, 2013 Red Hat, Inc.
I'd use 2012-2013, instead of comma notation.
ACK with the reshuffle of iso hunks and copyright fix.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:52 p.m.
New subject: [libvirt] [PATCH v2 2/2] Add basic support for VDI images
On 02/04/2013 11:06 PM, Eric Blake wrote:
On 02/04/2013 02:49 PM, Martin Kletzander wrote:
> QEMU is fully capable of handling VDI images and we just refuse to
> work with them. As qemu-img knows and supports this, there should be
> no problem with this addition.
>
> This is of course, just basic functionality, without searching for any
> backing files, etc.
> ---
> src/util/virstoragefile.c | 15 +++++++++++----
> src/util/virstoragefile.h | 3 ++-
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> @@ -122,9 +122,10 @@ qedGetBackingStore(char **, int *, const unsigned char *,
size_t);
> #define QED_F_BACKING_FILE 0x01
> #define QED_F_BACKING_FORMAT_NO_PROBE 0x04
>
> -/* VMDK needs at least this to find backing store,
> +/* VMDK needs at least 20*512 B to find backing store,
> + * ISO has 5 Byte magic on offset 32769,
> * other formats need less */
> -#define STORAGE_MAX_HEAD (20*512)
> +#define STORAGE_MAX_HEAD 32769+5
This hunk belongs in patch 1/2.
>
>
> static struct FileTypeInfo const fileTypeInfo[] = {
> @@ -164,7 +165,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
> },
> [VIR_STORAGE_FILE_ISO] = {
> 32769, "CD001", ".iso",
> - 0, -1, 0,
> + LV_LITTLE_ENDIAN, -2, 0,
> -1, 0, 0, -1, NULL
And this one, too.
> },
> + /* TODO: add getBackingStore function */
> + [VIR_STORAGE_FILE_VDI] = {
> + 64, "\x7f\x10\xda\xbe", ".vdi",
> + LV_LITTLE_ENDIAN, 68, 0x00010001,
> + 68 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL},
> +
Looks okay.
> /* Not direct file formats, but used for various drivers */
> [VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
> -1, 0, 0, 0, 0, 0, NULL },
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 618b028..83dc3f2 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -1,7 +1,7 @@
> /*
> * virstoragefile.h: file utility functions for FS storage backend
> *
> - * Copyright (C) 2007-2009, 2012 Red Hat, Inc.
> + * Copyright (C) 2007-2009, 2012, 2013 Red Hat, Inc.
I'd use 2012-2013, instead of comma notation.
I haven't noticed that. I guess 'copyright-update uses ranges only when
updating a range already.
ACK with the reshuffle of iso hunks and copyright fix.
Did all that and pushed both patches, thanks.
Martin
4309
days inactive
4309
days old
6 comments
2 participants
participants (2)
-
Eric Blake -
Martin Kletzander