From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47366) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qb3nZ-0007Vq-Oy for qemu-devel@nongnu.org; Mon, 27 Jun 2011 00:51:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qb3nX-000124-QI for qemu-devel@nongnu.org; Mon, 27 Jun 2011 00:51:49 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:53947) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qb3nX-00011w-AX for qemu-devel@nongnu.org; Mon, 27 Jun 2011 00:51:47 -0400 Received: by gxk26 with SMTP id 26so2264846gxk.4 for ; Sun, 26 Jun 2011 21:51:46 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1309146518-8998-5-git-send-email-famcool@gmail.com> References: <1309146518-8998-1-git-send-email-famcool@gmail.com> <1309146518-8998-5-git-send-email-famcool@gmail.com> Date: Mon, 27 Jun 2011 05:51:46 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 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: kwolf@redhat.com, qemu-devel@nongnu.org, hch@lst.de On Mon, Jun 27, 2011 at 4:48 AM, Fam Zheng wrote: > - =A0 =A0 =A0 =A0extent =3D s->extents; > - =A0 =A0 =A0 =A0extent->flat =3D false; > - =A0 =A0 =A0 =A0extent->file =3D bs->file; > - =A0 =A0 =A0 =A0extent->cluster_sectors =3D le32_to_cpu(header.granulari= ty); > - =A0 =A0 =A0 =A0extent->l2_size =3D 1 << 9; > - =A0 =A0 =A0 =A0extent->l1_size =3D 1 << 6; > - =A0 =A0 =A0 =A0extent->sectors =3D le32_to_cpu(header.disk_sectors); > - =A0 =A0 =A0 =A0extent->end_sector =3D le32_to_cpu(header.disk_sectors); > - =A0 =A0 =A0 =A0extent->l1_table_offset =3D le32_to_cpu(header.l1dir_off= set) << 9; > - =A0 =A0 =A0 =A0extent->l1_backup_table_offset =3D 0; > - =A0 =A0 =A0 =A0extent->l1_entry_sectors =3D extent->l2_size * extent->c= luster_sectors; Please move vmdk_add_extent() to the patch where you added this code. A nice patch series minimizes the amount of code that gets added temporarily and then removed again. > +/* Create and append extent to the entext array. Return the added VmdkEx= tent s/entext/extent/ > + * address. return NULL if allocation failed. */ > +static int vmdk_add_extent(BlockDriverState *bs, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BlockDriverState *f= ile, bool flat, int64_t sectors, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int64_t l1_offset, = int64_t l1_backup_offset, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 uint32_t l1_size, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int l2_size, unsign= ed int cluster_sectors, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 VmdkExtent **new_ex= tent) > +{ > + =A0 =A0VmdkExtent *extent, *p; > + =A0 =A0BDRVVmdkState *s =3D bs->opaque; > + > + =A0 =A0p =3D qemu_realloc(s->extents, (s->num_extents + 1) * sizeof(Vmd= kExtent)); > + =A0 =A0if (!p) { > + =A0 =A0 =A0 =A0return -ENOMEM; > + =A0 =A0} qemu_realloc() never returns NULL. > + =A0 =A0s->extents =3D p; > + =A0 =A0extent =3D &s->extents[s->num_extents]; > + =A0 =A0s->num_extents++; > + > + =A0 =A0memset(extent, 0, sizeof(VmdkExtent)); > + =A0 =A0extent->file =3D file; > + =A0 =A0extent->flat =3D flat; > + =A0 =A0extent->sectors =3D sectors; > + =A0 =A0extent->l1_table_offset =3D l1_offset; > + =A0 =A0extent->l1_backup_table_offset =3D l1_backup_offset; > + =A0 =A0extent->l1_size =3D l1_size; > + =A0 =A0extent->l1_entry_sectors =3D l2_size * cluster_sectors; > + =A0 =A0extent->l2_size =3D l2_size; > + =A0 =A0extent->cluster_sectors =3D cluster_sectors; > + > + =A0 =A0if (s->num_extents > 1) { > + =A0 =A0 =A0 =A0extent->end_sector =3D (*(extent - 1)).end_sector + exte= nt->sectors; > + =A0 =A0} else { > + =A0 =A0 =A0 =A0extent->end_sector =3D extent->sectors; > + =A0 =A0} > + =A0 =A0bs->total_sectors =3D extent->end_sector; > + =A0 =A0if (new_extent) { > + =A0 =A0 =A0 =A0*new_extent =3D extent; > + =A0 =A0} > + =A0 =A0return 0; Since this function cannot fail it would be simpler to make the return value VmdkExtent *. Stefan