On 17.05.21 16:06, Andy Shevchenko wrote: > On Mon, May 17, 2021 at 4:29 PM Tom Rini <tr...@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.g...@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.g...@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