From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WspwJ-0007rB-H1 for qemu-devel@nongnu.org; Fri, 06 Jun 2014 04:56:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WspwA-0007eC-0C for qemu-devel@nongnu.org; Fri, 06 Jun 2014 04:55:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64568) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wspw9-0007dV-OJ for qemu-devel@nongnu.org; Fri, 06 Jun 2014 04:55:45 -0400 Date: Fri, 6 Jun 2014 10:55:37 +0200 From: Kevin Wolf Message-ID: <20140606085537.GA4873@noname.redhat.com> References: <1401975393-7255-1-git-send-email-kwolf@redhat.com> <1401975393-7255-12-git-send-email-kwolf@redhat.com> <20140605145246.GB6287@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140605145246.GB6287@irqsave.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 11/21] qcow2: Handle failure for potentially large allocations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: qemu-devel@nongnu.org, stefanha@redhat.com Am 05.06.2014 um 16:52 hat Beno=EEt Canet geschrieben: > The Thursday 05 Jun 2014 =E0 15:36:23 (+0200), Kevin Wolf wrote : > > Some code in the block layer makes potentially huge allocations. Fail= ure > > is not completely unexpected there, so avoid aborting qemu and handle > > out-of-memory situations gracefully. > >=20 > > This patch addresses the allocations in the qcow2 block driver. > >=20 > > Signed-off-by: Kevin Wolf > > Reviewed-by: Stefan Hajnoczi > > --- > > block/qcow2-cache.c | 13 ++++++++++++- > > block/qcow2-cluster.c | 35 +++++++++++++++++++++++++++-------- > > block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++--= -------- > > block/qcow2-snapshot.c | 22 +++++++++++++++++----- > > block/qcow2.c | 41 +++++++++++++++++++++++++++++++++-------= - > > 5 files changed, 127 insertions(+), 32 deletions(-) > > @@ -1562,7 +1574,10 @@ static int expand_zero_clusters_in_l1(BlockDri= verState *bs, uint64_t *l1_table, > > if (!is_active_l1) { > > /* inactive L2 tables require a buffer to be stored in when = loading > > * them from disk */ > > - l2_table =3D qemu_blockalign(bs, s->cluster_size); > > + l2_table =3D qemu_try_blockalign(bs, s->cluster_size); > > + if (l2_table =3D=3D NULL) { > > + return -ENOMEM; > > + } > > } > > =20 > > for (i =3D 0; i < l1_size; i++) { > > @@ -1740,7 +1755,11 @@ int qcow2_expand_zero_clusters(BlockDriverStat= e *bs) > > =20 > > nb_clusters =3D size_to_clusters(s, bs->file->total_sectors * > > BDRV_SECTOR_SIZE); > I think we need asset(nb_clusters >=3D 1); else it will trigger a spuri= ous -ENOMEM > on very small files. To be precise, very small means size 0, because size_to_clusters() rounds up. An image file with size 0 wouldn't have a qcow2 header, though, so we wouldn't even have opened it in the first place. (Also, an assert() never fixes a bug. At best, it changes its symptom to a crash, except when built with NDEBUG defined.) > > - expanded_clusters =3D g_malloc0((nb_clusters + 7) / 8); > > + expanded_clusters =3D g_try_malloc0((nb_clusters + 7) / 8); > > + if (expanded_clusters =3D=3D NULL) { > > + ret =3D -ENOMEM; > > + goto fail; > > + } > > =20 > > ret =3D expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size, > > &expanded_clusters, &nb_cluster= s); Kevin