From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:35472) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QXw3s-0003Da-81 for qemu-devel@nongnu.org; Sat, 18 Jun 2011 09:59:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QXw3q-0005Xf-O9 for qemu-devel@nongnu.org; Sat, 18 Jun 2011 09:59:43 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:33431) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QXw3q-0005Xa-GN for qemu-devel@nongnu.org; Sat, 18 Jun 2011 09:59:42 -0400 Received: by gxk26 with SMTP id 26so2150015gxk.4 for ; Sat, 18 Jun 2011 06:59:41 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: Date: Sat, 18 Jun 2011 14:59:41 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 04/12] VMDK: separate vmdk_open by format version List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , qemu-devel@nongnu.org, Christoph Hellwig On Sat, Jun 4, 2011 at 1:41 AM, Fam Zheng wrote: > +static int vmdk_open_vmdk4(BlockDriverState *bs, int flags) > +{ > + =A0 =A0uint32_t magic; > + =A0 =A0VMDK4Header header; > + =A0 =A0BDRVVmdkState *s =3D bs->opaque; > + =A0 =A0VmdkExtent *extent; > + > + =A0 =A0s->extents =3D qemu_mallocz(sizeof(VmdkExtent)); > + =A0 =A0s->num_extents =3D 1; > + =A0 =A0if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) > + =A0 =A0 =A0 =A0!=3D sizeof(header)) { > + =A0 =A0 =A0 =A0goto fail; > + =A0 =A0} > + =A0 =A0extent =3D s->extents; > + =A0 =A0extent->file =3D bs->file; > + =A0 =A0extent->sectors =3D le64_to_cpu(header.capacity); > + =A0 =A0extent->cluster_sectors =3D le64_to_cpu(header.granularity); > + =A0 =A0extent->l2_size =3D le32_to_cpu(header.num_gtes_per_gte); > + =A0 =A0extent->l1_entry_sectors =3D extent->l2_size * extent->cluster_s= ectors; > + =A0 =A0if (extent->l1_entry_sectors <=3D 0) { > + =A0 =A0 =A0 =A0goto fail; > + =A0 =A0} > + =A0 =A0extent->l1_size =3D (extent->sectors + extent->l1_entry_sectors = - 1) > + =A0 =A0 =A0 =A0/ extent->l1_entry_sectors; > + =A0 =A0extent->l1_table_offset =3D le64_to_cpu(header.rgd_offset) << 9; > + =A0 =A0extent->l1_backup_table_offset =3D le64_to_cpu(header.gd_offset)= << 9; > + > + =A0 =A0// try to open parent images, if exist > + =A0 =A0if (vmdk_parent_open(bs) !=3D 0) { > + =A0 =A0 =A0 =A0goto fail; > + =A0 =A0} > + =A0 =A0// write the CID once after the image creation This comment is from vmdk_create() and can be removed? > + =A0 =A0extent->parent_cid =3D vmdk_read_cid(bs,1); > + =A0 =A0bs->total_sectors =3D extent->sectors; > + =A0 =A0if (vmdk_init_tables(bs, extent)) { > + =A0 =A0 =A0 =A0goto fail; > =A0 =A0 } > + =A0 =A0return 0; > + fail: > + =A0 =A0qemu_free(s->extents); > =A0 =A0 return -1; > =A0} > > +static int vmdk_open(BlockDriverState *bs, int flags) > +{ > + =A0 =A0uint32_t magic; > + > + =A0 =A0if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) !=3D sizeof(m= agic)) { > + =A0 =A0 =A0 =A0return -1; > + =A0 =A0} > + > + =A0 =A0magic =3D be32_to_cpu(magic); > + =A0 =A0if (magic =3D=3D VMDK3_MAGIC) { > + =A0 =A0 =A0 =A0return vmdk_open_vmdk3(bs, flags); > + =A0 =A0} else if (magic =3D=3D VMDK4_MAGIC) { > + =A0 =A0 =A0 =A0return vmdk_open_vmdk4(bs, flags); > + =A0 =A0} else { > + =A0 =A0 =A0 =A0return -1; > + =A0 =A0} This is a good opportunity to use -errno instead of -1. If you look at qcow2 or qed they return appropriate error numbers and not just -1. (Also remember -1 is -EPERM so callers that interpret the return value as an error will print junk error messages if -1 is returned.) Stefan