From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40560) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6Nxk-0007VD-2E for qemu-devel@nongnu.org; Thu, 23 Jan 2014 12:21:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W6Nxe-0003xb-Lo for qemu-devel@nongnu.org; Thu, 23 Jan 2014 12:21:08 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:48157) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6Nxe-0003wu-9t for qemu-devel@nongnu.org; Thu, 23 Jan 2014 12:21:02 -0500 Date: Thu, 23 Jan 2014 18:20:59 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140123172059.GE3519@irqsave.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 3/4] qcow2: check for NULL l2meta List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hu Tao Cc: Kevin Wolf , qemu-devel@nongnu.org Le Thursday 23 Jan 2014 =E0 11:04:07 (+0800), Hu Tao a =E9crit : > In case of do preallocating metadata with a large cluster size, In the case of a metadata preallocation with > qcow2_alloc_cluster_offset() can allocate nothing and returns > a NULL l2meta. This patch checks for it and link2 l2 with only > valid l2meta. >=20 > Replace 9 and 512 with BDRV_SECTOR_BITS, BDRV_SECTOR_SIZE > respectively while at the function. >=20 > Reviewed-by: Max Reitz > Signed-off-by: Hu Tao > --- > block/qcow2.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) >=20 > diff --git a/block/qcow2.c b/block/qcow2.c > index 0a310cc..f989247 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1390,22 +1390,24 @@ static int preallocate(BlockDriverState *bs) > int ret; > QCowL2Meta *meta; > =20 > - nb_sectors =3D bdrv_getlength(bs) >> 9; > + nb_sectors =3D bdrv_getlength(bs) >> BDRV_SECTOR_BITS; > offset =3D 0; > =20 > while (nb_sectors) { > - num =3D MIN(nb_sectors, INT_MAX >> 9); > + num =3D MIN(nb_sectors, INT_MAX >> BDRV_SECTOR_BITS); > ret =3D qcow2_alloc_cluster_offset(bs, offset, &num, > &host_offset, &meta); > if (ret < 0) { > return ret; > } > =20 > - ret =3D qcow2_alloc_cluster_link_l2(bs, meta); > - if (ret < 0) { > - qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_c= lusters, > - QCOW2_DISCARD_NEVER); > - return ret; > + if (meta) { > + ret =3D qcow2_alloc_cluster_link_l2(bs, meta); > + if (ret < 0) { > + qcow2_free_any_clusters(bs, meta->alloc_offset, > + meta->nb_clusters, QCOW2_DISCA= RD_NEVER); > + return ret; > + } > } Maybe if (meta) could include the following line to remove another extra = test. QLIST_REMOVE(meta, next_in_flight); > =20 > /* There are no dependent requests, but we need to remove our = request > @@ -1417,7 +1419,7 @@ static int preallocate(BlockDriverState *bs) > /* TODO Preallocate data if requested */ > =20 > nb_sectors -=3D num; > - offset +=3D num << 9; > + offset +=3D num << BDRV_SECTOR_BITS; > } > =20 > /* > @@ -1426,9 +1428,10 @@ static int preallocate(BlockDriverState *bs) > * EOF). Extend the image to the last allocated sector. > */ > if (host_offset !=3D 0) { > - uint8_t buf[512]; > - memset(buf, 0, 512); > - ret =3D bdrv_write(bs->file, (host_offset >> 9) + num - 1, buf= , 1); > + uint8_t buf[BDRV_SECTOR_SIZE]; > + memset(buf, 0, BDRV_SECTOR_SIZE); > + ret =3D bdrv_write(bs->file, (host_offset >> BDRV_SECTOR_BITS)= + num - 1, > + buf, 1); > if (ret < 0) { > return ret; > } > --=20 > 1.8.5.2.229.g4448466 >=20 >=20