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
>> ---
>> 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,
>> _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 thnk this makes sense, but hopefully someone else can give it the
> once-over too.
>
> Reviewed-by: John Snow
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
