Bug#737399: metoo
On 10/28/2014 12:26 PM, Michael Stone wrote:
> On Mon, Oct 27, 2014 at 11:51:44PM +, Pádraig Brady wrote:
>>> --- src/df.c.orig 2014-10-27 12:14:39.633167418 -0400
>>> +++ src/df.c2014-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?=
Date: Wed, 29 Oct 2014 02:49:17 +
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 chroo
Bug#737399: metoo
On Mon, Oct 27, 2014 at 11:51:44PM +, Pádraig Brady wrote:
--- src/df.c.orig 2014-10-27 12:14:39.633167418 -0400
+++ src/df.c2014-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?
Mike Stone
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On 10/27/2014 03:12 PM, Harald Dunkel wrote: > On 10/27/14 12:54, Pádraig Brady wrote: >> >> Good point about the man page. >> >> I've submitted a patch to mention that -a includes duplicate file systems. >> > > Thats exactly the point of this bug report: /home and > /data are not "duplicates". The server has a common > partition providing both exports, but I doubt that this > special case should matter on the client. To the client they're both the same file system (device ID). (If you fill up one, you fill up the other). > By "df" showing /data and hiding /home it seems that > /data is somehow superior to /home. df uses the path lengths to chose the most appropriate mount point. In this edge case since the lengths are the same it went with the last mounted entry as that's the most appropriate one in some other edge cases. thanks, Pádraig. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On 10/27/2014 05:21 PM, Michael Stone wrote:
> On Mon, Oct 27, 2014 at 11:54:57AM +, Pádraig Brady wrote:
>> On 10/27/2014 08:36 AM, Harald Dunkel wrote:
>>> On 10/24/14 18:24, Pádraig Brady wrote:
Note /home doesn't seem to be accessible above
which is another reason to prefer /data here.
>>>
>>> What do you mean by "not accessible"? Both /home and
>>> /data work fine.
>>
>> I was referring to the fact that "-" was shown for the /home entry:
>>
>> nfs-home:/space/home - - -- /home
>> nfs-home:/space/data13390666752 10768854016 1941581824 85% /data
>
> That's df's doing. Potentially something like this would get the suggested
> behavior:
>
> --- src/df.c.orig 2014-10-27 12:14:39.633167418 -0400
> +++ src/df.c2014-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.
> @@ -928,7 +932,7 @@
>if (stat (stat_file, &sb) == 0)
> {
>char const * devname = devname_for_dev (sb.st_dev);
> - if (devname && ! STREQ (devname, disk))
> + if (devname && ! STREQ (devname, disk) && !me_remote)
> {
>fstype = "-";
>fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
>
>
> The question then being whether making this case work breaks other cases, and
> whether making it that much harder to explain the logic of what df displays
> is worth it to avoid explaining why a particular set of mounts is captured by
> the current logic. :)
I agree that the "-" placeholders above should not have been output.
The attached patch is just a more general version of the hunk above.
thanks!
Pádraig.
>From 19c3646e1fd2964539fbb4240710e2262582c8d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?=
Date: Mon, 27 Oct 2014 23:37:08 +
Subject: [PATCH] df: ensure -a shows all remote file system entries
commit v8.22-125-g9d736f8 printed placeholder "-" values
for device names that didn't match the preferred device name
for a particular mount point. However that was seen to erroneously
suppress values for aliased host names or exports, common with
remote file systems.
* src/df.c (me_for_dev): Rename from devname_for_dev() so that
we can determine the remoteness as well as the name for the
preferred mount entry.
(get_dev): Don't output place holder values when both
current and preferred mount entries are remote.
Reported in http://bugs.debian.org/737399
---
src/df.c | 17 ++---
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/df.c b/src/df.c
index 5231676..a52afc4 100644
--- a/src/df.c
+++ b/src/df.c
@@ -703,17 +703,17 @@ filter_mount_list (bool devices_only)
}
/* Search a mount entry list for device id DEV.
- Return the corresponding device name if found or NULL if not. */
+ Return the corresponding mount entry if found or NULL if not. */
-static char const * _GL_ATTRIBUTE_PURE
-devname_for_dev (dev_t dev)
+static struct mount_entry const * _GL_ATTRIBUTE_PURE
+me_for_dev (dev_t dev)
{
struct devlist *dl = device_list;
while (dl)
{
if (dl->dev_num == dev)
-return dl->me->me_devname;
+return dl->me;
dl = dl->next;
}
@@ -928,12 +928,15 @@ get_dev (char const *disk, char const *mount_point, char const* file,
else if (process_all && show_all_fs)
{
/* Ensure we don't output incorrect stats for over-mounted directories.
- Discard stats when the device name doesn't match. */
+ Discard stats when the device name doesn't match. Though don't
+ discard when used and current mount entries are both remote due
+ to the possibility of aliased host names or exports. */
struct stat sb;
if (stat (stat_file, &sb) == 0)
{
- char const * devname = devname_for_dev (sb.st_dev);
- if (devname && ! STREQ (devname, disk))
+ struct mount_entry const * dev_me = me_for_dev (sb.st_dev);
+ if (dev_me && ! STREQ (dev_me->me_devname, disk)
+ && (! dev_me->me_remote || ! me_remote))
{
fstype = "-";
fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
--
1.7.7.6
Bug#737399: metoo
On Mon, Oct 27, 2014 at 11:54:57AM +, Pádraig Brady wrote:
On 10/27/2014 08:36 AM, Harald Dunkel wrote:
On 10/24/14 18:24, Pádraig Brady wrote:
Note /home doesn't seem to be accessible above
which is another reason to prefer /data here.
What do you mean by "not accessible"? Both /home and
/data work fine.
I was referring to the fact that "-" was shown for the /home entry:
nfs-home:/space/home - - -- /home
nfs-home:/space/data13390666752 10768854016 1941581824 85% /data
That's df's doing. Potentially something like this would get the
suggested behavior:
--- src/df.c.orig 2014-10-27 12:14:39.633167418 -0400
+++ src/df.c2014-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... */
@@ -928,7 +932,7 @@
if (stat (stat_file, &sb) == 0)
{
char const * devname = devname_for_dev (sb.st_dev);
- if (devname && ! STREQ (devname, disk))
+ if (devname && ! STREQ (devname, disk) && !me_remote)
{
fstype = "-";
fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
The question then being whether making this case work breaks other
cases, and whether making it that much harder to explain the logic of
what df displays is worth it to avoid explaining why a particular set of
mounts is captured by the current logic. :)
Mike Stone
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On Mon, Oct 27, 2014 at 04:12:04PM +0100, Harald Dunkel wrote: Thats exactly the point of this bug report: /home and /data are not "duplicates". The server has a common partition providing both exports, but I doubt that this special case should matter on the client. By "df" showing /data and hiding /home it seems that /data is somehow superior to /home. There is nothing available to df that shows that this is the case. It may not matter to you that they exports are on the same filesystem, but it does matter to the protocol. That was the point of my first email: it isn't clear that there is an algorithm that would show the filesystems that you want while simultaneously suppressing the empty/dummy/duplicate filesystems which a lot of people want (e.g., the rootfs in your example). Mike Stone -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On 10/27/14 12:54, Pádraig Brady wrote: > > Good point about the man page. > > I've submitted a patch to mention that -a includes duplicate file systems. > Thats exactly the point of this bug report: /home and /data are not "duplicates". The server has a common partition providing both exports, but I doubt that this special case should matter on the client. By "df" showing /data and hiding /home it seems that /data is somehow superior to /home. Regards Harri -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On 10/27/2014 08:36 AM, Harald Dunkel wrote: > On 10/24/14 18:24, Pádraig Brady wrote: >> >> Note /home doesn't seem to be accessible above >> which is another reason to prefer /data here. >> > > What do you mean by "not accessible"? Both /home and > /data work fine. I was referring to the fact that "-" was shown for the /home entry: nfs-home:/space/home - - -- /home nfs-home:/space/data13390666752 10768854016 1941581824 85% /data >> In general I think df is behaving correctly here, >> showing what file system space is available on the system. >> If it showed both there would be an ambiguity as to >> whether there were two such file systems available. >> >> If you want to see all nfs file systems you can do it explicitly like: >> >> df -a -t nfs >> > > Sure, unless I am on a "non-coreutils" system, e.g. on AIX: > Surely I am not asking you to support AIX' df flags. But it > would be nice if the central tools included in coreutils stay > in line with other systems. I don't think we should follow w AIX does in showing the duplicate file systems. > BTW, the coreutils man page says about "-a": "include dummy file > systems". Sorry to say, but this is misleading. "/home" is not > dummy at all. Its a valid mount point, seperate from others. Esp. > there is no local partition mounted for "/home", hidden by the > NFS mount. Good point about the man page. I've submitted a patch to mention that -a includes duplicate file systems. thanks, Pádraig. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On 10/24/14 18:24, Pádraig Brady wrote: > > Note /home doesn't seem to be accessible above > which is another reason to prefer /data here. > What do you mean by "not accessible"? Both /home and /data work fine. > In general I think df is behaving correctly here, > showing what file system space is available on the system. > If it showed both there would be an ambiguity as to > whether there were two such file systems available. > > If you want to see all nfs file systems you can do it explicitly like: > > df -a -t nfs > Sure, unless I am on a "non-coreutils" system, e.g. on AIX: bash-3.2# df Filesystem512-blocks Free %UsedIused %Iused Mounted on /dev/hd4 4194304 374 11%11555 3% / /dev/hd212582912 6930544 45%52597 7% /usr /dev/hd9var 2097152 480 48% 9899 7% /var /dev/hd316777216 10351952 39% 278259570% /tmp /dev/fwdump 2097152 20961601%5 1% /var/adm/ras/platform /dev/hd1 2097152 20905921% 96 1% /_home /dev/hd11admin2097152 20961121%5 1% /admin /proc - -- - - /proc /dev/hd10opt16777216 15234200 10%21122 2% /opt /dev/livedump2097152 20961761%4 1% /var/adm/ras/livedump /dev/fslv00268435456 2683935041%4 1% /usr/sys/inst.images /dev/fslv01 1073741824 203972016 82% 244679810% /export /dev/fslv02 16777216 164109683% 9267 1% /sample /dev/fslv05 4194304000 3139006384 26% 3308821 1% /local nfs-data:/space/data 26781332448 3661107344 87% 60070969 8% /data nfs-home:/space/home 26781332448 3661107344 87% 60070969 8% /home bash-3.2# df -a -t nfs df: Not a recognized flag: a Usage: df [-P] | [-IMitv] [-gkm] [-s] [filesystem ...] [file ...] Please note that both "nfs-data" and "nfs-home" are CNAMEs for the same NFS server. /etc/fstab on Linux says nfs-home:/space/home /home nfs noatime nfs-data:/space/data /data nfs noatime The AIX host from the example is setup similar to this. Surely I am not asking you to support AIX' df flags. But it would be nice if the central tools included in coreutils stay in line with other systems. BTW, the coreutils man page says about "-a": "include dummy file systems". Sorry to say, but this is misleading. "/home" is not dummy at all. Its a valid mount point, seperate from others. Esp. there is no local partition mounted for "/home", hidden by the NFS mount. Regards Harri -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On 10/24/2014 03:14 PM, Michael Stone wrote: > On Fri, Oct 24, 2014 at 03:38:35PM +0200, you wrote: >> rootfs- - -- / >> /dev/mapper/vg00-root 32896880 4781600 26421176 16% / > >> nfs-home:/space/home - - -- /home >> nfs-home:/space/data13390666752 10768854016 1941581824 85% /data > > I'm open to suggestions on a useful way to distinguish between these two > cases. (Without hard coding a bunch of site-specific rules into df.) Note /home doesn't seem to be accessible above which is another reason to prefer /data here. In general I think df is behaving correctly here, showing what file system space is available on the system. If it showed both there would be an ambiguity as to whether there were two such file systems available. If you want to see all nfs file systems you can do it explicitly like: df -a -t nfs thanks, Pádraig. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
On Fri, Oct 24, 2014 at 03:38:35PM +0200, you wrote: rootfs- - -- / /dev/mapper/vg00-root 32896880 4781600 26421176 16% / nfs-home:/space/home - - -- /home nfs-home:/space/data13390666752 10768854016 1941581824 85% /data I'm open to suggestions on a useful way to distinguish between these two cases. (Without hard coding a bunch of site-specific rules into df.) Mike Stone -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#737399: metoo
I've got the same problem: df doesn't show /home, which is pretty confusing, since NFS mounting /home is one of the major differences between the local server and desktop systems. # df Filesystem1K-blocksUsed Available Use% Mounted on /dev/mapper/vg00-root 32896880 4782132 26420644 16% / udev 10240 0 10240 0% /dev tmpfs405832 600 405232 1% /run tmpfs 5120 4 5116 1% /run/lock tmpfs811640 0 811640 0% /run/shm /dev/sda1 1014680 35160 910760 4% /boot /dev/mapper/vg00-export 396984544 67960 376727812 1% /export nfs-home:/space/data13390666752 10768152576 1942284288 85% /data # df -a Filesystem1K-blocksUsed Available Use% Mounted on rootfs- - -- / sysfs 0 0 0- /sys proc 0 0 0- /proc udev 10240 0 10240 0% /dev devpts0 0 0- /dev/pts tmpfs405832 600 405232 1% /run /dev/mapper/vg00-root 32896880 4781600 26421176 16% / tmpfs 5120 4 5116 1% /run/lock pstore0 0 0- /sys/fs/pstore tmpfs811640 0 811640 0% /run/shm /dev/sda1 1014680 35160 910760 4% /boot /dev/mapper/vg00-export 396984544 67960 376727812 1% /export rpc_pipefs0 0 0- /run/rpc_pipefs nfs-home:/space/home - - -- /home nfs-home:/space/data13390666752 10768854016 1941581824 85% /data # cat /proc/mounts | column -t rootfs /rootfs rw 0 0 sysfs/sys sysfs rw,nosuid,nodev,noexec,relatime 0 0 proc /procproc rw,nosuid,nodev,noexec,relatime 0 0 udev /dev devtmpfs rw,relatime,size=10240k,nr_inodes=504823,mode=755 0 0 devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0 tmpfs/run tmpfs rw,nosuid,noexec,relatime,size=405832k,mode=755 0 0 /dev/mapper/vg00-root/ext4rw,noatime,data=ordered 0 0 tmpfs/run/locktmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0 pstore /sys/fs/pstore pstore rw,relatime 0 0 tmpfs/run/shm tmpfs rw,nosuid,nodev,noexec,relatime,size=811640k 0 0 /dev/sda1/bootext4rw,noatime,data=ordered 0 0 /dev/mapper/vg00-export /export ext4rw,noatime,data=ordered 0 0 rpc_pipefs /run/rpc_pipefs rpc_pipefs rw,relatime
