Re: [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate()

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1846 bytes --]

On 18.09.19 11:51, Max Reitz wrote:
> Hi,
> 
> This series is supposed to pull out some of the problems from my
> “Generic file creation fallback” series.
> 
> The blk_truncate_for_formatting() function added there was buggy, as
> Maxim noted, in that it did not check whether blk_truncate() actually
> resized the block node to the target offset.  One way to fix this is to
> add a parameter to it that forces the block driver to do so, and that is
> done by this series.
> 
> I think this is generally useful (you can see the diff stat saldo is
> only +23 lines), because it allows us to drop a special check in
> qemu-img resize, and it fixes a bug in qed (which has relied on this
> behavior for over 8 years, but unfortunately bdrv_truncate()’s behavior
> changed quite exactly 8 years ago).
> 
> However, in the process I noticed we actually don’t need
> blk_truncate_for_formatting(): The underlying problem is that some
> format drivers truncate their underlying file node to 0 before
> formatting it to drop all data.  So they should pass exact=true, but
> they cannot, because this would break creation on block devices.  Hence
> blk_truncate_for_formatting().
> 
> It turns out, though, that three of the four drivers in question don’t
> need to truncate their file node at all.  The remaining one is qed which
> simply really should pass exact=true (it’s a bug fix).
> 
> (I do drop those blk_truncate() invocations in this series, because
> otherwise I feel like it is impossible to decide whether they should get
> exact=false or exact=true.  Either way is wrong.)

Thanks for the review, I’ve applied the series to my block branch and
changed the comment in qed.c as requested and suggested by Maxim on patch 7:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]