yast / yast-storage-ng

Rewrite of https://github.com/yast/yast-storage
http://yast.github.io/
GNU General Public License v2.0
14 stars 20 forks source link

Release sources before probing #1290

Closed joseivanlopez closed 2 years ago

joseivanlopez commented 2 years ago

Problem

An installation repo can be provided from a local disk by using install= boot option (e.g., install=hd:/isos/tw.iso). YaST mounts the device containing the local repository. If libstorage-ng has to unmount such a device (e.g., because the mount point of the repo device was edited in the Expert Partitioner), then the unmount action fails. Note that libstorage-ng would try to unmount the device by using a device path as the device would be mounted under the installation mount point (e.g., /mnt/var/adm/mount/AP_0xppFmPE).

Screenshot_SLE-15-SP4_2022-03-31_12:14:25

Solution

Always release all installation sources before probing. With that, libstorage-ng will never try to unmount the device. Note that YaST Packager will mount the source devices again under demand, for example, when the installation starts. Also note that this is only needed during installation. In a running system there are no chroot paths involved.

NOTE: this PR will be merged into pre-SLE-15-SP4 brach. Such a brach will be merged into master after branching SLE-15-SP4. The version should be bumped while merging pre-SLE-15-SP4 into master.

Testing

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.0002%) to 97.777% when pulling 99a7fcabdf4ad29ae9b1ee1018baf3f92a4adaa8 on joseivanlopez:release-resources into 695e7218febffe6b83c84b8595171133298717e5 on yast:pre-SLE-15-SP4.

joseivanlopez commented 2 years ago

lgtm

Just one question: this ensures the correctness of the probing data. But does this also ensure the media are detached at the proposal stage?

No, it doesn't, but ensuring the device is not mounted when probing is enough to avoid libstorage-ng tries to unmount the device. Also note that the proposal is not a problem at all, because it does not use devices containing sources.

wfeldt commented 2 years ago

The proposal is ok but the state doesn't change up to the commit phase. I was just worried it might cause problems during the commit. If it doesn't, I'm fine.

wfeldt commented 2 years ago

IIRC the issue was that libstorage goes to mount the partition (as it's part of the proposal and has a mount point) - and there fails as it's already mounted.

joseivanlopez commented 2 years ago

AFAIS in my tests, the issue happens when libstorage-ng has to unmount the mount point created for the source device (e.g., /var/adm/mount/AP_0xppFmPE, let's call it "source mount point"). During the commit phase, libstorage-ng will try to unmount the device if the source mount point was modified. For example, editing or removing the mount path in the Expert Partitioner. To unmount the device, libstorage-ng uses the source mount point prepended by the installation path (e.g., /mnt). The unmount action fails because such a mount point does not exist.

Another option to make libstroage-ng to unmount the device is to delete (or format, re-use, etc) the device, but in this case unmounting is the least of our problems because the installation media would be missing.

So, relaeasing the sources before probing ensures that libstorage-ng will not try to unmount the source mount point. What scenarios can be present during the commit?

a) The source device is currently mounted in the system: libstorage-ng will not unmount the device, but it will create a new mount point if needed (e.g., /mnt/data). b) The source device is not mounted: libstorage-ng will not try to unmount either and it will mount the device if needed. c) The source device was changed (formatted, removed): libstorage-ng will not try to unmount but YaST Packager will not be able to find the sources.

wfeldt commented 2 years ago

See a). In addition, libstorage will mount the sources at the mount point - and that fails as it's already mounted somewhere else.

joseivanlopez commented 2 years ago

See a). In addition, libstorage will mount the sources at the mount point - and that fails as it's already mounted somewhere else.

I have tested it, and libstorage-ng does not complain when the iso is mounted before the commit phase. Libstorage-ng mounts the device correctly and everything works as expected.