From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4v2s-0003UC-PD for qemu-devel@nongnu.org; Sun, 19 Jan 2014 11:16:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W4v2o-0001GZ-5a for qemu-devel@nongnu.org; Sun, 19 Jan 2014 11:16:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:23532) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4v2n-0001GV-SB for qemu-devel@nongnu.org; Sun, 19 Jan 2014 11:16:18 -0500 Message-ID: <52DBFABD.7010907@redhat.com> Date: Sun, 19 Jan 2014 17:18:05 +0100 From: Max Reitz MIME-Version: 1.0 References: <33b97a2f2e4860e69e658143d1e63927fa12a92e.1388381026.git.hutao@cn.fujitsu.com> In-Reply-To: <33b97a2f2e4860e69e658143d1e63927fa12a92e.1388381026.git.hutao@cn.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 3/3] qcow2: check for NULL l2meta List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hu Tao , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi On 30.12.2013 06:29, Hu Tao wrote: > In case of do preallocating metadata with a large cluster size, > qcow2_alloc_cluster_offset() can allocate nothing and returns > a NULL l2meta. This patch checks for it and link2 l2 with only > valid l2meta. > > Signed-off-by: Hu Tao > --- > block/qcow2.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 46860d5..380c240 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1399,18 +1399,20 @@ static int preallocate(BlockDriverState *bs) > offset = 0; > > while (nb_sectors) { > - num = MIN(nb_sectors, INT_MAX >> 9); > + num = MIN(nb_sectors, INT_MAX >> BDRV_SECTOR_BITS); Well, if you're already adjusting this here, you could also replace the other occurrences of 9 and 512 in this function. ;-) > ret = qcow2_alloc_cluster_offset(bs, offset, &num, > &host_offset, &meta); > if (ret < 0) { > return ret; > } > > - ret = qcow2_alloc_cluster_link_l2(bs, meta); > - if (ret < 0) { > - qcow2_free_any_clusters(bs, meta->alloc_offset, meta->nb_clusters, > - QCOW2_DISCARD_NEVER); > - return ret; > + if (meta) { > + ret = qcow2_alloc_cluster_link_l2(bs, meta); > + if (ret < 0) { > + qcow2_free_any_clusters(bs, meta->alloc_offset, > + meta->nb_clusters, QCOW2_DISCARD_NEVER); > + return ret; > + } > } > > /* There are no dependent requests, but we need to remove our request But this doesn't make this patch wrong, so: Reviewed-by: Max Reitz