`df` shows bind mounts instead of real mounts.

[Dave Chiluk]

[Impact]

 * df displays bind mounts instead of "real" mounts if the bind mount is mounted to a shorter directory.

 * justification - When trusty moved to using /proc/mounts this changed behavior from precise. Additionally it doesn't make sense that a bind mount should show up in df over a real root mount.

 * Explanation - These patches change behavior of df to rely on /proc/self/mountinfo which has more complete info than /proc/mounts. Such as what directory of the filesystem was used as the source of the mount. Additionally given this new information there is a patch on df itself to make use of this new information.

[Test Case]

 * $ mount
<snip>
192.168.1.2:/raid on /raid type nfs
/dev/sdc5 on /data type ext4 (rw)
<snip>
/data/a on /a type none (rw,bind)
/raid/temp on /b type none (rw,bind)

$df
Filesystem 1K-blocks Used Available Use% Mounted on
<snip>
/data/a 449830616 229975284 196982196 54% /a
/raid/temp 7752072192 5581343744 1780023296 76% /b
</snip>

I'd expect to see the real mount prioritized over the bind mount. Like so.
$df
Filesystem 1K-blocks Used Available Use% Mounted on
<snip>
/dev/sdc5 449830616 229975284 196982196 54% /data
192.168.1.2:/raid 7752072192 5581438976 1779929088 76% /raid
<snip>

[Regression Potential]

 * Patch is upstreamed.

 * df command now relies on /proc/self/mountinfo

[Other Info]

 * The behavior of df, mount and similar number of other commands has changed going forward. Previously these commands all processed /etc/mtab which was maintained by the mount command. Going forward they still process /etc/mtab, but this is simply a symlink to /proc/mounts now which is maintained by the kernel and slightly more accurate. Unlike the mount command, the kernel makes no distinction between bind mounts and normal mounts. This is evident by the fact that you can mount a device, bind mount from that device, and then unmount the original mount. The default behavior of df in this case is to simply pick the mounted directory for a device that is the shortest since it has no other information to go on from /proc/mounts. Moving to using /proc/self/mountinfo resolves this issue, and is what upstream is doing moving forward.

 * In order to solve this issue, I have written patches and got them integrated upstream.
gnulib commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=c6148bca89e9465fd6ba3a10d273ec4cb58c2dbe and
coreutils commit: http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=3babaf83875ceac896c8dd3a64248e955dfecef9
have been authored by me.

While attempting to push these patches into Ubuntu, it became apparent that our version of coreutils does not contain the requisite patches to correctly support /proc/self/mountinfo. /proc/self/mountinfo gives more complete information than /proc/self/mounts which /etc/mtab now points to. Using /proc/self/mountinfo is the upstream way of doing things, and it allows us to resolve this specific bug.

Patches required in order to support /proc/self/mountinfo are.
Origin: upstream, gnulib - http://git.savannah.gnu.org/gitweb/?p=gnulib.git
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=3ea43e02541ece750ffc6cd1dfe34195421b4ef3
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=2768ceb7994506e2cfba88be3b6bd13ef5440a90
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=de1cbdd48244c66c51a3e2bc1594ac3ad32ce038
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=3fb6e360363744462ce15c381f0b116c6fc4ce82

Origin: upstream, coreutils - http://git.savannah.gnu.org/gitweb/?p=coreutils.git
commit: http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=1b1c40e1d6f8cf30b6c7c9d31bbddbc3d5cc72e6
Original bug.
________________________________________________________________________
Depending on mount path length df sometimes prioritizes showing bind mounts over real mounts

for example.
$ mount
<snip>
192.168.1.2:/raid on /raid type nfs (rw,nosuid,nodev,noexec,vers=4,addr=192.168.1.2,clientaddr=192.168.1.3)
/dev/sdc5 on /data type ext4 (rw)
<snip>
/data/a on /a type none (rw,bind)
/raid/temp on /b type none (rw,bind)

$df
Filesystem 1K-blocks Used Available Use% Mounted on
<snip>
/data/a 449830616 229975284 196982196 54% /a
/raid/temp 7752072192 5581343744 1780023296 76% /b
</snip>

I'd expect to see the real mount prioritized over the bind mount. Like so.
$df
Filesystem 1K-blocks Used Available Use% Mounted on
<snip>
/dev/sdc5 449830616 229975284 196982196 54% /data
192.168.1.2:/raid 7752072192 5581438976 1779929088 76% /raid
<snip>

[Dave Chiluk (chiluk)]

Here is the fix that I will propose to upstream. Please do not do this upload until we have buy in from upstream.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "lp1432871.trusty.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Dave Chiluk (chiluk)]

Fixed 00list to include patch. I'm used to having quilt do that for me.

[Sebastien Bacher (seb128)]

@Dave, do you have a pointer to the upstream discussion/bug report? Unsubscribing sponsors as well, there is no need to have it in the sponsoring queue before upstream reviews it since you said to block on that

[Dave Chiluk (chiluk)]

So after further review this patch does not make sense upstream, so I never submitted it *(see SRU template).

Subscribing sponsors again to see if we want to Ubuntu Sauce patch this for trusty.

[Dave Chiluk (chiluk)]

Fixed version and added origin information to debdiff.

[Adam Conrad (adconrad)]

While I agree that the precise behaviour was probably slightly more intuitive, I don't think coreutils is a place we want to diverge from upstream. While you might argue that people who relied on the precise behaviour in local scripts need to fix their scripts on upgrade to trusty (true), I could readily argue that there's an unknown quantity of generic Linux software that relies on the upstream behaviour, and would break on precise but work on trusty.

In reality, both those groups are probably tiny but, again, diverging from upstream isn't the answer.

What we should do, IMO, is fix this to parse /proc/self/mountinfo instead (should be trivial, it's just moving the column numbers around a bit to extract the same info), and propose that patch upstream, with a rationale that the results match old mtab behaviour more closely and are, in general more human-readably "pretty".

[Dave Chiluk (chiluk)]

I have a new fix for this that I will attempt to upstream. I will submit it again once it gets accepted upstream.

[Dave Chiluk (chiluk)]

This same patch applies cleanly to both wily and vivid, but it has wily in the changelog.

[Dave Chiluk (chiluk)]

In order to solve this issue, I have written patches and got them integrated upstream.
gnulib commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=c6148bca89e9465fd6ba3a10d273ec4cb58c2dbe and
coreutils commit: http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=3babaf83875ceac896c8dd3a64248e955dfecef9
have been authored by me.

While attempting to push these patches into Ubuntu, it became apparent that our version of coreutils does not contain the requisite patches to correctly support /proc/self/mountinfo gives more complete information than /proc/self/mounts which /etc/mtab now points to. Using /proc/self/mountinfo is the upstream way of doing things, and it allows us to resolve this specific bug.

Patches required in order to support /proc/self/mountinfo are.
Origin: upstream, gnulib - http://git.savannah.gnu.org/gitweb/?p=gnulib.git
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=3ea43e02541ece750ffc6cd1dfe34195421b4ef3
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=2768ceb7994506e2cfba88be3b6bd13ef5440a90
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=de1cbdd48244c66c51a3e2bc1594ac3ad32ce038
commit: http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=3fb6e360363744462ce15c381f0b116c6fc4ce82

Origin: upstream, coreutils - http://git.savannah.gnu.org/gitweb/?p=coreutils.git
commit: http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=1b1c40e1d6f8cf30b6c7c9d31bbddbc3d5cc72e6

[Michael Terry (mterry)]

I've uploaded this patch (plus a bugfix merge from Debian) to wily. Thanks for the patch!

[Launchpad Janitor (janitor)]

This bug was fixed in the package coreutils - 8.23-4ubuntu2

---------------
coreutils (8.23-4ubuntu2) wily; urgency=medium

  * debian/patches/33_chroot_always.dpatch:
    - Update Debian's patch to also comment out the tests that are made
      obsolete by the same patch. We no longer have non-root support
      for "chroot /". Fixes FTBFS.

 -- Michael Terry <email address hidden> Wed, 14 Oct 2015 15:44:28 -0400

[Michael Terry (mterry)]

Uploaded the patch in comment #9 to vivid-proposed. Will sub ubuntu-sru to this bug.

[Dave Chiluk (chiluk)]

In case anyone is wondering. The patch does not cleanly apply against Trusty, and I'm working on a backport/appropriate cherrypick.

[Chris J Arges (arges)]

Hello Dave, or anyone else affected,

Accepted coreutils into vivid-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/coreutils/8.23-3ubuntu1.1 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 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!

[Dave Chiluk (chiluk)]

Here's the debdiff for trusty. The provenance of the patches are included in the header of the debian/patches. I've tested this patch on machines with the above issue as well as machines with varying configurations of disk mounts, and remote mounts even.

[Michael Terry (mterry)]

I reviewed and uploaded the trusty version of the patch. ~ubuntu-sru is already subscribed, so I believe that's it from my side.

[Dave Chiluk (chiluk)]

Thanks a ton mterry!

[Launchpad Janitor (janitor)]

This bug was fixed in the package coreutils - 8.23-3ubuntu1.1

---------------
coreutils (8.23-3ubuntu1.1) vivid; urgency=medium

  * Add support for correctly processing /proc/self/mountinfo.
  * Fix df to prioritize mounts of the root of a device over bind mounts.
    (LP: #1432871)

 -- Dave Chiluk <email address hidden> Wed, 30 Sep 2015 21:06:59 +0000

[Chris J Arges (arges)]

The verification of the Stable Release Update for coreutils 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.

[Dave Chiluk (chiluk)]

Subscribing sponsors and sru team, as both are still necessary for Trusty.

[Martin Pitt (pitti)]

Package got sponsored already, unsubscribing sponsors.

[Chris J Arges (arges)]

Hello Dave, or anyone else affected,

Accepted coreutils into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/coreutils/8.21-1ubuntu5.2 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 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!

[Dave Chiluk (chiluk)]

Fix ftbfs on tests/df/total-unprocessed.sh

Apparently the buildds are running with a slightly different environment than my local build machine which is causing this test to fail due to smart quotes in the set LANG. Explicitly setting LANG=C appears to resolve this.

[Dave Chiluk (chiluk)]

Alright so even though the buildds seem to be getting false failures, I did find a regression in the recent build. I'm going to work through that, and possibly open up a new bug for it, at least for vivid, wily, and xenial.

[Dave Chiluk (chiluk)]

So the regression I discovered was actually just behavior change. Due to the change to /proc/self/mountinfo the filesystem type is now more explicit. In this case that means that nfs mounts are now labeled nfs4.

Additionally I discovered that the test that is now failing was previously being skipped because df was not able to read mount entries on trusty *(which is now fixed in this bug). I'm still working through traces and building tests packages in a ppa to get more output.

[Dave Chiluk (chiluk)]

So after far too many builds with additional debug output, I realized that df itself was functioning exactly like the upstream version of df. However, it turns out the testcase was updated between 8.21 and 8.23. Updating the total-unprocessed testcase resolves the ftbfs.

It should be noted that these df testcases have failed to run prior to this change because df could not read the mount table within a chroot. So it is my determination that updating the total-unapproved testcase is the correct course of action. After fixing this testcase all of the other df testcases also now run, and PASS:.

[Dave Chiluk (chiluk)]

Patch

Successful test build here - https://launchpad.net/~chiluk/+archive/ubuntu/1432871/+build/8386436

[Chris J Arges (arges)]

Hello Dave, or anyone else affected,

Accepted coreutils into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/coreutils/8.21-1ubuntu5.3 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 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!

[Dave Chiluk (chiluk)]

Verified df has been fixed. yay.

[Launchpad Janitor (janitor)]

This bug was fixed in the package coreutils - 8.21-1ubuntu5.3

---------------
coreutils (8.21-1ubuntu5.3) trusty; urgency=medium

  * Fix ftbfs on buildd by updating total-unprocessed testcase from
    upstream. (LP: #1432871)

 -- Dave Chiluk <email address hidden> Thu, 03 Dec 2015 09:48:11 -0600

[Eric Jensen (ericcj)]

Perhaps it was reverted? This actually works fine for me on trusty in the older 8.21-1ubuntu5.1 but not the newer 8.21-1ubuntu5.3 or 8.21-1ubuntu5.4 I ran into the problem with it outputting UUID because amazon's aws ec2 cloudwatch script mon-put-instance-data.pl uses it as its Filesystem dimension and my alarms all of a sudden had insufficient data after apt-get upgrade:

ubuntu@ip-10-0-1-100:~$ dpkg --list coreutils
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name Version Architecture Description
+++-=======================================-========================-========================-===================================================================================
ii coreutils 8.21-1ubuntu5.4 amd64 GNU core utilities
ubuntu@ip-10-0-1-100:~$ /bin/df -k -l -P /
Filesystem 1024-blocks Used Available Capacity Mounted on
/dev/disk/by-uuid/aef583e5-28c9-4161-ac41-bc0f2c3f6b61 103071868 6235644 92542660 7% /
ubuntu@ip-10-0-1-100:~$ sudo dpkg -i coreutils_8.21-1ubuntu5.3_amd64.deb >/dev/null
ubuntu@ip-10-0-1-100:~$ /bin/df -k -l -P /
Filesystem 1024-blocks Used Available Capacity Mounted on
/dev/disk/by-uuid/aef583e5-28c9-4161-ac41-bc0f2c3f6b61 103071868 6235632 92542672 7% /
ubuntu@ip-10-0-1-100:~$ sudo dpkg -i coreutils_8.21-1ubuntu5.1_amd64.deb >/dev/null
ubuntu@ip-10-0-1-100:~$ /bin/df -k -l -P /
Filesystem 1024-blocks Used Available Capacity Mounted on
/dev/xvda1 103071868 6235628 92542676 7% /

[Eric Jensen (ericcj)]

Actually I was wrong, on some boxes with coreutils_8.21-1ubuntu5.4 it was working fine. This is really an initramfs-tools bug: https://bugs.launchpad.net/ubuntu/trusty/+source/coreutils/+bug/1535349

I had the version with the fix, but needed to reboot again to get it to take effect I guess. Rebooting after initramfs-tools - 0.103ubuntu4.3 fixed the issue everywhere for me even on coreutils_8.21-1ubuntu5.4