[Top][All Lists]

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

Re: [Qemu-block] [Qemu-devel] [PATCH v3 0/8] block: Ignore loosening per

From: Max Reitz
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v3 0/8] block: Ignore loosening perm restrictions failures
Date: Wed, 22 May 2019 20:37:50 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

On 22.05.19 20:24, Eric Blake wrote:
> On 5/22/19 12:03 PM, Max Reitz wrote:
>> Hi,
>> This series is mainly a fix for
>> https://bugzilla.redhat.com/show_bug.cgi?id=1703793.  The problem
>> described there is that mirroring to a gluster volume, then switching
>> off the volume makes qemu crash.  There are two problems here:
>> (1) file-posix reopens the FD all the time because it thinks the FD it
>>     has is RDONLY.  It actually isn’t after the first reopen, we just
>>     forgot to change the internal flags.  That’s what patch 1 is for.
>> (2) Even then, when mirror completes, it drops its write permission on
>>     the FD.  This requires a reopen, which will fail if the volume is
>>     down.  Mirror doesn’t expect that.  Nobody ever expects that
>>     dropping permissions can fail, and rightfully so because that’s what
>>     I think we have generally agreed on.
>>     Therefore, the block layer should hide this error.  This is what the
>>     last two patches are for.
>> The penultimate patch adds two assertions: bdrv_replace_child() (for the
>> old BDS) and bdrv_inactivate_recurse() assume they only ever drop
>> assertions.  This is now substantiated by these new assertions.
>> It turns out that this assumption was just plain wrong.  Patches 3 to 5
>> make it right.
>> v3:
>> - Received no reply to my “Hm, warnings break 'make check', so maybe we
>>   should just keep quiet if loosening restrictions fails?” question, so
>>   I assume silence means agreement.  Changed patch 7 accordingly.
> I don't know if there is an easy way to warn for normal users, but
> silence the warnings if run under test setups to keep 'make check'
> output unchanged (I know we've silenced warnings in the past when we
> detect we are running qtest, but this isn't necessarily the same setup).
>  So not a show-stopper for me.

Hm.  That doesn’t sound too bad.  I don’t think there is an easy way to
silence the warning in qemu, but we might be able to just modify the test.

But I don’t even know whether the warnings are even useful or whether
they would just confuse users more than anything.  So far, every case I
know where loosening restrictions failed was because the file is just
gone completely.  The only purpose of a warning is to show the user that
qemu might have locks on the file that it doesn’t need, so they will
know what’s up if they try to open the file in another qemu instance in
a way that should normally work but suddenly doesn’t.  But if the file’s
just gone, you can’t open it in another qemu, so I don’t even know
whether there’s actually any point in warning.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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