Subject: bug#17833: closed (Re: bug#17833: coreutils 8.22 df)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 10 Jul 2014 23:29:02 +0000

Your bug report

#17833: coreutils 8.22 df

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
17833: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17833
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#17833: coreutils 8.22 df
From: Pádraig Brady <[email-masked]>
To: "Gabor Z. Papp" <[email-masked]>
Cc: [email-masked]
Date: Fri, 11 Jul 2014 00:28:34 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On 07/08/2014 06:28 PM, Pádraig Brady wrote:
> On 06/25/2014 01:24 PM, Gabor Z. Papp wrote:
>> * Pádraig Brady <[email-masked]>:
>>
>> | > $ cat /etc/mtab 
>> | > /dev/md0 / ext3 rw,errors=remount-ro 0 0
>> | > proc /proc proc rw,relatime 0 0
>> | > sysfs /sys sysfs rw,relatime 0 0
>> | > tmpfs /dev tmpfs rw,relatime,size=4k,mode=755 0 0
>> | > devpts /dev/pts devpts rw,relatime,mode=620 0 0
>> | > /dev/md1 /var/archive ext3 rw,errors=remount-ro 0 1
>> | > /var/archive/home /home none rw,bind 0 0
>> | > /var/archive/spool /var/spool none rw,bind 0 0
>> | > /var/archive/www /var/lib/www none rw,bind 0 0
>>
>> | Is this mtab really a link to /proc/mounts?
>>
>> No, its a standalone file.
>>
>> | That's the normal case on modern Linux distros.
>>
>> Its not a modern linux. :-)
>>
>> | > $ df-8.22 -h
>> | > Filesystem      Size  Used Avail Use% Mounted on
>> | > /dev/md0         21G  8.8G   11G  46% /
>> | > tmpfs           4.0K     0  4.0K   0% /dev
>> | > 
>> | > And where is /dev/md1 ?
>>
>> | Not sure what's going on here.
>> | Since you've compiled various versions,
>> | could you replace src/df.c in your 8.22 build dir with:
>> | http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob_plain;f=src/df.c;hb=HEAD
>> | There have been many df changes since 8.22, so it would
>> | be good to check with the latest.
>>
>> $ /tmp/df-git -h
>> Filesystem      Size  Used Avail Use% Mounted on
>> /dev/md0         21G  9.0G   11G  47% /
>> tmpfs           4.0K     0  4.0K   0% /dev
>>
>> Same problem, /dev/md1 missing.
> 
> The missing entry is due to the bind mounts being classified as "dummy" devices.
> There is code in gnulib to explicitly avoid that for bind mounts,
> however that was negated with this change:
> http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=62bb7a8b
> Now that change is correct in isolation, but when combined
> with the incomplete original change, it triggered the issue.
> The attached patch should fix this up.
> 
>>
>> | > running df from coreutils 8.21 shows different result:
>> | > 
>> | > $ df-8.21 -h
>> | > Filesystem         Size  Used Avail Use% Mounted on
>> | > /dev/md0            21G  8.8G   11G  46% /
>> | > tmpfs              4.0K     0  4.0K   0% /dev
>> | > /var/archive/home  273G  244G   29G  90% /home
>> | > 
>> | > Still weird.
>>
>> | As part of the device de-duplication logic we used in 8.21
>> | we're favoring '/home' since it's shorter than '/var/archive'
>> | and thus we're using the '/var/archive/home' "device".
>>
>> | > Trying with 8.20:
>> | > 
>> | > $ df-8.20 -h
>> | > Filesystem      Size  Used Avail Use% Mounted on
>> | > /dev/md0         21G  8.8G   11G  46% /
>> | > tmpfs           4.0K     0  4.0K   0% /dev
>> | > /dev/md1        273G  244G   29G  90% /var/archive
>>
>> | I agree that this is the best output for your case.
>>
>> Right.
>>
>> | I suppose we could stat the sources in the de-duplication logic
>> | to favor real devices. I.E. stat '/var/archive/home' and '/dev/md1'
>> | in your case to favor the latter.
> 
> That's something we could still do in addition in coreutils.

The attached should handle this by giving precedence
to displaying real device nodes.

thanks,
Pádraig.
>From 840e7afb6d7d499d52aaac0f9d34cb90208b2183 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Thu, 10 Jul 2014 23:59:35 +0100
Subject: [PATCH] df: give precedence to real device nodes

This is significant when /etc/mtab is a real file
rather than a symlink to /proc/mounts, in which case
the bind mounted dir will be listed as the source "device"
rather than the real base device.  Therefore we stat
the device node, which should be local and a relatively
cheap operation, and give precedence to real device nodes.

For example given this setup:

  truncate -s10M bind.img
  mkfs -F bind.img
  mount bind.img /var/archive/
  mkdir -p /var/archive/home
  mkdir -p /t/home
  mount --bind /var/archive/home/ /t/home/

on an older system without /proc/mounts we would have displayed:

  Filesystem            Mounted on
  /var/archive/home/    /t/home

but we now display:

  Filesystem            Mounted on
  /dev/loop0            /var/archive

Note on a newer system using /proc/mounts we display:

  Filesystem            Mounted on
  /dev/loop0            /t/home

* src/df.c (filter_mount_list): stat() the device node for
each mount entry and use that to give precedence to real
device nodes.
* NEWS: Mention the bug fix.
Fixes http://bugs.gnu.org/17833
---
 NEWS     |    5 +++++
 src/df.c |   35 ++++++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index 4e90b79..e942fe8 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   than error messages or values for the wrong file system.
   [These bugs were present in "the beginning".]
 
+  df now give precedence to displaying real device nodes in the presence of
+  bind mounts, on systems where the base device is not listed for all entries
+  in the mount list.
+  [bug introduced in coreutils-8.21]
+
   du now silently ignores directory cycles introduced with bind mounts.
   Previously it would issue a warning and exit with a failure status.
   [bug introduced in coreutils-8.1]
diff --git a/src/df.c b/src/df.c
index 063cabf..46aeef9 100644
--- a/src/df.c
+++ b/src/df.c
@@ -48,6 +48,7 @@
 static struct devlist
 {
   dev_t dev_num;
+  bool real_dev;
   struct mount_entry *me;
   struct devlist *next;
 } *device_list;
@@ -618,35 +619,46 @@ filter_mount_list (bool devices_only)
   /* Sort all 'wanted' entries into the list device_list.  */
   for (me = mount_list; me;)
     {
-      struct stat buf;
+      struct stat mnt_buf;
       struct devlist *devlist;
       struct mount_entry *discard_me = NULL;
+      bool real_dev = false;
 
       /* TODO: On Linux we might avoid this stat() and another in get_dev()
          by using the device IDs available from /proc/self/mountinfo.
          read_file_system_list() could populate me_dev from those
          for efficiency and accuracy.  */
-      if (-1 == stat (me->me_mountdir, &buf))
+      if (-1 == stat (me->me_mountdir, &mnt_buf))
         {
           /* Stat failed - add ME to be able to complain about it later.  */
-          buf.st_dev = me->me_dev;
+          mnt_buf.st_dev = me->me_dev;
         }
       else
         {
+          /* when /etc/mtab is not linked to /proc/mounts,
+             then in the presence of bind mounts, the source will
+             be the bind mounted directory, rather than the base device.
+             In this case we want to give precedence to the base device.  */
+          struct stat sb;
+          real_dev = (! me->me_dummy
+                      && stat (me->me_devname, &sb) == 0
+                      && (S_ISBLK (sb.st_mode) || S_ISCHR (sb.st_mode)));
+
           /* If we've already seen this device...  */
           for (devlist = device_list; devlist; devlist = devlist->next)
-            if (devlist->dev_num == buf.st_dev)
+            if (devlist->dev_num == mnt_buf.st_dev)
               break;
 
           if (devlist)
             {
               /* ...let the shorter mountdir win.  */
-              if ((strchr (me->me_devname, '/')
-                   && ! strchr (devlist->me->me_devname, '/'))
-                  || (strlen (devlist->me->me_mountdir)
-                      > strlen (me->me_mountdir))
-                  /* or one overmounted on a different device.  */
-                  || ! STREQ (devlist->me->me_devname, me->me_devname))
+              if (real_dev >= devlist->real_dev
+                  && ((strchr (me->me_devname, '/')
+                       && ! strchr (devlist->me->me_devname, '/'))
+                      || (strlen (devlist->me->me_mountdir)
+                          > strlen (me->me_mountdir))
+                      /* or one overmounted on a different device.  */
+                      || ! STREQ (devlist->me->me_devname, me->me_devname)))
                 {
                   /* Discard mount entry for existing device.  */
                   discard_me = devlist->me;
@@ -672,7 +684,8 @@ filter_mount_list (bool devices_only)
           /* Add the device number to the global list devlist.  */
           devlist = xmalloc (sizeof *devlist);
           devlist->me = me;
-          devlist->dev_num = buf.st_dev;
+          devlist->dev_num = mnt_buf.st_dev;
+          devlist->real_dev = real_dev;
           devlist->next = device_list;
           device_list = devlist;
 
-- 
1.7.7.6

Subject: coreutils 8.22 df
From: "Gabor Z. Papp" <[email-masked]>
To: [email-masked]
Date: Sun, 22 Jun 2014 10:41:29 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

lo lo,

seems like I discovered a problem in 8.22's df util:

$ cat /etc/fstab 
/dev/md0 / ext3 defaults,errors=remount-ro 0 1
/dev/sda2 swap swap defaults 0 0
/dev/sdb2 swap swap defaults 0 0
/dev/md1 /var/archive ext3 defaults,errors=remount-ro 0 1
/dev/sr0 /mnt/cdrom iso9660 defaults,noauto 0 0

proc /proc proc defaults 0 0
none /proc/bus/usb usbfs defaults 0 0

$ cat /etc/mtab 
/dev/md0 / ext3 rw,errors=remount-ro 0 0
proc /proc proc rw,relatime 0 0
sysfs /sys sysfs rw,relatime 0 0
tmpfs /dev tmpfs rw,relatime,size=4k,mode=755 0 0
devpts /dev/pts devpts rw,relatime,mode=620 0 0
/dev/md1 /var/archive ext3 rw,errors=remount-ro 0 1
/var/archive/home /home none rw,bind 0 0
/var/archive/spool /var/spool none rw,bind 0 0
/var/archive/www /var/lib/www none rw,bind 0 0

$ df-8.22 -h
Filesystem      Size  Used Avail Use% Mounted on
/dev/md0         21G  8.8G   11G  46% /
tmpfs           4.0K     0  4.0K   0% /dev

And where is /dev/md1 ?

running df from coreutils 8.21 shows different result:

$ df-8.21 -h
Filesystem         Size  Used Avail Use% Mounted on
/dev/md0            21G  8.8G   11G  46% /
tmpfs              4.0K     0  4.0K   0% /dev
/var/archive/home  273G  244G   29G  90% /home

Still weird.

Trying with 8.20:

$ df-8.20 -h
Filesystem      Size  Used Avail Use% Mounted on
/dev/md0         21G  8.8G   11G  46% /
tmpfs           4.0K     0  4.0K   0% /dev
/dev/md1        273G  244G   29G  90% /var/archive

Now ok.