On 29.04.19 18: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.)
On second thought, I guess the actual reason it's safe is because the crypto code never yields. Max >> 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? > > Max > >> if (ret < 0) { >> + qemu_co_mutex_unlock(&s->lock); >> goto fail; >> } >> >
signature.asc
Description: OpenPGP digital signature