On 3/27/19 3:33 PM, John Snow wrote: > > > On 3/27/19 8:49 AM, Vladimir Sementsov-Ogievskiy wrote: >> bdrv_replace_child() calls bdrv_check_perm() with error_abort on >> loosening permissions. However file-locking operations may fail even >> in this case, for example on NFS. And this leads to Qemu crash. >> >> Let's ignore such errors, as we do already on permission update commit >> and abort. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> >> --- >> block/file-posix.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/block/file-posix.c b/block/file-posix.c >> index db4cccbe51..403e67fe90 100644 >> --- a/block/file-posix.c >> +++ b/block/file-posix.c >> @@ -815,6 +815,20 @@ static int raw_handle_perm_lock(BlockDriverState *bs, >> >> switch (op) { >> case RAW_PL_PREPARE: >> + if ((s->perm | new_perm) == s->perm &&
This says if new_perm does not add any bits beyond what s->perm had.
>> + (~s->shared_perm | ~new_perm) == ~s->shared_perm)
>
> Little strange to read, but ultimately "If we aren't changing anything"
> based on the call below.
'(~a | ~b)' is equivalent to '~(a & b)'.
'~(a & b) == ~a' is equivalent to '(a & b) == a'
That expression is much easier to read, as new_perm does not remove any
bits beyond what s->shared_perm already had.
But rewriting it in an easier form would indeed make the patch easier to
swallow.
>
>> + {
>> + /*
>> + * We are going to unlock bytes, it should not fail. If fail,
>> + * just report it and ignore, like we do for ABORT and COMMIT
>> + * anyway.
>> + */
>> + ret = raw_check_lock_bytes(s->fd, new_perm, new_shared,
>> &local_err);
>> + if (local_err) {
>> + error_report_err(local_err);
>> + }
>> + return 0;
>> + }
>> ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm,
>> ~s->shared_perm | ~new_shared,
>> false, errp);
>>
>
> I thiiiink this makes sense, but hopefully someone else can give it the
> once-over too.
>
> Reviewed-by: John Snow <[email protected]>
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
