qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Farhan Ali <alifm@linux.ibm.com>, qemu-block@nongnu.org
Cc: kwolf@redhat.com, "Qemu-devel@nongnu.org" <Qemu-devel@nongnu.org>,
	Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [RFC for 3.1? or 4 v2 1/1] qemu-iotests: Don't run the test when user is root
Date: Fri, 30 Nov 2018 11:50:27 -0600	[thread overview]
Message-ID: <eaba14ba-5c92-34d4-ab7c-3cc5aefdcccb@redhat.com> (raw)
In-Reply-To: <cb554c04ee50ac970fa10edaa090982bef18f673.1543593569.git.alifm@linux.ibm.com>

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).

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

       reply	other threads:[~2018-11-30 17:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1543593569.git.alifm@linux.ibm.com>
     [not found] ` <cb554c04ee50ac970fa10edaa090982bef18f673.1543593569.git.alifm@linux.ibm.com>
2018-11-30 17:50   ` Eric Blake [this message]
2018-11-30 19:45     ` [Qemu-devel] [RFC for 3.1? or 4 v2 1/1] qemu-iotests: Don't run the test when user is root Farhan Ali

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eaba14ba-5c92-34d4-ab7c-3cc5aefdcccb@redhat.com \
    --to=eblake@redhat.com \
    --cc=Qemu-devel@nongnu.org \
    --cc=alifm@linux.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).