13.09.2019 15:59, Maxim Levitsky wrote:
> This commit tries to clarify few function arguments,
> and add comments describing the encrypt/decrypt interface
>
> Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
> ---
> block/qcow2-cluster.c | 9 ++++---
> block/qcow2-threads.c | 62 ++++++++++++++++++++++++++++++++++---------
> block/qcow2.c | 5 ++--
> block/qcow2.h | 8 +++---
> 4 files changed, 61 insertions(+), 23 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index f09cc992af..46b0854d7e 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -463,8 +463,8 @@ static int coroutine_fn
> do_perform_cow_read(BlockDriverState *bs,
> }
>
> static bool coroutine_fn do_perform_cow_encrypt(BlockDriverState *bs,
> - uint64_t src_cluster_offset,
> - uint64_t cluster_offset,
> + uint64_t
> guest_cluster_offset,
> + uint64_t host_cluster_offset,
> unsigned offset_in_cluster,
> uint8_t *buffer,
> unsigned bytes)
> @@ -474,8 +474,9 @@ static bool coroutine_fn
> do_perform_cow_encrypt(BlockDriverState *bs,
> assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
> assert((bytes & ~BDRV_SECTOR_MASK) == 0);
> assert(s->crypto);
> - if (qcow2_co_encrypt(bs, cluster_offset,
> - src_cluster_offset + offset_in_cluster,
> + if (qcow2_co_encrypt(bs,
> + host_cluster_offset + offset_in_cluster,
> + guest_cluster_offset + offset_in_cluster,
oops, seems you accidentally fixed the bug, which you are going to fix in the
next
patch, as now correct offsets are given to qcow2_co_encrypt :)
and next patch no is a simple no-logic-change refactoring, so at least commit
message
should be changed.
> buffer, bytes) < 0) {
> return false;
> }
> diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
> index 3b1e63fe41..9646243a9b 100644
> --- a/block/qcow2-threads.c
> +++ b/block/qcow2-threads.c
> @@ -234,15 +234,15 @@ static int qcow2_encdec_pool_func(void *opaque)
> }
>
> static int coroutine_fn
> -qcow2_co_encdec(BlockDriverState *bs, uint64_t file_cluster_offset,
> - uint64_t offset, void *buf, size_t len, Qcow2EncDecFunc
> func)
> +qcow2_co_encdec(BlockDriverState *bs, uint64_t host_offset,
> + uint64_t guest_offset, void *buf, size_t len,
> + Qcow2EncDecFunc func)
> {
> BDRVQcow2State *s = bs->opaque;
> +
> Qcow2EncDecData arg = {
> .block = s->crypto,
> - .offset = s->crypt_physical_offset ?
> - file_cluster_offset + offset_into_cluster(s, offset) :
> - offset,
> + .offset = s->crypt_physical_offset ? host_offset : guest_offset,
Nice change. Obviously original design was worse.
> .buf = buf,
> .len = len,
> .func = func,
> @@ -251,18 +251,54 @@ qcow2_co_encdec(BlockDriverState *bs, uint64_t
> file_cluster_offset,
> return qcow2_co_process(bs, qcow2_encdec_pool_func, &arg);
> }
>
> +
> +/*
> + * qcow2_co_encrypt()
> + *
> + * Encrypts one or more contiguous aligned sectors
> + *
> + * @host_offset - underlying storage offset of the first sector of the
> + * data to be encrypted
> +
> + * @guest_offset - guest (virtual) offset of the first sector of the
> + * data to be encrypted
> + *
> + * @buf - buffer with the data to encrypt, that after encryption
> + * will be written to the underlying storage device at
> + * @host_offset
> + *
> + * @len - length of the buffer (must be a BDRV_SECTOR_SIZE multiple)
> + *
> + * Depending on the encryption method, @host_cluster_offset and/or
> @guest_offset
> + * may be used for generating the initialization vector for
> + * encryption.
> + *
> + * Note that while the whole range must be aligned on sectors, it
> + * does not have to be aligned on clusters and can also cross cluster
> + * boundaries
> + *
> + */
> int coroutine_fn
> -qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
> - uint64_t offset, void *buf, size_t len)
> +qcow2_co_encrypt(BlockDriverState *bs, uint64_t host_offset,
> + uint64_t guest_offset, void *buf, size_t len)
> {
> - return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
> - qcrypto_block_encrypt);
> + return qcow2_co_encdec(bs, host_offset, guest_offset, buf, len,
> + qcrypto_block_encrypt);
> }
>
> +
> +/*
> + * qcow2_co_decrypt()
> + *
> + * Decrypts one or more contiguous aligned sectors
> + * Similar to qcow2_co_encrypt
> + *
> + */
> +
> int coroutine_fn
> -qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
> - uint64_t offset, void *buf, size_t len)
> +qcow2_co_decrypt(BlockDriverState *bs, uint64_t host_offset,
> + uint64_t guest_offset, void *buf, size_t len)
> {
> - return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
> - qcrypto_block_decrypt);
> + return qcow2_co_encdec(bs, host_offset, guest_offset, buf, len,
> + qcrypto_block_decrypt);
> }
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 0882ff6e92..7ec50157e0 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2065,7 +2065,8 @@ static coroutine_fn int
> qcow2_co_preadv_part(BlockDriverState *bs,
>
> assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
> - if (qcow2_co_decrypt(bs, cluster_offset, offset,
> + if (qcow2_co_decrypt(bs, cluster_offset + offset_in_cluster,
> + offset,
> cluster_data, cur_bytes) < 0) {
> ret = -EIO;
> goto fail;
> @@ -2284,7 +2285,7 @@ static coroutine_fn int qcow2_co_pwritev_part(
> qemu_iovec_to_buf(qiov, qiov_offset + bytes_done,
> cluster_data, cur_bytes);
>
> - if (qcow2_co_encrypt(bs, cluster_offset, offset,
> + if (qcow2_co_encrypt(bs, cluster_offset + offset_in_cluster,
> offset,
> cluster_data, cur_bytes) < 0) {
> ret = -EIO;
> goto out_unlocked;
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 998bcdaef1..a488d761ff 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -758,10 +758,10 @@ ssize_t coroutine_fn
> qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
> const void *src, size_t src_size);
> int coroutine_fn
> -qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
> - uint64_t offset, void *buf, size_t len);
> +qcow2_co_encrypt(BlockDriverState *bs, uint64_t host_offset,
> + uint64_t guest_offset, void *buf, size_t len);
> int coroutine_fn
> -qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
> - uint64_t offset, void *buf, size_t len);
> +qcow2_co_decrypt(BlockDriverState *bs, uint64_t host_offset,
> + uint64_t guest_offset, void *buf, size_t len);
>
> #endif
>
--
Best regards,
Vladimir
