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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3095 bytes --]

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.

But actually, while avoiding the fake truncate outside of image creation
would avoid confusing situations where the user requested image
shrinking, gets success, but nothing happened, it would also break
actual resizes where the volume is resized externally and block_resize
is only called to notify qemu to pick up the size change.

So after all, we probably do need to keep the hack in bdrv_co_truncate()
instead of moving it to image creation only.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]