30.04.2019 11:38, Vladimir Sementsov-Ogievskiy wrote: > 29.04.2019 19:37, Max Reitz wrote: >> On 02.04.19 17:37, Vladimir Sementsov-Ogievskiy wrote: >>> Background: decryption will be done in threads, to take benefit of it, >>> we should move it out of the lock first. >> >> ...which is safe after your commit c972fa123c73501b4, I presume. >> >> (At first glance, the patched looked a bit weird to me because it >> doesn't give a reason why dropping the lock around >> qcrypto_block_decrypt() would be OK.) >> >>> But let's go further: it turns out, that for locking around switch >>> cases we have only two variants: when we just do memset(0) not >>> releasing the lock (it is useless) and when we actually can handle the >>> whole case out of the lock. So, refactor the whole thing to reduce >>> locked code region and make it clean. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>> Reviewed-by: Alberto Garcia <be...@igalia.com> >>> --- >>> block/qcow2.c | 46 ++++++++++++++++++++++------------------------ >>> 1 file changed, 22 insertions(+), 24 deletions(-) >>> >>> diff --git a/block/qcow2.c b/block/qcow2.c >>> index 46e8e39da5..fcf92a7eb6 100644 >>> --- a/block/qcow2.c >>> +++ b/block/qcow2.c >>> @@ -1983,6 +1983,7 @@ static coroutine_fn int >>> qcow2_co_preadv(BlockDriverState *bs, uint64_t offset, >>> ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes, >>> &cluster_offset); >> >> Isn't this the only function in the loop that actually needs the lock? >> Wouldn't it make more sense to just take it around this call? >> > > Hmm, looks correct, I'll resend. > >
Or not, actually, we may have several qcow2_get_data_offset calls under one lock, if clusters are different kinds of ZERO. So, I think better to keep it as is for now. -- Best regards, Vladimir
