[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] iotests: Skip read-only cases in 118 when run as root
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] iotests: Skip read-only cases in 118 when run as root |
Date: |
Fri, 18 Oct 2019 16:27:20 +0200 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
Am 18.10.2019 um 14:59 hat Philippe Mathieu-Daudé geschrieben:
> Hi Kevin,
>
> On 10/18/19 1:51 PM, Kevin Wolf wrote:
> > Some tests in 118 use chmod to remove write permissions from the file
> > and assume that the image can indeed not be opened read-write
> > afterwards. This doesn't work when the test is run as root, because root
> > can still open the file as writable even when the permission bit isn't
> > set.
> >
> > Introduce a @skip_if_root decorator and use it in 118 to skip the tests
> > in question when the script is run as root.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > tests/qemu-iotests/118 | 3 +++
> > tests/qemu-iotests/iotests.py | 10 ++++++++++
> > 2 files changed, 13 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
> > index ea0b326ae0..9eff46d189 100755
> > --- a/tests/qemu-iotests/118
> > +++ b/tests/qemu-iotests/118
> > @@ -446,6 +446,7 @@ class TestChangeReadOnly(ChangeBaseClass):
> > self.assert_qmp(result, 'return[0]/inserted/ro', True)
> > self.assert_qmp(result, 'return[0]/inserted/image/filename',
> > new_img)
> > + @iotests.skip_if_root
>
> Why not have case_notrun() return 'reason' and use:
>
> @unittest.skipIf(os.getuid() == 0, case_notrun("cannot be run as root"))
Because we can't skip test cases using unittest functionality, it
results in different output (the test is marked as 's' instead of '.'
and a message '(skipped=n)' is added), which means failure for
qemu-iotests.
Apart from that, it would duplicate the logic and the error message in
every place, which wouldn't be very nice anyway. With the necessary
iotests.case_notrun() the line becomes > 80 characters, too.
> > def test_rw_ro_retain(self):
> > os.chmod(new_img, 0o444)
> > self.vm.add_drive(old_img, 'media=disk', 'none')
> > @@ -530,6 +531,7 @@ class TestChangeReadOnly(ChangeBaseClass):
> > self.assert_qmp(result, 'return[0]/inserted/ro', True)
> > self.assert_qmp(result, 'return[0]/inserted/image/filename',
> > new_img)
> > + @iotests.skip_if_root
> > def test_make_ro_rw(self):
> > os.chmod(new_img, 0o444)
> > self.vm.add_drive(old_img, 'media=disk', 'none')
> > @@ -571,6 +573,7 @@ class TestChangeReadOnly(ChangeBaseClass):
> > self.assert_qmp(result, 'return[0]/inserted/ro', True)
> > self.assert_qmp(result, 'return[0]/inserted/image/filename',
> > new_img)
> > + @iotests.skip_if_root
> > def test_make_ro_rw_by_retain(self):
> > os.chmod(new_img, 0o444)
> > self.vm.add_drive(old_img, 'media=disk', 'none')
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index 3a8f378f90..9c66db613e 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -920,6 +920,16 @@ def skip_if_unsupported(required_formats=[],
> > read_only=False):
> > return func_wrapper
> > return skip_test_decorator
> > +def skip_if_root(func):
>
> skip_if_user_is_root() is slightly less confuse.
Ok, I can make this change.
Kevin