diff mbox

[v5,3/5] fs/ext2: rename BR2_TARGET_ROOTFS_EXT2_BLOCKS -> BR2_TARGET_ROOTFS_EXT2_SIZE

Message ID 20170704144729.19753-4-s.martin49@gmail.com
State Superseded
Headers show

Commit Message

Samuel Martin July 4, 2017, 2:47 p.m. UTC
This change deprecates the ext2/3/4 rootfs size in blocks symbol in
favor of one that mimic the fs-size argument behavior of mkfs (i.e.
size in a human readable format accepting k, m, g or t suffix or their
upper-case variants).

This change also updates the defconfigs that used to set
BR2_TARGET_ROOTFS_EXT2_BLOCKS symbol.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v4->v5:
- new patch (Arnout)
---
 Config.in.legacy                            | 11 +++++++++++
 configs/at91sam9x5ek_mmc_dev_defconfig      |  2 +-
 configs/beaglebone_qt5_defconfig            |  2 +-
 configs/firefly_rk3288_demo_defconfig       |  2 +-
 configs/minnowboard_max-graphical_defconfig |  2 +-
 configs/nanopi_neo_defconfig                |  2 +-
 configs/pc_x86_64_bios_defconfig            |  2 +-
 configs/pc_x86_64_efi_defconfig             |  2 +-
 configs/raspberrypi3_64_defconfig           |  2 +-
 fs/ext2/Config.in                           | 13 ++++++++-----
 fs/ext2/ext2.mk                             |  8 ++++++--
 11 files changed, 33 insertions(+), 15 deletions(-)

Comments

Arnout Vandecappelle July 4, 2017, 5:28 p.m. UTC | #1
On 04-07-17 16:47, Samuel Martin wrote:
> This change deprecates the ext2/3/4 rootfs size in blocks symbol in
> favor of one that mimic the fs-size argument behavior of mkfs (i.e.
> size in a human readable format accepting k, m, g or t suffix or their
> upper-case variants).
> 
> This change also updates the defconfigs that used to set
> BR2_TARGET_ROOTFS_EXT2_BLOCKS symbol.

 So I tried this and it is in fact very annoying that it changes. Anyone who was
using ext2 will be hit by this legacy stuff...


> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v4->v5:
> - new patch (Arnout)
> ---
>  Config.in.legacy                            | 11 +++++++++++
>  configs/at91sam9x5ek_mmc_dev_defconfig      |  2 +-
>  configs/beaglebone_qt5_defconfig            |  2 +-
>  configs/firefly_rk3288_demo_defconfig       |  2 +-
>  configs/minnowboard_max-graphical_defconfig |  2 +-
>  configs/nanopi_neo_defconfig                |  2 +-
>  configs/pc_x86_64_bios_defconfig            |  2 +-
>  configs/pc_x86_64_efi_defconfig             |  2 +-
>  configs/raspberrypi3_64_defconfig           |  2 +-
>  fs/ext2/Config.in                           | 13 ++++++++-----
>  fs/ext2/ext2.mk                             |  8 ++++++--
>  11 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/Config.in.legacy b/Config.in.legacy
> index 8ea468b64a..a26869ac64 100644
> --- a/Config.in.legacy
> +++ b/Config.in.legacy
> @@ -145,6 +145,17 @@ endif
>  ###############################################################################
>  comment "Legacy options removed in 2017.08"
>  
> +config BR2_TARGET_ROOTFS_EXT2_BLOCKS
> +	int "exact size in blocks has been removed"
> +	default 0
> +	help
> +	  This option has been removed in favor of BR2_TARGET_ROOTFS_EXT2_SIZE.

 Add:
	  It has been set automatically to the value you had before.

> +
> +config BR2_TARGET_ROOTFS_EXT2_BLOCKS_WRAP
> +	bool
> +	default y if BR2_TARGET_ROOTFS_EXT2_BLOCKS != 0

 So this will hit legacy handling for anyone using the default value in 2017.05,
because there the default is 61440. And if we change it into != 61440, then it
will hit anyone who upgrades from <2017.05. And if we add || != 61440, it will
still hit anyone who changed the default.

 So at the very least we should add || != 61440 so anyone who was using the
default before will not get hit by legacy.

 Ideally we don't change the symbol name but then we can't change it from int to
string. So I guess we'll have to do this after all :-(



> +	select BR2_LEGACY
> +

 Add a comment that fs/ext2/Config.in refers this symbol.

>  config BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES
>  	int "ext2 extra inodes has been removed" if BR2_TARGET_ROOTFS_EXT2_INODES = 0
>  	default 0
> diff --git a/configs/at91sam9x5ek_mmc_dev_defconfig b/configs/at91sam9x5ek_mmc_dev_defconfig
> index 0060111fca..1bf2983ee7 100644
> --- a/configs/at91sam9x5ek_mmc_dev_defconfig
> +++ b/configs/at91sam9x5ek_mmc_dev_defconfig
> @@ -81,7 +81,7 @@ BR2_PACKAGE_VIM=y
>  # Filesystem
>  BR2_TARGET_ROOTFS_EXT2=y
>  BR2_TARGET_ROOTFS_EXT2_4=y
> -BR2_TARGET_ROOTFS_EXT2_BLOCKS=120000
> +BR2_TARGET_ROOTFS_EXT2_SIZE="120000K"

 While you're at it, make it 120M instead. Not exactly the same but it's larger
so should be OK.

[snip]
> diff --git a/fs/ext2/Config.in b/fs/ext2/Config.in
> index 9c58ac62ed..b2363940f0 100644
> --- a/fs/ext2/Config.in
> +++ b/fs/ext2/Config.in
> @@ -44,12 +44,15 @@ config BR2_TARGET_ROOTFS_EXT2_REV
>  config BR2_TARGET_ROOTFS_EXT2_LABEL
>  	string "filesystem label"
>  
> -# 61440 = 60MB, i.e usually small enough to fit on a 64MB media
> -config BR2_TARGET_ROOTFS_EXT2_BLOCKS
> -	int "exact size in blocks"
> -	default 61440
> +config BR2_TARGET_ROOTFS_EXT2_SIZE
> +	string "exact size"
> +	default BR2_TARGET_ROOTFS_EXT2_BLOCKS if BR2_TARGET_ROOTFS_EXT2_BLOCKS != 0 # legacy 2017.08

 Make it instead "if BR2_TARGET_ROOTFS_EXT2_BLOCKS_WRAP" - simpler.

> +	default "61440K" # default size

 Use "60M" instead.

>  	help
> -	  Specify the file system size as a number of 1024-byte blocks.
> +	  The size of the filesystem image. If it does not have a suffix, it is
> +	  interpreted as power-of-two kilobytes. If it is suffixed by 'k', 'm',
> +	  'g', 't' (either upper-case or lower-case), then it is interpreted in
> +	  power-of-two kilobytes, megabytes, gigabytes, terabytes, etc.
>  
>  config BR2_TARGET_ROOTFS_EXT2_INODES
>  	int "exact number of inodes (leave at 0 for auto calculation)"
> diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
> index bff442ff18..c8da9c4451 100644
> --- a/fs/ext2/ext2.mk
> +++ b/fs/ext2/ext2.mk
> @@ -4,6 +4,11 @@
>  #
>  ################################################################################
>  
> +EXT2_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_SIZE))
> +ifeq ($(BR2_TARGET_ROOTFS_EXT)-$(EXT2_SIZE),y-)
                                ^2

> +$(error BR2_TARGET_ROOTFS_EXT2_SIZE cannot be empty)

 I wonder if it makes sense to check this explicitly. There are thousands of
ways it can be wrong, empty is just one of them. If it is empty, mke2fs will
complain that the argument is missing.

> +endif
> +
>  # qstrip results in stripping consecutive spaces into a single one. So the
>  # variable is not qstrip-ed to preserve the integrity of the string value.
>  EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
> @@ -20,8 +25,7 @@ ROOTFS_EXT2_DEPENDENCIES = host-e2fsprogs
>  
>  define ROOTFS_EXT2_CMD
>  	rm -f $@
> -	PATH=$(BR_PATH) mkfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN) $(EXT2_OPTS) $@ \
> -		 $(BR2_TARGET_ROOTFS_EXT2_BLOCKS)
> +	PATH=$(BR_PATH) mkfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN) $(EXT2_OPTS) $@ $(EXT2_SIZE)

 Put quotes around EXT2_SIZE - a user may have added spaces there (which will
never work, but better let mke2fs give the appropriate error message).

 Regards,
 Arnout

>  endef
>  
>  rootfs-ext2-symlink:
>
Peter Korsgaard July 4, 2017, 11:39 p.m. UTC | #2
>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:

 > This change deprecates the ext2/3/4 rootfs size in blocks symbol in
 > favor of one that mimic the fs-size argument behavior of mkfs (i.e.
 > size in a human readable format accepting k, m, g or t suffix or their
 > upper-case variants).

 > +++ b/fs/ext2/ext2.mk
 > @@ -4,6 +4,11 @@
 >  #
 >  ################################################################################
 
 > +EXT2_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_SIZE))
 > +ifeq ($(BR2_TARGET_ROOTFS_EXT)-$(EXT2_SIZE),y-)
 > +$(error BR2_TARGET_ROOTFS_EXT2_SIZE cannot be empty)
 > +endif

Hmm, shouldn't that be BR2_TARGET_ROOTFS_EXT2 instead of
BR2_TARGET_ROOTFS_EXT?
diff mbox

Patch

diff --git a/Config.in.legacy b/Config.in.legacy
index 8ea468b64a..a26869ac64 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -145,6 +145,17 @@  endif
 ###############################################################################
 comment "Legacy options removed in 2017.08"
 
+config BR2_TARGET_ROOTFS_EXT2_BLOCKS
+	int "exact size in blocks has been removed"
+	default 0
+	help
+	  This option has been removed in favor of BR2_TARGET_ROOTFS_EXT2_SIZE.
+
+config BR2_TARGET_ROOTFS_EXT2_BLOCKS_WRAP
+	bool
+	default y if BR2_TARGET_ROOTFS_EXT2_BLOCKS != 0
+	select BR2_LEGACY
+
 config BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES
 	int "ext2 extra inodes has been removed" if BR2_TARGET_ROOTFS_EXT2_INODES = 0
 	default 0
diff --git a/configs/at91sam9x5ek_mmc_dev_defconfig b/configs/at91sam9x5ek_mmc_dev_defconfig
index 0060111fca..1bf2983ee7 100644
--- a/configs/at91sam9x5ek_mmc_dev_defconfig
+++ b/configs/at91sam9x5ek_mmc_dev_defconfig
@@ -81,7 +81,7 @@  BR2_PACKAGE_VIM=y
 # Filesystem
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=120000
+BR2_TARGET_ROOTFS_EXT2_SIZE="120000K"
 
 # Bootloaders
 BR2_TARGET_AT91BOOTSTRAP3=y
diff --git a/configs/beaglebone_qt5_defconfig b/configs/beaglebone_qt5_defconfig
index 34063739a2..fd408f5d09 100644
--- a/configs/beaglebone_qt5_defconfig
+++ b/configs/beaglebone_qt5_defconfig
@@ -25,7 +25,7 @@  BR2_PACKAGE_TI_SGX_KM=y
 BR2_PACKAGE_TI_SGX_UM=y
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=126976
+BR2_TARGET_ROOTFS_EXT2_SIZE="126976K"
 BR2_TARGET_UBOOT=y
 BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
 BR2_TARGET_UBOOT_CUSTOM_VERSION=y
diff --git a/configs/firefly_rk3288_demo_defconfig b/configs/firefly_rk3288_demo_defconfig
index 0f4dd368c4..cd06aa784f 100644
--- a/configs/firefly_rk3288_demo_defconfig
+++ b/configs/firefly_rk3288_demo_defconfig
@@ -25,7 +25,7 @@  BR2_PACKAGE_QT5BASE_EGLFS=y
 BR2_PACKAGE_MALI_T76X=y
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=250000
+BR2_TARGET_ROOTFS_EXT2_SIZE="250000K"
 # BR2_TARGET_ROOTFS_TAR is not set
 BR2_TARGET_UBOOT=y
 BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
diff --git a/configs/minnowboard_max-graphical_defconfig b/configs/minnowboard_max-graphical_defconfig
index 1a2cd33be2..855d257bc8 100644
--- a/configs/minnowboard_max-graphical_defconfig
+++ b/configs/minnowboard_max-graphical_defconfig
@@ -68,6 +68,6 @@  BR2_PACKAGE_STARTUP_NOTIFICATION=y
 # Filesystem image
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=120000
+BR2_TARGET_ROOTFS_EXT2_SIZE="120000K"
 # BR2_TARGET_ROOTFS_TAR is not set
 
diff --git a/configs/nanopi_neo_defconfig b/configs/nanopi_neo_defconfig
index a22705269d..5fd08328f1 100644
--- a/configs/nanopi_neo_defconfig
+++ b/configs/nanopi_neo_defconfig
@@ -31,7 +31,7 @@  BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot-sunxi-with-spl.bin"
 # Build an sdcard image
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=32768
+BR2_TARGET_ROOTFS_EXT2_SIZE="32768K"
 BR2_TARGET_ROOTFS_EXT2_INODES=8192
 # BR2_TARGET_ROOTFS_TAR is not set
 BR2_PACKAGE_HOST_DOSFSTOOLS=y
diff --git a/configs/pc_x86_64_bios_defconfig b/configs/pc_x86_64_bios_defconfig
index d64f09a1dc..b68e49919a 100644
--- a/configs/pc_x86_64_bios_defconfig
+++ b/configs/pc_x86_64_bios_defconfig
@@ -17,7 +17,7 @@  BR2_TARGET_GRUB2=y
 # Filesystem / image
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=120000
+BR2_TARGET_ROOTFS_EXT2_SIZE="120000K"
 # BR2_TARGET_ROOTFS_TAR is not set
 BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh"
 
diff --git a/configs/pc_x86_64_efi_defconfig b/configs/pc_x86_64_efi_defconfig
index ea2abc39be..400dcffa64 100644
--- a/configs/pc_x86_64_efi_defconfig
+++ b/configs/pc_x86_64_efi_defconfig
@@ -20,7 +20,7 @@  BR2_TARGET_GRUB2_X86_64_EFI=y
 # Filesystem / image
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=120000
+BR2_TARGET_ROOTFS_EXT2_SIZE="120000K"
 # BR2_TARGET_ROOTFS_TAR is not set
 BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh"
 
diff --git a/configs/raspberrypi3_64_defconfig b/configs/raspberrypi3_64_defconfig
index 13a505b031..0ed9f6f708 100644
--- a/configs/raspberrypi3_64_defconfig
+++ b/configs/raspberrypi3_64_defconfig
@@ -30,7 +30,7 @@  BR2_PACKAGE_HOST_MTOOLS=y
 # Filesystem / image
 BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_BLOCKS=120000
+BR2_TARGET_ROOTFS_EXT2_SIZE="120000K"
 # BR2_TARGET_ROOTFS_TAR is not set
 BR2_ROOTFS_POST_BUILD_SCRIPT="board/raspberrypi3-64/post-build.sh"
 BR2_ROOTFS_POST_IMAGE_SCRIPT="board/raspberrypi3-64/post-image.sh"
diff --git a/fs/ext2/Config.in b/fs/ext2/Config.in
index 9c58ac62ed..b2363940f0 100644
--- a/fs/ext2/Config.in
+++ b/fs/ext2/Config.in
@@ -44,12 +44,15 @@  config BR2_TARGET_ROOTFS_EXT2_REV
 config BR2_TARGET_ROOTFS_EXT2_LABEL
 	string "filesystem label"
 
-# 61440 = 60MB, i.e usually small enough to fit on a 64MB media
-config BR2_TARGET_ROOTFS_EXT2_BLOCKS
-	int "exact size in blocks"
-	default 61440
+config BR2_TARGET_ROOTFS_EXT2_SIZE
+	string "exact size"
+	default BR2_TARGET_ROOTFS_EXT2_BLOCKS if BR2_TARGET_ROOTFS_EXT2_BLOCKS != 0 # legacy 2017.08
+	default "61440K" # default size
 	help
-	  Specify the file system size as a number of 1024-byte blocks.
+	  The size of the filesystem image. If it does not have a suffix, it is
+	  interpreted as power-of-two kilobytes. If it is suffixed by 'k', 'm',
+	  'g', 't' (either upper-case or lower-case), then it is interpreted in
+	  power-of-two kilobytes, megabytes, gigabytes, terabytes, etc.
 
 config BR2_TARGET_ROOTFS_EXT2_INODES
 	int "exact number of inodes (leave at 0 for auto calculation)"
diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index bff442ff18..c8da9c4451 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -4,6 +4,11 @@ 
 #
 ################################################################################
 
+EXT2_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_SIZE))
+ifeq ($(BR2_TARGET_ROOTFS_EXT)-$(EXT2_SIZE),y-)
+$(error BR2_TARGET_ROOTFS_EXT2_SIZE cannot be empty)
+endif
+
 # qstrip results in stripping consecutive spaces into a single one. So the
 # variable is not qstrip-ed to preserve the integrity of the string value.
 EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
@@ -20,8 +25,7 @@  ROOTFS_EXT2_DEPENDENCIES = host-e2fsprogs
 
 define ROOTFS_EXT2_CMD
 	rm -f $@
-	PATH=$(BR_PATH) mkfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN) $(EXT2_OPTS) $@ \
-		 $(BR2_TARGET_ROOTFS_EXT2_BLOCKS)
+	PATH=$(BR_PATH) mkfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN) $(EXT2_OPTS) $@ $(EXT2_SIZE)
 endef
 
 rootfs-ext2-symlink: