`df` shows bind mounts instead of real mounts.

Bug #1432871 reported by Dave Chiluk
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
coreutils (Ubuntu)
Fix Released
Low
Dave Chiluk
Declined for Utopic by Chris J Arges
Trusty
Fix Released
Low
Dave Chiluk
Vivid
Fix Released
Low
Dave Chiluk
Wily
Fix Released
Low
Dave Chiluk

Bug Description

[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>

Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

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.]

tags: added: patch
Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Revision history for this message
Sebastien Bacher (seb128) wrote :

@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

affects: ubuntu → coreutils (Ubuntu)
Dave Chiluk (chiluk)
Changed in coreutils (Ubuntu):
importance: Undecided → Low
Revision history for this message
Dave Chiluk (chiluk) wrote :

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.

description: updated
Dave Chiluk (chiluk)
description: updated
Revision history for this message
Dave Chiluk (chiluk) wrote :

Fixed version and added origin information to debdiff.

Revision history for this message
Adam Conrad (adconrad) wrote :

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".

Changed in coreutils (Ubuntu):
status: New → Won't Fix
Mathew Hodson (mhodson)
Changed in coreutils (Ubuntu):
milestone: trusty-updates → none
Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Changed in coreutils (Ubuntu):
status: Won't Fix → Incomplete
Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Dave Chiluk (chiluk)
description: updated
description: updated
Dave Chiluk (chiluk)
description: updated
Dave Chiluk (chiluk)
Changed in coreutils (Ubuntu):
status: Incomplete → In Progress
Revision history for this message
Michael Terry (mterry) wrote :

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

Dave Chiluk (chiluk)
Changed in coreutils (Ubuntu Vivid):
assignee: nobody → Dave Chiluk (chiluk)
Changed in coreutils (Ubuntu Trusty):
assignee: nobody → Dave Chiluk (chiluk)
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in coreutils (Ubuntu Wily):
status: In Progress → Fix Released
Dave Chiluk (chiluk)
Changed in coreutils (Ubuntu Vivid):
status: New → In Progress
Changed in coreutils (Ubuntu Trusty):
status: New → In Progress
Changed in coreutils (Ubuntu Wily):
status: Fix Released → Fix Committed
Dave Chiluk (chiluk)
Changed in coreutils (Ubuntu Wily):
status: Fix Committed → Fix Released
Revision history for this message
Michael Terry (mterry) wrote :

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

Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Mathew Hodson (mhodson)
Changed in coreutils (Ubuntu Trusty):
importance: Undecided → Low
Changed in coreutils (Ubuntu Vivid):
importance: Undecided → Low
Revision history for this message
Chris J Arges (arges) wrote : Please test proposed package

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!

Changed in coreutils (Ubuntu Vivid):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Dave Chiluk (chiluk) wrote :

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.

Dave Chiluk (chiluk)
tags: added: verification-done-vivid verification-done-wily
removed: verification-needed
Revision history for this message
Michael Terry (mterry) wrote :

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

Revision history for this message
Dave Chiluk (chiluk) wrote :

Thanks a ton mterry!

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in coreutils (Ubuntu Vivid):
status: Fix Committed → Fix Released
Revision history for this message
Chris J Arges (arges) wrote : Update Released

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.

Revision history for this message
Dave Chiluk (chiluk) wrote :

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

Revision history for this message
Martin Pitt (pitti) wrote :

Package got sponsored already, unsubscribing sponsors.

Revision history for this message
Chris J Arges (arges) wrote : Please test proposed package

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!

Changed in coreutils (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Dave Chiluk (chiluk) wrote :

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.

Revision history for this message
Dave Chiluk (chiluk) wrote :

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.

Revision history for this message
Dave Chiluk (chiluk) wrote :

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.

Revision history for this message
Dave Chiluk (chiluk) wrote :

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:.

Revision history for this message
Dave Chiluk (chiluk) wrote :
Revision history for this message
Chris J Arges (arges) wrote :

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!

Revision history for this message
Dave Chiluk (chiluk) wrote :

Verified df has been fixed. yay.

tags: added: verification-done verification-done-trusty
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in coreutils (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Eric Jensen (ericcj) wrote :

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% /

Revision history for this message
Eric Jensen (ericcj) wrote :

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

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.