From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43131)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@redhat.com>) id 1WnVX8-0002SK-TC
	for qemu-devel@nongnu.org; Thu, 22 May 2014 12:08:02 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@redhat.com>) id 1WnVX1-0005Tr-Dm
	for qemu-devel@nongnu.org; Thu, 22 May 2014 12:07:54 -0400
Received: from mx1.redhat.com ([209.132.183.28]:30977)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@redhat.com>) id 1WnVX1-0005Tm-6A
	for qemu-devel@nongnu.org; Thu, 22 May 2014 12:07:47 -0400
Received: from int-mx02.intmail.prod.int.phx2.redhat.com
	(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4MG7krp023841
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Thu, 22 May 2014 12:07:46 -0400
Date: Thu, 22 May 2014 18:07:44 +0200
From: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20140522160744.GM21238@stefanha-thinkpad.redhat.com>
References: <1400689698-3096-1-git-send-email-kwolf@redhat.com>
	<1400689698-3096-12-git-send-email-kwolf@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1400689698-3096-12-git-send-email-kwolf@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 11/20] qcow2: Handle failure for
 potentially large allocations
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org

On Wed, May 21, 2014 at 06:28:09PM +0200, Kevin Wolf wrote:
> diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> index 8ecbb5b..465ef24 100644
> --- a/block/qcow2-cache.c
> +++ b/block/qcow2-cache.c
> @@ -53,10 +53,20 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
>      c->entries = g_malloc0(sizeof(*c->entries) * num_tables);
>  
>      for (i = 0; i < c->size; i++) {
> -        c->entries[i].table = qemu_blockalign(bs, s->cluster_size);
> +        c->entries[i].table = qemu_try_blockalign(bs, s->cluster_size);
> +        if (c->entries[i].table == NULL) {
> +            goto fail;
> +        }
>      }
>  
>      return c;
> +
> +fail:
> +    for (i = 0; i < c->size; i++) {
> +        g_free(c->entries[i].table);

qemu_blockalign() must be paired with qemu_vfree().

Come to think of it, in the patches where you converted malloc to
blockalign, please check that the buffer is vfreed.

> diff --git a/block/qcow2.c b/block/qcow2.c
> index a4b97e8..1efdd17 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -676,8 +676,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>  
>  
>      if (s->l1_size > 0) {
> -        s->l1_table = g_malloc0(
> +        s->l1_table = qemu_try_blockalign(bs->file,

Is blockalign used consistently for s->l1_table?  Or places in this
patch have l1_table = g_try_malloc0().  We need to be careful because of
g_free() vs qemu_vfree().