GNU bug report logs - #8091
fiemap_capable_ misbehaves in a chroot

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8091 in the body.
You can then email your comments to 8091 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Adam Sampson <ats <at> offog.org>
To: bug-coreutils <at> gnu.org
Subject: fiemap_capable_ misbehaves in a chroot
Date: Mon, 21 Feb 2011 13:04:57 +0000

Hi,

A minor bug in the testsuite in coreutils 8.10...

I package software in a chroot. At the moment, my chroot doesn't contain
an mtab file, which means df can't tell what type the filesystem is:

$ df -T /
Filesystem    Type   1K-blocks      Used Available Use% Mounted on
-                -   288370908  90872412 194568804  32% /

The fiemap_capable_ function in tests/init.cfg uses df to tell whether
the filesystem it's running on supports fiemap, by asking it to only
list filesystems that it knows are fiemap-capable using -t. However, the
selected_fstype function in df.c will always include filesystems that it
doesn't know the type of, even when -t is specified:

$ df -T -t blahfs /
Filesystem    Type   1K-blocks      Used Available Use% Mounted on
-                -   288370908  90872400 194568816  32% /

The result is that fiemap_capable_ assumes my chrooted filesystem
supports fiemap when it doesn't, so the tests that use it fail.

I'm not sure at this point whether df -t is doing the wrong thing or
fiemap_capable_ is using it incorrectly. Any thoughts?

Thanks very much,

-- 
Adam Sampson <ats <at> offog.org>                         <http://offog.org/>

Message #10 received at 8091-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Adam Sampson <ats <at> offog.org>
Cc: 8091-done <at> debbugs.gnu.org
Subject: Re: bug#8091: fiemap_capable_ misbehaves in a chroot
Date: Mon, 21 Feb 2011 15:16:28 +0000

On 21/02/11 13:04, Adam Sampson wrote:
> Hi,
> 
> A minor bug in the testsuite in coreutils 8.10...
> 
> I package software in a chroot. At the moment, my chroot doesn't contain
> an mtab file, which means df can't tell what type the filesystem is:

Going forward /etc/mtab is going to be linked to /proc/mounts
so this should be less of an issue.

> 
> $ df -T /
> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
> -                -   288370908  90872412 194568804  32% /
> 
> The fiemap_capable_ function in tests/init.cfg uses df to tell whether
> the filesystem it's running on supports fiemap, by asking it to only
> list filesystems that it knows are fiemap-capable using -t. However, the
> selected_fstype function in df.c will always include filesystems that it
> doesn't know the type of, even when -t is specified:
> 
> $ df -T -t blahfs /
> Filesystem    Type   1K-blocks      Used Available Use% Mounted on
> -                -   288370908  90872400 194568816  32% /
> 
> The result is that fiemap_capable_ assumes my chrooted filesystem
> supports fiemap when it doesn't, so the tests that use it fail.

That is already addressed with:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5c3fd50a

> I'm not sure at this point whether df -t is doing the wrong thing or
> fiemap_capable_ is using it incorrectly. Any thoughts?

We may adjust what df does in future when it can't get the fs list.
Perhaps we could resort to statfs(), though that has its own
ambiguities, like ext[234] all having the same type.
Hmm, that reminds me that we probably want to:

diff --git a/src/stat.c b/src/stat.c
index f26dced..5458e48 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -273,7 +273,7 @@ human_fstype (STRUCT_STATVFS const *statfsbuf)
     case S_MAGIC_EXT: /* 0x137D */
       return "ext";
     case S_MAGIC_EXT2: /* 0xEF53 */
-      return "ext2/ext3";
+      return "ext2/ext3/ext4";
     case S_MAGIC_EXT2_OLD: /* 0xEF51 */
       return "ext2";
     case S_MAGIC_FAT: /* 0x4006 */

cheers,
Pádraig.

Message #13 received at 8091 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 8091 <at> debbugs.gnu.org
Cc: P <at> draigBrady.com
Subject: Re: bug#8091: fiemap_capable_ misbehaves in a chroot
Date: Mon, 21 Feb 2011 17:25:22 +0100

Pádraig Brady wrote:
...
>> I'm not sure at this point whether df -t is doing the wrong thing or
>> fiemap_capable_ is using it incorrectly. Any thoughts?
>
> We may adjust what df does in future when it can't get the fs list.
> Perhaps we could resort to statfs(), though that has its own
> ambiguities, like ext[234] all having the same type.
> Hmm, that reminds me that we probably want to:
>
> diff --git a/src/stat.c b/src/stat.c
> index f26dced..5458e48 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -273,7 +273,7 @@ human_fstype (STRUCT_STATVFS const *statfsbuf)
>      case S_MAGIC_EXT: /* 0x137D */
>        return "ext";
>      case S_MAGIC_EXT2: /* 0xEF53 */
> -      return "ext2/ext3";
> +      return "ext2/ext3/ext4";
>      case S_MAGIC_EXT2_OLD: /* 0xEF51 */
>        return "ext2";
>      case S_MAGIC_FAT: /* 0x4006 */

We talked about this two years ago:

  Re: [PATCH] stat: add support for more file system types
    http://www.mail-archive.com/bug-coreutils <at> gnu.org/msg18898.html
  Re: [Bug 485507] RFE: add ext4 to "stat -f" output
    http://lists.gnu.org/archive/html/bug-coreutils/2009-02/msg00160.html

I guess the question should be "do we ever want to change this?".
If so, sooner may be better than later, but I'm not sure it's worth
the risk.

Message #16 received at 8091 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 8091 <at> debbugs.gnu.org
Subject: Re: bug#8091: fiemap_capable_ misbehaves in a chroot
Date: Mon, 21 Feb 2011 17:22:51 +0000

On 21/02/11 16:25, Jim Meyering wrote:
> Pádraig Brady wrote:
>> Hmm, that reminds me that we probably want to:
>>
>> diff --git a/src/stat.c b/src/stat.c
>> index f26dced..5458e48 100644
>> --- a/src/stat.c
>> +++ b/src/stat.c
>> @@ -273,7 +273,7 @@ human_fstype (STRUCT_STATVFS const *statfsbuf)
>>      case S_MAGIC_EXT: /* 0x137D */
>>        return "ext";
>>      case S_MAGIC_EXT2: /* 0xEF53 */
>> -      return "ext2/ext3";
>> +      return "ext2/ext3/ext4";
>>      case S_MAGIC_EXT2_OLD: /* 0xEF51 */
>>        return "ext2";
>>      case S_MAGIC_FAT: /* 0x4006 */
> 
> We talked about this two years ago:
> 
>   Re: [PATCH] stat: add support for more file system types
>     http://www.mail-archive.com/bug-coreutils <at> gnu.org/msg18898.html
>   Re: [Bug 485507] RFE: add ext4 to "stat -f" output
>     http://lists.gnu.org/archive/html/bug-coreutils/2009-02/msg00160.html
> 
> I guess the question should be "do we ever want to change this?".
> If so, sooner may be better than later, but I'm not sure it's worth
> the risk.

Hmm yes, this is debatable.

So your argument is script might be keying on "ext2/ext3"
rather than "ext2/ext3/*". Unlikely, but possible.

It's a pity ext4 didn't pick a new ID, given that
it's not forwards compatible when various features are used,
unlike ext[23] where one could always mount as ext2.

I guess it's safer to leave alone.

cheers,
Pádraig.

Message #17 received at 8091-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 8091-done <at> debbugs.gnu.org
Subject: Re: bug#8091: fiemap_capable_ misbehaves in a chroot
Date: Mon, 21 Feb 2011 18:29:29 +0100

Pádraig Brady wrote:

> On 21/02/11 16:25, Jim Meyering wrote:
>> Pádraig Brady wrote:
>>> Hmm, that reminds me that we probably want to:
>>>
>>> diff --git a/src/stat.c b/src/stat.c
>>> index f26dced..5458e48 100644
>>> --- a/src/stat.c
>>> +++ b/src/stat.c
>>> @@ -273,7 +273,7 @@ human_fstype (STRUCT_STATVFS const *statfsbuf)
>>>      case S_MAGIC_EXT: /* 0x137D */
>>>        return "ext";
>>>      case S_MAGIC_EXT2: /* 0xEF53 */
>>> -      return "ext2/ext3";
>>> +      return "ext2/ext3/ext4";
>>>      case S_MAGIC_EXT2_OLD: /* 0xEF51 */
>>>        return "ext2";
>>>      case S_MAGIC_FAT: /* 0x4006 */
>>
>> We talked about this two years ago:
>>
>>   Re: [PATCH] stat: add support for more file system types
>>     http://www.mail-archive.com/bug-coreutils <at> gnu.org/msg18898.html
>>   Re: [Bug 485507] RFE: add ext4 to "stat -f" output
>>     http://lists.gnu.org/archive/html/bug-coreutils/2009-02/msg00160.html
>>
>> I guess the question should be "do we ever want to change this?".
>> If so, sooner may be better than later, but I'm not sure it's worth
>> the risk.
>
> Hmm yes, this is debatable.
>
> So your argument is script might be keying on "ext2/ext3"
> rather than "ext2/ext3/*". Unlikely, but possible.

Right, but "ext2/ext3*".

> It's a pity ext4 didn't pick a new ID, given that
> it's not forwards compatible when various features are used,
> unlike ext[23] where one could always mount as ext2.
>
> I guess it's safer to leave alone.

So I'm closing the ticket.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.