Re: [Qemu-devel] [RFC for 3.1? or 4 v2 1/1] qemu-iotests: Don't run the test when user is root

[Farhan Ali <alifm@linux.ibm.com>]

On 11/30/2018 12:50 PM, Eric Blake wrote:
> Adding qemu-devel - all patches should go there, especially if you want 
> to get Peter's attention that this might be a 3.1 candidate if we have 
> other reasons to spin -rc4.
> 
> On 11/30/18 10:04 AM, Farhan Ali wrote:
>> Test 232 creates image files with read-only permission and
>> expects an error message when trying to access the image
>> files with read-only and auto-read-only turned off.
>>
>> Don't run as root user, since root can open files with read/write
>> access for read-only files.
>>
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   tests/qemu-iotests/232 | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/232 b/tests/qemu-iotests/232
>> index 0708b8b..05d5f2f 100755
>> --- a/tests/qemu-iotests/232
>> +++ b/tests/qemu-iotests/232
>> @@ -41,6 +41,14 @@ _supported_fmt generic
>>   _supported_proto file
>>   _supported_os Linux
>> +tmp='file'
>> +touch $tmp
>> +chmod a-w $tmp
>> +if [ -w $tmp ]
>> +then
>> +    _notrun "Cannot run this test as root user"
>> +fi
>> +
> 
> I know you just copied from my suggestion, but now looking at it, this 
> leaves 'tmp' around in the directory for both success and skip. Better 
> might be to just check whether $TEST_IMG is writable, immediately after 
> the existing 'chmod a-w $TEST_IMG' line (Hmm - that line is already 
> broken for not quoting "$TEST_IMG" in case it contains whitespace).

Ah yes, I forgot about the fact the 'file' will linger around. I will 
spin up a v3.

> 
> I don't see this being a reason for -rc4 on its own (most people don't 
> run iotests as root); and the fact that we're still working on the final 
> contents of what the patch should contain, as well as the fact that the 
> patch doesn't affect the main binaries, means that if it were up to me, 
> I'd defer it to 4.0. Kevin may have a different opinion, though, since 
> it is his test, and new to 3.1.
>