pengutronix / genimage

tool to generate multiple filesystem and flash images from a tree
GNU General Public License v2.0
308 stars 110 forks source link

ext4: option "features" ignored when "use-mke2fs" is true #81

Closed LeSpocky closed 4 years ago

LeSpocky commented 4 years ago

After migrating a ptxdist based BSP from ptxdist-2019.09.0 to ptxdist-2019.10.0 our target board complained when booting with U-Boot 2016.09.x from SD-Card with ext4 partitions:

Unsupported feature found (64bit, possibly metadata_csum), not mounting
Failed to mount ext2 filesystem...
** Unrecognized filesystem type **

Comparing the ext4 flags of the created root.ext2 images revealed this:

-Filesystem features:      has_journal dir_index extent uninit_bg
+Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit flex_bg sparse_super dir_nlink extra_isize metadata_csum

The reason was ptxdist switched on use-mke2fs = true in the default genimage config for **ext* images, so the old image was build with genext2fs and the new one with mke2fs which is the intend of this option. However, the latter ignores the option features** as can be seen when you compare output of genimage (stripped to the necessary lines):

INFO: ext4(root.ext2): cmd: "genext2fs -d '/tmp/ptxdist.Llj64K/genimage.M7vSVP/root' --size-in-blocks=76800 -i 16384 '/home/adahl/Work/bsp/***/platform-v7a/images/root.ext2' " (stderr):
INFO: ext4(root.ext2): cmd: "tune2fs -O 'extents,uninit_bg,dir_index,has_journal' '/home/adahl/Work/bsp/***platform-v7a/images/root.ext2'" (stderr):

vs. just

INFO: ext4(root.ext2): cmd: "mke2fs -t ext4 -E 'root_owner=0:0,lazy_itable_init=0,lazy_journal_init=0' -O '^large_file' -O '^huge_file' -d '/tmp/ptxdist.T2Gf50/genimage.sJEXH6/root'   '/home/adahl/Work/bsp/***/platform-v7a/images/root.ext2' 60416" (stderr):

Without use-mke2fs = true genimage passes the options from "features" to a separate tune2fs run, which is not done in the case where mke2fs is called.

I could confirm this adding the following line to the genimage config:

features = "uninit_bg"

When checking the resulting image with tune2fs that option is not shown.

Maybe one could just pass "features" to the -O option of "mke2fs"?

LeSpocky commented 4 years ago

Maybe add a test which calls tune2fs and checks if things set/unset in features option matches with the result?

michaelolbrich commented 4 years ago

More tests are always good :-)

You know, I was reluctant to change the default in PTXdist because I was afraid of stuff like that. Then https://github.com/systemd/systemd/issues/13609 happened. And while that has been fixed in systemd, I'm pretty sure, that other software gets it wrong as well so I changed the default. And of course not the issues show up...

LeSpocky commented 4 years ago

Maybe add a test which calls tune2fs and checks if things set/unset in features option matches with the result?

That might not work. Some features I passed with mke2fs -O manually did not show up with tune2fs later. 🤔

michaelolbrich commented 4 years ago

That might not work. Some features I passed with mke2fs -O manually did not show up with tune2fs later. thinking

Which features did not work? I think some features are mutually exclusive, so that might be the reason.

LeSpocky commented 4 years ago

Which features did not work? I think some features are mutually exclusive, so that might be the reason.

I set "^64bit,extents,uninit_bg,dir_index,has_journal" and the result was:

has_journal ext_attr resize_inode dir_index filetype extent flex_bg sparse_super dir_nlink extra_isize metadata_csum

This is the (relevant?) part of platform-v7a/sysroot-host/etc/mke2fs.conf:

[defaults]
        base_features = sparse_super,large_file,filetype,resize_inode,dir_index,ext_attr
        default_mntopts = acl,user_xattr
        enable_periodic_fsck = 0
        blocksize = 4096
        inode_size = 256
        inode_ratio = 16384

[fs_types]
        ext3 = {
                features = has_journal
        }
        ext4 = {
                features = has_journal,extent,huge_file,flex_bg,metadata_csum,64bit,dir_nlink,extra_isize
                inode_size = 256
        }

The feature which did not show up: uninit_bg

michaelolbrich commented 4 years ago

try ^metadata_csum.

From the documentation: "The checksum algorithm used for the metadata blocks is different than the one used for group descriptors with the uninit_bg feature. These two features are incompatible and metadata_csum will be used preferentially instead of uninit_bg."

LeSpocky commented 4 years ago

try ^metadata_csum.

I could, but I'd rather have metadata_csum than uninit_bg.

From the documentation: "The checksum algorithm used for the metadata blocks is different than the one used for group descriptors with the uninit_bg feature. These two features are incompatible and metadata_csum will be used preferentially instead of uninit_bg."

Okay then this should be considered if someone writes a test.