Package: coreutils;
Reported by: crubel <at> compro.net
Date: Fri, 24 Jan 2014 20:39:01 UTC
Severity: normal
Done: Bernhard Voelker <mail <at> bernhard-voelker.de>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16539 in the body.
You can then email your comments to 16539 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Fri, 24 Jan 2014 20:39:01 GMT) Full text and rfc822 format available.crubel <at> compro.net:bug-coreutils <at> gnu.org.
(Fri, 24 Jan 2014 20:39:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Curtis Rubel <crubel <at> compro.net> To: <bug-coreutils <at> gnu.org> Subject: df command, possible bug? Date: Fri, 24 Jan 2014 15:23:02 -0500
The df command as distributed with OpenSuSE 13.1 -- package coreutils-8.21-7.8.1.x86_64 does not return all currently mounted nfs mounts when specifying the command with no command line arguments. Specifying the df command: df -at nfs works perfectly and returns the data for all mounted nfs filesystems. uname -a output for your reference: Linux otw-l0 3.11.6-4-desktop #1 SMP PREEMPT Wed Oct 30 18:04:56 UTC 2013 (e6d4a27) x86_64 x86_64 x86_64 GNU/Linux If you need further information please let me know. Thank you, Curtis -- Curtis Rubel Senior Development Engineer Compro Computer Services, Inc. 105 East Drive - Melbourne, Florida, 32904 Phone: 321-727-2211 email: crubel <at> compro.net Web: http://www.compro.net "An ISO 9001:2008 Registered Company"
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Fri, 24 Jan 2014 20:48:01 GMT) Full text and rfc822 format available.Message #8 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Curtis Rubel <crubel <at> compro.net> To: <16539 <at> debbugs.gnu.org> Subject: More details on df command output for you Date: Fri, 24 Jan 2014 15:47:14 -0500
df command output with no args: only 1 nfs mount is listed amx <at> otw-l0:~> df Filesystem 1K-blocks Used Available Use% Mounted on /dev/sda3 173232952 44850568 127310032 27% / devtmpfs 4049732 32 4049700 1% /dev tmpfs 4095336 76 4095260 1% /dev/shm tmpfs 4095336 4688 4090648 1% /run tmpfs 4095336 0 4095336 0% /sys/fs/cgroup tmpfs 4095336 4688 4090648 1% /var/run tmpfs 4095336 4688 4090648 1% /var/lock /dev/sda1 1035084 45272 920848 5% /boot host:/usr/local/bin 110217536 40202928 68990448 37% /usr/local/muse amx <at> otw-l0:~> df command with options to show nfs mounted filesystems showing all nfs mounts. amx <at> otw-l0:~> df -at nfs Filesystem 1K-blocks Used Available Use% Mounted on host:/usr/local/muse 110217536 40203472 68989896 37% /usr/local/muse host:/usr/local/LINUX_FILES 110217536 40203472 68989896 37% /usr/local/LINUX_FILES host:/usr/local/3rdparty 110217536 40203472 68989896 37% /usr/local/3rdparty amx <at> otw-l0:~> Thank you. -- Curtis Rubel Senior Development Engineer Compro Computer Services, Inc. 105 East Drive - Melbourne, Florida, 32904 Phone: 321-727-2211 email: crubel <at> compro.net Web: http://www.compro.net "An ISO 9001:2008 Registered Company" --------------------------------------------------------------------------- CONFIDENTIALITY NOTICE: This email transmission, and any documents, files or previous email messages attached to it may contain confidential information that is legally privileged. If you are not the intended recipient or a person responsible for delivering it to the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of any of the information contained in or attached to this transmission is STRICTLY PROHIBITED. If you have received this transmission in error, please immediately notify the sender by email or call 321-727-2211. Please destroy the original transmission and its attachments without reading or saving it in any manner.
Bernhard Voelker <mail <at> bernhard-voelker.de>:crubel <at> compro.net:Message #13 received at 16539-done <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: crubel <at> compro.net, 16539-done <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Fri, 24 Jan 2014 23:27:11 +0100
tag 16539 notabug close thanks On 01/24/2014 09:47 PM, Curtis Rubel wrote: > > df command output with no args: > > only 1 nfs mount is listed > > amx <at> otw-l0:~> df > Filesystem 1K-blocks Used Available Use% Mounted on > /dev/sda3 173232952 44850568 127310032 27% / > devtmpfs 4049732 32 4049700 1% /dev > tmpfs 4095336 76 4095260 1% /dev/shm > tmpfs 4095336 4688 4090648 1% /run > tmpfs 4095336 0 4095336 0% /sys/fs/cgroup > tmpfs 4095336 4688 4090648 1% /var/run > tmpfs 4095336 4688 4090648 1% /var/lock > /dev/sda1 1035084 45272 920848 5% /boot > host:/usr/local/bin 110217536 40202928 68990448 37% /usr/local/muse > amx <at> otw-l0:~> > > > df command with options to show nfs mounted filesystems > showing all nfs mounts. > > amx <at> otw-l0:~> df -at nfs > Filesystem 1K-blocks Used Available Use% Mounted > on > host:/usr/local/muse 110217536 40203472 68989896 37% > /usr/local/muse > host:/usr/local/LINUX_FILES 110217536 40203472 68989896 37% > /usr/local/LINUX_FILES > host:/usr/local/3rdparty 110217536 40203472 68989896 37% > /usr/local/3rdparty > amx <at> otw-l0:~> Just for info: df in openSUSE-13.1 is currently identical to that in coreutils-v8.21. The above is the result of df suppressing duplicate entries like bind mounts. This filtering is done based on the device number. As this example shows, a few exports of directories of the same file system from "host" are mounted - yet it's the same file system. We already had a few discussions about this filtering of duplicate mount entries. For me, this example shows that the current implementation is not that bad because df's job is to show block and inode usage statistics about mounted file systems. When it comes to information about mount points, then I think findmnt(1) from util-linux is the right tool. I'm therefore tagging this bug as "notabug" and mark it as done. If you or someone else wants to continue this discussion, then this can of course be done in this thread. And if someone has the *golden* idea how to solve this problem, then just stand up and let use know. Have a nice day, Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Sat, 25 Jan 2014 03:21:02 GMT) Full text and rfc822 format available.Message #16 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: 16539 <at> debbugs.gnu.org, mail <at> bernhard-voelker.de, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Sat, 25 Jan 2014 03:19:57 +0000
[Message part 1 (text/plain, inline)]
On 01/24/2014 10:27 PM, Bernhard Voelker wrote: > tag 16539 notabug > close > thanks > > On 01/24/2014 09:47 PM, Curtis Rubel wrote: >> >> df command output with no args: >> >> only 1 nfs mount is listed > Just for info: df in openSUSE-13.1 is currently identical to that > in coreutils-v8.21. > > The above is the result of df suppressing duplicate entries like > bind mounts. This filtering is done based on the device number. > As this example shows, a few exports of directories of the same file > system from "host" are mounted - yet it's the same file system. Right. Essentially df is showing storage for available file systems. Noting that df also has a --total option, it makes sense by default to not repeat file systems. This can be overridden easily with the -a option as noted above. Actually we should in fact be merging more entries! Notice the following: >> tmpfs 4095336 4688 4090648 1% /run >> tmpfs 4095336 4688 4090648 1% /var/run >> tmpfs 4095336 4688 4090648 1% /var/lock Hopefully the attached patch addresses this (and a couple of other test issues). thanks, Pádraig.
[df-tmpfs-merge.patch (text/x-patch, attachment)]
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Sat, 25 Jan 2014 23:56:05 GMT) Full text and rfc822 format available.Message #19 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com>, 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Sun, 26 Jan 2014 00:55:16 +0100
On 01/25/2014 04:19 AM, Pádraig Brady wrote:
> On 01/24/2014 10:27 PM, Bernhard Voelker wrote:
>> The above is the result of df suppressing duplicate entries like
>> bind mounts. This filtering is done based on the device number.
>> As this example shows, a few exports of directories of the same file
>> system from "host" are mounted - yet it's the same file system.
>
> Right. Essentially df is showing storage for available file systems.
> Noting that df also has a --total option, it makes sense by default
> to not repeat file systems. This can be overridden easily with the
> -a option as noted above.
>
> Actually we should in fact be merging more entries!
> Notice the following:
>
>>> tmpfs 4095336 4688 4090648 1% /run
>>> tmpfs 4095336 4688 4090648 1% /var/run
>>> tmpfs 4095336 4688 4090648 1% /var/lock
>
> Hopefully the attached patch addresses this
> (and a couple of other test issues).
Good idea.
I think we're on the right way - and the patch looks good to me.
Thanks.
However, I remember some other corner cases with eclipsed file
systems in the Fedora bug tracker. I think we're quite close
to solve them all this time (hopefully).
The idea was to trust the order of mount entries returned by
the kernel, i.e. in the loop over the mount entries, if the
mount point is the same one as a previous one, then we should
process the one mounted later.
E.g. the situation where 2 file systems are mounted on the
same mount point:
$ findmnt | grep loop
└─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
└─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
└─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
df - the new one with your patch - still shows the wrong device:
$ src/df | grep loop
/dev/loop0 122835 1551 112110 2% /mnt
/dev/loop1 122835 1550 112111 2% /mnt/dir
It should say /dev/loop2 here. BTW the numbers are correct.
Have a nice day,
Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Sun, 26 Jan 2014 00:05:02 GMT) Full text and rfc822 format available.Message #22 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com>, 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Sun, 26 Jan 2014 01:04:05 +0100
On 01/26/2014 12:55 AM, Bernhard Voelker wrote: > I think we're on the right way - and the patch looks good to me. oops, too early: $ make syntax-check ... tests/df/skip-duplicates.sh:72: if (strcmp (mntents[done-2].mnt_dir, "/NONROOT") == 0) maint.mk: replace strcmp calls above with STREQ/STRNEQ make: *** [sc_prohibit_strcmp] Error 1 make: *** Waiting for unfinished jobs.... --- ./po/POTFILES.in +++ ./po/POTFILES.in @@ -16,11 +16,14 @@ lib/rpmatch.c lib/set-acl.c lib/siglist.h +lib/spawn-pipe.c lib/strsignal.c lib/unicodeio.c lib/userspec.c lib/verror.c lib/version-etc.c +lib/w32spawn.h +lib/wait-process.c lib/xalloc-die.c lib/xfreopen.c lib/xmemcoll.c maint.mk: you have changed the set of files with translatable diagnostics; apply the above patch make: *** [sc_po_check] Error 1 The latter sc error is not related to this patch, obviously. Have a nice day, Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Sun, 26 Jan 2014 11:30:02 GMT) Full text and rfc822 format available.Message #25 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Sun, 26 Jan 2014 11:28:52 +0000
On 01/25/2014 11:55 PM, Bernhard Voelker wrote: > On 01/25/2014 04:19 AM, Pádraig Brady wrote: >> On 01/24/2014 10:27 PM, Bernhard Voelker wrote: >>> The above is the result of df suppressing duplicate entries like >>> bind mounts. This filtering is done based on the device number. >>> As this example shows, a few exports of directories of the same file >>> system from "host" are mounted - yet it's the same file system. >> >> Right. Essentially df is showing storage for available file systems. >> Noting that df also has a --total option, it makes sense by default >> to not repeat file systems. This can be overridden easily with the >> -a option as noted above. >> >> Actually we should in fact be merging more entries! >> Notice the following: >> >>>> tmpfs 4095336 4688 4090648 1% /run >>>> tmpfs 4095336 4688 4090648 1% /var/run >>>> tmpfs 4095336 4688 4090648 1% /var/lock >> >> Hopefully the attached patch addresses this >> (and a couple of other test issues). > > Good idea. > > I think we're on the right way - and the patch looks good to me. Thanks for the review. I'll handle the syntax check too. > However, I remember some other corner cases with eclipsed file > systems in the Fedora bug tracker. I think we're quite close > to solve them all this time (hopefully). > The idea was to trust the order of mount entries returned by > the kernel, i.e. in the loop over the mount entries, if the > mount point is the same one as a previous one, then we should > process the one mounted later. > > E.g. the situation where 2 file systems are mounted on the > same mount point: > > $ findmnt | grep loop > └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered > └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered > └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered > > df - the new one with your patch - still shows the wrong device: > > $ src/df | grep loop > /dev/loop0 122835 1551 112110 2% /mnt > /dev/loop1 122835 1550 112111 2% /mnt/dir > > It should say /dev/loop2 here. BTW the numbers are correct. Right, that could be handled easy enough. loop1 is not accessible above and so should be hidden. But consider a bind mount resulting in something like: > └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered > └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered > └─/some/place/else /dev/loop1 ext4 rw,relatime,data=ordered > └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered If we did a linear scan through that, we'd lose the /some/place/else due to it being a longer mount dir, and then also the original loop1 as we took /dev/loop2 for /mnt/dir. Seems like when discarding we would need to see if this was the last entry for a device and then see if there are any other candidate mount points for that device? thanks, Pádraig.
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Sun, 26 Jan 2014 23:37:01 GMT) Full text and rfc822 format available.Message #28 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Mon, 27 Jan 2014 00:35:58 +0100
On 01/26/2014 12:28 PM, Pádraig Brady wrote:
> On 01/25/2014 11:55 PM, Bernhard Voelker wrote:
>> However, I remember some other corner cases with eclipsed file
>> systems in the Fedora bug tracker. I think we're quite close
>> to solve them all this time (hopefully).
>> The idea was to trust the order of mount entries returned by
>> the kernel, i.e. in the loop over the mount entries, if the
>> mount point is the same one as a previous one, then we should
>> process the one mounted later.
>>
>> E.g. the situation where 2 file systems are mounted on the
>> same mount point:
>>
>> $ findmnt | grep loop
>> └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
>> └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
>> └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
>>
>> df - the new one with your patch - still shows the wrong device:
>>
>> $ src/df | grep loop
>> /dev/loop0 122835 1551 112110 2% /mnt
>> /dev/loop1 122835 1550 112111 2% /mnt/dir
>>
>> It should say /dev/loop2 here. BTW the numbers are correct.
BTW: the fstype is wrong, too (which can only be seen with -T or --output,
and if it differs, of course).
> Right, that could be handled easy enough.
> loop1 is not accessible above and so should be hidden.
> But consider a bind mount resulting in something like:
>
>> └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
>> └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
>> └─/some/place/else /dev/loop1 ext4 rw,relatime,data=ordered
>> └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
>
> If we did a linear scan through that, we'd lose the /some/place/else
> due to it being a longer mount dir, and then also the original loop1
> as we took /dev/loop2 for /mnt/dir.
> Seems like when discarding we would need to see if this was the
> last entry for a device and then see if there are any other candidate
> mount points for that device?
Hi Padraig,
thanks.
Again, mount_list is a little beast - more below.
The following patch (on top of yours) would handle both cases
without a problem. Feel free to squash it in, if you like.
diff --git a/src/df.c b/src/df.c
index 23b5156..78768cc 100644
--- a/src/df.c
+++ b/src/df.c
@@ -631,9 +631,20 @@ filter_mount_list (void)
else
{
/* If we've already seen this device... */
+ struct devlist *d = NULL;
for (devlist = devlist_head; devlist; devlist = devlist->next)
if (devlist->dev_num == buf.st_dev)
- break;
+ {
+ d = devlist;
+ if (!STREQ (devlist->me->me_devname, me->me_devname))
+ {
+ /* Fix the devname if the mount dir has been
+ mounted over by a different devname. */
+ free (devlist->me->me_devname);
+ devlist->me->me_devname = xstrdup (me->me_devname);
+ }
+ }
+ devlist = d;
if (devlist)
{
But there is yet another issue with the -a mode for such
over-mounted and therefore eclipsed file systems:
# Create 2 file system images: 1 ext4, 1 xfs.
$ dd if=/dev/zero bs=1M status=none count=128 of=img1
$ dd if=/dev/zero bs=1M status=none count=256 of=img2
$ mkfs -t ext4 -F img1 >/dev/null 2>&1
$ mkfs -t xfs -f img2 >/dev/null 2>&1
$ mkdir /mnt{1,2}
# Mount both on /mnt1.
$ mount -o loop img1 /mnt1
$ mount -o loop img2 /mnt1
# Mount the former (ext4) also on /mnt2 via its loop device.
$ mount /dev/loop0 /mnt2
# Result:
$ findmnt --output=TARGET,SOURCE,FSTYPE | grep loop
├─/mnt1 /dev/loop0 ext4
│ └─/mnt1 /dev/loop1 xfs
└─/mnt2 /dev/loop0 ext4
Everything is fine now with the filtered df run ...
$ src/df --out -h | grep loop
/dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
/dev/loop0 ext4 32K 11 32K 1% 120M 1.6M 110M 2% - /mnt2
...but "df -a" prints the wrong statistics for the "over-mounted" /mnt1!
$ src/df --out -h -a | grep loop
/dev/loop0 ext4 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
/dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
/dev/loop0 ext4 32K 11 32K 1% 120M 1.6M 110M 2% - /mnt2
Okay, this is nothing new.
BTW: strictly speaking, also the output of today's "df -t rootfs -a"
is wrong because the numbers are definitely not that of the early-boot
rootfs file system.
Now, how should df handle this?
a)
df silently filters out the mount entries of all eclipsed mount dirs,
even with -a.
--> Hmm, I think this would probably contradict to POSIX.
b)
df prints an error diagnostic for each eclipsed mount dir, and exits
non-Zero.
--> Well, there are probably such mounts on every system, e.g. on my box:
TARGET SOURCE FSTYPE
/proc/sys/fs/binfmt_misc systemd-1 autofs
/proc/sys/fs/binfmt_misc binfmt_misc binfmt_misc
Therefore, a "df -a" would always fail. ;-(
At least on my system, there are
c)
df prints a warning diagnostic for each eclipsed mount dir, and exits
Zero (unless another error occurs).
--> Due to the same reason as in b), these warning might be messy
and users will probably be irritated.
d)
df outputs "-" for all numbers of such eclipsed file systems, e.g.
$ src/df --out -h -a | grep mnt1
/dev/loop0 ext4 - - - - - - - - - /mnt1
/dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
Maybe d) is the best solution, as it mirrors what df can know:
it knows source, target and the file system type, but it doesn't
have access to the block and inode numbers.
WDYT?
Have a nice day,
Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Mon, 27 Jan 2014 01:07:02 GMT) Full text and rfc822 format available.Message #31 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Mon, 27 Jan 2014 01:06:16 +0000
On 01/26/2014 11:35 PM, Bernhard Voelker wrote:
> On 01/26/2014 12:28 PM, Pádraig Brady wrote:
>> On 01/25/2014 11:55 PM, Bernhard Voelker wrote:
>>> However, I remember some other corner cases with eclipsed file
>>> systems in the Fedora bug tracker. I think we're quite close
>>> to solve them all this time (hopefully).
>>> The idea was to trust the order of mount entries returned by
>>> the kernel, i.e. in the loop over the mount entries, if the
>>> mount point is the same one as a previous one, then we should
>>> process the one mounted later.
>>>
>>> E.g. the situation where 2 file systems are mounted on the
>>> same mount point:
>>>
>>> $ findmnt | grep loop
>>> └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
>>> └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
>>> └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
>>>
>>> df - the new one with your patch - still shows the wrong device:
>>>
>>> $ src/df | grep loop
>>> /dev/loop0 122835 1551 112110 2% /mnt
>>> /dev/loop1 122835 1550 112111 2% /mnt/dir
>>>
>>> It should say /dev/loop2 here. BTW the numbers are correct.
>
> BTW: the fstype is wrong, too (which can only be seen with -T or --output,
> and if it differs, of course).
>
>> Right, that could be handled easy enough.
>> loop1 is not accessible above and so should be hidden.
>> But consider a bind mount resulting in something like:
>>
>>> └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
>>> └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
>>> └─/some/place/else /dev/loop1 ext4 rw,relatime,data=ordered
>>> └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
>>
>> If we did a linear scan through that, we'd lose the /some/place/else
>> due to it being a longer mount dir, and then also the original loop1
>> as we took /dev/loop2 for /mnt/dir.
>> Seems like when discarding we would need to see if this was the
>> last entry for a device and then see if there are any other candidate
>> mount points for that device?
>
> Hi Padraig,
>
> thanks.
> Again, mount_list is a little beast - more below.
>
> The following patch (on top of yours) would handle both cases
> without a problem. Feel free to squash it in, if you like.
>
> diff --git a/src/df.c b/src/df.c
> index 23b5156..78768cc 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -631,9 +631,20 @@ filter_mount_list (void)
> else
> {
> /* If we've already seen this device... */
> + struct devlist *d = NULL;
> for (devlist = devlist_head; devlist; devlist = devlist->next)
> if (devlist->dev_num == buf.st_dev)
> - break;
> + {
> + d = devlist;
> + if (!STREQ (devlist->me->me_devname, me->me_devname))
> + {
> + /* Fix the devname if the mount dir has been
> + mounted over by a different devname. */
> + free (devlist->me->me_devname);
> + devlist->me->me_devname = xstrdup (me->me_devname);
> + }
> + }
> + devlist = d;
>
> if (devlist)
> {
>
> But there is yet another issue with the -a mode for such
> over-mounted and therefore eclipsed file systems:
>
> # Create 2 file system images: 1 ext4, 1 xfs.
> $ dd if=/dev/zero bs=1M status=none count=128 of=img1
> $ dd if=/dev/zero bs=1M status=none count=256 of=img2
> $ mkfs -t ext4 -F img1 >/dev/null 2>&1
> $ mkfs -t xfs -f img2 >/dev/null 2>&1
> $ mkdir /mnt{1,2}
>
> # Mount both on /mnt1.
> $ mount -o loop img1 /mnt1
> $ mount -o loop img2 /mnt1
>
> # Mount the former (ext4) also on /mnt2 via its loop device.
> $ mount /dev/loop0 /mnt2
>
> # Result:
> $ findmnt --output=TARGET,SOURCE,FSTYPE | grep loop
> ├─/mnt1 /dev/loop0 ext4
> │ └─/mnt1 /dev/loop1 xfs
> └─/mnt2 /dev/loop0 ext4
>
> Everything is fine now with the filtered df run ...
>
> $ src/df --out -h | grep loop
> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
> /dev/loop0 ext4 32K 11 32K 1% 120M 1.6M 110M 2% - /mnt2
>
> ...but "df -a" prints the wrong statistics for the "over-mounted" /mnt1!
>
> $ src/df --out -h -a | grep loop
> /dev/loop0 ext4 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
> /dev/loop0 ext4 32K 11 32K 1% 120M 1.6M 110M 2% - /mnt2
>
> Okay, this is nothing new.
> BTW: strictly speaking, also the output of today's "df -t rootfs -a"
> is wrong because the numbers are definitely not that of the early-boot
> rootfs file system.
>
> Now, how should df handle this?
>
> a)
> df silently filters out the mount entries of all eclipsed mount dirs,
> even with -a.
> --> Hmm, I think this would probably contradict to POSIX.
>
> b)
> df prints an error diagnostic for each eclipsed mount dir, and exits
> non-Zero.
> --> Well, there are probably such mounts on every system, e.g. on my box:
>
> TARGET SOURCE FSTYPE
> /proc/sys/fs/binfmt_misc systemd-1 autofs
> /proc/sys/fs/binfmt_misc binfmt_misc binfmt_misc
>
> Therefore, a "df -a" would always fail. ;-(
> At least on my system, there are
>
> c)
> df prints a warning diagnostic for each eclipsed mount dir, and exits
> Zero (unless another error occurs).
>
> --> Due to the same reason as in b), these warning might be messy
> and users will probably be irritated.
>
> d)
> df outputs "-" for all numbers of such eclipsed file systems, e.g.
>
> $ src/df --out -h -a | grep mnt1
> /dev/loop0 ext4 - - - - - - - - - /mnt1
> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
>
>
> Maybe d) is the best solution, as it mirrors what df can know:
> it knows source, target and the file system type, but it doesn't
> have access to the block and inode numbers.
>
> WDYT?
Thanks for the nice analysis and tests.
d) seems like the best option here, though we'd have to be careful
about cases where /proc/mounts was giving a system wide view,
while df wasn't privy to that due to mount namespaces or
overmounts etc. I'm not thinking of a specific issue here,
just the general problem.
wrt c) and annoying warnings, I also notice `df -a` on a default Fedora 20 install here,
giving multiple duplicate warnings like:
df: ‘net:[4026532416]’: No such file or directory
df: ‘net:[4026532416]’: No such file or directory
That's due to:
$ grep net: /proc/mounts
proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0
proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0
Which is due to support for namespaces.
Seems like we should not try to lookup non absolute mount points?
Also a general point is that a lot of stuff has changed underneath us recently,
and perhaps we should be looking at abstracting that away somewhere
(like libmount that is part of util-linux). In the short term anyway
we should fix up the above warts within df.
thanks,
Pádraig.
thanks,
Pádraig.
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Mon, 27 Jan 2014 22:48:02 GMT) Full text and rfc822 format available.Message #34 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Mon, 27 Jan 2014 23:47:18 +0100
On 01/27/2014 02:06 AM, Pádraig Brady wrote: > On 01/26/2014 11:35 PM, Bernhard Voelker wrote: >> Now, how should df handle this? >> >> a) >> df silently filters out the mount entries of all eclipsed mount dirs, >> even with -a. >> --> Hmm, I think this would probably contradict to POSIX. >> >> b) >> df prints an error diagnostic for each eclipsed mount dir, and exits >> non-Zero. >> --> Well, there are probably such mounts on every system, e.g. on my box: >> >> TARGET SOURCE FSTYPE >> /proc/sys/fs/binfmt_misc systemd-1 autofs >> /proc/sys/fs/binfmt_misc binfmt_misc binfmt_misc >> >> Therefore, a "df -a" would always fail. ;-( >> At least on my system, there are >> >> c) >> df prints a warning diagnostic for each eclipsed mount dir, and exits >> Zero (unless another error occurs). >> >> --> Due to the same reason as in b), these warning might be messy >> and users will probably be irritated. >> >> d) >> df outputs "-" for all numbers of such eclipsed file systems, e.g. >> >> $ src/df --out -h -a | grep mnt1 >> /dev/loop0 ext4 - - - - - - - - - /mnt1 >> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1 >> >> >> Maybe d) is the best solution, as it mirrors what df can know: >> it knows source, target and the file system type, but it doesn't >> have access to the block and inode numbers. >> >> WDYT? > > Thanks for the nice analysis and tests. > d) seems like the best option here, though we'd have to be careful > about cases where /proc/mounts was giving a system wide view, > while df wasn't privy to that due to mount namespaces or > overmounts etc. I'm not thinking of a specific issue here, > just the general problem. okay, let's try it that way. I think this is not much related to the issue your patch (with my amendment) is trying to fix. Therefore, I suggest to finish that and handle the overmounts in a separate commit. > wrt c) and annoying warnings, I also notice `df -a` on a default Fedora 20 install here, > giving multiple duplicate warnings like: > df: ‘net:[4026532416]’: No such file or directory > df: ‘net:[4026532416]’: No such file or directory > That's due to: > $ grep net: /proc/mounts > proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0 > proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0 > Which is due to support for namespaces. Ugh ... and that's a third problm class for df. ;-( > Seems like we should not try to lookup non absolute mount points? yes, that's probably our best chance. > Also a general point is that a lot of stuff has changed underneath us recently, > and perhaps we should be looking at abstracting that away somewhere > (like libmount that is part of util-linux). In the short term anyway > we should fix up the above warts within df. I'm not sure about such an abstraction. Actually, the mount_list is already abstracted ... by gnulib. And that's the biggest difference to the situation in util-linux: while util-linux only has to run on GNU/Linux, coreutils' df has to run on a big variety of systems including far-off systems like cygwin. I think if we find a way to ignore such non-accessible file systems without producing too many annoying warnings, then I'd say we're off the hook. Namespaces have been invented to hide mounts, so people shouldn't wonder that they are hidden to df. ;-) Have a nice day, Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Tue, 28 Jan 2014 11:14:01 GMT) Full text and rfc822 format available.Message #37 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Tue, 28 Jan 2014 12:13:00 +0100
On 01/27/2014 11:47 PM, Bernhard Voelker wrote: > On 01/27/2014 02:06 AM, Pádraig Brady wrote: >> wrt c) and annoying warnings, I also notice `df -a` on a default Fedora 20 install here, >> giving multiple duplicate warnings like: >> df: ‘net:[4026532416]’: No such file or directory >> df: ‘net:[4026532416]’: No such file or directory >> That's due to: >> $ grep net: /proc/mounts >> proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0 >> proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0 >> Which is due to support for namespaces. > > Ugh ... and that's a third problm class for df. ;-( > >> Seems like we should not try to lookup non absolute mount points? > > yes, that's probably our best chance. Thinking further, this is mandatory: $ touch 'net:[4026532416]' This would make the stat() in df succeed and lead to wrong output. Have a nice day, Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Tue, 28 Jan 2014 11:35:02 GMT) Full text and rfc822 format available.Message #40 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org, crubel <at> compro.net Subject: Re: bug#16539: More details on df command output for you Date: Tue, 28 Jan 2014 11:34:22 +0000
On 01/28/2014 11:13 AM, Bernhard Voelker wrote: > On 01/27/2014 11:47 PM, Bernhard Voelker wrote: >> On 01/27/2014 02:06 AM, Pádraig Brady wrote: >>> wrt c) and annoying warnings, I also notice `df -a` on a default Fedora 20 install here, >>> giving multiple duplicate warnings like: >>> df: ‘net:[4026532416]’: No such file or directory >>> df: ‘net:[4026532416]’: No such file or directory >>> That's due to: >>> $ grep net: /proc/mounts >>> proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0 >>> proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0 >>> Which is due to support for namespaces. >> >> Ugh ... and that's a third problm class for df. ;-( >> >>> Seems like we should not try to lookup non absolute mount points? >> >> yes, that's probably our best chance. > > Thinking further, this is mandatory: > > $ touch 'net:[4026532416]' > > This would make the stat() in df succeed and lead to wrong output. OK I'll do a patchset for these df issue this evening hopefully. thanks, Pádraig.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Tue, 25 Feb 2014 12:24:04 GMT) Full text and rfc822 format available.Pádraig Brady <P <at> draigBrady.com>
to control <at> debbugs.gnu.org.
(Mon, 12 May 2014 15:08:01 GMT) Full text and rfc822 format available.bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Mon, 12 May 2014 15:12:03 GMT) Full text and rfc822 format available.Message #47 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Mon, 12 May 2014 16:10:55 +0100
[Message part 1 (text/plain, inline)]
On 01/27/2014 01:06 AM, Pádraig Brady wrote:
> On 01/26/2014 11:35 PM, Bernhard Voelker wrote:
>> On 01/26/2014 12:28 PM, Pádraig Brady wrote:
>>> On 01/25/2014 11:55 PM, Bernhard Voelker wrote:
>>>> However, I remember some other corner cases with eclipsed file
>>>> systems in the Fedora bug tracker. I think we're quite close
>>>> to solve them all this time (hopefully).
>>>> The idea was to trust the order of mount entries returned by
>>>> the kernel, i.e. in the loop over the mount entries, if the
>>>> mount point is the same one as a previous one, then we should
>>>> process the one mounted later.
>>>>
>>>> E.g. the situation where 2 file systems are mounted on the
>>>> same mount point:
>>>>
>>>> $ findmnt | grep loop
>>>> └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
>>>> └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
>>>> └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
>>>>
>>>> df - the new one with your patch - still shows the wrong device:
>>>>
>>>> $ src/df | grep loop
>>>> /dev/loop0 122835 1551 112110 2% /mnt
>>>> /dev/loop1 122835 1550 112111 2% /mnt/dir
>>>>
>>>> It should say /dev/loop2 here. BTW the numbers are correct.
>>
>> BTW: the fstype is wrong, too (which can only be seen with -T or --output,
>> and if it differs, of course).
>>
>>> Right, that could be handled easy enough.
>>> loop1 is not accessible above and so should be hidden.
>>> But consider a bind mount resulting in something like:
>>>
>>>> └─/mnt /dev/loop0 ext4 rw,relatime,data=ordered
>>>> └─/mnt/dir /dev/loop1 ext4 rw,relatime,data=ordered
>>>> └─/some/place/else /dev/loop1 ext4 rw,relatime,data=ordered
>>>> └─/mnt/dir /dev/loop2 ext4 rw,relatime,data=ordered
>>>
>>> If we did a linear scan through that, we'd lose the /some/place/else
>>> due to it being a longer mount dir, and then also the original loop1
>>> as we took /dev/loop2 for /mnt/dir.
>>> Seems like when discarding we would need to see if this was the
>>> last entry for a device and then see if there are any other candidate
>>> mount points for that device?
>>
>> Hi Padraig,
>>
>> thanks.
>> Again, mount_list is a little beast - more below.
>>
>> The following patch (on top of yours) would handle both cases
>> without a problem. Feel free to squash it in, if you like.
>>
>> diff --git a/src/df.c b/src/df.c
>> index 23b5156..78768cc 100644
>> --- a/src/df.c
>> +++ b/src/df.c
>> @@ -631,9 +631,20 @@ filter_mount_list (void)
>> else
>> {
>> /* If we've already seen this device... */
>> + struct devlist *d = NULL;
>> for (devlist = devlist_head; devlist; devlist = devlist->next)
>> if (devlist->dev_num == buf.st_dev)
>> - break;
>> + {
>> + d = devlist;
>> + if (!STREQ (devlist->me->me_devname, me->me_devname))
>> + {
>> + /* Fix the devname if the mount dir has been
>> + mounted over by a different devname. */
>> + free (devlist->me->me_devname);
>> + devlist->me->me_devname = xstrdup (me->me_devname);
>> + }
>> + }
>> + devlist = d;
>>
>> if (devlist)
>> {
>>
>> But there is yet another issue with the -a mode for such
>> over-mounted and therefore eclipsed file systems:
>>
>> # Create 2 file system images: 1 ext4, 1 xfs.
>> $ dd if=/dev/zero bs=1M status=none count=128 of=img1
>> $ dd if=/dev/zero bs=1M status=none count=256 of=img2
>> $ mkfs -t ext4 -F img1 >/dev/null 2>&1
>> $ mkfs -t xfs -f img2 >/dev/null 2>&1
>> $ mkdir /mnt{1,2}
>>
>> # Mount both on /mnt1.
>> $ mount -o loop img1 /mnt1
>> $ mount -o loop img2 /mnt1
>>
>> # Mount the former (ext4) also on /mnt2 via its loop device.
>> $ mount /dev/loop0 /mnt2
>>
>> # Result:
>> $ findmnt --output=TARGET,SOURCE,FSTYPE | grep loop
>> ├─/mnt1 /dev/loop0 ext4
>> │ └─/mnt1 /dev/loop1 xfs
>> └─/mnt2 /dev/loop0 ext4
>>
>> Everything is fine now with the filtered df run ...
>>
>> $ src/df --out -h | grep loop
>> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
>> /dev/loop0 ext4 32K 11 32K 1% 120M 1.6M 110M 2% - /mnt2
>>
>> ...but "df -a" prints the wrong statistics for the "over-mounted" /mnt1!
>>
>> $ src/df --out -h -a | grep loop
>> /dev/loop0 ext4 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
>> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
>> /dev/loop0 ext4 32K 11 32K 1% 120M 1.6M 110M 2% - /mnt2
>>
>> Okay, this is nothing new.
>> BTW: strictly speaking, also the output of today's "df -t rootfs -a"
>> is wrong because the numbers are definitely not that of the early-boot
>> rootfs file system.
>>
>> Now, how should df handle this?
>>
>> a)
>> df silently filters out the mount entries of all eclipsed mount dirs,
>> even with -a.
>> --> Hmm, I think this would probably contradict to POSIX.
>>
>> b)
>> df prints an error diagnostic for each eclipsed mount dir, and exits
>> non-Zero.
>> --> Well, there are probably such mounts on every system, e.g. on my box:
>>
>> TARGET SOURCE FSTYPE
>> /proc/sys/fs/binfmt_misc systemd-1 autofs
>> /proc/sys/fs/binfmt_misc binfmt_misc binfmt_misc
>>
>> Therefore, a "df -a" would always fail. ;-(
>> At least on my system, there are
>>
>> c)
>> df prints a warning diagnostic for each eclipsed mount dir, and exits
>> Zero (unless another error occurs).
>>
>> --> Due to the same reason as in b), these warning might be messy
>> and users will probably be irritated.
>>
>> d)
>> df outputs "-" for all numbers of such eclipsed file systems, e.g.
>>
>> $ src/df --out -h -a | grep mnt1
>> /dev/loop0 ext4 - - - - - - - - - /mnt1
>> /dev/loop1 xfs 256K 3 256K 1% 252M 13M 239M 6% - /mnt1
>>
>>
>> Maybe d) is the best solution, as it mirrors what df can know:
>> it knows source, target and the file system type, but it doesn't
>> have access to the block and inode numbers.
>>
>> WDYT?
>
> Thanks for the nice analysis and tests.
> d) seems like the best option here, though we'd have to be careful
> about cases where /proc/mounts was giving a system wide view,
> while df wasn't privy to that due to mount namespaces or
> overmounts etc. I'm not thinking of a specific issue here,
> just the general problem.
>
> wrt c) and annoying warnings, I also notice `df -a` on a default Fedora 20 install here,
> giving multiple duplicate warnings like:
> df: ‘net:[4026532416]’: No such file or directory
> df: ‘net:[4026532416]’: No such file or directory
> That's due to:
> $ grep net: /proc/mounts
> proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0
> proc net:[4026532416] proc rw,nosuid,nodev,noexec,relatime 0 0
> Which is due to support for namespaces.
> Seems like we should not try to lookup non absolute mount points?
Hi Bernhard,
I've attached 4 patches for df to:
df: also deduplicate virtual file systems
df: fix handling of symlinks in mount list
df: ignore non file system entries in /proc/mounts
maint: avoid clang -Wtautological-constant-out-of-range-compare warning
Not included is your "overmount change" or the "d)" adjustment above,
as I was unsure how you wanted to handle exactly.
thanks,
Pádraig.
[df-8.23-fixes.patch (text/x-patch, attachment)]
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Tue, 13 May 2014 22:21:01 GMT) Full text and rfc822 format available.Message #50 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 14 May 2014 00:20:08 +0200
On 05/12/2014 05:10 PM, Pádraig Brady wrote: > I've attached 4 patches for df to: > > df: also deduplicate virtual file systems > df: fix handling of symlinks in mount list > df: ignore non file system entries in /proc/mounts > maint: avoid clang -Wtautological-constant-out-of-range-compare warning They are look good to me - great work, thanks! AFAIR, the last one was also warned about by Coverity (but I can't check now as their website is down for maintenance). > Not included is your "overmount change" or the "d)" adjustment above, > as I was unsure how you wanted to handle exactly. To recap, this was the problem: $ mount /dev/sda5 /tmp/mnt $ mount -o loop tmp.img /tmp/mnt df - even including your 4 patches - shows wrong results: $ src/df | grep mnt /dev/sda5 59365 1308 53471 3% /tmp/mnt $ src/df -a | grep mnt /dev/sda5 59365 1308 53471 3% /tmp/mnt /dev/loop0 59365 1308 53471 3% /tmp/mnt The idea was to trust the order of /proc/mounts $ tail -n2 /proc/mounts /dev/sda5 /tmp/mnt ext2 rw,relatime 0 0 /dev/loop0 /tmp/mnt ext3 rw,relatime,data=ordered 0 0 i.e., loop through the mount list leaving only the last unique (maybe canonicalized) mount point. Another special case is when a mount point is not reachable at all anymore: /dev/sda5 /tmp/mnt/some/subdir ext2 rw,relatime 0 0 /dev/loop0 /tmp/mnt ext3 rw,relatime,data=ordered 0 0 I recommend discarding eclipsed mounts in normal output, and output "-" values with -a. Thanks & have a nice day, Berny
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Wed, 11 Jun 2014 11:24:04 GMT) Full text and rfc822 format available.Pádraig Brady <P <at> draigBrady.com>
to control <at> debbugs.gnu.org.
(Tue, 17 Jun 2014 16:02:01 GMT) Full text and rfc822 format available.bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Tue, 17 Jun 2014 16:05:01 GMT) Full text and rfc822 format available.Message #57 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Tue, 17 Jun 2014 17:03:39 +0100
[Message part 1 (text/plain, inline)]
On 05/13/2014 11:20 PM, Bernhard Voelker wrote: > On 05/12/2014 05:10 PM, Pádraig Brady wrote: >> I've attached 4 patches for df to: >> >> df: also deduplicate virtual file systems >> df: fix handling of symlinks in mount list >> df: ignore non file system entries in /proc/mounts >> maint: avoid clang -Wtautological-constant-out-of-range-compare warning > > They are look good to me - great work, thanks! > AFAIR, the last one was also warned about by Coverity (but > I can't check now as their website is down for maintenance). > >> Not included is your "overmount change" or the "d)" adjustment above, >> as I was unsure how you wanted to handle exactly. > > To recap, this was the problem: > > $ mount /dev/sda5 /tmp/mnt > $ mount -o loop tmp.img /tmp/mnt > > df - even including your 4 patches - shows wrong results: > > $ src/df | grep mnt > /dev/sda5 59365 1308 53471 3% /tmp/mnt > > $ src/df -a | grep mnt > /dev/sda5 59365 1308 53471 3% /tmp/mnt > /dev/loop0 59365 1308 53471 3% /tmp/mnt > > The idea was to trust the order of /proc/mounts > > $ tail -n2 /proc/mounts > /dev/sda5 /tmp/mnt ext2 rw,relatime 0 0 > /dev/loop0 /tmp/mnt ext3 rw,relatime,data=ordered 0 0 > > i.e., loop through the mount list leaving only the > last unique (maybe canonicalized) mount point. > > Another special case is when a mount point is not reachable > at all anymore: > > /dev/sda5 /tmp/mnt/some/subdir ext2 rw,relatime 0 0 > /dev/loop0 /tmp/mnt ext3 rw,relatime,data=ordered 0 0 > > I recommend discarding eclipsed mounts in normal output, > and output "-" values with -a. The attached (on top of df-fix-last-device.diff) should put "-" placeholder values in the needed cases. That should be it for df fixes for coreutils-8.23. cheers, Pádraig.
[df-fix-wrong-device.diff (text/x-patch, attachment)]
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Wed, 18 Jun 2014 01:20:02 GMT) Full text and rfc822 format available.Message #60 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com>, 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 18 Jun 2014 03:19:04 +0200
On 06/17/2014 06:03 PM, Pádraig Brady wrote:
> From 0a4b8027049f6746a237c9fc34a0e0a4afdcfc62 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com>
> Date: Wed, 4 Jun 2014 00:09:11 +0100
> Subject: [PATCH] df: output placeholder values for inaccessible mount points
>
> A system provided mount entry may be unavailable due to TOCTOU race,
> or if another device has been overmounted at that position, or due to
> access permissions. In all these cases output "-" placeholder values
> rather than either producing an error, or in the overmount case
> outputting values for the wrong device.
Cool: finally GNU df fulfills the first sentence of the POSIX spec ;-)
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/df.html
The df utility shall write the amount of available space ... for
file systems on which the invoking user has appropriate read access.
I think we already discussed whether a warning would be better than
a line with "-"s (or even silently omit the line) - and seeing the
result is pleasing:
Before:
/usr/bin/df: ‘/root/backup’: Permission denied
After:
/dev/sda3 - - - - /root/backup
> * src/df.c (device_list): A new global list updated from ...
... from what?
> (filter_mount_list): Adjust to take a parameter as to whether
> update the global mount list, or only the mount <-> device ID mapping.
> (get_dev): Use the device ID mapping to ensure we're not outputting
> stats for the wrong device. Also output plaeholder values when we
s/plaeholder/placeholder/
> can't access a system specified mount point.
> (devname_for_dev): A new function to search the mount <-> dev mapping.
get_all_entries was also changed.
> * test/df/skip-duplicates.sh: Adjust accordingly.
> * NEWS: Mention the bug fixes.
>
> Discussed at: http://bugs.gnu.org/16539
> diff --git a/src/df.c b/src/df.c
> index 10047ce..c7da208 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -45,12 +45,12 @@
>
> /* Filled with device numbers of examined file systems to avoid
> duplicities in output. */
> -struct devlist
> +static struct devlist
> {
> dev_t dev_num;
> struct mount_entry *me;
> struct devlist *next;
> -};
> +} *device_list;
Maybe I'm old-fashioned, but I'd have separated the struct definition
from the variable definition ... well, personal style.
> @@ -610,20 +610,21 @@ excluded_fstype (const char *fstype)
> me_mountdir wins. */
>
> static void
> -filter_mount_list (void)
> +filter_mount_list (bool devices_only)
The parameter name isn't very intuitive (to me).
Would it harm to add a sentence in the function description?
> @@ -878,9 +900,37 @@ get_dev (char const *disk, char const *mount_point, char const* file,
> fsu = *force_fsu;
> else if (get_fs_usage (stat_file, disk, &fsu))
> {
> - error (0, errno, "%s", quote (stat_file));
> - exit_status = EXIT_FAILURE;
> - return;
> + /* If we can't access a system provided entry due
> + to it not being present (now), or due to permissions,
> + just output placeholder values rather than failing. */
> + if (process_all && (errno == EACCES || errno == ENOENT))
> + {
> + fstype = "-";
> + fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
> + fsu.fsu_bavail = fsu.fsu_files = fsu.fsu_ffree = UINTMAX_MAX;
> + }
> + else
> + {
> + error (0, errno, "%s", quote (stat_file));
> + exit_status = EXIT_FAILURE;
> + return;
> + }
Is it possible that errno is leaking from one get_dev() invocation
to another, i.e. that one failure eclipses another?
> + }
> + else if (process_all && show_all_fs)
> + {
> + /* Ensure we don't output incorrect stats for overmounted directories.
> + Discard stats when the device name doesn't match. */
> + struct stat sb;
> + if (stat (stat_file, &sb) == 0)
> + {
> + char const * devname = devname_for_dev (sb.st_dev);
> + if (devname && ! STREQ (devname, disk))
> + {
> + fstype = "-";
> + fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
> + fsu.fsu_bavail = fsu.fsu_files = fsu.fsu_ffree = UINTMAX_MAX;
> + }
> + }
> }
>
> if (fsu.fsu_blocks == 0 && !show_all_fs && !show_listed_fs)
> @@ -1230,8 +1280,10 @@ get_all_entries (void)
> {
> struct mount_entry *me;
>
> - if (!show_all_fs)
> - filter_mount_list ();
> + if (! show_all_fs)
> + filter_mount_list (false);
> + else
> + filter_mount_list (true);
Shorter?
+ filter_mount_list (show_all_fs);
> for (me = mount_list; me; me = me->me_next)
> get_dev (me->me_devname, me->me_mountdir, NULL, NULL, me->me_type,
> diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh
> index a620e73..1b19149 100755
> --- a/tests/df/skip-duplicates.sh
> +++ b/tests/df/skip-duplicates.sh
> @@ -96,9 +96,10 @@ 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; }
>
> -# Ensure we fail when unable to stat invalid 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; }
> +# Ensure we don't fail when unable to stat (currently) unavailable entries
> +# instead outputting placeholder information
> +LD_PRELOAD=./k.so CU_TEST_DUPE_INVALID=1 df -T >out || fail=1
> +test $(wc -l <out) -eq $(expr 2 + $unique_entries) || { fail=1; cat out; }
>
> # df should also prefer "/fsname" over "fsname"
> if test "$unique_entries" = 2; then
> @@ -113,6 +114,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; }
> +# Ensure placeholder "-" values used for the overmounted "virtfs"
my Thunderbird suggests: s/overmounted/over-mounted/
> +test $(grep -c 'virtfs *-' <out) -eq 1 || { fail=1; cat out; }
>
> # Ensure that filtering duplicates does not affect
> # argument processing (now without the fake getmntent()).
> -- 1.7.7.6
Great, we're getting closer!
Finally, I have an edge case on my PC which is not yet covered:
I have a backup partition mounted on /root/backup (which is not
accessible by a normal user), and a read-only bind-mount of it:
$ mount /dev/sda1 /root/backup
$ mount --bind /root/backup /media/backup
$ mount -o ro,remount,bind /media/backup
(The 3rd command is just there for setting the read-only flag).
df-8.21 perfectly skipped the inaccessible entry and went ahead
with the read-only copy
$ /usr/bin/df /dev/sda3
Filesystem 1K-blocks Used Available Use% Mounted on
/dev/sda3 155396036 124425132 23054156 85% /media/backup
while the latest one from git plus these 2 patches is falling over:
$ src/df /dev/sda3
src/df: ‘/root/backup’: Permission denied
This is a little regression.
Thus said, I'm not 100% happy with the filtering yet.
I don't have an idea right now how to solve it. Maybe in a
couple of days ... I have to think about it.
Thanks again & have a nice day,
Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Wed, 18 Jun 2014 12:11:02 GMT) Full text and rfc822 format available.Message #63 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 18 Jun 2014 13:09:56 +0100
On 06/18/2014 02:19 AM, Bernhard Voelker wrote: > On 06/17/2014 06:03 PM, Pádraig Brady wrote: >> From 0a4b8027049f6746a237c9fc34a0e0a4afdcfc62 Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com> >> Date: Wed, 4 Jun 2014 00:09:11 +0100 >> Subject: [PATCH] df: output placeholder values for inaccessible mount points I agree with all your suggested tweaks here. > Finally, I have an edge case on my PC which is not yet covered: > I have a backup partition mounted on /root/backup (which is not > accessible by a normal user), and a read-only bind-mount of it: > > $ mount /dev/sda1 /root/backup > $ mount --bind /root/backup /media/backup > $ mount -o ro,remount,bind /media/backup > > (The 3rd command is just there for setting the read-only flag). > > df-8.21 perfectly skipped the inaccessible entry and went ahead > with the read-only copy > > $ /usr/bin/df /dev/sda3 > Filesystem 1K-blocks Used Available Use% Mounted on > /dev/sda3 155396036 124425132 23054156 85% /media/backup > > while the latest one from git plus these 2 patches is falling over: > > $ src/df /dev/sda3 > src/df: ‘/root/backup’: Permission denied > > This is a little regression. > Thus said, I'm not 100% happy with the filtering yet. The above is a separate case actually, triggered in 8.22 with: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=2091f449 Note that only triggers the issue when the last mount point for a device is accessible. If the /proc/mounts order was different, then you'd get the issue anyway. I'll fix this by adding a stat() to the selection criteria in get_disk(). Thanks for the excellent review. Pádraig.
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Wed, 18 Jun 2014 17:23:01 GMT) Full text and rfc822 format available.Message #66 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 18 Jun 2014 18:21:50 +0100
[Message part 1 (text/plain, inline)]
On 06/18/2014 01:09 PM, Pádraig Brady wrote: > On 06/18/2014 02:19 AM, Bernhard Voelker wrote: >> On 06/17/2014 06:03 PM, Pádraig Brady wrote: >>> From 0a4b8027049f6746a237c9fc34a0e0a4afdcfc62 Mon Sep 17 00:00:00 2001 >>> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P <at> draigBrady.com> >>> Date: Wed, 4 Jun 2014 00:09:11 +0100 >>> Subject: [PATCH] df: output placeholder values for inaccessible mount points > > I agree with all your suggested tweaks here. > >> Finally, I have an edge case on my PC which is not yet covered: >> I have a backup partition mounted on /root/backup (which is not >> accessible by a normal user), and a read-only bind-mount of it: >> >> $ mount /dev/sda1 /root/backup >> $ mount --bind /root/backup /media/backup >> $ mount -o ro,remount,bind /media/backup >> >> (The 3rd command is just there for setting the read-only flag). >> >> df-8.21 perfectly skipped the inaccessible entry and went ahead >> with the read-only copy >> >> $ /usr/bin/df /dev/sda3 >> Filesystem 1K-blocks Used Available Use% Mounted on >> /dev/sda3 155396036 124425132 23054156 85% /media/backup >> >> while the latest one from git plus these 2 patches is falling over: >> >> $ src/df /dev/sda3 >> src/df: ‘/root/backup’: Permission denied >> >> This is a little regression. >> Thus said, I'm not 100% happy with the filtering yet. > > The above is a separate case actually, triggered in 8.22 with: > http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=2091f449 > Note that only triggers the issue when the last mount point for a device > is accessible. If the /proc/mounts order was different, then you'd get the issue anyway. > I'll fix this by adding a stat() to the selection criteria in get_disk(). The attached 2 patches on top of current master, should address your suggested tweaks and above ordering issue. thanks, Pádraig.
[df-fix-wrong-device.diff (text/x-patch, attachment)]
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Fri, 20 Jun 2014 00:24:01 GMT) Full text and rfc822 format available.Message #69 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Fri, 20 Jun 2014 02:23:12 +0200
On 06/18/2014 07:21 PM, Pádraig Brady wrote: > The attached 2 patches on top of current master, > should address your suggested tweaks and above ordering issue. Thanks. Both patches look good. The whole story is already rather long, so I almost don't dare to come up with it ... but there is one last issue left with eclipsed file systems: $ mount /dev/sda5 /mnt $ src/df /mnt Filesystem 1K-blocks Used Available Use% Mounted on /dev/sda5 206424760 440500 195482116 1% /mnt $ mount -o loop tmp.img /mnt $ src/df /mnt # correct: search by mount point Filesystem 1K-blocks Used Available Use% Mounted on /dev/loop0 59365 1308 53471 3% /mnt $ src/df /dev/sda5 # wrong: search by device Filesystem 1K-blocks Used Available Use% Mounted on /dev/sda5 59365 1308 53471 3% /mnt Have a nice day, Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Tue, 24 Jun 2014 16:37:01 GMT) Full text and rfc822 format available.Message #72 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Tue, 24 Jun 2014 17:36:08 +0100
[Message part 1 (text/plain, inline)]
On 06/20/2014 01:23 AM, Bernhard Voelker wrote: > On 06/18/2014 07:21 PM, Pádraig Brady wrote: >> The attached 2 patches on top of current master, >> should address your suggested tweaks and above ordering issue. > > Thanks. Both patches look good. > > The whole story is already rather long, so I almost don't dare > to come up with it ... but there is one last issue left with > eclipsed file systems: > > $ mount /dev/sda5 /mnt > > $ src/df /mnt > Filesystem 1K-blocks Used Available Use% Mounted on > /dev/sda5 206424760 440500 195482116 1% /mnt > > $ mount -o loop tmp.img /mnt > > $ src/df /mnt # correct: search by mount point > Filesystem 1K-blocks Used Available Use% Mounted on > /dev/loop0 59365 1308 53471 3% /mnt > > $ src/df /dev/sda5 # wrong: search by device > Filesystem 1K-blocks Used Available Use% Mounted on > /dev/sda5 59365 1308 53471 3% /mnt Fingers crossed this is the last df issue. df is getting quite messy and could do with a refactoring to use prepopulate hashes from /proc/self/mountinfo, but not for this iteration at least. Proposed test attached. thanks, Pádraig.
[df-dev-eclipse.patch (text/x-patch, attachment)]
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Wed, 25 Jun 2014 08:40:02 GMT) Full text and rfc822 format available.Message #75 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 25 Jun 2014 10:35:28 +0200
On 06/24/2014 06:36 PM, Pádraig Brady wrote: > Fingers crossed this is the last df issue. +1 > df is getting quite messy and could do with a refactoring > to use prepopulate hashes from /proc/self/mountinfo, > but not for this iteration at least. Good idea. Maybe we should wait for user feedback (with yet other strange test cases) anyway. The patch looks perfect, thanks! Have a nice day, Berny
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Wed, 25 Jun 2014 08:55:02 GMT) Full text and rfc822 format available.Message #78 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Pádraig Brady <P <at> draigBrady.com> To: Bernhard Voelker <mail <at> bernhard-voelker.de> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 25 Jun 2014 09:54:15 +0100
On 06/25/2014 09:35 AM, Bernhard Voelker wrote:
> On 06/24/2014 06:36 PM, Pádraig Brady wrote:
>> Fingers crossed this is the last df issue.
>
> +1
>
>> df is getting quite messy and could do with a refactoring
>> to use prepopulate hashes from /proc/self/mountinfo,
>> but not for this iteration at least.
>
> Good idea. Maybe we should wait for user feedback (with yet
> other strange test cases) anyway.
>
> The patch looks perfect, thanks!
I submitted a slightly stale patch to the list.
The one I'll push now has this change.
diff --git a/src/df.c b/src/df.c
index 25a6df5..063cabf 100644
--- a/src/df.c
+++ b/src/df.c
@@ -1168,7 +1168,7 @@ get_disk (char const *disk)
if (STREQ (disk, devname))
{
char *last_device = last_device_for_mount (me->me_mountdir);
- eclipsed_device = ! STREQ (last_device, devname);
+ eclipsed_device = last_device && ! STREQ (last_device, devname);
size_t len = strlen (me->me_mountdir);
if (! eclipsed_device
thanks,
Pádraig.
bug-coreutils <at> gnu.org:bug#16539; Package coreutils.
(Wed, 25 Jun 2014 09:16:02 GMT) Full text and rfc822 format available.Message #81 received at 16539 <at> debbugs.gnu.org (full text, mbox):
From: Bernhard Voelker <mail <at> bernhard-voelker.de> To: Pádraig Brady <P <at> draigBrady.com> Cc: 16539 <at> debbugs.gnu.org Subject: Re: bug#16539: More details on df command output for you Date: Wed, 25 Jun 2014 11:11:00 +0200
On 06/25/2014 10:54 AM, Pádraig Brady wrote:
> I submitted a slightly stale patch to the list.
> The one I'll push now has this change.
>
> diff --git a/src/df.c b/src/df.c
> index 25a6df5..063cabf 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -1168,7 +1168,7 @@ get_disk (char const *disk)
> if (STREQ (disk, devname))
> {
> char *last_device = last_device_for_mount (me->me_mountdir);
> - eclipsed_device = ! STREQ (last_device, devname);
> + eclipsed_device = last_device && ! STREQ (last_device, devname);
> size_t len = strlen (me->me_mountdir);
>
> if (! eclipsed_device
oops, correct.
Thanks & have a nice day,
Berny
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org.
(Wed, 23 Jul 2014 11:24:04 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.