Re: [Qemu-devel] [PATCH v7 2/3] block/qcow2: refactor encryption code

[Vladimir Sementsov-Ogievskiy]

15.09.2019 23:36, Maxim Levitsky wrote:
> * Change the qcow2_co_{encrypt|decrypt} to just receive full host and
>    guest offsets and use this function directly instead of calling
>    do_perform_cow_encrypt (which is removed by that patch).
> 
> * Adjust qcow2_co_encdec to take full host and guest offsets as well.
> 
> * Document the qcow2_co_{encrypt|decrypt} arguments
>    to prevent the bug fixed in former commit from hopefully
>    happening again.
> 
> Signed-off-by: Maxim Levitsky <address@hidden>
> ---
>   block/qcow2-cluster.c | 41 ++++++++++------------------
>   block/qcow2-threads.c | 63 +++++++++++++++++++++++++++++++++----------
>   block/qcow2.c         |  5 ++--
>   block/qcow2.h         |  8 +++---
>   4 files changed, 70 insertions(+), 47 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index bfeb0241d7..a2d4909024 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c

[..]

>   static int coroutine_fn do_perform_cow_write(BlockDriverState *bs,
>                                                uint64_t cluster_offset,
>                                                unsigned offset_in_cluster,
> @@ -891,11 +869,20 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta 
> *m)
>   
>       /* Encrypt the data if necessary before writing it */
>       if (bs->encrypted) {
> -        if (!do_perform_cow_encrypt(bs, m->offset, m->alloc_offset,
> -                                    start->offset, start_buffer,
> -                                    start->nb_bytes) ||
> -            !do_perform_cow_encrypt(bs, m->offset, m->alloc_offset,
> -                                    end->offset, end_buffer, end->nb_bytes)) 
> {
> +        ret = qcow2_co_encrypt(bs,
> +                               m->alloc_offset + start->offset,
> +                               m->offset + start->offset,
> +                               start_buffer, start->nb_bytes);
> +        if (ret < 0) {
> +            ret = -EIO;
> +            goto fail;

Just go to fail I think, don't reassign ret variable.

> +        }
> +
> +        ret = qcow2_co_encrypt(bs,
> +                               m->alloc_offset + end->offset,
> +                               m->offset + end->offset,
> +                               end_buffer, end->nb_bytes);
> +        if (ret < 0) {
>               ret = -EIO;

and here.

with these two places fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

(I think, these simple changes may be applied while queuing, so you don't
need to resend, if there no other reasons)




-- 
Best regards,
Vladimir