13.09.2019 17:07, Maxim Levitsky wrote: > On Fri, 2019-09-13 at 14:01 +0000, Vladimir Sementsov-Ogievskiy wrote: >> 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. > > Yep :-( I am trying my best in addition to fixing the bug, also clarify the > area to > avoid this from happening again.
And this is really great! I'm sorry that I've broken these things, we actually don't use encryption (don't ask me why I've implemented threaded encryption :), so, I hope you are not only damaged by my patches but also have some benefit. > > What do you think that I fold these two patches together after all? OK for me (but I'm not a maintainer). -- Best regards, Vladimir