Am 15.06.2010 13:08, schrieb Stefan Hajnoczi:
> On Tue, Jun 15, 2010 at 11:36 AM, Kevin Wolf <kw...@redhat.com> wrote:
>> + /*
>> + * And now open the image and make it consistent first (i.e. increase
>> the
>> + * refcount of the cluster that is occupied by the header and the
>> refcount
>> + * table)
>> + */
>> + BlockDriver* drv = bdrv_find_format("qcow2");
>> + assert(drv != NULL);
>> + ret = bdrv_open(bs, filename, BDRV_O_RDWR | BDRV_O_NO_FLUSH, drv);
>> + if (ret < 0) {
>> + goto out;
>> + }
>
> Here I think we should really return directly on error.
> bdrv_delete(bs) doesn't work since bs isn't initialized when
> bdrv_open() fails.
I did consider returning directly here at first, but decided against it
because usually you expect that a function that uses some goto does so
consistently. Also, I noticed later that returning directly we would
leak the BlockDriverState which is malloc'd in bdrv_file_open.
bs should still be initialized at this point and bs->drv = NULL after
the bdrv_close() above, so that bdrv_delete(bs) will just free the
BlockDriverState.
Kevin
