Re: [Qemu-devel] [PATCH 1/7] block: only force IO completion in .bdrv_truncate if we are shrinking

[Peter Lieven]

Am 06.03.2013 19:42, schrieb Jeff Cody:
> On Wed, Mar 06, 2013 at 07:25:39PM +0100, Peter Lieven wrote:
>> Am 06.03.2013 19:14, schrieb Jeff Cody:
>>> On Wed, Mar 06, 2013 at 07:06:24PM +0100, Paolo Bonzini wrote:
>>>> Il 06/03/2013 18:50, Peter Lieven ha scritto:
>>>>>>> Commit 9a665b2b made bdrv_truncate() call bdrv_drain_all(), but this 
>>>>>>> breaks
>>>>>>> QCOW images, as well other future image formats (such as VHDX) that may 
>>>>>>> call
>>>>>>> bdrv_truncate(bs->file) from within a read/write operation.  For 
>>>>>>> example, QCOW
>>>>>>> will cause an assert, due to tracked_requests not being empty (since the
>>>>>>> read/write that called bdrv_truncate() is still in progress).
>>>>
>>>> I'm not sure such bdrv_truncate calls are necessary.  QCOW2 doesn't have
>>>> them (almost; there is one in qcow2_write_compressed, I'm not even sure
>>>> that one is necessary though), and I think QCOW's breaks using it with a
>>>> block device as a backing file.
>>>>
>>>> Paolo
>>>
>>> QCOW breaks with it using a normal raw posix file as a device.  As a
>>> test: qemu-img create -f qcow test.qcow 5G.  Now run qemu with that
>>> drive mounted, and try to partition and format it.  QEMU now asserts.
>>>
>>> The nicety of being able to using truncate during a write call,
>>> especially for VHDX (which can have relatively large block/cluster
>>> sizes), so to grow the file sparsely in a dynamically allocated file.
>>>
>>
>> why don't you call vhdx_truncate() there instead of bdrv_truncate()?
>>
>> Peter
>>
> 
> What I want to do is grow the underlying file, so bdrv_truncate() is
> called on bs->file.  vhdx_truncate() would primarily care about
> updating the vhdx file structures / headers to reflect the new file
> size (which is what vhdx_block_allocate() does), but I need to
> actually make the underlying file larger (and ideally, in a sparse
> fashion). 
> 

As Paolo suggested we probably need to fix the underlying misuage of
bdrv_truncate() for non growable devices and for that we additionally
need to set bs->growable not to 1 per default, but only if the devices
are really growable: qcow2, raw (only on regular file), vhdx? I maybe
missing some.

Paolo?