[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock byte
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes |
Date: |
Fri, 29 Mar 2019 17:31:43 +0000 |
29.03.2019 20:15, Max Reitz wrote:
> On 29.03.19 12:04, 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 avoid such errors. Note, that we ignore such things anyway on
>> permission update commit and abort.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>> block/file-posix.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index db4cccbe51..1cf4ee49eb 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -815,6 +815,18 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
>>
>> switch (op) {
>> case RAW_PL_PREPARE:
>> + if ((s->perm | new_perm) == s->perm &&
>> + (s->shared_perm & new_shared) == s->shared_perm)
>> + {
>> + /*
>> + * We are going to unlock bytes, it should not fail. If it fail
>> due
>> + * to some fs-dependent permission-unrelated reasons (which
>> occurs
>> + * sometimes on NFS and leads to abort in bdrv_replace_child) we
>> + * can't prevent such errors by any check here. And we ignore
>> them
>> + * anyway in ABORT and COMMIT.
>> + */
>> + return 0;
>> + }
>> ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm,
>> ~s->shared_perm | ~new_shared,
>> false, errp);
>
> Help me understand the exact issue, please. I understand that there are
> operations like bdrv_replace_child() that pass &error_abort to
> bdrv_check_perm() because they just loosen the permissions, so it should
> not fail.
>
> However, if the whole effect really would be to loosen permissions,
> raw_apply_lock_bytes() wouldn't have failed here in PREPARE anyway:
> @unlock is passed as false, so no bytes will be unlocked. And if
> permissions are just loosened (as your condition checks), it should not
> lock any bytes.
but it does qemu_lock_fd unconditionally on any locked byte (it may be
loosening,
but not of all bytes). and fcntl is called, which may fail.
>
> So why does it attempt lock any bytes in the first place? There must be
> some discrepancy between s->perm and s->locked_perm, or ~s->shared_perm
> and s->locked_shared_perm. How does that occur?
>
> Max
>
--
Best regards,
Vladimir
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, (continued)
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Kevin Wolf, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Max Reitz, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Kevin Wolf, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Max Reitz, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Vladimir Sementsov-Ogievskiy, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Kevin Wolf, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Max Reitz, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Vladimir Sementsov-Ogievskiy, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Vladimir Sementsov-Ogievskiy, 2019/03/29
- Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes, Kevin Wolf, 2019/03/29
Re: [Qemu-block] [PATCH v3] block/file-posix: do not fail on unlock bytes,
Vladimir Sementsov-Ogievskiy <=