Am 13.09.2019 um 16:37 hat Maxim Levitsky geschrieben: > On Fri, 2019-09-13 at 16:24 +0200, Kevin Wolf wrote: > > Am 13.09.2019 um 16:07 hat Maxim Levitsky geschrieben: > > > 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. > > > > > > What do you think that I fold these two patches together after all? > > > > No, just make sure that your refactoring patch is really just > > refactoring without semantic change, i.e. make sure to preserve the bug > > in this patch. > > > > Maybe you should actually have two refactoring patches (this one without > > the addition of offset_in_cluster, and patch 2) and an additional > > one-liner for the actual fix. > > > > Kevin > > Let me do it simplier I'll just split it to one liner patch that fixes it > and second patch that does all the refactoring.
Fine with me, as long as the fix is kept separate from the refactoring. Kevin
