Re: [PATCH] iotests: Skip read-only cases in 118 when run as root

[Philippe Mathieu-Daudé]

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


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

> +    '''Skip Test Decorator
> +       Runs the test only without root permissions'''
> +    def func_wrapper(*args, **kwargs):
> +        if os.getuid() == 0:
> +            case_notrun('{}: cannot be run as root'.format(args[0]))
> +        else:
> +            return func(*args, **kwargs)
> +    return func_wrapper
> +
>   def execute_unittest(output, verbosity, debug):
>       runner = unittest.TextTestRunner(stream=output, descriptions=True,
>                                        verbosity=verbosity)