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

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.)


Max Reitz (8):
  block: Handle filter truncation like native impl.
  block/cor: Drop cor_co_truncate()
  block: Do not truncate file node when formatting
  block: Add @exact parameter to bdrv_co_truncate()
  block: Evaluate @exact in protocol drivers
  block: Let format drivers pass @exact
  block: Pass truncate exact=true where reasonable
  Revert "qemu-img: Check post-truncation size"

 include/block/block.h          |  6 ++---
 include/block/block_int.h      | 17 ++++++++++++-
 include/sysemu/block-backend.h |  4 +--
 block/block-backend.c          |  6 ++---
 block/commit.c                 |  5 ++--
 block/copy-on-read.c           |  8 ------
 block/crypto.c                 |  8 +++---
 block/file-posix.c             | 11 ++++++--
 block/file-win32.c             |  3 ++-
 block/gluster.c                |  1 +
 block/io.c                     | 29 ++++++++++++---------
 block/iscsi.c                  | 10 ++++++--
 block/mirror.c                 |  4 +--
 block/nfs.c                    |  2 +-
 block/parallels.c              | 18 +++++++------
 block/qcow.c                   |  9 ++-----
 block/qcow2-refcount.c         |  2 +-
 block/qcow2.c                  | 45 +++++++++++++++++++++------------
 block/qed.c                    |  3 ++-
 block/raw-format.c             |  5 ++--
 block/rbd.c                    |  1 +
 block/sheepdog.c               |  5 ++--
 block/ssh.c                    |  3 ++-
 block/vdi.c                    |  2 +-
 block/vhdx-log.c               |  4 +--
 block/vhdx.c                   |  7 +++---
 block/vmdk.c                   |  8 +++---
 block/vpc.c                    |  2 +-
 blockdev.c                     |  2 +-
 qemu-img.c                     | 46 ++++++++--------------------------
 qemu-io-cmds.c                 |  7 +++++-
 tests/test-block-iothread.c    |  8 +++---
 32 files changed, 157 insertions(+), 134 deletions(-)

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