From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:53034)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jcody@redhat.com>) id 1W6SXa-00067S-TM
	for qemu-devel@nongnu.org; Thu, 23 Jan 2014 17:14:31 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jcody@redhat.com>) id 1W6SXW-0001Ok-Ak
	for qemu-devel@nongnu.org; Thu, 23 Jan 2014 17:14:26 -0500
Received: from mx1.redhat.com ([209.132.183.28]:22444)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jcody@redhat.com>) id 1W6SXW-0001Ob-0o
	for qemu-devel@nongnu.org; Thu, 23 Jan 2014 17:14:22 -0500
Date: Thu, 23 Jan 2014 17:14:17 -0500
From: Jeff Cody <jcody@redhat.com>
Message-ID: <20140123221417.GD13722@localhost.localdomain>
References: <cover.1390513348.git.jcody@redhat.com>
	<cbd2d430e1eb75264289b243a7863f8ed6570ea2.1390513348.git.jcody@redhat.com>
	<20140123220014.GC22578@irqsave.net> <52E192B0.1030804@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <52E192B0.1030804@redhat.com>
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v3 1/3] block: resize backing file image
 during offline commit, if necessary
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: Eric Blake <eblake@redhat.com>
Cc: =?iso-8859-1?Q?Beno=EEt?= Canet <benoit.canet@irqsave.net>, kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com

On Thu, Jan 23, 2014 at 03:07:44PM -0700, Eric Blake wrote:
> On 01/23/2014 03:00 PM, Beno=EEt Canet wrote:
> > Le Thursday 23 Jan 2014 =E0 16:48:55 (-0500), Jeff Cody a =E9crit :
> >> Currently, if an image file is logically larger than its backing fil=
e,
> >> commiting it via 'qemu-img commit' will fail.
>=20
> s/commiting/committing/
>=20

I could respin, or Kevin / Stefan could fix this up whenever it is
applied to their branches.

>=20
> >> +    uint8_t *buf =3D NULL;
> >=20
> > Why assign NULL to buf ? Is it related to the rest of the patch ?
> >=20
> > Reviewed-by: Benoit Canet <benoit@irqsave.net>
> >=20
> >>      char filename[PATH_MAX];
> >> =20
> >>      if (!drv)
> >> @@ -1904,7 +1904,24 @@ int bdrv_commit(BlockDriverState *bs)
> >>          }
> >>      }
> >> =20
> >> -    total_sectors =3D bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
> >> +    length =3D bdrv_getlength(bs);
> >> +    backing_length =3D bdrv_getlength(bs->backing_hd);
> >> +
> >> +    if (length < 0 || backing_length < 0) {
> >> +        goto ro_cleanup;
>=20
> Because this goto now reaches the ro_cleanup label with buf
> uninitialized, if we don't assign NULL originally.
>=20

Yup, exactly.

> >> +    total_sectors =3D length >> BDRV_SECTOR_BITS;
> >>      buf =3D g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
>=20
> The old code only ever reached ro_cleanup after assigning buf, and
> ro_cleanup blindly frees buf.
>=20
> --=20
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>=20