[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH v2 1/2] qcow2: Simplify image creation
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] Re: [PATCH v2 1/2] qcow2: Simplify image creation |
Date: |
Tue, 15 Jun 2010 12:53:35 +0100 |
On Tue, Jun 15, 2010 at 12:31 PM, Kevin Wolf <address@hidden> wrote:
> Am 15.06.2010 13:08, schrieb Stefan Hajnoczi:
>> On Tue, Jun 15, 2010 at 11:36 AM, Kevin Wolf <address@hidden> 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.
I see, you're right.
Looks good.
Stefan
[Qemu-devel] [PATCH 2/2] qcow2: Remove old image creation function, Kevin Wolf, 2010/06/14