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?= <[email protected]>
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
