[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 5/7] block: convert crypto driver to bdrv_co_
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v3 5/7] block: convert crypto driver to bdrv_co_preadv|pwritev |
Date: |
Sat, 16 Sep 2017 18:54:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 2017-09-12 13:28, Daniel P. Berrange wrote:
> Make the crypto driver implement the bdrv_co_preadv|pwritev
> callbacks, and also use bdrv_co_preadv|pwritev for I/O
> with the protocol driver beneath.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> block/crypto.c | 104
> +++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 56 insertions(+), 48 deletions(-)
Reviewed-by: Max Reitz <address@hidden>
Some notes below.
> diff --git a/block/crypto.c b/block/crypto.c
> index 49d6d4c058..d004e9cef4 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
[...]
> @@ -410,37 +414,33 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t
> sector_num,
> goto cleanup;
> }
>
> - while (remaining_sectors) {
> - cur_nr_sectors = remaining_sectors;
> + while (bytes) {
> + cur_bytes = bytes;
>
> - if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) {
> - cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS;
> + if (cur_bytes > (BLOCK_CRYPTO_MAX_SECTORS * sector_size)) {
> + cur_bytes = BLOCK_CRYPTO_MAX_SECTORS * sector_size;
It's a bit weird that now the bounce buffer's size is now no longer
fixed at 1 MB but variable depending on the crypto driver's block size.
It also doesn't seem too intentional when looking at the first patch's
commit message...
In any case, that would be an issue in the previous patch, though. In
general, I'm wondering whether maybe you should squash this patch into
the previous one... Yes, that would make the for a larger patch, but it
wouldn't leave some not-quite-right state in between where sector_size
is generally assumed to be equal to BDRV_SECTOR_SIZE -- which it is in
practice, but not necessarily in theory.
> }
Also, just a note: It would be shorter with a MIN(). :-)
(cur_bytes = MIN(bytes, BLOCK_CRYPTO_MAX_SECTORS * sector_size))
Max
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v3 3/7] block: fix data type casting for crypto payload offset, (continued)
- [Qemu-devel] [PATCH v3 1/7] block: use 1 MB bounce buffers for crypto instead of 16KB, Daniel P. Berrange, 2017/09/12
- [Qemu-devel] [PATCH v3 4/7] block: don't use constant 512 as sector size in crypto driver, Daniel P. Berrange, 2017/09/12
- [Qemu-devel] [PATCH v3 5/7] block: convert crypto driver to bdrv_co_preadv|pwritev, Daniel P. Berrange, 2017/09/12
- [Qemu-devel] [PATCH v3 7/7] block: support passthrough of BDRV_REQ_FUA in crypto driver, Daniel P. Berrange, 2017/09/12
- [Qemu-devel] [PATCH v3 6/7] block: convert qcrypto_block_encrypt|decrypt to take bytes offset, Daniel P. Berrange, 2017/09/12
- Re: [Qemu-devel] [PATCH v3 0/7] Misc improvements to crypto block driver, no-reply, 2017/09/12