Re: [Qemu-devel] [RFC 2/5] block: Generic truncation fallback

[Max Reitz]

On 12.07.19 13:48, Max Reitz wrote:
> On 12.07.19 13:17, Kevin Wolf wrote:
>> Am 12.07.2019 um 12:58 hat Max Reitz geschrieben:
>>> On 12.07.19 11:49, Kevin Wolf wrote:
>>>> Am 11.07.2019 um 21:58 hat Max Reitz geschrieben:
>>>>> If a protocol driver does not support truncation, we call fall back to
>>>>> effectively not doing anything if the new size is less than the actual
>>>>> file size.  This is what we have been doing for some host device drivers
>>>>> already.
>>>>
>>>> Specifically, we're doing it for drivers that access a fixed-size image,
>>>> i.e. block devices rather than regular files. We don't want to do this
>>>> for drivers where the file size could be changed, but just didn't
>>>> implement it.
>>>>
>>>> So I would suggest calling the function more specifically something like
>>>> bdrv_co_truncate_blockdev(), and not using it as an automatic fallback
>>>> in bdrv_co_truncate(), but just make it the BlockDriver.bdrv_co_truncate
>>>> implementation for those drivers where it makes sense.
>>>
>>> I was thinking about this, but the problem is that .bdrv_co_truncate()
>>> does not get a BdrvChild, so an implementation for it cannot generically
>>> zero the first sector (without bypassing the permission system, which
>>> would be wrong).
>>
>> Hm, I see. The next best thing would then probably having a bool in
>> BlockDriver that enables the fallback.
>>
>>> So the function pointer would actually need to be set to something like
>>> (int (*)(BlockDriverState *, int64_t, PreallocMode, Error **))42ul, or a
>>> dummy function that just aborts, and then bdrv_co_truncate() would
>>> recognize this magic constant.  But that seemed so weird to me that I
>>> decided just not to do it, mostly because I was wondering what would be
>>> so bad about treating images whose size we cannot change because we
>>> haven’t implemented it exactly like fixed-size images.
>>>
>>> (Also, “fixed-size” is up to interpretation.  You can change an LVM
>>> volume’s size.  qemu doesn’t do it, obviously.  But that is the reason
>>> for the warning qemu-img resize emits when it sees that the file size
>>> did not change.)
>>>
>>>> And of course, we only need these fake implementations because qemu-img
>>>> (or .bdrv_co_create_opts) always wants to create the protocol level. If
>>>> we could avoid this, then we wouldn't need any of this.
>>>
>>> It’s trivial to avoid this.  I mean, patch 4 does exactly that.
>>>
>>> So it isn’t about avoiding creating the protocol level, it’s about
>>> avoiding the truncation there.  But why would you do that?
>>
>> Because we can't actually truncate there. We can only do the fake thing
>> and claim we have truncated even though the size didn't change.
> 
> You’re right.  I actually didn’t realize that we have no drivers that
> support truncation, but not image creation.
> 
> Yes, then it’s stupid.
> 
> I thought it was a reasonable thing to do for such drivers.
> 
> So I suppose the best thing is to do what I hinted at in my reply to
> your reply to patch 3, which is to drop patches 2 and 3 and instead make
> the creation fallback work around blk_truncate() failures.

Oh no.  Now I remember.  The problem with that is that nowadays all
format drivers truncate the protocol file to 0 in their
.bdrv_co_create() implementation.  Aw, man.

Max

signature.asc
Description: OpenPGP digital signature