[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] block: Simplify bdrv_can_write_zeroes_with_unma
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-block] [PATCH] block: Simplify bdrv_can_write_zeroes_with_unmap() |
Date: |
Mon, 29 Jan 2018 11:08:44 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Fri, Jan 26, 2018 at 01:34:39PM -0600, Eric Blake wrote:
> We don't need the can_write_zeroes_with_unmap field in
> BlockDriverInfo, because it is redundant information with
> supported_zero_flags & BDRV_REQ_MAY_UNMAP. Note that
> BlockDriverInfo and supported_zero_flags are both per-device
> settings, rather than global state about the driver as a
> whole, which means one or both of these bits of information
> can already be conditional. Let's audit how they were set:
>
> crypto: always setting can_write_ to false is pointless (the
> struct starts life zero-initialized), no use of supported_
>
> nbd: just recently fixed to set can_write_ if supported_
> includes MAY_UNMAP (thus this commit effectively reverts
> bca80059e and solves the problem mentioned there in a more
> global way)
>
> file-posix, iscsi, qcow2: can_write_ is conditional, while
> supported_ was unconditional; but passing MAY_UNMAP would
> fail with ENOTSUP if the condition wasn't met
>
> qed: can_write_ is unconditional, but pwrite_zeroes lacks
> support for MAY_UNMAP and supported_ is not set. Perhaps
> support can be added later (since it would be similar to
> qcow2), but for now claiming false is no real loss
>
> all other drivers: can_write_ is not set, and supported_ is
> either unset or a passthrough
>
> Simplify the code by moving the conditional into
> supported_zero_flags for all drivers, then dropping the
> now-unused BDI field. For callers that relied on
> bdrv_can_write_zeroes_with_unmap(), we return the same
> per-device settings for drivers that had conditions (no
> observable change in behavior there); and can now return
> true (instead of false) for drivers that support passthrough
> (for example, the commit driver) which gives those drivers
> the same fix as nbd just got in bca80059e. For callers that
> relied on supported_zero_flags, we now have a few more places
> that can avoid a wasted call to pwrite_zeroes() that will
> just fail with ENOTSUP.
>
> Suggested-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> The commit id mentioned above is dependent on me not having to respin
> my latest NBD pull request:
>
> Based-on: <address@hidden>
> ([PULL 0/8] NBD patches through 26 Jan)
> ---
> include/block/block.h | 11 -----------
> block.c | 8 +-------
> block/crypto.c | 1 -
> block/file-posix.c | 3 +--
> block/iscsi.c | 6 ++++--
> block/nbd.c | 11 -----------
> block/qcow2.c | 3 +--
> block/qed.c | 1 -
> 8 files changed, 7 insertions(+), 37 deletions(-)
Reviewed-by: Stefan Hajnoczi <address@hidden>
signature.asc
Description: PGP signature