* [Qemu-devel] [PATCH 0/2] Small improvements to VMDK4 file handling @ 2009-12-18 15:56 Richard W.M. Jones 2009-12-18 15:57 ` [Qemu-devel] [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs Richard W.M. Jones 2009-12-18 15:58 ` [Qemu-devel] [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field Richard W.M. Jones 0 siblings, 2 replies; 5+ messages in thread From: Richard W.M. Jones @ 2009-12-18 15:56 UTC (permalink / raw) To: qemu-devel We're tracing a bug in "qemu-img convert" where it will silently fail to convert certain types of VMDK file. The VMDK files produced by VMWare vSphere / ESX 4.0, which have the VMDK subformat "streamOptimized" are one notable case: https://bugzilla.redhat.com/show_bug.cgi?id=548723 These two patches certainly don't fix this, but they do at least prevent "qemu-img convert" from silently failing. Unfortunately to do this I had to look much more deeply into the VMDK format than the current VMDK block driver does. We have to parse the VMDK4 plain text descriptor which is embedded in these binary files. So the first patch adds the parsing and has the benefit of improving how the CID and parentCID fields are read and updated. The second patch parses the createType field from the descriptor, and refuses to continue if it's not on a short whitelist of subformats which we are likely to be able to parse. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs 2009-12-18 15:56 [Qemu-devel] [PATCH 0/2] Small improvements to VMDK4 file handling Richard W.M. Jones @ 2009-12-18 15:57 ` Richard W.M. Jones 2009-12-18 16:06 ` Richard W.M. Jones 2009-12-18 15:58 ` [Qemu-devel] [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field Richard W.M. Jones 1 sibling, 1 reply; 5+ messages in thread From: Richard W.M. Jones @ 2009-12-18 15:57 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 260 bytes --] -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v [-- Attachment #2: 0001-VMDK4-Parse-the-VMDK-descriptor-explicitly-improve-h.patch --] [-- Type: text/plain, Size: 8240 bytes --] >From 69701355561c328fac5e7986d6122c8411137b7e Mon Sep 17 00:00:00 2001 From: Richard Jones <rjones@redhat.com> Date: Fri, 18 Dec 2009 15:30:03 +0000 Subject: [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs The VMDK monolithic file format[1] contains an embedded text descriptor. This is at a variable location in the file, pointed to by fields in the header. An example of this embedded descriptor might be: # Disk DescriptorFile version=1 CID=12345678 parentCID=87654321 createType="monolithicSparse" # Extent description RW 16777216 SPARSE "generated-stream.vmdk" The current code assumes the descriptor is at offset 0x200 with a fixed size, and doesn't really parse the descriptor properly eg when reading and writing the CID and parentCID fields. This patch contains code to properly locate and parse the embedded descriptor in monolithic files (qemu doesn't support "split" VMDK files at all). Also this modifies the code which updates the CID fields so it doesn't rewrite the whole of the descriptor. [1] The VMDK 1.1 (ie. "VMDK4") format is partially described here: http://www.vmware.com/app/vmdk/?src=vmdk [PDF link] --- block/vmdk.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 142 insertions(+), 31 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 4e48622..71680f0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -77,6 +77,10 @@ typedef struct BDRVVmdkState { unsigned int cluster_sectors; uint32_t parent_cid; int is_parent; + + /* VMDK4 fields */ + int64_t cid_offset; + int64_t parent_cid_offset; } BDRVVmdkState; typedef struct VmdkMetaData { @@ -112,57 +116,159 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename) #define CHECK_CID 1 #define SECTOR_SIZE 512 -#define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each #define HEADER_SIZE 512 // first sector of 512 bytes +/* Read and parse the VMDK4 descriptor. + * + * So-called "monolithic" VMDK4 files (which are the only ones we can + * currently read) have a text descriptor section embedded within the + * file at an offset described in the header. + * + * The other format for VMDK4 files ("split") has a toplevel .vmdk + * file which is the plain text descriptor, and that points to other + * files that contain the disk data. We can't read this sort of file + * yet. + */ +#define MAX_DESC_SIZE 100*512 + +#define VMDK4_ACCESS_RW 1 +#define VMDK4_ACCESS_RDONLY 2 +#define VMDK4_ACCESS_NOACCESS 3 + +static int vmdk4_parse_desc_extent(BlockDriverState *bs, VMDK4Header *header, + int access_type, char *p) +{ + /* We should probably check that the monolithic file + * has only one extent. + */ + return 0; +} + +static int vmdk4_parse_desc_assignment(BlockDriverState *bs, + VMDK4Header *header, char *p, size_t len, + int64_t file_offset) +{ + BDRVVmdkState *s = bs->opaque; + + if (strncmp(p, "version=", 8) == 0 && strcmp(p, "version=1") != 0) + fprintf (stderr, + "warning: vmdk4_parse_desc: descriptor version != 1 (%s)\n", + p); + + if (!strncmp(p, "CID=", 4)) + s->cid_offset = file_offset + 4; + + if (!strncmp(p, "parentCID=", 10)) + s->parent_cid_offset = file_offset + 10; + + return 0; +} + +static int vmdk4_parse_desc(BlockDriverState *bs, VMDK4Header *header) +{ + BDRVVmdkState *s = bs->opaque; + int64_t desc_offset, desc_size; + char desc[MAX_DESC_SIZE]; + char *p, *pnext; + size_t len; + + s->cid_offset = 0; + s->parent_cid_offset = 0; + + /* Stored in the header are the offset/size in sectors. */ + desc_size = le64_to_cpu(header->desc_size) * SECTOR_SIZE; + desc_offset = le64_to_cpu(header->desc_offset) * SECTOR_SIZE; + + if (desc_size > MAX_DESC_SIZE) { + fprintf (stderr, + "vmdk4_parse_desc: descriptor size > maximum size " + "(%" PRIi64 " > %d)\n", desc_size, MAX_DESC_SIZE); + return -1; + } + + if (bdrv_pread(s->hd, desc_offset, (uint8_t *)desc, desc_size) != desc_size) { + fprintf (stderr, + "vmdk4_parse_desc: could not read descriptor from " + "%" PRIi64 " size %" PRIi64 "\n", + desc_offset, desc_size); + return -1; + } + + if (!strncmp(desc, "# Disk DescriptorFile", 22)) { + fprintf (stderr, "vmdk4_parse_desc: unrecognized descriptor"); + return -1; + } + + /* Parse the descriptor line by line. */ + for (p = desc; p && *p; p = pnext) { + pnext = strchr (p, '\n'); + if (pnext != NULL) { + *pnext = '\0'; + pnext++; + } + + while (*p && isspace (*p)) + p++; + while ((len = strlen (p)) > 0 && isspace (p[len-1])) + p[len-1] = '\0'; + + if (!*p) + continue; + if (p[0] == '#') + continue; + + /* Ignore any lines that we can't parse. */ + if (!strncmp(p, "RW ", 3)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_RW, p + 3); + else if (!strncmp(p, "RDONLY ", 7)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_RDONLY, p + 7); + else if (!strncmp(p, "NOACCESS ", 9)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_NOACCESS, p + 9); + else /* else expecting name = value */ + vmdk4_parse_desc_assignment (bs, header, p, len, + p-desc+desc_offset); + } + + return 0; +} + static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent) { BDRVVmdkState *s = bs->opaque; - char desc[DESC_SIZE]; + int64_t offset; + char str[9]; uint32_t cid; - const char *p_name, *cid_str; - size_t cid_str_size; - /* the descriptor offset = 0x200 */ - if (bdrv_pread(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) - return 0; + if (parent) + offset = s->parent_cid_offset; + else + offset = s->cid_offset; - if (parent) { - cid_str = "parentCID"; - cid_str_size = sizeof("parentCID"); - } else { - cid_str = "CID"; - cid_str_size = sizeof("CID"); - } + if (offset == 0) + return 0; - if ((p_name = strstr(desc,cid_str)) != NULL) { - p_name += cid_str_size; - sscanf(p_name,"%x",&cid); - } + if (bdrv_pread(s->hd, offset, str, 8) != 8) + return 0; + sscanf(str, "%x", &cid); return cid; } static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid) { BDRVVmdkState *s = bs->opaque; - char desc[DESC_SIZE], tmp_desc[DESC_SIZE]; - char *p_name, *tmp_str; + int64_t offset; + char str[9]; - /* the descriptor offset = 0x200 */ - if (bdrv_pread(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) - return -1; + offset = s->cid_offset; + if (offset == 0) + return 0; - tmp_str = strstr(desc,"parentCID"); - pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str); - if ((p_name = strstr(desc,"CID")) != NULL) { - p_name += sizeof("CID"); - snprintf(p_name, sizeof(desc) - (p_name - desc), "%x\n", cid); - pstrcat(desc, sizeof(desc), tmp_desc); - } + snprintf(str, sizeof str, "%x\n", cid); - if (bdrv_pwrite(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) + if (bdrv_pwrite(s->hd, offset, str, 8) != 8) return -1; + return 0; } @@ -184,6 +290,8 @@ static int vmdk_is_cid_valid(BlockDriverState *bs) return 1; } +#define DESC_SIZE 20*SECTOR_SIZE + static int vmdk_snapshot_create(const char *filename, const char *backing_file) { int snp_fd, p_fd; @@ -410,6 +518,9 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9; s->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9; + if (vmdk4_parse_desc(bs, &header) == -1) + goto fail; + if (parent_open) s->is_parent = 1; else -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs 2009-12-18 15:57 ` [Qemu-devel] [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs Richard W.M. Jones @ 2009-12-18 16:06 ` Richard W.M. Jones 0 siblings, 0 replies; 5+ messages in thread From: Richard W.M. Jones @ 2009-12-18 16:06 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 324 bytes --] Let's fix a buffer overflow, and add the Signed-off-by line ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top [-- Attachment #2: 0001-VMDK4-Parse-the-VMDK-descriptor-explicitly-improve-h.patch --] [-- Type: text/plain, Size: 8310 bytes --] >From b22afc3464ec124d131ab21fc4270ef575c7b47d Mon Sep 17 00:00:00 2001 From: Richard Jones <rjones@redhat.com> Date: Fri, 18 Dec 2009 15:30:03 +0000 Subject: [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs The VMDK monolithic file format[1] contains an embedded text descriptor. This is at a variable location in the file, pointed to by fields in the header. An example of this embedded descriptor might be: # Disk DescriptorFile version=1 CID=12345678 parentCID=87654321 createType="monolithicSparse" # Extent description RW 16777216 SPARSE "generated-stream.vmdk" The current code assumes the descriptor is at offset 0x200 with a fixed size, and doesn't really parse the descriptor properly eg when reading and writing the CID and parentCID fields. This patch contains code to properly locate and parse the embedded descriptor in monolithic files (qemu doesn't support "split" VMDK files at all). Also this modifies the code which updates the CID fields so it doesn't rewrite the whole of the descriptor. [1] The VMDK 1.1 (ie. "VMDK4") format is partially described here: http://www.vmware.com/app/vmdk/?src=vmdk [PDF link] Signed-off-by: Richard Jones <rjones@redhat.com> --- block/vmdk.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 143 insertions(+), 31 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 4e48622..0f59ea5 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -77,6 +77,10 @@ typedef struct BDRVVmdkState { unsigned int cluster_sectors; uint32_t parent_cid; int is_parent; + + /* VMDK4 fields */ + int64_t cid_offset; + int64_t parent_cid_offset; } BDRVVmdkState; typedef struct VmdkMetaData { @@ -112,57 +116,160 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename) #define CHECK_CID 1 #define SECTOR_SIZE 512 -#define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each #define HEADER_SIZE 512 // first sector of 512 bytes +/* Read and parse the VMDK4 descriptor. + * + * So-called "monolithic" VMDK4 files (which are the only ones we can + * currently read) have a text descriptor section embedded within the + * file at an offset described in the header. + * + * The other format for VMDK4 files ("split") has a toplevel .vmdk + * file which is the plain text descriptor, and that points to other + * files that contain the disk data. We can't read this sort of file + * yet. + */ +#define MAX_DESC_SIZE 100*512 + +#define VMDK4_ACCESS_RW 1 +#define VMDK4_ACCESS_RDONLY 2 +#define VMDK4_ACCESS_NOACCESS 3 + +static int vmdk4_parse_desc_extent(BlockDriverState *bs, VMDK4Header *header, + int access_type, char *p) +{ + /* We should probably check that the monolithic file + * has only one extent. + */ + return 0; +} + +static int vmdk4_parse_desc_assignment(BlockDriverState *bs, + VMDK4Header *header, char *p, size_t len, + int64_t file_offset) +{ + BDRVVmdkState *s = bs->opaque; + + if (strncmp(p, "version=", 8) == 0 && strcmp(p, "version=1") != 0) + fprintf (stderr, + "warning: vmdk4_parse_desc: descriptor version != 1 (%s)\n", + p); + + if (!strncmp(p, "CID=", 4)) + s->cid_offset = file_offset + 4; + + if (!strncmp(p, "parentCID=", 10)) + s->parent_cid_offset = file_offset + 10; + + return 0; +} + +static int vmdk4_parse_desc(BlockDriverState *bs, VMDK4Header *header) +{ + BDRVVmdkState *s = bs->opaque; + int64_t desc_offset, desc_size; + char desc[MAX_DESC_SIZE]; + char *p, *pnext; + size_t len; + + s->cid_offset = 0; + s->parent_cid_offset = 0; + + /* Stored in the header are the offset/size in sectors. */ + desc_size = le64_to_cpu(header->desc_size) * SECTOR_SIZE; + desc_offset = le64_to_cpu(header->desc_offset) * SECTOR_SIZE; + + if (desc_size > MAX_DESC_SIZE) { + fprintf (stderr, + "vmdk4_parse_desc: descriptor size > maximum size " + "(%" PRIi64 " > %d)\n", desc_size, MAX_DESC_SIZE); + return -1; + } + + if (bdrv_pread(s->hd, desc_offset, (uint8_t *)desc, desc_size) != desc_size) { + fprintf (stderr, + "vmdk4_parse_desc: could not read descriptor from " + "%" PRIi64 " size %" PRIi64 "\n", + desc_offset, desc_size); + return -1; + } + + if (!strncmp(desc, "# Disk DescriptorFile", 22)) { + fprintf (stderr, "vmdk4_parse_desc: unrecognized descriptor"); + return -1; + } + + /* Parse the descriptor line by line. */ + for (p = desc; p && *p; p = pnext) { + pnext = strchr (p, '\n'); + if (pnext != NULL) { + *pnext = '\0'; + pnext++; + } + + while (*p && isspace (*p)) + p++; + while ((len = strlen (p)) > 0 && isspace (p[len-1])) + p[len-1] = '\0'; + + if (!*p) + continue; + if (p[0] == '#') + continue; + + /* Ignore any lines that we can't parse. */ + if (!strncmp(p, "RW ", 3)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_RW, p + 3); + else if (!strncmp(p, "RDONLY ", 7)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_RDONLY, p + 7); + else if (!strncmp(p, "NOACCESS ", 9)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_NOACCESS, p + 9); + else /* else expecting name = value */ + vmdk4_parse_desc_assignment (bs, header, p, len, + p-desc+desc_offset); + } + + return 0; +} + static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent) { BDRVVmdkState *s = bs->opaque; - char desc[DESC_SIZE]; + int64_t offset; + char str[9]; uint32_t cid; - const char *p_name, *cid_str; - size_t cid_str_size; - /* the descriptor offset = 0x200 */ - if (bdrv_pread(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) - return 0; + if (parent) + offset = s->parent_cid_offset; + else + offset = s->cid_offset; - if (parent) { - cid_str = "parentCID"; - cid_str_size = sizeof("parentCID"); - } else { - cid_str = "CID"; - cid_str_size = sizeof("CID"); - } + if (offset == 0) + return 0; - if ((p_name = strstr(desc,cid_str)) != NULL) { - p_name += cid_str_size; - sscanf(p_name,"%x",&cid); - } + if (bdrv_pread(s->hd, offset, str, 8) != 8) + return 0; + str[8] = '\0'; + sscanf(str, "%x", &cid); return cid; } static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid) { BDRVVmdkState *s = bs->opaque; - char desc[DESC_SIZE], tmp_desc[DESC_SIZE]; - char *p_name, *tmp_str; + int64_t offset; + char str[9]; - /* the descriptor offset = 0x200 */ - if (bdrv_pread(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) - return -1; + offset = s->cid_offset; + if (offset == 0) + return 0; - tmp_str = strstr(desc,"parentCID"); - pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str); - if ((p_name = strstr(desc,"CID")) != NULL) { - p_name += sizeof("CID"); - snprintf(p_name, sizeof(desc) - (p_name - desc), "%x\n", cid); - pstrcat(desc, sizeof(desc), tmp_desc); - } + snprintf(str, sizeof str, "%08x", cid); - if (bdrv_pwrite(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) + if (bdrv_pwrite(s->hd, offset, str, 8) != 8) return -1; + return 0; } @@ -184,6 +291,8 @@ static int vmdk_is_cid_valid(BlockDriverState *bs) return 1; } +#define DESC_SIZE 20*SECTOR_SIZE + static int vmdk_snapshot_create(const char *filename, const char *backing_file) { int snp_fd, p_fd; @@ -410,6 +519,9 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9; s->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9; + if (vmdk4_parse_desc(bs, &header) == -1) + goto fail; + if (parent_open) s->is_parent = 1; else -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field. 2009-12-18 15:56 [Qemu-devel] [PATCH 0/2] Small improvements to VMDK4 file handling Richard W.M. Jones 2009-12-18 15:57 ` [Qemu-devel] [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs Richard W.M. Jones @ 2009-12-18 15:58 ` Richard W.M. Jones 2009-12-18 16:07 ` Richard W.M. Jones 1 sibling, 1 reply; 5+ messages in thread From: Richard W.M. Jones @ 2009-12-18 15:58 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 260 bytes --] -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v [-- Attachment #2: 0002-VMDK4-Parse-and-check-the-createType-VMDK-file-type-.patch --] [-- Type: text/plain, Size: 2864 bytes --] >From bc09a7473b0382fbac7c3702b668acc1b04f1e9c Mon Sep 17 00:00:00 2001 From: Richard Jones <rjones@redhat.com> Date: Fri, 18 Dec 2009 15:36:17 +0000 Subject: [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field. Currently qemu fails to read VMDK subformats such as streamOptimized. It silently corrupts these files when it reads them. >From the embedded descriptor, parse the createType field and check it against a short list of subformats which are likely to work. --- block/vmdk.c | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 71680f0..c0b3160 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -81,6 +81,7 @@ typedef struct BDRVVmdkState { /* VMDK4 fields */ int64_t cid_offset; int64_t parent_cid_offset; + char *create_type; } BDRVVmdkState; typedef struct VmdkMetaData { @@ -161,6 +162,13 @@ static int vmdk4_parse_desc_assignment(BlockDriverState *bs, if (!strncmp(p, "parentCID=", 10)) s->parent_cid_offset = file_offset + 10; + if (strncmp(p, "createType=\"", 12) == 0) { + p[len-1] = '\0'; /* Remove trailing double quote. */ + if (s->create_type) + qemu_free(s->create_type); + s->create_type = qemu_strdup(p+12); + } + return 0; } @@ -174,6 +182,7 @@ static int vmdk4_parse_desc(BlockDriverState *bs, VMDK4Header *header) s->cid_offset = 0; s->parent_cid_offset = 0; + s->create_type = NULL; /* Stored in the header are the offset/size in sectors. */ desc_size = le64_to_cpu(header->desc_size) * SECTOR_SIZE; @@ -521,6 +530,16 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) if (vmdk4_parse_desc(bs, &header) == -1) goto fail; + if (s->create_type && + !(!strcmp(s->create_type, "monolithicSparse") || + !strcmp(s->create_type, "monolithicFlat") || + !strcmp(s->create_type, "twoGbMaxExtentSparse") || + !strcmp(s->create_type, "twoGbMaxExtentFlat"))) { + fprintf (stderr, "vmdk: disk type '%s' is not supported by qemu\n", + s->create_type); + goto fail; + } + if (parent_open) s->is_parent = 1; else @@ -559,6 +578,7 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) qemu_free(s->l1_backup_table); qemu_free(s->l1_table); qemu_free(s->l2_cache); + qemu_free(s->create_type); bdrv_delete(s->hd); return -1; } @@ -927,6 +947,7 @@ static void vmdk_close(BlockDriverState *bs) qemu_free(s->l1_table); qemu_free(s->l2_cache); + qemu_free(s->create_type); // try to close parent image, if exist vmdk_parent_close(s->hd); bdrv_delete(s->hd); -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field. 2009-12-18 15:58 ` [Qemu-devel] [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field Richard W.M. Jones @ 2009-12-18 16:07 ` Richard W.M. Jones 0 siblings, 0 replies; 5+ messages in thread From: Richard W.M. Jones @ 2009-12-18 16:07 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 330 bytes --] Add Signed-off-by line. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html [-- Attachment #2: 0002-VMDK4-Parse-and-check-the-createType-VMDK-file-type-.patch --] [-- Type: text/plain, Size: 2914 bytes --] >From 1d2352173501bef4f95d4a3c17566934eb5abbe4 Mon Sep 17 00:00:00 2001 From: Richard Jones <rjones@redhat.com> Date: Fri, 18 Dec 2009 15:36:17 +0000 Subject: [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field. Currently qemu fails to read VMDK subformats such as streamOptimized. It silently corrupts these files when it reads them. >From the embedded descriptor, parse the createType field and check it against a short list of subformats which are likely to work. Signed-off-by: Richard Jones <rjones@redhat.com> --- block/vmdk.c | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 0f59ea5..4cb126b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -81,6 +81,7 @@ typedef struct BDRVVmdkState { /* VMDK4 fields */ int64_t cid_offset; int64_t parent_cid_offset; + char *create_type; } BDRVVmdkState; typedef struct VmdkMetaData { @@ -161,6 +162,13 @@ static int vmdk4_parse_desc_assignment(BlockDriverState *bs, if (!strncmp(p, "parentCID=", 10)) s->parent_cid_offset = file_offset + 10; + if (strncmp(p, "createType=\"", 12) == 0) { + p[len-1] = '\0'; /* Remove trailing double quote. */ + if (s->create_type) + qemu_free(s->create_type); + s->create_type = qemu_strdup(p+12); + } + return 0; } @@ -174,6 +182,7 @@ static int vmdk4_parse_desc(BlockDriverState *bs, VMDK4Header *header) s->cid_offset = 0; s->parent_cid_offset = 0; + s->create_type = NULL; /* Stored in the header are the offset/size in sectors. */ desc_size = le64_to_cpu(header->desc_size) * SECTOR_SIZE; @@ -522,6 +531,16 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) if (vmdk4_parse_desc(bs, &header) == -1) goto fail; + if (s->create_type && + !(!strcmp(s->create_type, "monolithicSparse") || + !strcmp(s->create_type, "monolithicFlat") || + !strcmp(s->create_type, "twoGbMaxExtentSparse") || + !strcmp(s->create_type, "twoGbMaxExtentFlat"))) { + fprintf (stderr, "vmdk: disk type '%s' is not supported by qemu\n", + s->create_type); + goto fail; + } + if (parent_open) s->is_parent = 1; else @@ -560,6 +579,7 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) qemu_free(s->l1_backup_table); qemu_free(s->l1_table); qemu_free(s->l2_cache); + qemu_free(s->create_type); bdrv_delete(s->hd); return -1; } @@ -928,6 +948,7 @@ static void vmdk_close(BlockDriverState *bs) qemu_free(s->l1_table); qemu_free(s->l2_cache); + qemu_free(s->create_type); // try to close parent image, if exist vmdk_parent_close(s->hd); bdrv_delete(s->hd); -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-12-18 16:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-18 15:56 [Qemu-devel] [PATCH 0/2] Small improvements to VMDK4 file handling Richard W.M. Jones 2009-12-18 15:57 ` [Qemu-devel] [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs Richard W.M. Jones 2009-12-18 16:06 ` Richard W.M. Jones 2009-12-18 15:58 ` [Qemu-devel] [PATCH 2/2] VMDK4: Parse and check the createType (VMDK file type) field Richard W.M. Jones 2009-12-18 16:07 ` Richard W.M. Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).