[libvirt] [PATCH] 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. --- It's very much possible that I missed something when adding this "functionality", I just tested running a machine and creating a volume using 'virsh vol-create-as'. So feel free to point out mymistakes. --- src/util/virstoragefile.c | 9 +++++++-- src/util/virstoragefile.h | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cdac5b1..33e72c9 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 @@ -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 */ @@ -193,6 +193,11 @@ static struct FileTypeInfo const fileTypeInfo[] = { -1, 0, 0, 0, 0, 0, NULL }, [VIR_STORAGE_FILE_VHD] = { NULL, NULL, LV_LITTLE_ENDIAN, -1, 0, 0, 0, 0, 0, NULL }, + + /* not fully supported, but qemu knows it, so we should be able to + * handle this at least basically */ + [VIR_STORAGE_FILE_VDI] = { NULL, ".vdi", LV_LITTLE_ENDIAN, + -2, 0, 0, 0, 0, -1, NULL}, }; verify(ARRAY_CARDINALITY(fileTypeInfo) == VIR_STORAGE_FILE_LAST); 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

On Mon, Feb 04, 2013 at 04:46:53PM +0100, 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. ---
It's very much possible that I missed something when adding this "functionality", I just tested running a machine and creating a volume using 'virsh vol-create-as'. So feel free to point out mymistakes.
--- src/util/virstoragefile.c | 9 +++++++-- src/util/virstoragefile.h | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cdac5b1..33e72c9 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 @@ -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 */ @@ -193,6 +193,11 @@ static struct FileTypeInfo const fileTypeInfo[] = { -1, 0, 0, 0, 0, 0, NULL }, [VIR_STORAGE_FILE_VHD] = { NULL, NULL, LV_LITTLE_ENDIAN, -1, 0, 0, 0, 0, 0, NULL }, + + /* not fully supported, but qemu knows it, so we should be able to + * handle this at least basically */ + [VIR_STORAGE_FILE_VDI] = { NULL, ".vdi", LV_LITTLE_ENDIAN, + -2, 0, 0, 0, 0, -1, NULL},
We can do better than that - look at block/vdi.c in QEMU GIT tree to find out the offsets / values for version number, magic signature at least, even if you ignore backing files for now. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/04/2013 04:53 PM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 04:46:53PM +0100, Martin Kletzander wrote: [...]
+ + /* not fully supported, but qemu knows it, so we should be able to + * handle this at least basically */ + [VIR_STORAGE_FILE_VDI] = { NULL, ".vdi", LV_LITTLE_ENDIAN, + -2, 0, 0, 0, 0, -1, NULL},
We can do better than that - look at block/vdi.c in QEMU GIT tree to find out the offsets / values for version number, magic signature at least, even if you ignore backing files for now.
Thanks for pointing that out, I've got a v2 and will send it after some testing. However, it looks like VDI doesn't have a magic, because there is a 40 Byte string at the start that cacn contain anything the creating binary puts there. For example qemu-img uses "<<< QEMU VM Virtual Disk Image >>>\n". According to [1], I'm adding "<<<" as the magic, because we don't fail if it doesn't match and we can still match it by the extension then. [1] Cite from block/vdi.c: /* Innotek / SUN images use these strings in header.text: * "<<< innotek VirtualBox Disk Image >>>\n" * "<<< Sun xVM VirtualBox Disk Image >>>\n" * "<<< Sun VirtualBox Disk Image >>>\n" * The value does not matter, so QEMU created images use a different text. */ #define VDI_TEXT "<<< QEMU VM Virtual Disk Image >>>\n" I added all the rest I could, though. Martin

On Mon, Feb 04, 2013 at 06:53:34PM +0100, Martin Kletzander wrote:
On 02/04/2013 04:53 PM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 04:46:53PM +0100, Martin Kletzander wrote: [...]
+ + /* not fully supported, but qemu knows it, so we should be able to + * handle this at least basically */ + [VIR_STORAGE_FILE_VDI] = { NULL, ".vdi", LV_LITTLE_ENDIAN, + -2, 0, 0, 0, 0, -1, NULL},
We can do better than that - look at block/vdi.c in QEMU GIT tree to find out the offsets / values for version number, magic signature at least, even if you ignore backing files for now.
Thanks for pointing that out, I've got a v2 and will send it after some testing. However, it looks like VDI doesn't have a magic, because there is a 40 Byte string at the start that cacn contain anything the creating binary puts there. For example qemu-img uses "<<< QEMU VM Virtual Disk Image >>>\n". According to [1], I'm adding "<<<" as the magic, because we don't fail if it doesn't match and we can still match it by the extension then.
No, you need to look at the 'signature' field of the header instead, which has a fixed magic value #define VDI_SIGNATURE 0xbeda107f Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/04/2013 07:03 PM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 06:53:34PM +0100, Martin Kletzander wrote:
On 02/04/2013 04:53 PM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 04:46:53PM +0100, Martin Kletzander wrote: [...]
+ + /* not fully supported, but qemu knows it, so we should be able to + * handle this at least basically */ + [VIR_STORAGE_FILE_VDI] = { NULL, ".vdi", LV_LITTLE_ENDIAN, + -2, 0, 0, 0, 0, -1, NULL},
We can do better than that - look at block/vdi.c in QEMU GIT tree to find out the offsets / values for version number, magic signature at least, even if you ignore backing files for now.
Thanks for pointing that out, I've got a v2 and will send it after some testing. However, it looks like VDI doesn't have a magic, because there is a 40 Byte string at the start that cacn contain anything the creating binary puts there. For example qemu-img uses "<<< QEMU VM Virtual Disk Image >>>\n". According to [1], I'm adding "<<<" as the magic, because we don't fail if it doesn't match and we can still match it by the extension then.
No, you need to look at the 'signature' field of the header instead, which has a fixed magic value
#define VDI_SIGNATURE 0xbeda107f
But the struct mapped to the header starts like this: typedef struct { char text[0x40]; uint32_t signature; ... Of course that would just mean adding one more parameter to the FileTypeInfo struct, but I thought it's not magic then. However, I checked that and you're right, I was wrong, because according to magic(5) it can be anywhere. The magic file '/usr/share/misc/magic.mgc' also matches the signature, so I'm using that. Martin
participants (2)
-
Daniel P. Berrange
-
Martin Kletzander