From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34914) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVAh2-0000G0-BX for qemu-devel@nongnu.org; Tue, 01 Apr 2014 22:14:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WVAgw-0001PF-5F for qemu-devel@nongnu.org; Tue, 01 Apr 2014 22:14:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26232) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVAgv-0001P1-Ss for qemu-devel@nongnu.org; Tue, 01 Apr 2014 22:14:14 -0400 Date: Wed, 2 Apr 2014 10:14:19 +0800 From: Fam Zheng Message-ID: <20140402021419.GA16128@T430.nay.redhat.com> References: <20140401214954.GA2902@smaugslair> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140401214954.GA2902@smaugslair> Subject: Re: [Qemu-devel] [PATCH v2] Fix for qemu-img info to supply FORMAT values for SPARSE extents List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Shwetha Mathangi Chandra Choodamani Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On Tue, 04/01 17:49, Shwetha Mathangi Chandra Choodamani wrote: > This patch fixes the bug in qemu-img info that wouldn't populate the extent type for default formats. > The extent type has now been set where necessary. This is the second version in the series after inputs > from Fam Zheng(famz@redhat.com). > > Signed-off-by: Shwetha Mathangi Chandra Choodamani > --- > block/vmdk.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 49 insertions(+), 4 deletions(-) > > diff --git a/block/vmdk.c b/block/vmdk.c > index b69988d..d4a37ad 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -499,6 +499,18 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, > VMDK3Header header; > VmdkExtent *extent; > IIRC VMFS always comes with a description file, so we won't miss the extent type and this part is not necessary. > + char access[11]; > + char type[11]; > + char fname[512]; > + int64_t sectors = 0; > + int64_t flat_offset; > + int64_t size; > + size = bdrv_getlength(file); > + char *buf; > + buf = g_malloc0(size + 1); > + bdrv_pread(file, sizeof(magic), buf, size); > + sscanf(buf, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64, > + access, §ors, type, fname, &flat_offset); > ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); > if (ret < 0) { > error_setg_errno(errp, -ret, > @@ -515,6 +527,18 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, > le32_to_cpu(header.granularity), > &extent, > errp); > + while (strcmp(access, "RW")) { > + while (*buf) { > + sscanf(buf, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64, > + access, §ors, type, fname, &flat_offset); > + if (*buf == '\n') { > + buf++; > + break; > + } > + buf++; > + } > + } > + extent->type = g_strdup(type); > if (ret < 0) { > return ret; > } > @@ -523,6 +547,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, > /* free extent allocated by vmdk_add_extent */ > vmdk_free_last_extent(bs); > } > + g_free(buf); > return ret; > } > > @@ -566,7 +591,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, > VmdkExtent *extent; > BDRVVmdkState *s = bs->opaque; > int64_t l1_backup_offset = 0; > - > + char access[11]; > + char type[11]; > + char fname[512]; > + int64_t sectors = 0; > + int64_t flat_offset; > ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); > if (ret < 0) { > error_setg_errno(errp, -ret, > @@ -586,11 +615,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, > return ret; > } > } > - Unnecessary blank line change. > if (!s->create_type) { > s->create_type = g_strdup("monolithicSparse"); > } > - Same here. > if (le64_to_cpu(header.gd_offset) == VMDK4_GD_AT_END) { > /* > * The footer takes precedence over the header, so read it in. The > @@ -694,6 +721,25 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, > g_free(s->create_type); > s->create_type = g_strdup("streamOptimized"); > } > + > + if (!extent->type) { > + uint64_t desc_offset = le64_to_cpu(header.desc_offset); > + char *buf = vmdk_read_desc(file, desc_offset<<9, errp); > + sscanf(buf, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64, > + access, §ors, type, fname, &flat_offset); > + while (strcmp(access, "RW")) { > + while (*buf) { > + sscanf(buf, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64, > + access, §ors, type, fname, &flat_offset); > + if (*buf == '\n') { > + buf++; > + break; > + } > + buf++; > + } > + } > + } No. Why do you need to read the description? Here we already know the extent type by looking at s->create_type, it would be "monolithicSparse" or "streamOptimized" in the cases that we are trying to fix in this patch, so no need to parse the description text. Just set extent->type according to s->create_type. > + extent->type = g_strdup(type); > extent->has_marker = le32_to_cpu(header.flags) & VMDK4_FLAG_MARKER; > extent->version = le32_to_cpu(header.version); > extent->has_zero_grain = le32_to_cpu(header.flags) & VMDK4_FLAG_ZERO_GRAIN; > @@ -711,7 +757,6 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, > { > char *opt_pos, *opt_end; > const char *end = desc + strlen(desc); > - Blank line change, please drop it. Fam > opt_pos = strstr(desc, opt_name); > if (!opt_pos) { > return VMDK_ERROR; > -- > 1.7.9.5 > >