Subject: Re: bug#21372: [PATCH] df: fix prioritize real mounts over bind mounts

From: Pádraig Brady <[email-masked]>
Date: Sat, 29 Aug 2015 01:37:55 +0100

On 28/08/15 21:42, Dave Chiluk wrote:
> Fixes an issue where bind mounts with shorter mount directories than the
> original mount are prioritized when running df.  The root cause of this
> is that /proc/self/mountinfo now lists the filesystem device with bind
> mounts rather than the source directory. With /etc/mtab the source
> device was listed as the originating directory so this was not an issue.
> 
> More information is available here.
> https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/1432871

It's debatable as to whether we should prefer standard mounts
over bind mounts, but I'm 60:40 for preferring mount points
(more towards) the root of the device.

> ---
>  src/df.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/df.c b/src/df.c
> index 2e541b9..13e2661 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -652,9 +652,12 @@ filter_mount_list (bool devices_only)
>                else if ((strchr (me->me_devname, '/')
>                         /* let "real" devices with '/' in the name win.  */
>                          && ! strchr (devlist->me->me_devname, '/'))
> -                       /* let a shorter mountdir win.  */
> -                       || (strlen (devlist->me->me_mountdir)
> +                       /* let a shorter mountdir win. */
> +                       /* Only if it's not a bind mount.*/
> +                       || ((strlen (devlist->me->me_mountdir)
>                             > strlen (me->me_mountdir))
> +                          && (strlen (devlist->me->me_mountroot)
> +                           > strlen(me->me_mountroot)))

I think this should be:       >= strlen (me->me_mountroot)

>                         /* let an entry overmounted on a new device win...  */
>                         || (! STREQ (devlist->me->me_devname, me->me_devname)
>                             /* ... but only when matching an existing mnt point,
> 

Cool, so that depends on http://lists.gnu.org/archive/html/bug-gnulib/2015-08/msg00030.html

I had some personal notes RE the mountroot element,
which I'll add here for reference.

  "$ findmnt --df shows bind mounts (and BTRFS subvols) like:
    SOURCE          FSTYPE            SIZE  USED  AVAIL USE% TARGET
    /dev/sdb2       xfs              12.7G  9.4G   3.3G  74% /
    /dev/sdb2[/etc] xfs                                      /root/chroot
  Note the blank stats for the bind mount (df would probably use - - - here).
  The mountroot is presented in mountinfo and we could adjust mountlist.c
  to return this element, to allow df to present the bindmount info?
  Not sure about that actually since findmnt already does this, and also df is
  DEVICE oriented as per the name."

So while we mightn't distinguish the bind mounts like findmnt does,
it seems it's still useful to return the mountroot element.
This should not introduce API concerns since gnulib is not
released as a separate lib.

thanks!
Pádraig.