[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Don't lock /dev/null and /dev/zero autom
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH] block: Don't lock /dev/null and /dev/zero automatically |
Date: |
Sun, 22 Jul 2018 16:06:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-07-22 04:37, Fam Zheng wrote:
> On Sun, Jul 22, 2018 at 5:08 AM Max Reitz <address@hidden> wrote:
>>
>> On 2018-07-19 05:41, Fam Zheng wrote:
>>> On my Fedora 28, /dev/null is locked by some other process (couldn't
>>> inspect it due to the current lslocks limitation), so iotests 226 fails
>>> with some unexpected image locking errors because it uses qemu-io to
>>> open it.
>>>
>>> Actually it's safe to not use any lock on /dev/null or /dev/zero.
>>>
>>> Signed-off-by: Fam Zheng <address@hidden>
>>> ---
>>> block/file-posix.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/file-posix.c b/block/file-posix.c
>>> index 60af4b3d51..8bf034108a 100644
>>> --- a/block/file-posix.c
>>> +++ b/block/file-posix.c
>>> @@ -503,7 +503,12 @@ static int raw_open_common(BlockDriverState *bs, QDict
>>> *options,
>>> s->use_lock = false;
>>> break;
>>> case ON_OFF_AUTO_AUTO:
>>> - s->use_lock = qemu_has_ofd_lock();
>>> + if (!strcmp(filename, "/dev/null") ||
>>> + !strcmp(filename, "/dev/zero")) {
>>
>> I’m not sure I like a strcmp() based on filename (though it should work
>> for all of the cases where someone would want to specify either of those
>> for a qemu block device). Isn’t there some specific major/minor number
>> we can use to check for these two files? Or are those Linux-specific?
>
> Yeah, I guess major/minor numbers are Linux-specific.
>
>>
>> I could also imagine just not locking any host_device, but since people
>> do use qcow2 immediately on block devices, maybe we do want to lock them.
>
> That's right.
>
>>
>> Finally, if really all you are trying to do is to make test code easier,
>> then I don’t know exactly why. Just disabling locking in 226 shouldn’t
>> be too hard.
>
> The tricky thing is in remembering to do that for future test cases.
> My machine seems to be somehow different than John's so that my 226
> cannot lock /dev/null, but I'm not sure that is the case for other
> releases, distros or system instances.
Usually we don’t need to use /dev/null, though, because null-co:// is
better suited for most purposes. This is a very specific test of host
devices.
Maybe we should start a doc file with common good practices about
writing iotests?
Max
signature.asc
Description: OpenPGP digital signature