[trusty] mount -o loop is limited to 8 loop devices

For the record: Commit 0b14bf7a is not sufficient. That neither fixes "losetup" nor "mount -o loop", and systemd's .mount units also just get translated into a /bin/mount -o loop" call.

Hello Martin, or anyone else affected,

Accepted util-linux into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/util-linux/2.20.1-5.1ubuntu20.8 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

I successfully ran the test case with the packages from -proposed. The VM also still boots successfully and shows no other sign of trouble.

This felt unusually risk to me, so I took a look at the patch before releasing. I found a number of problems:

1. This patch leaks a file descriptor every time the new code path runs. loop_ctl_fd is opened but not closed.

2. "int devnr = -1" is defined but never used.

3.

> ll->ncur = ioctl(loop_ctl_fd, LOOP_CTL_GET_FREE);

This is wrong. ncur is an integer iterator, not necessarily a loop number. It keeps state between multiple calls to looplist_next, and you're clobbering it. It may be the case that it happens not to get used again when LLFLG_LOOPCTL is set, but that's no reason to re-use an existing variable for a different purpose. This makes the code much more difficult to check for unexpected side effects and thus may hide bugs.

4. looplist_next() is called to acquire both free and used loop device fds (based on LLFLG_USEDONLY/LLFLG_FREEONLY; see looplist_open_dev()). The fashion in which it does this is quite convulated. Additionally, it appears designed to be called multiple times (like a C++ iterator). The new code will always return a new loop device when requested. This is of course the point of this SRU. However, a consequence of this is that calls to looplist_next with LLFLG_FREEONLY are now unbounded, and this may have implications throughout the code. I can't find any, but this is certainly Regression Potential territory, and IMHO all known use cases that call looplist_next() should be checked for correct behaviour in SRU verification.

5. The "mount" and "umount" commands also call functions defined by lomount.c, so loop-affecting behaviour should be checked in these, too.

It feels like luck to me that this code didn't introduce a bug that I could find. However, we want to minimise regression risk in SRUs. Even though I couldn't find a specific bug, issues 1 and 3 above unnecessarily add to the risk of regression in this SRU, IMHO, and are easy to mitigate. So I'm marking this bug verification-failed. You might as well fix issue 2 while you're there. For issues 4 and 5, I'll update the test plan in the bug description.

Thanks for your detailed feedback, I revisited the patch (please see the diff here http://paste.ubuntu.com/23522718/). I think (1.) - (3.) are covered.

Hello Martin, or anyone else affected,

Accepted util-linux into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/util-linux/2.20.1-5.1ubuntu20.9 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

I successfully ran the test case against ubuntu20.9 in -proposed. I also verified that using losetup and mount on loop devices still works:

$ sudo losetup -f --show debian-stretch-DI-alpha7-amd64-netinst.iso
/dev/loop0
$ sudo mount /dev/loop0 /mnt
mount: block device /dev/loop0 is write-protected, mounting read-only
$ ls /mnt
autorun.inf debian [...]

With the -proposed version, losetup -f also succeeds to use /dev/loop-control, so you can do the above a few times and it works with /dev/loop12 too. When moving /dev/loop-control out of the way, the above also works up to loop7, and then errors out with "losetup: could not find any free loop device" as expected. "losetup -a" correctly shows the bound devices/files.

16:49 <rbasak> bdmurray: I hadn't looked at bug 1640823 again, no. Shall I review and release if appropriate during my SRU day tomorrow?

16:50 <bdmurray> rbasak: Since you had an opinion I think that's appropriate

The verification of the Stable Release Update for util-linux has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package util-linux - 2.20.1-5.1ubuntu20.9

---------------
util-linux (2.20.1-5.1ubuntu20.9) trusty; urgency=medium

  * mount/lomount.c: Query /dev/loop-control for next free loopback device.
    (LP: #1640823)

 -- Thomas Voß <email address hidden> Tue, 15 Nov 2016 09:52:37 +0100