Re: [Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image

2011-07-08 Thread Stefan Hajnoczi
On Tue, Jul 5, 2011 at 12:31 PM, Fam Zheng  wrote:
> +        ret = sscanf(p, "%10s %lld %10s %512s",
[...]
> +            ret = sscanf(p, "%10s %lld %10s %511s %lld",

%512s -> %511s

But instead of duplicating the format string and sscanf(3), I suggest
doing sscanf(p, "%10s %lld %10s %511s %lld", ...) once only.

After it returns you can check:
if (ret < 4) {
...fail...
} else if (!strcmp(access, "FLAT")) {
if (ret != 5 || flat_offset < 0) {
...fail...
}
} else {
if (ret != 4) {
...fail...
}
}

Stefan



[Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image

2011-07-05 Thread Fam Zheng
Parse vmdk decriptor file and open mono flat image.
Read/write the flat extent.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  172 +-
 1 files changed, 159 insertions(+), 13 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index f637d98..2183ace 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -65,6 +65,7 @@ typedef struct VmdkExtent {
 bool flat;
 int64_t sectors;
 int64_t end_sector;
+int64_t flat_start_offset;
 int64_t l1_table_offset;
 int64_t l1_backup_table_offset;
 uint32_t *l1_table;
@@ -407,9 +408,10 @@ fail:
 static int vmdk_parent_open(BlockDriverState *bs)
 {
 char *p_name;
-char desc[DESC_SIZE];
+char desc[DESC_SIZE + 1];
 BDRVVmdkState *s = bs->opaque;
 
+desc[DESC_SIZE] = '\0';
 if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
 return -1;
 }
@@ -584,6 +586,145 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int 
flags)
 return ret;
 }
 
+/* find an option value out of descriptor file */
+static int vmdk_parse_description(const char *desc, const char *opt_name,
+char *buf, int buf_size)
+{
+char *opt_pos, *opt_end;
+const char *end = desc + strlen(desc);
+
+opt_pos = strstr(desc, opt_name);
+if (!opt_pos) {
+return -1;
+}
+/* Skip "=\"" following opt_name */
+opt_pos += strlen(opt_name) + 2;
+if (opt_pos >= end) {
+return -1;
+}
+opt_end = opt_pos;
+while (opt_end < end && *opt_end != '"') {
+opt_end++;
+}
+if (opt_end == end || buf_size < opt_end - opt_pos + 1) {
+return -1;
+}
+pstrcpy(buf, opt_end - opt_pos + 1, opt_pos);
+return 0;
+}
+
+static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
+const char *desc_file_path)
+{
+int ret;
+char access[11];
+char type[11];
+char fname[512];
+const char *p = desc;
+int64_t sectors = 0;
+int64_t flat_offset;
+
+while (*p) {
+/* parse extent line:
+ * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
+ * or
+ * RW [size in sectors] SPARSE "file-name.vmdk"
+ */
+flat_offset = -1;
+ret = sscanf(p, "%10s %lld %10s %512s",
+access, §ors, type, fname);
+if (ret != 4) {
+goto next_line;
+}
+if (!strcmp(type, "FLAT")) {
+ret = sscanf(p, "%10s %lld %10s %511s %lld",
+access, §ors, type, fname, &flat_offset);
+if (ret != 5 || flat_offset < 0) {
+return -EINVAL;
+}
+}
+
+/* trim the quotation marks around */
+if (fname[0] == '"') {
+memmove(fname, fname + 1, strlen(fname));
+if (strlen(fname) <= 1 || fname[strlen(fname) - 1] != '"') {
+return -EINVAL;
+}
+fname[strlen(fname) - 1] = '\0';
+}
+if (sectors <= 0 ||
+(strcmp(type, "FLAT") && strcmp(type, "SPARSE")) ||
+(strcmp(access, "RW"))) {
+goto next_line;
+}
+
+/* save to extents array */
+if (!strcmp(type, "FLAT")) {
+/* FLAT extent */
+char extent_path[PATH_MAX];
+BlockDriverState *extent_file;
+VmdkExtent *extent;
+
+path_combine(extent_path, sizeof(extent_path),
+desc_file_path, fname);
+ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags);
+if (ret) {
+return ret;
+}
+extent = vmdk_add_extent(bs, extent_file, true, sectors,
+0, 0, 0, 0, sectors);
+extent->flat_start_offset = flat_offset;
+} else {
+/* SPARSE extent, not supported for now */
+fprintf(stderr,
+"VMDK: Not supported extent type \"%s\""".\n", type);
+return -ENOTSUP;
+}
+next_line:
+/* move to next line */
+while (*p && *p != '\n') {
+p++;
+}
+p++;
+}
+return 0;
+}
+
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
+{
+int ret;
+char buf[2048];
+char ct[128];
+BDRVVmdkState *s = bs->opaque;
+
+ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
+if (ret < 0) {
+return ret;
+}
+buf[2047] = '\0';
+if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
+return -EINVAL;
+}
+if (strcmp(ct, "monolithicFlat")) {
+fprintf(stderr,
+"VMDK: Not supported image type \"%s\""".\n", ct);
+return -ENOTSUP;
+}
+s->desc_offset = 0;
+ret = vmdk_parse_extents(buf, bs, bs->file->filename);
+if (ret) {
+return ret;
+}
+
+/* try to open parent images, if exist */
+if (vmdk_parent_open(bs)) {
+qemu_free(s->extents);
+return -EINVAL;
+}
+s