On 10/28/2014 12:26 PM, Michael Stone wrote:
> On Mon, Oct 27, 2014 at 11:51:44PM +0000, Pádraig Brady wrote:
>>> --- src/df.c.orig       2014-10-27 12:14:39.633167418 -0400
>>> +++ src/df.c    2014-10-27 13:16:54.524752800 -0400
>>> @@ -631,6 +631,10 @@
>>>            /* Stat failed - add ME to be able to complain about it later.  
>>> */
>>>            buf.st_dev = me->me_dev;
>>>          }
>>> +      else if (me->me_remote)
>>> +        {
>>> +          /* ignore duplicate network mounts */
>>> +        }
>>>        else
>>>          {
>>>            /* If we've already seen this device...  */
>>
>> Still not convinced about that hunk.
> 
> I'm increasingly coming to the position that this is something that should 
> basically be opaque to the client. The two exports have independent access 
> control policies, and from the user's perspective are two different "things". 
> The admin on the client system treats them as different, and doesn't 
> necessarily have any insight into how the server is configured. Suppressing 
> one as a duplicate is probably more confusing than helpful. And if the server 
> is reconfigured, the you'll suddenly have two entries in df rather than one, 
> even though nothing on the client side has changed. Though it would 
> complicate things even more, maybe a reasonable heuristic would be to 
> suppress remote mounts only if the remote path is a duplicate?

Yes the separate exports argument has merit and detecting those should handle 
this case.
I've attached a proposed patch against upstream

thanks,
Pádraig.

>From b391963bc9e392ced266ef5f91e9c329d460af55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Wed, 29 Oct 2014 02:49:17 +0000
Subject: [PATCH] df: avoid suppressing remote mounts of separate exports

* src/df.c (filter_mount_list): Separate remote locations may
have different ACLs etc. so list each even if they share
the same remote device and thus storage.
* NEWS: Mention the change in behavior.

Reported in http://bugs.debian.org/737399
Reported in http://bugzilla.redhat.com/920806
---
 NEWS     |    6 ++++++
 src/df.c |   33 +++++++++++++++++++++------------
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index 5fbdc6a..6dda9f5 100644
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,12 @@ GNU coreutils NEWS                                    -*- outline -*-
   dd accepts a new status=progress level to print data transfer statistics
   on stderr approximately every second.
 
+** Changes in behavior
+
+  df no longer suppresses separate exports of the same remote device,
+  as these are probably explicitly mounted and may have separate ACLs etc.
+  [suppression was introduced in coreutils-8.21]
+
 ** Improvements
 
   cp,install,mv will convert smaller runs of NULs in the input to holes,
diff --git a/src/df.c b/src/df.c
index a52afc4..cf0d433 100644
--- a/src/df.c
+++ b/src/df.c
@@ -640,18 +640,27 @@ filter_mount_list (bool devices_only)
 
           if (devlist)
             {
-                  /* let "real" devices with '/' in the name win.  */
-              if ((strchr (me->me_devname, '/')
-                   && ! strchr (devlist->me->me_devname, '/'))
-                  /* let a shorter mountdir win.  */
-                  || (strlen (devlist->me->me_mountdir)
-                      > strlen (me->me_mountdir))
-                  /* let an entry overmounted on a different device win...  */
-                  || (! STREQ (devlist->me->me_devname, me->me_devname)
-                      /* ... but only when matching an existing mount point, to
-                      avoid problematic replacement when given inaccurate mount
-                      lists, seen with some chroot environments for example.  */
-                      && STREQ (me->me_mountdir, devlist->me->me_mountdir)))
+              if (me->me_remote && devlist->me->me_remote
+                  && ! STREQ (devlist->me->me_devname, me->me_devname))
+                {
+                  /* Don't discard remote entries with different locations,
+                     as there may be differing ACLs etc. per remote path, and
+                     also these are more likely to be explicitly mounted.  */
+                }
+              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)
+                           > strlen (me->me_mountdir))
+                       /* 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,
+                              to avoid problematic replacement when given
+                              inaccurate mount lists, seen with some chroot
+                              environments for example.  */
+                           && STREQ (me->me_mountdir,
+                                     devlist->me->me_mountdir)))
                 {
                   /* Discard mount entry for existing device.  */
                   discard_me = devlist->me;
-- 
1.7.7.6

Reply via email to