[parted-devel] [PATCH] Detect Ext4 (Parted Alioth #188)

[Jim Meyering]

"Debarshi Ray" <debarshi.ray at gmail.com> wrote:

>> Here ([1] and below) is the 'git format-patch ...' with an added test case.
>>
>> [...]
>>
>> [1] http://rishi.fedorapeople.org/gnu/0001-Differentiate-between-Ext4-and-Ext3-file-systems.-Fi.patch
>
> I have updated the patch at the above URL as per some of Jim
> Meyering's suggestions on IRC.
>
> While looking at the way TestDisk [2] does the same thing, I found
> that they look for the following features in src/ext2.c:
> EXT4_FEATURE_RO_COMPAT_HUGE_FILE
> EXT4_FEATURE_RO_COMPAT_GDT_CSUM
> EXT4_FEATURE_RO_COMPAT_DIR_NLINK
> EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE
> EXT4_FEATURE_INCOMPAT_64BIT
> EXT4_FEATURE_INCOMPAT_MMP
>
> Interestingly, they don't look for EXT4_FEATURE_INCOMPAT_EXTENTS. Must
> be a mistake on their part since they have defined the constant and
> not used it. Looking at the Ext4 wiki [3], I added checks for the
> following features in Parted:
> EXT4_FEATURE_RO_COMPAT_HUGE_FILE
> EXT4_FEATURE_RO_COMPAT_DIR_NLINK
> EXT4_FEATURE_INCOMPAT_EXTENTS
> EXT4_FEATURE_INCOMPAT_64BIT
>
> What do you think?

Thanks for the quick changes.
Here are a few more requests:

Please make the one-line summary fit within ~70 columns:
i.e., make this the summary,

  Differentiate between Ext4 and Ext3 file systems.

insert a blank line, then this:

  Fixes http://parted.alioth.debian.org/cgi-bin/trac.cgi/ticket/188
------------------------
remove the redundant test of is_ext3:

          if (is_ext3) {
+                        is_ext4
+                            = is_ext3
...

------------------------
adjust C indentation; it's not quite right in places
(i.e., use TABs (not spaces) in ext2_fs.h, when surrounding code does that)

------------------------
make the test "skip" (not fail) if mkfs.ext4 fails.
That may save us some reports of test failures on systems
lacking ext4 support.