On Dienstag, 16. August 2016 13:41:21 CEST you wrote:
Hi Stefan,
Am 2016-08-14 03:50, schrieb Stefan Bruens:
On Freitag, 12. August 2016 15:16:20 CEST Michael Walle wrote:
All fields were accessed directly instead of using the proper byte swap functions. Thus, ext4 write support was only usable on little-endian architectures. Fix this.
Signed-off-by: Michael Walle michael@walle.cc
I have tested this on sandbox (x86_64), no regressions found. Some remarks below.
Reviewed-by: Stefan Brüns stefan.bruens@rwth-aachen.de Tested-by: Stefan Brüns stefan.bruens@rwth-aachen.de
[snip]
@@ -2234,7 +2246,7 @@ int ext4fs_mount(unsigned part_length)
* and we do not support metadata_csum (and cannot reliably find * files when it is set. Refuse to mount. */
- if (data->sblock.feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT) {
- if (le32_to_cpu(data->sblock.feature_incompat) &
EXT4_FEATURE_INCOMPAT_64BIT) { printf("Unsupported feature found (64bit, possibly metadata_csum), not mounting\n"); goto fail;
}
This should have a if ((data->sblock.revision_level !=0) && ... in front, features are not defined for revision 0. Applies to other places as well ...
are you sure about that? I can't find any code in the kernel where features are only valid if revision > 0. Also, I couldn't find anything in the ext4 documentation wiki:
This document states that inode_size and all later fields are only defined for revision 1.
The statement seems to originate from here: http://git.kernel.org/cgit/fs/ext2/e2fsprogs.git/tree/lib/ext2fs/ ext2_fs.h#n651
Now, if the remainder of the superblock after def_resgid is guaranteed to be zeroed the feature checks are ok for any revision, whereas a zero inode size would be bad and defaulting to 128 is needed.
As e2fsprogs is skipping any revision-level tests for feature checks, it seems to be ok to assume remaining fields to be zeroed.
Kind regards,
Stefan