diff mbox

[2/4] fs/ext2: add ability to build ext3/4 too

Message ID 1a995bdbc65141cf4e8a540c41cb56f4a43fba5f.1361142401.git.yann.morin.1998@free.fr
State Changes Requested
Headers show

Commit Message

Yann E. MORIN Feb. 17, 2013, 11:10 p.m. UTC 
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 fs/ext2/Config.in    |   37 ++++++++++++++++++++++++++++++-------
 fs/ext2/ext2.mk      |    4 ++--
 fs/ext2/genext2fs.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 78 insertions(+), 11 deletions(-)

Comments

Arnout Vandecappelle Feb. 19, 2013, 7:04 a.m. UTC | #1 
Great feature addition!

On 18/02/13 00:10, Yann E. MORIN wrote:
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>   fs/ext2/Config.in    |   37 ++++++++++++++++++++++++++++++-------
>   fs/ext2/ext2.mk      |    4 ++--
>   fs/ext2/genext2fs.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 78 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ext2/Config.in b/fs/ext2/Config.in
> index cb4beed..00f11a2 100644
> --- a/fs/ext2/Config.in
> +++ b/fs/ext2/Config.in
> @@ -1,10 +1,33 @@
>   config BR2_TARGET_ROOTFS_EXT2
> -	bool "ext2 root filesystem"
> +	bool "ext2/3/4 root filesystem"
>   	help
> -	  Build an ext2 root filesystem
> +	  Build an ext2/3/4 root filesystem
>
>   if BR2_TARGET_ROOTFS_EXT2
>
> +choice
> +	bool "ext generation"

  Given the way it appears in menuconfig, I think this will be hard to 
understand for many users. Perhaps "ext generation (ext2, ext3 or ext4)".

> +	default BR2_TARGET_ROOTFS_EXT2_2

  Although this matches the current default, doesn't it make more sense 
to "bump" to ext4?

> +
> +config BR2_TARGET_ROOTFS_EXT2_2
> +	bool "ext2"
> +
> +config BR2_TARGET_ROOTFS_EXT2_3
> +	bool "ext3"
> +	select BR2_PACKAGE_HOST_E2FSPROGS

  We don't usually select the host package. On the other hand, the 
support for user-selectable host packages is pretty recent, so we don't 
have a real tradition for this.

  Peter?

> +
> +config BR2_TARGET_ROOTFS_EXT2_4
> +	bool "ext4"
> +	select BR2_PACKAGE_HOST_E2FSPROGS
> +
> +endchoice
> +
> +config BR2_TARGET_ROOTFS_EXT2_GEN
> +	int
> +	default 2 if BR2_TARGET_ROOTFS_EXT2_2
> +	default 3 if BR2_TARGET_ROOTFS_EXT2_3
> +	default 4 if BR2_TARGET_ROOTFS_EXT2_4
> +
>   config BR2_TARGET_ROOTFS_EXT2_BLOCKS
>   	int "size in blocks (leave at 0 for auto calculation)"
>   	default 0
[snip]
> diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
> index 7b71592..80ad93f 100644
> --- a/fs/ext2/ext2.mk
> +++ b/fs/ext2/ext2.mk
> @@ -18,10 +18,10 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
>   EXT2_OPTS += -m $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
>   endif
>
> -ROOTFS_EXT2_DEPENDENCIES = host-genext2fs
> +ROOTFS_EXT2_DEPENDENCIES = host-genext2fs $(if $(BR2_PACKAGE_HOST_E2FSPROGS),host-e2fsprogs)

  Although this is correct, I think it looks confusing. I prefer a more 
explicit

ifeq ($(BR2_TARGET_ROOTFS_EXT2_3)$(BR2_TARGET_ROOTFS_EXT2_4),y)
ROOTFS_EXT2_DEPENDENCIES += host-e2fsprogs
endif

>
>   define ROOTFS_EXT2_CMD
> -	PATH=$(TARGET_PATH) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) $@
> +	PATH=$(TARGET_PATH) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) -$(BR2_TARGET_ROOTFS_EXT2_GEN) $@

  Minor nit: I would prefer
EXT2_OPTS += -$(BR2_TARGET_ROOTFS_EXT2_GEN)

>   endef
>
>   $(eval $(call ROOTFS_TARGET,ext2))
> diff --git a/fs/ext2/genext2fs.sh b/fs/ext2/genext2fs.sh
> index 7a518ae..fcbd43c 100755
> --- a/fs/ext2/genext2fs.sh
> +++ b/fs/ext2/genext2fs.sh
> @@ -1,19 +1,22 @@
>   #!/bin/sh
>   # genext2fs wrapper calculating needed blocks/inodes values if not specified
> +set -e
>
>   export LC_ALL=C
>
>   CALC_BLOCKS=1
>   CALC_INODES=1
>
> -while getopts x:d:D:b:i:N:m:g:e:zfqUPhVv f
> +while getopts x:d:D:b:i:N:m:g:e:zfqUPhVv234 f
>   do
>       case $f in
> +	2|3|4) GEN=$f ;;
>   	b) CALC_BLOCKS=0 ;;
>   	N) CALC_INODES=0; INODES=$OPTARG ;;
>   	d) TARGET_DIR=$OPTARG ;;
>       esac
>   done
> +eval IMG="\"\${${OPTIND}}\""
>
>   # calculate needed inodes
>   if [ $CALC_INODES -eq 1 ];
> @@ -30,7 +33,48 @@ then
>       # we scale inodes / blocks with 10% to compensate for bitmaps size + slack
>       BLOCKS=$(du -s -c -k $TARGET_DIR | grep total | sed -e "s/total//")
>       BLOCKS=$(expr 500 + \( $BLOCKS + $INODES / 8 \) \* 11 / 10)
> +    # we add 1081 blocks (a bit more than 1 MiB, assuming 1KiB blocks) for
> +    # the journal if ext3/4

  Is this based on anything? Could you add something like "This allows 
filesystems up to 4GiB"?

> +    if [ ${GEN} -ge 3 ]; then
> +        BLOCKS=$(expr 1081 + $BLOCKS )
> +    fi
>       set -- $@ -b $BLOCKS
>   fi
>
> -exec genext2fs $@
> +# Remove -{2,3,4} from the arguments, they are not recognised
> +# by genext2fs and we handle them manually later

  Wouldn't it be a lot simpler to pass the generation through the 
environment instead?

> +first=1
> +for o; do
> +    case "${o}" in
> +	-2|-3|-4)  ;;
> +	*)  if [ ${first} -eq 1 ]; then
> +		set --
> +		first=0
> +	    fi
> +	    set -- "$@" "${o}"
> +            ;;
> +    esac
> +done
> +
> +# Generate the base ext2 file system
> +genext2fs "$@"
> +
> +# Upgrade to ext3 if needed
> +if [ ${GEN} -ge 3 ]; then
> +    tune2fs -j -J size=1 "${IMG}" >/dev/null

  Ah, this is where the 1081 blocks come from.  There should be a comment 
pointing to that 1081 so it's easier to find this back if it is ever 
changed to a different value.

  Why does it have to be >/dev/null? We don't usually do that...

  In the script I used, I also added -c 0 (max mount count) and -i 0 
(interval between checks).  That's not for this patch of course, but I 
think it's something useful to have in general.

> +fi
> +
> +# Upgrade to ext4 if needed
> +if [ ${GEN} -ge 4 ]; then
> +    tune2fs -O extents,uninit_bg,dir_index "${IMG}" >/dev/null
> +    ret=0
> +    fsck.ext4 -pDf "${IMG}" >/dev/null || ret=$?

  This fsck is needed for ext3 as well, just to set rev0 -> rev1. Of 
course, patch 4/4 does that already.

  You should add a comment why fsck is needed.

  I would use e2fsck rather than fsck.ext4, but that's a minor thing.

> +    # Exit codes 1 & 2 are OK, it means fs errors
> +    # were successfully corrected
> +    case ${ret} in
> +	0|1|2) ;;
> +	*)   exit 1;;
> +    esac
> +    # fsck.ext4 will force a UUID, which we do not want
> +    tune2fs -U clear "${IMG}" >/dev/null

  Why don't we want a UUID? We have it for other filesystems, e.g. ubifs...


  Regards,
  Arnout

> +fi
>
Peter Korsgaard Feb. 19, 2013, 12:03 p.m. UTC | #2 
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> if BR2_TARGET_ROOTFS_EXT2
 >> 
 >> +choice
 >> +	bool "ext generation"

 Arnout>  Given the way it appears in menuconfig, I think this will be hard to
 Arnout> understand for many users. Perhaps "ext generation (ext2, ext3 or
 Arnout> ext4)".

Or simply ext variant?

 >> +	default BR2_TARGET_ROOTFS_EXT2_2

 Arnout>  Although this matches the current default, doesn't it make
 Arnout> more sense to "bump" to ext4?

Possibly. It would break existing configs though.

 >> +config BR2_TARGET_ROOTFS_EXT2_2
 >> +	bool "ext2"
 >> +
 >> +config BR2_TARGET_ROOTFS_EXT2_3
 >> +	bool "ext3"
 >> +	select BR2_PACKAGE_HOST_E2FSPROGS

 Arnout>  We don't usually select the host package. On the other hand, the
 Arnout> support for user-selectable host packages is pretty recent, so we
 Arnout> don't have a real tradition for this.

 Arnout>  Peter?

I would argue that we should keep kconfig and makefiles consistent when
possible, so when we have a user selectable host package (kconfig
symbol), we should select it.
Arnout Vandecappelle Feb. 19, 2013, 3:33 p.m. UTC | #3 
On 19/02/13 13:03, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
>
> Hi,
>
>   >> if BR2_TARGET_ROOTFS_EXT2
>   >>
>   >> +choice
>   >> +	bool "ext generation"
>
>   Arnout>  Given the way it appears in menuconfig, I think this will be hard to
>   Arnout> understand for many users. Perhaps "ext generation (ext2, ext3 or
>   Arnout> ext4)".
>
> Or simply ext variant?
>
>   >> +	default BR2_TARGET_ROOTFS_EXT2_2
>
>   Arnout>  Although this matches the current default, doesn't it make
>   Arnout> more sense to "bump" to ext4?
>
> Possibly. It would break existing configs though.

  No it doesn't. It just _changes_ existing _defconfigs_. It is similar 
to (though a bit more invasive than) bumping the default kernel version.


>
>   >> +config BR2_TARGET_ROOTFS_EXT2_2
>   >> +	bool "ext2"
>   >> +
>   >> +config BR2_TARGET_ROOTFS_EXT2_3
>   >> +	bool "ext3"
>   >> +	select BR2_PACKAGE_HOST_E2FSPROGS
>
>   Arnout>  We don't usually select the host package. On the other hand, the
>   Arnout> support for user-selectable host packages is pretty recent, so we
>   Arnout> don't have a real tradition for this.
>
>   Arnout>  Peter?
>
> I would argue that we should keep kconfig and makefiles consistent when
> possible, so when we have a user selectable host package (kconfig
> symbol), we should select it.

  OK. We probably have quite a few violations of this convention, but we 
can look out for it when changes are made.

  Regards,
  Arnout
Yann E. MORIN Feb. 19, 2013, 5:33 p.m. UTC | #4 
Arnout, All,

On Tuesday 19 February 2013 Arnout Vandecappelle wrote:
>   Great feature addition!

:-)

> On 18/02/13 00:10, Yann E. MORIN wrote:
[--SNIP--]
> >   config BR2_TARGET_ROOTFS_EXT2
> > -	bool "ext2 root filesystem"
> > +	bool "ext2/3/4 root filesystem"
> >   	help
> > -	  Build an ext2 root filesystem
> > +	  Build an ext2/3/4 root filesystem
> >
> >   if BR2_TARGET_ROOTFS_EXT2
> >
> > +choice
> > +	bool "ext generation"
> 
>   Given the way it appears in menuconfig, I think this will be hard to 
> understand for many users. Perhaps "ext generation (ext2, ext3 or ext4)".

OK, will look at it.

> > +	default BR2_TARGET_ROOTFS_EXT2_2
> 
>   Although this matches the current default, doesn't it make more sense 
> to "bump" to ext4?

No. There are stuff that does have to be ext2 (mostly because of dumb
bootloaders that can't read ext3/4).

> > +
> > +config BR2_TARGET_ROOTFS_EXT2_2
> > +	bool "ext2"
> > +
> > +config BR2_TARGET_ROOTFS_EXT2_3
> > +	bool "ext3"
> > +	select BR2_PACKAGE_HOST_E2FSPROGS
> 
>   We don't usually select the host package. On the other hand, the 
> support for user-selectable host packages is pretty recent, so we don't 
> have a real tradition for this.

OK, so only depend on e2fsprogs from the .mk, then.

[--SNIP--]
> > -ROOTFS_EXT2_DEPENDENCIES = host-genext2fs
> > +ROOTFS_EXT2_DEPENDENCIES = host-genext2fs $(if $(BR2_PACKAGE_HOST_E2FSPROGS),host-e2fsprogs)
> 
>   Although this is correct, I think it looks confusing. I prefer a more 
> explicit
> 
> ifeq ($(BR2_TARGET_ROOTFS_EXT2_3)$(BR2_TARGET_ROOTFS_EXT2_4),y)
> ROOTFS_EXT2_DEPENDENCIES += host-e2fsprogs
> endif

OK.

> >   define ROOTFS_EXT2_CMD
> > -	PATH=$(TARGET_PATH) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) $@
> > +	PATH=$(TARGET_PATH) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) -$(BR2_TARGET_ROOTFS_EXT2_GEN) $@
> 
>   Minor nit: I would prefer
> EXT2_OPTS += -$(BR2_TARGET_ROOTFS_EXT2_GEN)

Right.

> > diff --git a/fs/ext2/genext2fs.sh b/fs/ext2/genext2fs.sh
> > index 7a518ae..fcbd43c 100755
> > --- a/fs/ext2/genext2fs.sh
> > +++ b/fs/ext2/genext2fs.sh
[--SNIP--]
> > @@ -30,7 +33,48 @@ then
> >       # we scale inodes / blocks with 10% to compensate for bitmaps size + slack
> >       BLOCKS=$(du -s -c -k $TARGET_DIR | grep total | sed -e "s/total//")
> >       BLOCKS=$(expr 500 + \( $BLOCKS + $INODES / 8 \) \* 11 / 10)
> > +    # we add 1081 blocks (a bit more than 1 MiB, assuming 1KiB blocks) for
> > +    # the journal if ext3/4
> 
>   Is this based on anything? Could you add something like "This allows 
> filesystems up to 4GiB"?

I've "bisected" the number of blocks required to add a journal, and
1081 was the strict minimum. 1080 blocks, and tune2fs would yell.

This is a purely test-and-check process with two different ext2
filesystems, one ~10 MiBs, another 128MiB.

YMMV, as they use to say! ;-)

> > +    if [ ${GEN} -ge 3 ]; then
> > +        BLOCKS=$(expr 1081 + $BLOCKS )
> > +    fi
> >       set -- $@ -b $BLOCKS
> >   fi
> >
> > -exec genext2fs $@
> > +# Remove -{2,3,4} from the arguments, they are not recognised
> > +# by genext2fs and we handle them manually later
> 
>   Wouldn't it be a lot simpler to pass the generation through the 
> environment instead?

I'd like to avoid 'scanning' .config if possible.
But I'll see what to do about it.

> > +first=1
> > +for o; do
> > +    case "${o}" in
> > +	-2|-3|-4)  ;;
> > +	*)  if [ ${first} -eq 1 ]; then
> > +		set --
> > +		first=0
> > +	    fi
> > +	    set -- "$@" "${o}"
> > +            ;;
> > +    esac
> > +done
> > +
> > +# Generate the base ext2 file system
> > +genext2fs "$@"
> > +
> > +# Upgrade to ext3 if needed
> > +if [ ${GEN} -ge 3 ]; then
> > +    tune2fs -j -J size=1 "${IMG}" >/dev/null
> 
>   Ah, this is where the 1081 blocks come from.  There should be a comment 
> pointing to that 1081 so it's easier to find this back if it is ever 
> changed to a different value.

OK.

>   Why does it have to be >/dev/null? We don't usually do that...

I usually >/dev/null to only see errors, as I don't give a dime about
the output when all goes well. I'll strip it off before I resend.

>   In the script I used, I also added -c 0 (max mount count) and -i 0 
> (interval between checks).  That's not for this patch of course, but I 
> think it's something useful to have in general.

Well, I'd rather leave that for a post-image script. I think it is highly
context-specific whether you want time- or count-based checks, or not.

But that's trivial enough to add, so I'll give it a whirl.

> > +fi
> > +
> > +# Upgrade to ext4 if needed
> > +if [ ${GEN} -ge 4 ]; then
> > +    tune2fs -O extents,uninit_bg,dir_index "${IMG}" >/dev/null
> > +    ret=0
> > +    fsck.ext4 -pDf "${IMG}" >/dev/null || ret=$?
> 
>   This fsck is needed for ext3 as well, just to set rev0 -> rev1. Of 
> course, patch 4/4 does that already.

No, because *this* patch does not set rev1, and fsck is not needed
after adding a journal. It's needed only when setting rev1, or upgrading
to ext4.

>   You should add a comment why fsck is needed.

I'll do.

>   I would use e2fsck rather than fsck.ext4, but that's a minor thing.

Well, does e2fsck behave properly when called e2fsck, and not through
its symlinks mkfs.extX ?

I'll check that.

> > +    # Exit codes 1 & 2 are OK, it means fs errors
> > +    # were successfully corrected
> > +    case ${ret} in
> > +	0|1|2) ;;
> > +	*)   exit 1;;
> > +    esac
> > +    # fsck.ext4 will force a UUID, which we do not want
> > +    tune2fs -U clear "${IMG}" >/dev/null
> 
>   Why don't we want a UUID? We have it for other filesystems, e.g. ubifs...

Reproducible builds. The UUID added by e2fsck is random, so one can not
reproduce the same images.

Explicitly adding a UUID can then be done in a post-image hook, but is
context-specific.

Thanks for the review. :-)

Regards,
Yann E. MORIN.
Yann E. MORIN Feb. 19, 2013, 7:01 p.m. UTC | #5 
Arnout, All,

On Tuesday 19 February 2013 Yann E. MORIN wrote:
> On Tuesday 19 February 2013 Arnout Vandecappelle wrote:
[--SNIP--]
> > > +# Remove -{2,3,4} from the arguments, they are not recognised
> > > +# by genext2fs and we handle them manually later
> > 
> >   Wouldn't it be a lot simpler to pass the generation through the 
> > environment instead?
> 
> I'd like to avoid 'scanning' .config if possible.
> But I'll see what to do about it.

Forget it, I've seen The Light! ;-)

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/fs/ext2/Config.in b/fs/ext2/Config.in
index cb4beed..00f11a2 100644
--- a/fs/ext2/Config.in
+++ b/fs/ext2/Config.in
@@ -1,10 +1,33 @@ 
 config BR2_TARGET_ROOTFS_EXT2
-	bool "ext2 root filesystem"
+	bool "ext2/3/4 root filesystem"
 	help
-	  Build an ext2 root filesystem
+	  Build an ext2/3/4 root filesystem
 
 if BR2_TARGET_ROOTFS_EXT2
 
+choice
+	bool "ext generation"
+	default BR2_TARGET_ROOTFS_EXT2_2
+
+config BR2_TARGET_ROOTFS_EXT2_2
+	bool "ext2"
+
+config BR2_TARGET_ROOTFS_EXT2_3
+	bool "ext3"
+	select BR2_PACKAGE_HOST_E2FSPROGS
+
+config BR2_TARGET_ROOTFS_EXT2_4
+	bool "ext4"
+	select BR2_PACKAGE_HOST_E2FSPROGS
+
+endchoice
+
+config BR2_TARGET_ROOTFS_EXT2_GEN
+	int
+	default 2 if BR2_TARGET_ROOTFS_EXT2_2
+	default 3 if BR2_TARGET_ROOTFS_EXT2_3
+	default 4 if BR2_TARGET_ROOTFS_EXT2_4
+
 config BR2_TARGET_ROOTFS_EXT2_BLOCKS
 	int "size in blocks (leave at 0 for auto calculation)"
 	default 0
@@ -21,27 +44,27 @@  choice
 	prompt "Compression method"
 	default BR2_TARGET_ROOTFS_EXT2_NONE
 	help
-	  Select compressor for ext2 filesystem of the root filesystem
+	  Select compressor for ext2/3/4 filesystem of the root filesystem
 
 config BR2_TARGET_ROOTFS_EXT2_NONE
 	bool "no compression"
 	help
-	  Do not compress the ext2 filesystem.
+	  Do not compress the ext2/3/4 filesystem.
 
 config BR2_TARGET_ROOTFS_EXT2_GZIP
 	bool "gzip"
 	help
-	  Do compress the ext2 filesystem with gzip.
+	  Do compress the ext2/3/4 filesystem with gzip.
 
 config BR2_TARGET_ROOTFS_EXT2_BZIP2
 	bool "bzip2"
 	help
-	  Do compress the ext2 filesystem with bzip2.
+	  Do compress the ext2/3/4 filesystem with bzip2.
 
 config BR2_TARGET_ROOTFS_EXT2_LZMA
 	bool "lzma"
 	help
-	  Do compress the ext2 filesystem with lzma.
+	  Do compress the ext2/3/4 filesystem with lzma.
 
 endchoice
 
diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 7b71592..80ad93f 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -18,10 +18,10 @@  ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
 EXT2_OPTS += -m $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
 endif
 
-ROOTFS_EXT2_DEPENDENCIES = host-genext2fs
+ROOTFS_EXT2_DEPENDENCIES = host-genext2fs $(if $(BR2_PACKAGE_HOST_E2FSPROGS),host-e2fsprogs)
 
 define ROOTFS_EXT2_CMD
-	PATH=$(TARGET_PATH) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) $@
+	PATH=$(TARGET_PATH) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) -$(BR2_TARGET_ROOTFS_EXT2_GEN) $@
 endef
 
 $(eval $(call ROOTFS_TARGET,ext2))
diff --git a/fs/ext2/genext2fs.sh b/fs/ext2/genext2fs.sh
index 7a518ae..fcbd43c 100755
--- a/fs/ext2/genext2fs.sh
+++ b/fs/ext2/genext2fs.sh
@@ -1,19 +1,22 @@ 
 #!/bin/sh
 # genext2fs wrapper calculating needed blocks/inodes values if not specified
+set -e
 
 export LC_ALL=C
 
 CALC_BLOCKS=1
 CALC_INODES=1
 
-while getopts x:d:D:b:i:N:m:g:e:zfqUPhVv f
+while getopts x:d:D:b:i:N:m:g:e:zfqUPhVv234 f
 do
     case $f in
+	2|3|4) GEN=$f ;;
 	b) CALC_BLOCKS=0 ;;
 	N) CALC_INODES=0; INODES=$OPTARG ;;
 	d) TARGET_DIR=$OPTARG ;;
     esac
 done
+eval IMG="\"\${${OPTIND}}\""
 
 # calculate needed inodes
 if [ $CALC_INODES -eq 1 ];
@@ -30,7 +33,48 @@  then
     # we scale inodes / blocks with 10% to compensate for bitmaps size + slack
     BLOCKS=$(du -s -c -k $TARGET_DIR | grep total | sed -e "s/total//")
     BLOCKS=$(expr 500 + \( $BLOCKS + $INODES / 8 \) \* 11 / 10)
+    # we add 1081 blocks (a bit more than 1 MiB, assuming 1KiB blocks) for
+    # the journal if ext3/4
+    if [ ${GEN} -ge 3 ]; then
+        BLOCKS=$(expr 1081 + $BLOCKS )
+    fi
     set -- $@ -b $BLOCKS
 fi
 
-exec genext2fs $@
+# Remove -{2,3,4} from the arguments, they are not recognised
+# by genext2fs and we handle them manually later
+first=1
+for o; do
+    case "${o}" in
+	-2|-3|-4)  ;;
+	*)  if [ ${first} -eq 1 ]; then
+		set --
+		first=0
+	    fi
+	    set -- "$@" "${o}"
+            ;;
+    esac
+done
+
+# Generate the base ext2 file system
+genext2fs "$@"
+
+# Upgrade to ext3 if needed
+if [ ${GEN} -ge 3 ]; then
+    tune2fs -j -J size=1 "${IMG}" >/dev/null
+fi
+
+# Upgrade to ext4 if needed
+if [ ${GEN} -ge 4 ]; then
+    tune2fs -O extents,uninit_bg,dir_index "${IMG}" >/dev/null
+    ret=0
+    fsck.ext4 -pDf "${IMG}" >/dev/null || ret=$?
+    # Exit codes 1 & 2 are OK, it means fs errors
+    # were successfully corrected
+    case ${ret} in
+	0|1|2) ;;
+	*)   exit 1;;
+    esac
+    # fsck.ext4 will force a UUID, which we do not want
+    tune2fs -U clear "${IMG}" >/dev/null
+fi