* [Buildroot] [PATCH 1/2] Adding btrfs-progs host package. @ 2018-08-21 16:04 Robert J. Heywood 2018-08-21 16:04 ` [Buildroot] [PATCH 2/2] Adding btrfs rootfs option Robert J. Heywood 2018-08-21 21:42 ` [Buildroot] [PATCH 1/2] Adding btrfs-progs host package Thomas Petazzoni 0 siblings, 2 replies; 7+ messages in thread From: Robert J. Heywood @ 2018-08-21 16:04 UTC (permalink / raw) To: buildroot Adding a new option to build the btrfs toolset for the host. Signed-off-by: Robert J. Heywood <robert.heywood@codethink.co.uk> --- package/Config.in.host | 1 + package/btrfs-progs/Config.in.host | 6 ++++++ package/btrfs-progs/btrfs-progs.mk | 4 ++++ 3 files changed, 11 insertions(+) create mode 100644 package/btrfs-progs/Config.in.host diff --git a/package/Config.in.host b/package/Config.in.host index 7838ffc219..432597ed31 100644 --- a/package/Config.in.host +++ b/package/Config.in.host @@ -2,6 +2,7 @@ menu "Host utilities" source "package/aespipe/Config.in.host" source "package/android-tools/Config.in.host" + source "package/btrfs-progs/Config.in.host" source "package/cargo/Config.in.host" source "package/cbootimage/Config.in.host" source "package/checkpolicy/Config.in.host" diff --git a/package/btrfs-progs/Config.in.host b/package/btrfs-progs/Config.in.host new file mode 100644 index 0000000000..cdcd2e4ec4 --- /dev/null +++ b/package/btrfs-progs/Config.in.host @@ -0,0 +1,6 @@ +config BR2_PACKAGE_HOST_BTRFS_PROGS + bool "host btrfs-progs" + help + Btrfs filesystem utilities + + https://btrfs.wiki.kernel.org/index.php/Main_Page diff --git a/package/btrfs-progs/btrfs-progs.mk b/package/btrfs-progs/btrfs-progs.mk index b4cc839352..3129ecc714 100644 --- a/package/btrfs-progs/btrfs-progs.mk +++ b/package/btrfs-progs/btrfs-progs.mk @@ -21,4 +21,8 @@ BTRFS_PROGS_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) install-static BTRFS_PROGS_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) install-static endif +HOST_BTRFS_PROGS_DEPENDENCIES = host-e2fsprogs host-lzo host-zlib +HOST_BTRFS_PROGS_CONF_OPTS = --disable-backtrace --disable-zstd --disable-python + $(eval $(autotools-package)) +$(eval $(host-autotools-package)) -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/2] Adding btrfs rootfs option. 2018-08-21 16:04 [Buildroot] [PATCH 1/2] Adding btrfs-progs host package Robert J. Heywood @ 2018-08-21 16:04 ` Robert J. Heywood 2018-08-21 20:27 ` Yann E. MORIN 2018-08-21 21:42 ` [Buildroot] [PATCH 1/2] Adding btrfs-progs host package Thomas Petazzoni 1 sibling, 1 reply; 7+ messages in thread From: Robert J. Heywood @ 2018-08-21 16:04 UTC (permalink / raw) To: buildroot This patch makes it possible to format the rootfs using btrfs. Introduces the option; BR2_TARGET_ROOTFS_BTRFS When selected, the user is able to specify the filesystem size, label, options, and node and sector sizes. The new files are based on fs/ext2/{Config.in,ext2.mk} Signed-off-by: Robert J. Heywood <robert.heywood@codethink.co.uk> --- fs/Config.in | 1 + fs/btrfs/Config.in | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ fs/btrfs/btrfs.mk | 45 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 fs/btrfs/Config.in create mode 100644 fs/btrfs/btrfs.mk diff --git a/fs/Config.in b/fs/Config.in index c25b01c3de..24f22fd7e3 100644 --- a/fs/Config.in +++ b/fs/Config.in @@ -1,6 +1,7 @@ menu "Filesystem images" source "fs/axfs/Config.in" +source "fs/btrfs/Config.in" source "fs/cloop/Config.in" source "fs/cpio/Config.in" source "fs/cramfs/Config.in" diff --git a/fs/btrfs/Config.in b/fs/btrfs/Config.in new file mode 100644 index 0000000000..cd563235ec --- /dev/null +++ b/fs/btrfs/Config.in @@ -0,0 +1,48 @@ +config BR2_TARGET_ROOTFS_BTRFS + bool "btrfs root filesystem" + select BR2_PACKAGE_HOST_BTRFS_PROGS + help + Build a btrfs root filesystem. If you enable this option, you + probably want to enable the btrfs-progs package too. + + +if BR2_TARGET_ROOTFS_BTRFS + +config BR2_TARGET_ROOTFS_BTRFS_LABEL + string "filesystem label" + +config BR2_TARGET_ROOTFS_BTRFS_SIZE + string "filesystem size" + default "100M" + help + 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_BTRFS_SIZE_NODE + string "btree node size" + help + The tree block size in which btrfs stores metadata. The default + value is 16KiB (16384) or the page size, whichever is bigger. + Must be a multiple of the sectorsize, but not larger than 64KiB + (65536). + +config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR + string "sector size" + help + The default value is the page size and is autodetected. If the + sectorsize differs from the page size, the created filesystem + may not be mountable by the kernel. Therefore it is not + recommended to use this option unless you are going to mount it + on a system with the appropriate page size. + +config BR2_TARGET_ROOTFS_BTRFS_FEATURES + string "Filesystem Features" + help + A comma separated string of features that can be enabled + during creation time. + For a list of available options, use; `mkfs.btrfs -O list-all` + +endif # BR2_TARGET_ROOTFS_BTRFS diff --git a/fs/btrfs/btrfs.mk b/fs/btrfs/btrfs.mk new file mode 100644 index 0000000000..9f474edac3 --- /dev/null +++ b/fs/btrfs/btrfs.mk @@ -0,0 +1,45 @@ +################################################################################ +# +# Build the btrfs root filesystem image +# +################################################################################ + +BTRFS_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE)) +ifeq ($(BR2_TARGET_ROOTFS_BTRFS)-$(BTRFS_SIZE),y-) +$(error BR2_TARGET_ROOTFS_BTRFS_SIZE cannot be empty) +endif + +BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE)) +BTRFS_SIZE_NODE_FLAG = $(if $(BTRFS_SIZE_NODE),--nodesize "$(BTRFS_SIZE_NODE)",) + +BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR)) +BTRFS_SIZE_SECTOR_FLAG = $(if $(BTRFS_SIZE_SECTOR),--sectorsize "$(BTRFS_SIZE_SECTOR)",) + +BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES)) +BTRFS_FEATURES_FLAG = $(if $(BTRFS_FEATURES),--features "$(BTRFS_FEATURES)",) + +# 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. +BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL)) +## ") + +BTRFS_OPTS = \ + -r $(TARGET_DIR) \ + -L "$(BTRFS_LABEL)" \ + $(BTRFS_SIZE_NODE_FLAG) \ + $(BTRFS_SIZE_SECTOR_FLAG) \ + $(BTRFS_FEATURES_FLAG) + +ROOTFS_BTRFS_DEPENDENCIES = host-btrfs-progs + +define ROOTFS_BTRFS_CMD + rm -f $@ + fallocate -l $(BTRFS_SIZE) $@ + $(HOST_DIR)/bin/mkfs.btrfs $(BTRFS_OPTS) $@ \ + || { ret=$$?; \ + echo "*** Maybe you need to increase the filesystem size (BR2_TARGET_ROOTFS_BTRFS_SIZE)" 1>&2; \ + exit $$ret; \ + } +endef + +$(eval $(rootfs)) -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/2] Adding btrfs rootfs option. 2018-08-21 16:04 ` [Buildroot] [PATCH 2/2] Adding btrfs rootfs option Robert J. Heywood @ 2018-08-21 20:27 ` Yann E. MORIN 2018-08-22 13:10 ` Robert Heywood 0 siblings, 1 reply; 7+ messages in thread From: Yann E. MORIN @ 2018-08-21 20:27 UTC (permalink / raw) To: buildroot Robert, All, On 2018-08-21 17:04 +0100, Robert J. Heywood spake thusly: > This patch makes it possible to format the rootfs using btrfs. > Introduces the option; BR2_TARGET_ROOTFS_BTRFS > > When selected, the user is able to specify the filesystem size, > label, options, and node and sector sizes. > The new files are based on fs/ext2/{Config.in,ext2.mk} Nice addition! :-) > Signed-off-by: Robert J. Heywood <robert.heywood@codethink.co.uk> [--SNIP--] > +config BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE > + string "btree node size" > + help > + The tree block size in which btrfs stores metadata. The default > + value is 16KiB (16384) or the page size, whichever is bigger. How is the page size detected? I'd like to be sure this is detecting the page size of the target, not that of the build machine. Also, is it possible to indicate the size with a 'k' like for the fs size, or must it be only bytes? > + Must be a multiple of the sectorsize, but not larger than 64KiB > + (65536). > + > +config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR > + string "sector size" > + help > + The default value is the page size and is autodetected. If the > + sectorsize differs from the page size, the created filesystem > + may not be mountable by the kernel. Therefore it is not > + recommended to use this option unless you are going to mount it > + on a system with the appropriate page size. Given that the filsystem is specifically made for the target that will mount it, does it make sense to ffer that option at all? I.e. if the target uses a 4KiB page size, does it make sense to generatea rootfs that has a different secotr size, and thus not mountable on the target at all? My suggestion: don't add that option. > +config BR2_TARGET_ROOTFS_BTRFS_FEATURES > + string "Filesystem Features" > + help > + A comma separated string of features that can be enabled > + during creation time. > + For a list of available options, use; `mkfs.btrfs -O list-all` Nice. I guess that has to be done using the host-btrfs we built, as its feature list may be different than the one from the build machine. > +endif # BR2_TARGET_ROOTFS_BTRFS > diff --git a/fs/btrfs/btrfs.mk b/fs/btrfs/btrfs.mk > new file mode 100644 > index 0000000000..9f474edac3 > --- /dev/null > +++ b/fs/btrfs/btrfs.mk > @@ -0,0 +1,45 @@ > +################################################################################ > +# > +# Build the btrfs root filesystem image > +# > +################################################################################ > + > +BTRFS_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE)) > +ifeq ($(BR2_TARGET_ROOTFS_BTRFS)-$(BTRFS_SIZE),y-) > +$(error BR2_TARGET_ROOTFS_BTRFS_SIZE cannot be empty) > +endif > + > +BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE)) > +BTRFS_SIZE_NODE_FLAG = $(if $(BTRFS_SIZE_NODE),--nodesize "$(BTRFS_SIZE_NODE)",) > > +BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR)) > +BTRFS_SIZE_SECTOR_FLAG = $(if $(BTRFS_SIZE_SECTOR),--sectorsize "$(BTRFS_SIZE_SECTOR)",) > + > +BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES)) > +BTRFS_FEATURES_FLAG = $(if $(BTRFS_FEATURES),--features "$(BTRFS_FEATURES)",) > + > +# 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. > +BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL)) > +## ") Why double comment here? > +BTRFS_OPTS = \ > + -r $(TARGET_DIR) \ > + -L "$(BTRFS_LABEL)" \ > + $(BTRFS_SIZE_NODE_FLAG) \ > + $(BTRFS_SIZE_SECTOR_FLAG) \ > + $(BTRFS_FEATURES_FLAG) It is a bit annoying that you have to qstrip the variables, just to re-add double quotes right after. However, I prefer the way you do it, because then it is obvious the strings are quotted when passed to the shell. Furhtermore, I would quote with single quotes, to avoid shell expansion. Also, no need for the trailing comma when there is no 'else' clause. No need for intermediate variables for each option; just directly set the BTRFS_OPTS variable. Finally, the label case is a bit annyoing, but here is no better solution, and is exactly what we do in ext2.mk. BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE)) BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR)) BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES)) # 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. BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL)) # ") BTRFS_OPTS = \ -r '$(TARGET_DIR)' \ -L '$(BTRFS_LABEL)' \ $(if $(BTRFS_SIZE_NODE),--nodesize '$(BTRFS_SIZE_NODE)') \ $(if $(BTRFS_SIZE_SECTOR),--sectorsize '$(BTRFS_SIZE_SECTOR)') \ $(if $(BTRFS_FEATURES),--features '$(BTRFS_FEATURES)') > + > +ROOTFS_BTRFS_DEPENDENCIES = host-btrfs-progs > + > +define ROOTFS_BTRFS_CMD > + rm -f $@ > + fallocate -l $(BTRFS_SIZE) $@ Is fallocate really necessary here? If the host's filesystem is too small to accomodate the image, then the mkfs.btrfs below fill fail; we don't need fallocate to fail. Besides, is fallocate readilly available on all distros? Usually, we use truncate(1) to that effect; see for example: support/testing/tests/fs/test_ubi.py at 29 Regards, Yann E. MORIN. > + $(HOST_DIR)/bin/mkfs.btrfs $(BTRFS_OPTS) $@ \ > + || { ret=$$?; \ > + echo "*** Maybe you need to increase the filesystem size (BR2_TARGET_ROOTFS_BTRFS_SIZE)" 1>&2; \ > + exit $$ret; \ > + } > +endef > + > +$(eval $(rootfs)) > -- > 2.11.0 > > _______________________________________________ > buildroot mailing list > buildroot at busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/2] Adding btrfs rootfs option. 2018-08-21 20:27 ` Yann E. MORIN @ 2018-08-22 13:10 ` Robert Heywood 2018-08-22 16:57 ` Yann E. MORIN 0 siblings, 1 reply; 7+ messages in thread From: Robert Heywood @ 2018-08-22 13:10 UTC (permalink / raw) To: buildroot Hi Yann, thanks for the through review :) Comments inline. On 21/08/18 21:27, Yann E. MORIN wrote: > Robert, All, > > On 2018-08-21 17:04 +0100, Robert J. Heywood spake thusly: >> This patch makes it possible to format the rootfs using btrfs. >> Introduces the option; BR2_TARGET_ROOTFS_BTRFS >> >> When selected, the user is able to specify the filesystem size, >> label, options, and node and sector sizes. >> The new files are based on fs/ext2/{Config.in,ext2.mk} > > Nice addition! :-) > >> Signed-off-by: Robert J. Heywood <robert.heywood@codethink.co.uk> > [--SNIP--] >> +config BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE >> + string "btree node size" >> + help >> + The tree block size in which btrfs stores metadata. The default >> + value is 16KiB (16384) or the page size, whichever is bigger. > > How is the page size detected? I'd like to be sure this is detecting the > page size of the target, not that of the build machine. > > Also, is it possible to indicate the size with a 'k' like for the fs > size, or must it be only bytes? > Good question. I guess because the makefile creates a sparse file with fallocate which mkfs.btrfs then formats, it won't be able to detect any page size (host or otherwise) and so will always be set to a default value of 16384. I'll set that in the Config file explicitly. It's possible to specify the size in kilobytes, ie 4k for 4096. I can change the help text to reflect this. >> + Must be a multiple of the sectorsize, but not larger than 64KiB >> + (65536). >> + >> +config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR >> + string "sector size" >> + help >> + The default value is the page size and is autodetected. If the >> + sectorsize differs from the page size, the created filesystem >> + may not be mountable by the kernel. Therefore it is not >> + recommended to use this option unless you are going to mount it >> + on a system with the appropriate page size. > > Given that the filsystem is specifically made for the target that will > mount it, does it make sense to ffer that option at all? I.e. if the > target uses a 4KiB page size, does it make sense to generatea rootfs > that has a different secotr size, and thus not mountable on the target > at all? > > My suggestion: don't add that option. > On my system (building for a raspi0) it autodetects the sector size to be 4k, and the resulting image is bootable. However I'm worried the host won't be able to detect the correct value for the target. In such a case I can see why it would be useful to set the sector size explicitly. Is there another place where the sector size is specified? If so, it might be better to use the existing value instead. >> +config BR2_TARGET_ROOTFS_BTRFS_FEATURES >> + string "Filesystem Features" >> + help >> + A comma separated string of features that can be enabled >> + during creation time. >> + For a list of available options, use; `mkfs.btrfs -O list-all` > > Nice. I guess that has to be done using the host-btrfs we built, as its > feature list may be different than the one from the build machine. > I can change the help text to include the full path to the binary. >> +endif # BR2_TARGET_ROOTFS_BTRFS >> diff --git a/fs/btrfs/btrfs.mk b/fs/btrfs/btrfs.mk >> new file mode 100644 >> index 0000000000..9f474edac3 >> --- /dev/null >> +++ b/fs/btrfs/btrfs.mk >> @@ -0,0 +1,45 @@ >> +################################################################################ >> +# >> +# Build the btrfs root filesystem image >> +# >> +################################################################################ >> + >> +BTRFS_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE)) >> +ifeq ($(BR2_TARGET_ROOTFS_BTRFS)-$(BTRFS_SIZE),y-) >> +$(error BR2_TARGET_ROOTFS_BTRFS_SIZE cannot be empty) >> +endif >> + >> +BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE)) >> +BTRFS_SIZE_NODE_FLAG = $(if $(BTRFS_SIZE_NODE),--nodesize "$(BTRFS_SIZE_NODE)",) >> >> +BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR)) >> +BTRFS_SIZE_SECTOR_FLAG = $(if $(BTRFS_SIZE_SECTOR),--sectorsize "$(BTRFS_SIZE_SECTOR)",) >> + >> +BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES)) >> +BTRFS_FEATURES_FLAG = $(if $(BTRFS_FEATURES),--features "$(BTRFS_FEATURES)",) >> + >> +# 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. >> +BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL)) >> +## ") > > Why double comment here? > There is no reason for that. It was added when I first copied the ext2.mk file and commented out everything. I'll remove one of them. >> +BTRFS_OPTS = \ >> + -r $(TARGET_DIR) \ >> + -L "$(BTRFS_LABEL)" \ >> + $(BTRFS_SIZE_NODE_FLAG) \ >> + $(BTRFS_SIZE_SECTOR_FLAG) \ >> + $(BTRFS_FEATURES_FLAG) > > It is a bit annoying that you have to qstrip the variables, just to > re-add double quotes right after. However, I prefer the way you do it, > because then it is obvious the strings are quotted when passed to the > shell. > > Furhtermore, I would quote with single quotes, to avoid shell expansion. Sure thing. > > Also, no need for the trailing comma when there is no 'else' clause. I didn't know that! :) > > No need for intermediate variables for each option; just directly set > the BTRFS_OPTS variable. > > Finally, the label case is a bit annyoing, but here is no better > solution, and is exactly what we do in ext2.mk. > > BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE)) > BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR)) > BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES)) > # 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. > BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL)) > # ") > > BTRFS_OPTS = \ > -r '$(TARGET_DIR)' \ > -L '$(BTRFS_LABEL)' \ > $(if $(BTRFS_SIZE_NODE),--nodesize '$(BTRFS_SIZE_NODE)') \ > $(if $(BTRFS_SIZE_SECTOR),--sectorsize '$(BTRFS_SIZE_SECTOR)') \ > $(if $(BTRFS_FEATURES),--features '$(BTRFS_FEATURES)') > Good suggestions. >> + >> +ROOTFS_BTRFS_DEPENDENCIES = host-btrfs-progs >> + >> +define ROOTFS_BTRFS_CMD >> + rm -f $@ >> + fallocate -l $(BTRFS_SIZE) $@ > > Is fallocate really necessary here? If the host's filesystem is too > small to accomodate the image, then the mkfs.btrfs below fill fail; we > don't need fallocate to fail. > > Besides, is fallocate readilly available on all distros? > > Usually, we use truncate(1) to that effect; see for example: > support/testing/tests/fs/test_ubi.py at 29 > fallocate is used here because (unlike other mkfs programs) you can't specify a size and have mkfs.btrfs create an image file for you. It needs a sparse file to work with. I'll be happy to change this to truncate, as per your example. > Regards, > Yann E. MORIN. > >> + $(HOST_DIR)/bin/mkfs.btrfs $(BTRFS_OPTS) $@ \ >> + || { ret=$$?; \ >> + echo "*** Maybe you need to increase the filesystem size (BR2_TARGET_ROOTFS_BTRFS_SIZE)" 1>&2; \ >> + exit $$ret; \ >> + } >> +endef >> + >> +$(eval $(rootfs)) >> -- >> 2.11.0 >> regards, rob ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/2] Adding btrfs rootfs option. 2018-08-22 13:10 ` Robert Heywood @ 2018-08-22 16:57 ` Yann E. MORIN 2018-08-22 17:21 ` Yann E. MORIN 0 siblings, 1 reply; 7+ messages in thread From: Yann E. MORIN @ 2018-08-22 16:57 UTC (permalink / raw) To: buildroot Robert, All, On 2018-08-22 14:10 +0100, Robert Heywood spake thusly: > On 21/08/18 21:27, Yann E. MORIN wrote: > >On 2018-08-21 17:04 +0100, Robert J. Heywood spake thusly: > >>This patch makes it possible to format the rootfs using btrfs. > >>Introduces the option; BR2_TARGET_ROOTFS_BTRFS > >> > >>When selected, the user is able to specify the filesystem size, > >>label, options, and node and sector sizes. > >>The new files are based on fs/ext2/{Config.in,ext2.mk} > > > >Nice addition! :-) > > > >>Signed-off-by: Robert J. Heywood <robert.heywood@codethink.co.uk> > >[--SNIP--] > >>+config BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE > >>+ string "btree node size" > >>+ help > >>+ The tree block size in which btrfs stores metadata. The default > >>+ value is 16KiB (16384) or the page size, whichever is bigger. > > > >How is the page size detected? I'd like to be sure this is detecting the > >page size of the target, not that of the build machine. > Good question. > I guess because the makefile creates a sparse file with fallocate which > mkfs.btrfs then formats, it won't be able to detect any page size (host or > otherwise) and so will always be set to a default value of 16384. > I'll set that in the Config file explicitly. So, the mkfs.btrfs is a host tool; it has no knowledge of the target at all, so it can't possibly know the target page size. I believe the node size should default to 16384, so that we get an explicit sane default. Users who would change that value would know what they would be doing. > >>+config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR > >>+ string "sector size" > >>+ help > >>+ The default value is the page size and is autodetected. If the > >>+ sectorsize differs from the page size, the created filesystem > >>+ may not be mountable by the kernel. Therefore it is not > >>+ recommended to use this option unless you are going to mount it > >>+ on a system with the appropriate page size. > > > >Given that the filsystem is specifically made for the target that will > >mount it, does it make sense to ffer that option at all? I.e. if the > >target uses a 4KiB page size, does it make sense to generatea rootfs > >that has a different secotr size, and thus not mountable on the target > >at all? > > > >My suggestion: don't add that option. > > > > On my system (building for a raspi0) it autodetects the sector size to be > 4k, and the resulting image is bootable. However I'm worried the host won't > be able to detect the correct value for the target. In such a case I can see > why it would be useful to set the sector size explicitly. Well, basically all architectures have a 4K page size, so does your x86_64 and so does the rpi (0/1/2/3), so the default for your host is valid for your target. What architectures have a page-size that is not 4K? > Is there another place where the sector size is specified? If so, it might > be better to use the existing value instead. I would also add a default here, to 4096. Users who want to change that value would know what they would be doing. > >>+config BR2_TARGET_ROOTFS_BTRFS_FEATURES > >>+ string "Filesystem Features" > >>+ help > >>+ A comma separated string of features that can be enabled > >>+ during creation time. > >>+ For a list of available options, use; `mkfs.btrfs -O list-all` > > > >Nice. I guess that has to be done using the host-btrfs we built, as its > >feature list may be different than the one from the build machine. > > I can change the help text to include the full path to the binary. For a list of available options, see: .../host/sbin/mkfs.btrfs -O list-all [--SNIP--] > >Is fallocate really necessary here? If the host's filesystem is too > >small to accomodate the image, then the mkfs.btrfs below fill fail; we > >don't need fallocate to fail. > > > >Besides, is fallocate readilly available on all distros? > > > >Usually, we use truncate(1) to that effect; see for example: > > support/testing/tests/fs/test_ubi.py at 29 > > > > fallocate is used here because (unlike other mkfs programs) you can't > specify a size and have mkfs.btrfs create an image file for you. Yeah, I know. I was just questionning the use of fallocate for that, rather than the usual truncate we use everywhere else. > It needs a > sparse file to work with. Except fallocate will not create a sparse file at all; it will really allocate all blocks the file needs. > I'll be happy to change this to truncate, as per your example. Yes, please, for consistency with the other parts of the code in Buildroot. Thanks for the feedback; eager to read your v2. ;-) Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/2] Adding btrfs rootfs option. 2018-08-22 16:57 ` Yann E. MORIN @ 2018-08-22 17:21 ` Yann E. MORIN 0 siblings, 0 replies; 7+ messages in thread From: Yann E. MORIN @ 2018-08-22 17:21 UTC (permalink / raw) To: buildroot Robert, All, On 2018-08-22 18:57 +0200, Yann E. MORIN spake thusly: > On 2018-08-22 14:10 +0100, Robert Heywood spake thusly: > > On 21/08/18 21:27, Yann E. MORIN wrote: [--SNIP--] > > >Given that the filsystem is specifically made for the target that will > > >mount it, does it make sense to ffer that option at all? I.e. if the > > >target uses a 4KiB page size, does it make sense to generatea rootfs > > >that has a different secotr size, and thus not mountable on the target > > >at all? > > > > > >My suggestion: don't add that option. > > > > On my system (building for a raspi0) it autodetects the sector size to be > > 4k, and the resulting image is bootable. However I'm worried the host won't > > be able to detect the correct value for the target. In such a case I can see > > why it would be useful to set the sector size explicitly. > > Well, basically all architectures have a 4K page size, so does your > x86_64 and so does the rpi (0/1/2/3), so the default for your host is > valid for your target. > > What architectures have a page-size that is not 4K? After discussing on IRC with Paul (Cc), it turns out that quite a few architectures can have a page size different from 4K, like mips, arc and a few others. Sometimes it is even configrable. Thanks Paul. 4K just happens to be the most common, but definitely not the only one. > > Is there another place where the sector size is specified? If so, it might > > be better to use the existing value instead. > I would also add a default here, to 4096. Users who want to change > that value would know what they would be doing. So yeah, just keep that option, and let set the default to 4K. People who have a non 4K arch or kernel option will know to change that value. Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 1/2] Adding btrfs-progs host package. 2018-08-21 16:04 [Buildroot] [PATCH 1/2] Adding btrfs-progs host package Robert J. Heywood 2018-08-21 16:04 ` [Buildroot] [PATCH 2/2] Adding btrfs rootfs option Robert J. Heywood @ 2018-08-21 21:42 ` Thomas Petazzoni 1 sibling, 0 replies; 7+ messages in thread From: Thomas Petazzoni @ 2018-08-21 21:42 UTC (permalink / raw) To: buildroot Hello Robert, Thanks for your contribution! On Tue, 21 Aug 2018 17:04:11 +0100, Robert J. Heywood wrote: > Adding a new option to build the btrfs toolset for the host. > > Signed-off-by: Robert J. Heywood <robert.heywood@codethink.co.uk> > --- > package/Config.in.host | 1 + > package/btrfs-progs/Config.in.host | 6 ++++++ > package/btrfs-progs/btrfs-progs.mk | 4 ++++ > 3 files changed, 11 insertions(+) > create mode 100644 package/btrfs-progs/Config.in.host Applied to next after fixing the commit title to: btrfs-progs: add host package variant Indeed, we want all our commit title to look like this: <package>: <description> This should also be fixed for your PATCH 2/2, along with the other comments made by Yann E. Morin. Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-08-22 17:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-21 16:04 [Buildroot] [PATCH 1/2] Adding btrfs-progs host package Robert J. Heywood 2018-08-21 16:04 ` [Buildroot] [PATCH 2/2] Adding btrfs rootfs option Robert J. Heywood 2018-08-21 20:27 ` Yann E. MORIN 2018-08-22 13:10 ` Robert Heywood 2018-08-22 16:57 ` Yann E. MORIN 2018-08-22 17:21 ` Yann E. MORIN 2018-08-21 21:42 ` [Buildroot] [PATCH 1/2] Adding btrfs-progs host package Thomas Petazzoni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox