Package: coreutils;
Reported by: jidanni <at> jidanni.org
Date: Sun, 25 Dec 2011 00:44:02 UTC
Severity: normal
Tags: fixed
Done: Assaf Gordon <assafgordon <at> gmail.com>
Bug is archived. No further changes may be made.
Message #23 received at 10363 <at> debbugs.gnu.org (full text, mbox):
From: Jim Meyering <jim <at> meyering.net> To: Paul Eggert <eggert <at> cs.ucla.edu> Cc: 10363 <at> debbugs.gnu.org, coreutils <at> gnu.org Subject: Re: bug#10363: df vs. long-named /dev/disk/by-uuid/... file system device names Date: Fri, 30 Dec 2011 14:52:13 +0100
Paul Eggert wrote:
> On 12/29/11 06:09, Jim Meyering wrote:
>> + /* If dev_name is a long-named symlink like
>> + /dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7196a2edb66 and its
>> + canonical name is shorter, use the shorter name. But don't bother
>> + checking when DEV_NAME is no longer than e.g., "/dev/sda1" */
>> + if (resolve_device_symlink && 9 < orig_len
>> + && (resolved_dev = canonicalize_filename_mode (dev_name, CAN_EXISTING)))
>> + {
>> + if (strlen (resolved_dev) < orig_len)
>> + {
>> + free (dev_name);
>> + dev_name = resolved_dev;
>> + }
>> + else
>> + {
>> + free (resolved_dev);
>> + }
>> + }
>
> I have some qualms about that "is shorter" part;
> couldn't that lead to confusing results, on systems
> where the canonical name is sometimes a bit shorter and sometimes
> a bit longer?
>
> Also, that "9 < orig_len" could also cause confusion.
>
> The flag "resolve_device_symlink" suggests that
> the name should always be resolved, at any rate.
>
> In short, how a simpler approach, that always resolves
> symlinks? Something like this:
>
> /* If dev_name is a symlink use the resolved name.
> On recent GNU/Linux systems we often see a symlink from, e.g.,
> /dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7196a2edb66
> tp /dev/sda3 and it's better to output /dev/sda3. */
> if (resolve_device_symlink
> && (resolved_dev = canonicalize_filename_mode (dev_name, CAN_EXISTING)))
> {
> free (dev_name);
> dev_name = resolved_dev;
> }
Thanks for the suggestion. I've dropped the 9 < ... part, but since this
is (at least planned) to be the default, I want to limit the scope of the
change to those very long by-uuid symlinks, so I've adjusted it like this:
/* On some systems, dev_name is a long-named symlink like
/dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7196a2edb66 pointing
to a much shorter and more useful name like /dev/sda1.
When resolve_device_symlink is true and dev_name is a symlink whose
name starts with /dev/disk/by-uuid/ use the resolved name instead. */
if (resolve_device_symlink
&& STRPREFIX (dev_name, "/dev/disk/by-uuid/")
&& (resolved_dev = canonicalize_filename_mode (dev_name, CAN_EXISTING)))
{
free (dev_name);
dev_name = resolved_dev;
}
I considered matching only "/dev/disk/by-", in case some initrd
uses by-id, by-label or by-path, but I doubt that will happen often
enough, so will keep this change very precisely targeted.
I've also changed the parameter name from resolve_device_symlink to
process_all. I don't particular like that name either.
Suggestions welcome.
From fb52b3722433d88af8ca8912d3cac531e04bf585 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Thu, 29 Dec 2011 14:49:00 +0100
Subject: [PATCH] df: work around long-named /dev/disk/by-uuid/... symlinks
On systems with recent kernel/tools, a symlink from /etc/mtab to
/proc/mounts, and a by-UUID mount (i.e., soon, nearly everyone),
you will see something like the following when running "df -hT":
(this has been truncated to fit in a width-limited ChangeLog file)
Filesystem Type Siz...
rootfs rootfs 11G
udev devtmpfs 3.8G
tmpfs tmpfs 774M
/dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7096a2edb66 ext4 11G
tmpfs tmpfs 1.6G
/dev/sda2 ext3 494M
/dev/sda5 ext4 12G
/dev/sda6 ext4 9.9G
Contrast that with what we're used to seeing (modulo the
two entries mounted on "/", which is a separate problem):
Filesystem Type Size Used Avail Use% Mounted on
rootfs rootfs 11G 1.9G 8.0G 19% /
udev devtmpfs 3.8G 0 3.8G 0% /dev
tmpfs tmpfs 774M 376K 774M 1% /run
/dev/sda3 ext4 11G 1.9G 8.0G 19% /
tmpfs tmpfs 1.6G 8.0K 1.6G 1% /run/shm
/dev/sda2 ext3 494M 78M 392M 17% /boot
/dev/sda5 ext4 12G 7.6G 3.7G 68% /usr
/dev/sda6 ext4 9.9G 6.6G 2.8G 71% /var
When that long /dev/disk/by-uuid/... name is merely a symlink
to a much shorter (and often more useful) device name like
"/dev/sda3", and when it's part of a listing of all file systems,
I would much prefer to see only the latter.
I.e., if I explicitly run
"df -hT /dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7096a2edb66",
then, it's fine -- and expected -- to print to the long name.
It was explicitly given. However, with no non-option argument,
df should print the shorter name. Note that performing this
translation at a lower level (via a change to gnulib's mountlist.c)
would make it impossible to distinguish those two cases.
* src/df.c: Include "canonicalize.h".
(get_dev): Add a parameter, telling when we're in process-all-
mount-points mode; update all callers. When true, resolve
/dev/disk/by-uuid/... symlinks.
Reported by Dan Jacobson in http://bugs.gnu.org/10363
---
src/df.c | 37 +++++++++++++++++++++++++++++--------
1 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/src/df.c b/src/df.c
index 982d607..cedb583 100644
--- a/src/df.c
+++ b/src/df.c
@@ -25,6 +25,7 @@
#include <assert.h>
#include "system.h"
+#include "canonicalize.h"
#include "error.h"
#include "fsusage.h"
#include "human.h"
@@ -428,13 +429,16 @@ add_uint_with_neg_flag (uintmax_t *dest, bool *dest_neg,
If FSTYPE is non-NULL, it is the type of the file system on DISK.
If MOUNT_POINT is non-NULL, then DISK may be NULL -- certain systems may
not be able to produce statistics in this case.
- ME_DUMMY and ME_REMOTE are the mount entry flags. */
+ ME_DUMMY and ME_REMOTE are the mount entry flags.
+ Caller must set PROCESS_ALL to true when iterating over all entries, as
+ when df is invoked with no non-option argument. See below for details. */
static void
get_dev (char const *disk, char const *mount_point,
char const *stat_file, char const *fstype,
bool me_dummy, bool me_remote,
- const struct fs_usage *force_fsu)
+ const struct fs_usage *force_fsu,
+ bool process_all)
{
struct fs_usage fsu;
char buf[LONGEST_HUMAN_READABLE + 2];
@@ -488,6 +492,23 @@ get_dev (char const *disk, char const *mount_point,
if (! disk)
disk = "-"; /* unknown */
+
+ char *dev_name = xstrdup (disk);
+ char *resolved_dev;
+
+ /* On some systems, dev_name is a long-named symlink like
+ /dev/disk/by-uuid/828fc648-9f30-43d8-a0b1-f7196a2edb66 pointing
+ to a much shorter and more useful name like /dev/sda1.
+ When process_all is true and dev_name is a symlink whose name starts
+ with /dev/disk/by-uuid/ use the resolved name instead. */
+ if (process_all
+ && STRPREFIX (dev_name, "/dev/disk/by-uuid/")
+ && (resolved_dev = canonicalize_filename_mode (dev_name, CAN_EXISTING)))
+ {
+ free (dev_name);
+ dev_name = resolved_dev;
+ }
+
if (! fstype)
fstype = "-"; /* unknown */
@@ -537,7 +558,7 @@ get_dev (char const *disk, char const *mount_point,
switch (field)
{
case DEV_FIELD:
- cell = xstrdup (disk);
+ cell = dev_name;
break;
case TYPE_FIELD:
@@ -648,7 +669,7 @@ get_disk (char const *disk)
{
get_dev (best_match->me_devname, best_match->me_mountdir, NULL,
best_match->me_type, best_match->me_dummy,
- best_match->me_remote, NULL);
+ best_match->me_remote, NULL, false);
return true;
}
@@ -734,7 +755,7 @@ get_point (const char *point, const struct stat *statp)
if (best_match)
get_dev (best_match->me_devname, best_match->me_mountdir, point,
best_match->me_type, best_match->me_dummy, best_match->me_remote,
- NULL);
+ NULL, false);
else
{
/* We couldn't find the mount entry corresponding to POINT. Go ahead and
@@ -745,7 +766,7 @@ get_point (const char *point, const struct stat *statp)
char *mp = find_mount_point (point, statp);
if (mp)
{
- get_dev (NULL, mp, NULL, NULL, false, false, NULL);
+ get_dev (NULL, mp, NULL, NULL, false, false, NULL, false);
free (mp);
}
}
@@ -774,7 +795,7 @@ get_all_entries (void)
for (me = mount_list; me; me = me->me_next)
get_dev (me->me_devname, me->me_mountdir, NULL, me->me_type,
- me->me_dummy, me->me_remote, NULL);
+ me->me_dummy, me->me_remote, NULL, true);
}
/* Add FSTYPE to the list of file system types to display. */
@@ -1066,7 +1087,7 @@ main (int argc, char **argv)
{
if (inode_format)
grand_fsu.fsu_blocks = 1;
- get_dev ("total", NULL, NULL, NULL, false, false, &grand_fsu);
+ get_dev ("total", NULL, NULL, NULL, false, false, &grand_fsu, false);
}
print_table ();
--
1.7.7.3
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.