Re: [Qemu-devel] [PATCH v3 1/3] block: resize backing file image during offline commit, if necessary

[Jeff Cody <jcody@redhat.com>]

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

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

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

Yup, exactly.

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