From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41649) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsZDl-000766-4J for qemu-devel@nongnu.org; Thu, 05 Jun 2014 11:04:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsZDf-00030O-6Z for qemu-devel@nongnu.org; Thu, 05 Jun 2014 11:04:49 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:46246 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsZDe-000308-U1 for qemu-devel@nongnu.org; Thu, 05 Jun 2014 11:04:43 -0400 Date: Thu, 5 Jun 2014 17:04:39 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140605150439.GB6430@irqsave.net> References: <1401975393-7255-1-git-send-email-kwolf@redhat.com> <1401975393-7255-11-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1401975393-7255-11-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 10/21] qcow1: Handle failure for potentially large allocations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: benoit.canet@irqsave.net, qemu-devel@nongnu.org, stefanha@redhat.com The Thursday 05 Jun 2014 =E0 15:36:22 (+0200), Kevin Wolf wrote : > Some code in the block layer makes potentially huge allocations. Failur= e > is not completely unexpected there, so avoid aborting qemu and handle > out-of-memory situations gracefully. >=20 > This patch addresses the allocations in the qcow1 block driver. >=20 > Signed-off-by: Kevin Wolf > Reviewed-by: Stefan Hajnoczi > --- > block/qcow.c | 33 ++++++++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 7 deletions(-) >=20 > diff --git a/block/qcow.c b/block/qcow.c > index 7fd57d7..31db585 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -182,7 +182,12 @@ static int qcow_open(BlockDriverState *bs, QDict *= options, int flags, > } > =20 > s->l1_table_offset =3D header.l1_table_offset; > - s->l1_table =3D g_malloc(s->l1_size * sizeof(uint64_t)); > + s->l1_table =3D g_try_malloc(s->l1_size * sizeof(uint64_t)); > + if (s->l1_table =3D=3D NULL) { > + error_setg(errp, "Could not allocate memory for L1 table"); > + ret =3D -ENOMEM; > + goto fail; > + } I notice we don't sett errp on the above bdrv_pread failure. > =20 > ret =3D bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, > s->l1_size * sizeof(uint64_t)); > @@ -193,8 +198,16 @@ static int qcow_open(BlockDriverState *bs, QDict *= options, int flags, > for(i =3D 0;i < s->l1_size; i++) { > be64_to_cpus(&s->l1_table[i]); > } > - /* alloc L2 cache */ > - s->l2_cache =3D g_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint6= 4_t)); > + > + /* alloc L2 cache (max. 64k * 16 * 8 =3D 8 MB) */ > + s->l2_cache =3D > + qemu_try_blockalign(bs->file, > + s->l2_size * L2_CACHE_SIZE * sizeof(uint64= _t)); > + if (s->l2_cache =3D=3D NULL) { > + error_setg(errp, "Could not allocate L2 table cache"); > + ret =3D -ENOMEM; > + goto fail; > + } > s->cluster_cache =3D g_malloc(s->cluster_size); > s->cluster_data =3D g_malloc(s->cluster_size); > s->cluster_cache_offset =3D -1; > @@ -226,7 +239,7 @@ static int qcow_open(BlockDriverState *bs, QDict *o= ptions, int flags, > =20 > fail: o> g_free(s->l1_table); > - g_free(s->l2_cache); > + qemu_vfree(s->l2_cache); > g_free(s->cluster_cache); > g_free(s->cluster_data); > return ret; > @@ -517,7 +530,10 @@ static coroutine_fn int qcow_co_readv(BlockDriverS= tate *bs, int64_t sector_num, > void *orig_buf; > =20 > if (qiov->niov > 1) { > - buf =3D orig_buf =3D qemu_blockalign(bs, qiov->size); > + buf =3D orig_buf =3D qemu_try_blockalign(bs, qiov->size); > + if (buf =3D=3D NULL) { > + return -ENOMEM; > + } > } else { > orig_buf =3D NULL; > buf =3D (uint8_t *)qiov->iov->iov_base; > @@ -619,7 +635,10 @@ static coroutine_fn int qcow_co_writev(BlockDriver= State *bs, int64_t sector_num, > s->cluster_cache_offset =3D -1; /* disable compressed cache */ > =20 > if (qiov->niov > 1) { > - buf =3D orig_buf =3D qemu_blockalign(bs, qiov->size); > + buf =3D orig_buf =3D qemu_try_blockalign(bs, qiov->size); > + if (buf =3D=3D NULL) { > + return -ENOMEM; > + } > qemu_iovec_to_buf(qiov, 0, buf, qiov->size); > } else { > orig_buf =3D NULL; > @@ -685,7 +704,7 @@ static void qcow_close(BlockDriverState *bs) > BDRVQcowState *s =3D bs->opaque; > =20 > g_free(s->l1_table); > - g_free(s->l2_cache); > + qemu_vfree(s->l2_cache); > g_free(s->cluster_cache); > g_free(s->cluster_data); > =20 > --=20 > 1.8.3.1 >=20 >=20 Reviewed-by: Benoit Canet