[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 13/13] block: Enforce non-zero bl.max_transfe
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v2 13/13] block: Enforce non-zero bl.max_transfer |
Date: |
Thu, 15 Nov 2018 17:24:28 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Am 15.11.2018 um 03:03 hat Eric Blake geschrieben:
> The raw format driver and the filter drivers default to picking
> up the same limits as what they wrap, and I've audited that they
> are otherwise simple enough in their passthrough to be 64-bit
> clean; it's not worth changing their .bdrv_refresh_limits to
> advertise anything different. Previous patches changed all
> remaining byte-based format and protocol drivers to supply an
> explicit max_transfer consistent with an audit (or lack thereof)
> of their code. And for drivers still using sector-based
> callbacks (gluster, iscsi, parallels, qed, replication, sheepdog,
> ssh, vhdx), we can easily supply a 2G default limit while waiting
> for a later per-driver conversion and auditing while moving to
> byte-based callbacks.
>
> With that, we can assert that max_transfer is now always set (either
> by sector-based default, by merge with a backing layer, or by
> explicit settings), and enforce that it be non-zero by the end
> of bdrv_refresh_limits(). This in turn lets us simplify portions
> of the block layer to use MIN() instead of MIN_NON_ZERO(), while
> still fragmenting things to no larger than 2G chunks. Also, we no
> longer need to rewrite the driver's bl.max_transfer to a value below
> 2G. There should be no semantic change from this patch, although
> it does open the door if a future patch wants to let the block layer
> choose a larger value than 2G while still honoring bl.max_transfer
> for fragmentation.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
>
> [hmm - in sending this email, I realize that I didn't specifically
> check whether quorum always picks up a sane limit from its children,
> since it is byte-based but lacks a .bdrv_refresh_limits]
>
> include/block/block_int.h | 10 +++++-----
> block/io.c | 25 ++++++++++++-------------
> 2 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index b19d94dac5f..410553bb0cc 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -548,11 +548,11 @@ typedef struct BlockLimits {
> uint32_t opt_transfer;
>
> /* Maximal transfer length in bytes. Need not be power of 2, but
> - * must be multiple of opt_transfer and bl.request_alignment, or 0
> - * for no 32-bit limit. The block layer fragments all actions
> - * below 2G, so setting this value to anything larger than INT_MAX
> - * implies that the driver has been audited for 64-bit
> - * cleanness. */
> + * must be larger than opt_transfer and request_alignment (the
> + * block layer rounds it down as needed). The block layer
> + * fragments all actions below 2G, so setting this value to
> + * anything larger than INT_MAX implies that the driver has been
> + * audited for 64-bit cleanness. */
> uint64_t max_transfer;
>
> /* memory alignment, in bytes so that no bounce buffer is needed */
> diff --git a/block/io.c b/block/io.c
> index 4911a1123eb..0024be0bf31 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -129,6 +129,9 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error
> **errp)
> /* Default alignment based on whether driver has byte interface */
> bs->bl.request_alignment = (drv->bdrv_co_preadv ||
> drv->bdrv_aio_preadv) ? 1 : 512;
> + /* Default cap at 2G for drivers without byte interface */
> + if (bs->bl.request_alignment == 1)
> + bs->bl.max_transfer = BDRV_REQUEST_MAX_BYTES;
Missing braces, and comment and code don't match (the comment suggests
that the condition should be != 1).
Kevin
- [Qemu-block] [PATCH v2 06/13] blkdebug: Audit for read/write 64-bit cleanness, (continued)
- [Qemu-block] [PATCH v2 06/13] blkdebug: Audit for read/write 64-bit cleanness, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 07/13] blklogwrites: Audit for read/write 64-bit cleanness, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 11/13] qcow2: Audit for read/write 64-bit cleanness, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 08/13] crypto: Audit for read/write 64-bit cleanness, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 09/13] RFC: crypto: Rely on block layer for fragmentation, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 10/13] file-posix: Audit for read/write 64-bit cleanness, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 12/13] block: Document need for audit of read/write 64-bit cleanness, Eric Blake, 2018/11/14
- [Qemu-block] [PATCH v2 13/13] block: Enforce non-zero bl.max_transfer, Eric Blake, 2018/11/14
- Re: [Qemu-block] [PATCH v2 13/13] block: Enforce non-zero bl.max_transfer,
Kevin Wolf <=
- Re: [Qemu-block] [Qemu-devel] [PATCH v2 for-4.0 00/13] block: byte-based blocking read/write, no-reply, 2018/11/15
- Re: [Qemu-block] [Qemu-devel] [PATCH v2 for-4.0 00/13] block: byte-based blocking read/write, no-reply, 2018/11/15