On 31/10/14 00:52, Bernhard Voelker wrote: > On 10/30/2014 05:59 AM, Pádraig Brady wrote: >> On 10/30/2014 03:44 AM, Pádraig Brady wrote: >>> On 10/30/2014 02:23 AM, Bernhard Voelker wrote: >>>> After all, I don't have a strong preference, so I'm probably >>>> okay with re-introducing duplicate remote file systems. >>> >>> I'm 50:50 too. In that case we should probably err >>> on the side of less code and edge cases. >>> I.E. not go with with patch. >> >> A compromise would be to list the separate exports by default >> since this is what people are used to, and suppress them >> in --total mode to support showing the individual file systems >> which are available and to have an accurate total in that case. > > I somehow don't like the idea that local file systems are treated > differently than remote ones. I'm either for showing all or none > of the duplicates.
I definitely see your point, though we've had _many_ complaints about this. I think keying the remote deduplication on --total is an acceptable compromise, and that will also allow us to be even more aggressive in future with excluding loop devices from the total values if --type is not specified etc. Attached is a patch is reinstate the separate remote mounts by default. thanks, Pádraig.
>From 838318741e094db41210f37ab2f2a8d216749e92 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: only suppress remote mounts of separate exports with --total * src/df.c (filter_mount_list): Separate remote locations are generally explicitly mounted so list each even if they share the same remote device and thus storage. However with --total keep the suppression to give a more accurate value for the total storage available. (usage): Expand on the new implications of --total and move it in the options list according to alphabetic order. doc/coreutils.texi (df invocation): Mention that --total impacts on deduplication of remote file systems and also move location according to alphabetic order. * tests/df/skip-duplicates.sh: Add remote test cases. * NEWS: Mention the change in behavior. Reported in http://bugs.debian.org/737399 Reported in http://bugzilla.redhat.com/920806 --- NEWS | 5 +++++ doc/coreutils.texi | 31 ++++++++++++++++--------------- src/df.c | 41 ++++++++++++++++++++++++++++------------- tests/df/skip-duplicates.sh | 31 ++++++++++++++++++++++++++++--- 4 files changed, 77 insertions(+), 31 deletions(-) diff --git a/NEWS b/NEWS index 5d3bc58..3491a9d 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,11 @@ GNU coreutils NEWS -*- outline -*- ** Changes in behavior + df no longer suppresses separate exports of the same remote device, + these are probably explicitly mounted. The --total option does still + suppress duplicate remote file systems. + [suppression was introduced in coreutils-8.21] + mv no longer supports moving a file to a hardlink, instead issuing an error. The implementation was susceptible to races in the presence of multiple mv instances, which could result in both hardlinks being deleted. Also on case diff --git a/doc/coreutils.texi b/doc/coreutils.texi index db24a75..e9008c0 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -11205,8 +11205,7 @@ Non-integer quantities are rounded up to the next higher unit. For bind mounts and without arguments, @command{df} only outputs the statistics for that device with the shortest mount point name in the list of file systems (@var{mtab}), i.e., it hides duplicate entries, unless the @option{-a} option is -specified. Remote file systems, such as NFS, are treated the same way as local -ones; only one mount entry per remote file system is shown by default. +specified. With the same logic, @command{df} elides a mount entry of a dummy pseudo device if there is another mount entry of a real block device for that mount point with @@ -11248,19 +11247,6 @@ due to permissions of the mount point etc. Scale sizes by @var{size} before printing them (@pxref{Block size}). For example, @option{-BG} prints sizes in units of 1,073,741,824 bytes. -@item --total -@opindex --total -@cindex grand total of disk size, usage and available space -Print a grand total of all arguments after all arguments have -been processed. This can be used to find out the total disk size, usage -and available space of all listed devices. - -For the grand total line, @command{df} prints @samp{"total"} into the -@var{source} column, and @samp{"-"} into the @var{target} column. -If there is no @var{source} column (see @option{--output}), then -@command{df} prints @samp{"total"} into the @var{target} column, -if present. - @optHumanReadable @item -H @@ -11401,6 +11387,21 @@ some systems (notably SunOS), doing this yields more up to date results, but in general this option makes @command{df} much slower, especially when there are many or very busy file systems. +@item --total +@opindex --total +@cindex grand total of disk size, usage and available space +Print a grand total of all arguments after all arguments have +been processed. This can be used to find out the total disk size, usage +and available space of all listed devices. If no arguments are specified +df will try harder to elide file systems insignificant to the total +available space, by suppressing duplicate remote file systems. + +For the grand total line, @command{df} prints @samp{"total"} into the +@var{source} column, and @samp{"-"} into the @var{target} column. +If there is no @var{source} column (see @option{--output}), then +@command{df} prints @samp{"total"} into the @var{target} column, +if present. + @item -t @var{fstype} @itemx --type=@var{fstype} @opindex -t diff --git a/src/df.c b/src/df.c index a52afc4..7bac184 100644 --- a/src/df.c +++ b/src/df.c @@ -640,18 +640,28 @@ 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 (! print_grand_total && me->me_remote && devlist->me->me_remote + && ! STREQ (devlist->me->me_devname, me->me_devname)) + { + /* Don't discard remote entries with different locations, + as these are more likely to be explicitly mounted. + However avoid this when producing a total to give + a more accurate value in that case. */ + } + 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; @@ -1403,7 +1413,6 @@ or all file systems by default.\n\ -B, --block-size=SIZE scale sizes by SIZE before printing them; e.g.,\n\ '-BM' prints sizes in units of 1,048,576 bytes;\n\ see SIZE format below\n\ - --total produce a grand total\n\ -h, --human-readable print sizes in powers of 1024 (e.g., 1023M)\n\ -H, --si print sizes in powers of 1000 (e.g., 1.1G)\n\ "), stdout); @@ -1419,6 +1428,12 @@ or all file systems by default.\n\ or print all fields if FIELD_LIST is omitted.\n\ -P, --portability use the POSIX output format\n\ --sync invoke sync before getting usage info\n\ +"), stdout); + fputs (_("\ + --total elide all entries insignificant to available space,\n\ + and produce a grand total\n\ +"), stdout); + fputs (_("\ -t, --type=TYPE limit listing to file systems of type TYPE\n\ -T, --print-type print file system type\n\ -x, --exclude-type=TYPE limit listing to file systems not of type TYPE\n\ diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh index 52b9014..6b984ad 100755 --- a/tests/df/skip-duplicates.sh +++ b/tests/df/skip-duplicates.sh @@ -26,7 +26,12 @@ require_gcc_shared_ df --local || skip_ "df fails" export CU_NONROOT_FS=$(df --local --output=target 2>&1 | grep /. | head -n1) -test -z "$CU_NONROOT_FS" && unique_entries=1 || unique_entries=2 +export CU_REMOTE_FS=$(df --local --output=target 2>&1 | grep /. | + tail -n+2 | head -n1) + +unique_entries=1 +test -z "$CU_NONROOT_FS" || unique_entries=$(expr $unique_entries + 1) +test -z "$CU_REMOTE_FS" || unique_entries=$(expr $unique_entries + 2) grep '^#define HAVE_MNTENT_H 1' $CONFIG_HEADER > /dev/null \ || skip_ "no mntent.h available to confirm the interface" @@ -46,6 +51,7 @@ cat > k.c <<'EOF' || framework_failure_ struct mntent *getmntent (FILE *fp) { static char *nonroot_fs; + static char *remote_fs; static int done; /* Prove that LD_PRELOAD works. */ @@ -63,6 +69,9 @@ struct mntent *getmntent (FILE *fp) {.mnt_fsname="virtfs", .mnt_dir="/NONROOT", .mnt_type="fstype1"}, {.mnt_fsname="virtfs2", .mnt_dir="/NONROOT", .mnt_type="fstype2"}, {.mnt_fsname="netns", .mnt_dir="net:[1234567]"}, + {.mnt_fsname="rem:ote1",.mnt_dir="/REMOTE"}, + {.mnt_fsname="rem:ote1",.mnt_dir="/REMOTE"}, + {.mnt_fsname="rem:ote2",.mnt_dir="/REMOTE"}, }; if (done == 1) @@ -70,17 +79,26 @@ struct mntent *getmntent (FILE *fp) nonroot_fs = getenv ("CU_NONROOT_FS"); if (!nonroot_fs || !*nonroot_fs) nonroot_fs = "/"; /* merge into / entries. */ + + remote_fs = getenv ("CU_REMOTE_FS"); } if (done == 1 && !getenv ("CU_TEST_DUPE_INVALID")) done++; /* skip the first entry. */ - while (done++ <= 7) + while (done++ <= 10) { if (!mntents[done-2].mnt_type) mntents[done-2].mnt_type = "-"; if (STREQ (mntents[done-2].mnt_dir, "/NONROOT")) mntents[done-2].mnt_dir = nonroot_fs; + if (STREQ (mntents[done-2].mnt_dir, "/REMOTE")) + { + if (!remote_fs || !*remote_fs) + continue; + else + mntents[done-2].mnt_dir = remote_fs; + } return &mntents[done-2]; } @@ -102,6 +120,12 @@ test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?" LD_PRELOAD=./k.so df -T >out || fail=1 test $(wc -l <out) -eq $(expr 1 + $unique_entries) || { fail=1; cat out; } +# With --total we should suppress the duplicate but separate remote file system +LD_PRELOAD=./k.so df --total >out || fail=1 +test "$CU_REMOTE_FS" && elide_remote=1 || elide_remote=0 +test $(wc -l <out) -eq $(expr 2 + $unique_entries - $elide_remote) || + { fail=1; cat out; } + # Ensure we don't fail when unable to stat (currently) unavailable entries LD_PRELOAD=./k.so CU_TEST_DUPE_INVALID=1 df -T >out || fail=1 test $(wc -l <out) -eq $(expr 1 + $unique_entries) || { fail=1; cat out; } @@ -118,7 +142,8 @@ test $(grep -c 'virtfs2.*fstype2' <out) -eq 1 || { fail=1; cat out; } # Ensure that filtering duplicates does not affect -a processing. LD_PRELOAD=./k.so df -a >out || fail=1 -test $(wc -l <out) -eq 6 || { fail=1; cat out; } +total_fs=6; test "$CU_REMOTE_FS" && total_fs=$(expr $total_fs + 3) +test $(wc -l <out) -eq $total_fs || { fail=1; cat out; } # Ensure placeholder "-" values used for the eclipsed "virtfs" test $(grep -c 'virtfs *-' <out) -eq 1 || { fail=1; cat out; } -- 2.1.0
