On Mar 6, 2018, at 8:18 AM, Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx> wrote:
>
> Inodes count and free inodes count should be 64 bit long.
You might consider splitting this into a "add wrappers for inode access"
patch and a separate "change s_inodes_* to 64-bit" patch, like the ext4
patches that went in upstream, to simplify inspection.
> @@ -128,7 +129,7 @@ ext2_ino_t string_to_inode(char *str)
> com_err(str, retval, 0);
> return 0;
> }
> - if (ino > current_fs->super->s_inodes_count) {
> + if (ino > ext2fs_get_inodes_count(current_fs->super)) {
> com_err(str, 0, "resolves to an illegal inode number: %u\n",
> ino);
Should this be "%llu"?
> @@ -433,8 +433,9 @@ static void check_if_skip(e2fsck_t ctx)
> /* Print the summary message when we're skipping a full check */
> log_out(ctx, _("%s: clean, %u/%u files, %llu/%llu blocks"),
> ctx->device_name,
> - fs->super->s_inodes_count - fs->super->s_free_inodes_count,
> - fs->super->s_inodes_count,
> + ext2fs_get_inodes_count(fs->super) -
> + ext2fs_get_free_inodes_count(fs->super),
> + ext2fs_get_inodes_count(fs->super),
> ext2fs_blocks_count(fs->super) -
> ext2fs_free_blocks_count(fs->super),
> ext2fs_blocks_count(fs->super));
Also "%llu/%llu"?
> diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
> index 1a5b2ebc..9ed1c193 100644
> --- a/e2fsck/pass2.c
> +++ b/e2fsck/pass2.c
> @@ -411,8 +411,15 @@ int e2fsck_check_dirent_data(e2fsck_t ctx, struct ext2_dir_entry *de,
> if ((de->name_len >> 8) & ~EXT2_FT_MASK) {
> /* clear dirent extra data flags. */
> if (fix_problem(ctx, PR_2_CLEAR_DIRDATA, pctx)) {
> - de->name_len &= (EXT2_FT_MASK << 8) |
> + if ((de->name_len >> 8) & EXT2_DIRENT_INODE &&
> + ctx->fs->super->s_feature_incompat &
> + EXT4_FEATURE_INCOMPAT_INODE64) {
> + ctx->fs->super->s_feature_incompat |=
> + EXT4_FEATURE_INCOMPAT_DIRDATA;
Isn't it pretty much a given that INCOMPAT_INODE64 requires INCOMPAT_DIRDATA?
That could be checked early on in e2fsck, rather than waiting until finding
a dirent that has the high bits set.
> diff --git a/lib/e2p/ls.c b/lib/e2p/ls.c
> index a7586e09..91a3b0ae 100644
> --- a/lib/e2p/ls.c
> +++ b/lib/e2p/ls.c
> @@ -269,14 +269,17 @@ void list_super2(struct ext2_super_block * sb, FILE *f)
> str = e2p_os2string(sb->s_creator_os);
> fprintf(f, "Filesystem OS type: %s\n", str);
> free(str);
> - fprintf(f, "Inode count: %u\n", sb->s_inodes_count);
> + fprintf(f, "Inode count: %u\n",
> + ext2fs_get_inodes_count(sb));
Should this be "%llu"?
> fprintf(f, "Block count: %llu\n", e2p_blocks_count(sb));
> fprintf(f, "Reserved block count: %llu\n", e2p_r_blocks_count(sb));
> if (sb->s_overhead_blocks)
> fprintf(f, "Overhead blocks: %u\n",
> sb->s_overhead_blocks);
> - fprintf(f, "Free blocks: %llu\n", e2p_free_blocks_count(sb));
> - fprintf(f, "Free inodes: %u\n", sb->s_free_inodes_count);
> + fprintf(f, "Free blocks: %llu\n",
> + e2p_free_blocks_count(sb));
> + fprintf(f, "Free inodes: %u\n",
> + ext2fs_get_free_inodes_count(sb));
Similarly, should this be "%lu" or "%llu"? While the kernel has a limit
of "unsigned long" for inode numbers, it isn't clear whether the same
limit should exist in userspace? In any case, "%u" is for unsigned int,
which is typically a 32-bit value, and we need at least "%lu" for
64-bit inode numbers.
I suspect that there is a lot more code that needs to be changed to allow
64-bit inode numbers everywhere, since any code using "ext2_ino_t" is
not going to be working correctly if it finds a 64-bit inode.
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
