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. Max > 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 >
signature.asc
Description: OpenPGP digital signature