[PATCH 1/1] test: revert Don't unmount not (yet) mounted system
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon May 17 19:08:26 CEST 2021
On 17.05.21 16:06, Andy Shevchenko wrote:
> On Mon, May 17, 2021 at 4:29 PM Tom Rini <trini at konsulko.com> wrote:
>> On Mon, May 17, 2021 at 03:21:41PM +0200, Heinrich Schuchardt wrote:
>>> On 17.05.21 13:44, Andy Shevchenko wrote:
>>>> On Mon, May 17, 2021 at 2:35 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>
>>>>> On 17.05.21 13:16, Andy Shevchenko wrote:
>>>>>> On Mon, May 17, 2021 at 10:48:33AM +0200, Heinrich Schuchardt wrote:
>>>>>>> On 17.05.21 08:33, Andy Shevchenko wrote:
>>>>>>>> On Thu, May 13, 2021 at 2:41 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>>>>>
>>>>>>>>> Since commit 1ba21bb06b08 ("test: Don't unmount not (yet) mounted system")
>>>>>>>>> the following tests are skipped:
>>>>>>>>>
>>>>>>>>> test/py/tests/test_fs/test_basic.py
>>>>>>>>> test/py/tests/test_fs/test_ext.py
>>>>>>>>>
>>>>>>>>> SKIPPED [13] test/py/tests/test_fs/conftest.py:350: Setup failed for
>>>>>>>>> filesystem: ext4. Command 'guestmount -a
>>>>>>>>> build-sandbox/persistent-data/3GB.ext4.img -m /dev/sda
>>>>>>>>> build-sandbox/persistent-data/mnt' returned non-zero exit status 1.
>>>>>>>>>
>>>>>>>>> Let's revert the patch to get our tests back.
>>>>>>>>
>>>>>>>> Probably we may understand first what is the root cause of this issue?
>>>>>>>>
>>>>>>>> In my case I can't allow this to happen, because it will annoy system
>>>>>>>> administrators as I mentioned earlier in the commit message.
>>>>>>>>
>>>>>>>> So, NAK from me and let's investigate.
>>>>>>>> Can you provide a command line that I may run on my environment w/o root access?
>>>>>>>
>>>>>>> Hello Andy,
>>>>>>>
>>>>>>> The tests don't require root access if you have installed the
>>>>>>> libguestfs-tools package and a Linux kernel.
>>>>>>>
>>>>>>> How can I reproduce the problem with duplicate umount?
>>>>>>
>>>>>> I was running this 2+ times in a row (*)
>>>>>>
>>>>>> ./test/py/test.py --bd sandbox --build
>>>>
>>>> (1)
>>>>
>>>>>>
>>>>>> *) I can't run tests right now due to they are more or less constantly broken
>>>>>> one way or the other, now
>>>>>>
>>>>>> ============================================== test session starts ==============================================
>>>>>> platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
>>>>>> rootdir: /home/andy/prj/u-boot/test/py, configfile: pytest.ini
>>>>>> collected 810 items / 1 error / 809 selected
>>>>>>
>>>>>> ___________________________________ ERROR collecting tests/test_fit_ecdsa.py ____________________________________
>>>>>> E ModuleNotFoundError: No module named 'Cryptodome'
>>>>>
>>>>> The missing package is available via
>>>>>
>>>>> apt-get install python3-pycryptodome # Debian/Ubuntu
>>>>>
>>>>> or
>>>>>
>>>>> dnf install python3-pycryptodomex # Fedora
>>>>
>>>> Thanks.
>>>>
>>>> So, I have run above mentioned line (1) with current U-Boot (see
>>>> above), everything is fine, then I have reverted the commit (as your
>>>> patch does, correct), and oops
>>>>
>>>> test/py/tests/test_efi_secboot/test_unsigned.py sss
>>>> [ 88%]
>>>> test/py/tests/test_fs/test_basic.py [sudo] password for andy:
>>>
>>> If you are asked for a sudo password, you have not install libguestfs.
>>>
>>> Please, install the missing package.
>>>
>>>> Sorry, try again.
>>>> [sudo] password for andy:
>>>> Sorry, try again.
>>>> [sudo] password for andy:
>>>> sssssssssssss[sudo] password for andy:
>>>>
>>>> Now I'm waiting for a punishment from the admin, thanks to this test round.
>>>
>>> make tests (on my local machine)
>>>
>>> with origin/master:
>>>
>>> test/py/tests/test_efi_secboot/test_unsigned.py ...
>>> test/py/tests/test_fs/test_basic.py sssssssssssssssssssssssssssssssssssssss
>>> test/py/tests/test_fs/test_ext.py ssssssssssssssssssssss
>>> test/py/tests/test_fs/test_fs_cmd.py .
>>> test/py/tests/test_fs/test_mkdir.py ............
>>> test/py/tests/test_fs/test_symlink.py ssss
>>> test/py/tests/test_fs/test_unlink.py ssssssssssssss
>>>
>>> with your patch reverted
>>>
>>> test/py/tests/test_efi_secboot/test_unsigned.py ...
>>> test/py/tests/test_fs/test_basic.py F............F.........................
>>> test/py/tests/test_fs/test_ext.py ......................
>>> test/py/tests/test_fs/test_fs_cmd.py .
>>> test/py/tests/test_fs/test_mkdir.py ............
>>> test/py/tests/test_fs/test_symlink.py ....
>>> test/py/tests/test_fs/test_unlink.py ..............
>>>
>>> The failures are caused by dd being called with conv=fsync before
>>> mounting with guestfs.
>>>
>>> Obviously we have two scenarios to test separately:
>>>
>>> 1) using sudo for mounting
>>> 2) using guestfs for mounting
>>>
>>>>
>>>> I'm not going to repeat this again, please understand me correctly.
>>>
>>> I assume that you possess a private laptop where your are the admin.
>>> Where is the problem?
>>
>> The problem here is that we have a number of different development /
>> testing scenarios that we need to support. Tests need to run in CI.
>> They need to run in developer-controlled machines. They need to run in
>> corporate-IT controlled machines.
>>
>> The next problem is that as Andy has said, our python logic to handle
>> these cases is, to be polite, not working. Check CI, we're skipping the
>> tests again, which I had missed.
>>
>> Once we get these the fs tests working finally in all cases, we need to
>> update the relevant EFI tests too.
>>
>> Finally, my python skills are not up to getting all of this correct, so
>> help greatly appreciated here.
>
> Thanks, Tom, for summarizing it.
>
> I would like to be helpful here and when I have time, I'll look at it
> closer if nobody beats me up to it. Currently I checked the reason why
> we skip them in my scenario:
> ============================================ short test summary info
> ============================================
> SKIPPED [13] test/py/tests/test_fs/conftest.py:274: Creating failed
> for filesystem: fat16. Command 'mkfs.vfat -F
> 16 /home/andy/prj/u-boot/build-sandbox/persistent-data/3GB.fat16.img'
> returned non-zero exit status 127.
> SKIPPED [13] test/py/tests/test_fs/conftest.py:274: Creating failed
> for filesystem: fat32. Command 'mkfs.vfat -F
> 32 /home/andy/prj/u-boot/build-sandbox/persistent-data/3GB.fat32.img'
> returned non-zero exit status 127.
> SKIPPED [13] test/py/tests/test_fs/conftest.py:274: Creating failed
> for filesystem: ext4. Command 'mkfs.ext4 /ho
> me/andy/prj/u-boot/build-sandbox/persistent-data/3GB.ext4.img'
> returned non-zero exit status 127.
> SKIPPED [11] test/py/tests/test_fs/conftest.py:390: Creating failed
> for filesystem: fat16. Command 'mkfs.vfat -F
> 16 /home/andy/prj/u-boot/build-sandbox/persistent-data/128MB.fat16.img'
> returned non-zero exit status 127.
> SKIPPED [11] test/py/tests/test_fs/conftest.py:390: Creating failed
> for filesystem: fat32. Command 'mkfs.vfat -F
> 32 /home/andy/prj/u-boot/build-sandbox/persistent-data/128MB.fat32.img'
> returned non-zero exit status 127.
> SKIPPED [6] test/py/tests/test_fs/conftest.py:479: Setup failed for
> filesystem: fat16
> SKIPPED [6] test/py/tests/test_fs/conftest.py:479: Setup failed for
> filesystem: fat32
> SKIPPED [4] test/py/tests/test_fs/conftest.py:590: Creating failed for
> filesystem: ext4. Command 'mkfs.ext4 /hom
> e/andy/prj/u-boot/build-sandbox/persistent-data/1GB.ext4.img' returned
> non-zero exit status 127.
> SKIPPED [7] test/py/tests/test_fs/conftest.py:513: Creating failed for
> filesystem: fat16. Command 'mkfs.vfat -F 1
> 6 /home/andy/prj/u-boot/build-sandbox/persistent-data/128MB.fat16.img'
> returned non-zero exit status 127.
> SKIPPED [7] test/py/tests/test_fs/conftest.py:513: Creating failed for
> filesystem: fat32. Command 'mkfs.vfat -F 3
> 2 /home/andy/prj/u-boot/build-sandbox/persistent-data/128MB.fat32.img'
> returned non-zero exit status 127.
> ======================================== 3 passed, 91 skipped in
> 10.02s =========================================
>
>
>
Let's look at the code without your patch:
We have multiple functions ending with:
umount_fs(mount_dir)
except CalledProcessError as err:
pytest.skip('Setup failed for filesystem: ' + fs_type + \
'. {}'.format(err))
return
else:
yield [fs_ubtype, fs_img, md5val]
finally:
umount_fs(mount_dir)
call('rmdir %s' % mount_dir, shell=True)
if fs_img:
call('rm -f %s' % fs_img, shell=True)
If no exception occurs, unmount_fs() is called twice.
Both the unmount_fs() before except and the return statement should be
removed. Then umount_fs() will be called only in the finally branch.
I hope this change to the original code is enough to solve your problem.
I will look into creating a patch.
Best regards
Heinrich
More information about the U-Boot
mailing list