[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2] block/file-posix: ignore fail on unlock byte

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH v2] block/file-posix: ignore fail on unlock bytes
Date: Fri, 29 Mar 2019 11:06:08 +0000

29.03.2019 13:55, Vladimir Sementsov-Ogievskiy wrote:
> 29.03.2019 13:12, Kevin Wolf wrote:
>> Am 29.03.2019 um 10:53 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>> 28.03.2019 21:40, Kevin Wolf wrote:
>>>> Am 28.03.2019 um 08:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>>> 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 <address@hidden>
>>>> I think this would better be fixed in block.c code so that unlock never
>>>> fails for any block driver.
>>> Hmm. We now only have one .bdrv_check_perm handler - raw_check_perm. And
>>> in this particular case, yes, the only thing we can do is ignoring error
>>> and do not fail on loosening permissions..
>>> If we have more drivers with this callback, what should be the common
>>> behavior?
>>> Do you propose to ignore .bdrv_check_perm errors in common case?
>>> Isn't it better to require, that .bdrv_check_perm handler do not fail on
>>> loosening permissions, and abort if it fail in this case, like it actually
>>> works after this patch?
>> Maybe an assertion in the common code is actually better, yes.
>> I do think that the common behaviour we want is to ignore
>> .bdrv_check_perm errors for unlock, but it might be surprising for
>> drivers that .bdrv_set_perm is called when .bdrv_check_perm failed. So
>> we need the drivers to be aware of the problem anyway.
>> Back to your patch: Why do you need to call raw_check_lock_bytes() in
>> the unlock case? We don't acquire any new permissions and hold the locks
>> for everything, so nobody else should have taken a conflicting lock.
> Hmm.. it check not same arguments, so I didn't drop it as 
> raw_apply_lock_bytes.
> On the other hand, the only meaning of this raw_check_lock_bytes, is that 
> we'll
> print error if it come (when it should not).
> Seems OK for me to drop it too and just return 0 immediately.

sent corresponding v3, renamed a bit:
[PATCH v3] block/file-posix: do not fail on unlock bytes

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]