Re: [PATCH] test: Have test_fs work with non-functional guestmount tools

7 Jul 2020

On 7/7/20 12:46 PM, Tom Rini wrote:
...
On Tue, Jul 07, 2020 at 12:34:19PM -0600, Stephen Warren wrote:
...
On 7/7/20 9:53 AM, Tom Rini wrote:
...
Since 2011 Ubuntu has intentionally broken support for guestmount[1] by
default and requires sysadmin intervention to re-enable support.  This
in turn exposed that in our tests if guestmount is available but fails
we do not fall back to trying to use sudo.  Restructure our code to try
sudo if guestmount fails rather than only when it is not in our path.
Further, only note that we are using fuse on success of the call.
That is ... an interesting bug!
This change looks conceptually fine.
...

diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
...
...
@@ -206,10 +206,11 @@ def mount_fs(fs_type, device, mount_point):
...

   try:



...
...
         mount_opt = 'loop,rw'
         if re.match('fat', fs_type):
             mount_opt += ',umask=0000'

...
...
         # may not be effective for some file systems
         check_call('sudo chmod a+rw %s' % mount_point, shell=True)


   except CalledProcessError:


       raise



That last/inner try...except/raise clause doesn't seem useful. Perhaps
just remote try...except and keep the body?
I don't quite follow, sorry.  What we have today is if/else on
guestmount, but since check_call() throws CalledProcessError when it
fails to run we never try sudo'ing.  That's why I put the inner
try/except/raise in.
It's the outer try/except that switches between the two options, not the
inner one.
It makes sense to have one try/except to catch errors running guestmount
and switch to sudo mount. However, the second try/except block that
solely wraps the sudo mount call doesn't do anything; it simply catches
an exception and then immediately re-raises it. This same issue is
actually present in the original code, except that there is applies to a
larger region of code.
So, wherever the code does this:
try:
        something
    except CalledProcessError:
        raise
Just replace that with this:
something
So...
fuse_mounted = False
     try:
         if tool_is_in_path('guestmount'):
             check_call('guestmount -a %s -m /dev/sda %s'
                 % (device, mount_point), shell=True)
            fuse_mounted = True
    except CalledProcessError:
        # TODO: Remove the try here
        # TODO: Remove 1 indent level from the rest of this code
             mount_opt = 'loop,rw'
             if re.match('fat', fs_type):
                 mount_opt += ',umask=0000'
@@ -219,8 +220,8 @@ def mount_fs(fs_type, device, mount_point):
# may not be effective for some file systems
             check_call('sudo chmod a+rw %s' % mount_point, shell=True)
        # TODO: Remove the except/raise here

    