| Message ID | 1401138097-11024-1-git-send-email-guido@vanguardiasur.com.ar |
|---|---|
| State | Rejected |
| Headers | show |
Hi Guido, Thanks for your patch. However, I don't think it is appropriate. See my comments below. On 26/05/14 23:01, Guido Martínez wrote: > This patch allows building the kernel with an external initramfs. This > is useful in cases where one wants to generate the kernel and rootfs > with BR, and the kernel needs to contain an initramfs different to the > rootfs. In that case, don't you typically want to copy part of the rootfs in the initramfs? Of course, it's possible to do that with a post-build script, but if we add a feature like this to buildroot, I think the copying part should be included. > > The path specified can be a cpio archive or a directory, just like > the kernel config. The path is specified w.r.t. BRs topdir, and made > absolute since the kernel builds in another directory. This patch does nothing more than update the kernel config. Since you can already supply a custom kernel config, I don't think there is much added value to moving these options to the buildroot config. The only feature you add is that the patch can be relative to the buildroot directory. Not enough to be relevant, IMHO, especially since the patch adds non-negligible complexity. > > Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar> > --- > Hi, > > I'm marking this as RFC because I don't particularly like what I wrote > to detect if the path is relative, but I found no other way. There is a very easy way: $(abspath $(call qstrip,$(BR2_LINUX_KERNEL_INITRAMFS_SOURCE))) this will evaluate the path relative to the buildroot directly. It will also expand ~ I think (but I'm not sure, possibly you need to use wildcard for that). [snip] > define LINUX_CONFIGURE_CMDS > $(INSTALL) -m 0644 $(KERNEL_SOURCE_CONFIG) $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig > $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) buildroot_defconfig > + $(if $(BR2_LINUX_KERNEL_EXTERNAL_INITRAMFS), > + $(call LINUX_CONFIGURE_INITRAMFS)) You don't need a $(call ...) here. Regards, Arnout > rm $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig > $(if $(BR2_arm)$(BR2_armeb), > $(call KCONFIG_ENABLE_OPT,CONFIG_AEABI,$(@D)/.config)) >
Hello Arnout, On 28 May 06:47 PM, Arnout Vandecappelle wrote: [..] > > > > The path specified can be a cpio archive or a directory, just like > > the kernel config. The path is specified w.r.t. BRs topdir, and made > > absolute since the kernel builds in another directory. > > This patch does nothing more than update the kernel config. Since you can > already supply a custom kernel config, I don't think there is much added value > to moving these options to the buildroot config. > > The only feature you add is that the patch can be relative to the buildroot > directory. Not enough to be relevant, IMHO, especially since the patch adds > non-negligible complexity. > Our software stack includes a kernel with an appended initramfs to take care of software upgrade and find, check and switch to the "real" rootfs. Both the initramfs and rootfs are Buildroot-based, but each of them with their own configuration and history (in separate git repos). Hence we came up with this solution: the initramfs would live in a different repo, and after it's built we use this new option to append it in the kernel. Maybe you have a better idea, and there's a cleaner way to accomplish this with Buildroot? Thanks for the feedback!
On 04/06/14 22:45, Ezequiel Garcia wrote: > Hello Arnout, > > On 28 May 06:47 PM, Arnout Vandecappelle wrote: > [..] >>> >>> The path specified can be a cpio archive or a directory, just like >>> the kernel config. The path is specified w.r.t. BRs topdir, and made >>> absolute since the kernel builds in another directory. >> >> This patch does nothing more than update the kernel config. Since you can >> already supply a custom kernel config, I don't think there is much added value >> to moving these options to the buildroot config. >> >> The only feature you add is that the patch can be relative to the buildroot >> directory. Not enough to be relevant, IMHO, especially since the patch adds >> non-negligible complexity. >> > > Our software stack includes a kernel with an appended initramfs to take care > of software upgrade and find, check and switch to the "real" rootfs. Both > the initramfs and rootfs are Buildroot-based, but each of them with their > own configuration and history (in separate git repos). > > Hence we came up with this solution: the initramfs would live in a different > repo, and after it's built we use this new option to append it in the kernel. > > Maybe you have a better idea, and there's a cleaner way to accomplish this > with Buildroot? Use a custom kernel config, and code the path to the external initramfs in that custom kernel config instead of in the buildroot config. You can use $(BASE_DIR)/../initramfs-output-dir if you want to avoid absolute paths in the config. Actually, in your case the easiest solution would be to build the kernel together with the initramfs-buildroot and use the initramfs rootfs instead of the cpio one. In the real-rootfs-buildroot you don't need to build a kernel. Or am I missing something? Regards, Arnout > > Thanks for the feedback! >
Dear Guido Martínez, On Mon, 26 May 2014 18:01:37 -0300, Guido Martínez wrote: > This patch allows building the kernel with an external initramfs. This > is useful in cases where one wants to generate the kernel and rootfs > with BR, and the kernel needs to contain an initramfs different to the > rootfs. > > The path specified can be a cpio archive or a directory, just like > the kernel config. The path is specified w.r.t. BRs topdir, and made > absolute since the kernel builds in another directory. > > Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar> Since I agree with the feedback from Arnout, I'll mark this as Rejected in patchwork. There's no point in duplicating each and every kernel configuration option at the Buildroot level. Just make sure to tune your kernel configuration properly, and pass it to Buildroot as the kernel configuration file. Thanks! Thomas
On 05 Jun 12:29 AM, Arnout Vandecappelle wrote: [..] > > Actually, in your case the easiest solution would be to build the kernel > together with the initramfs-buildroot and use the initramfs rootfs instead of > the cpio one. In the real-rootfs-buildroot you don't need to build a kernel. > Right, that sounds like a better plan. Thanks a lot for the feedback,
diff --git a/linux/Config.in b/linux/Config.in index 069a7d6..1efb845 100644 --- a/linux/Config.in +++ b/linux/Config.in @@ -155,6 +155,38 @@ config BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE help Path to the kernel configuration file +config BR2_LINUX_KERNEL_EXTERNAL_INITRAMFS + depends on !BR2_TARGET_ROOTFS_INITRAMFS + bool "Use an external initramfs" + help + Append an external initramfs to the kernel image. + +comment "using BR initramfs, can't use an external one" + depends on BR2_TARGET_ROOTFS_INITRAMFS + +config BR2_LINUX_KERNEL_INITRAMFS_SOURCE + string "Appended initramfs source" + depends on BR2_LINUX_KERNEL_EXTERNAL_INITRAMFS + help + Path to use as the external initramfs in the kernel. Can be + a CPIO archive or a directory. + +config BR2_LINUX_KERNEL_INITRAMFS_ROOT_UID + string "Initramfs squash UID" + depends on BR2_LINUX_KERNEL_EXTERNAL_INITRAMFS + help + If "Appended initramfs source" is a directory, select a UID + to be squashed to 0 (root). This allows creating an archive + with root-owned files as a user. + +config BR2_LINUX_KERNEL_INITRAMFS_ROOT_GID + string "Initramfs squash GID" + depends on BR2_LINUX_KERNEL_EXTERNAL_INITRAMFS + help + If "Appended initramfs source" is a directory, select a GID + to be squashed to 0 (root). This allows creating an archive + with root-owned files as a user. + # # Binary format # diff --git a/linux/linux.mk b/linux/linux.mk index bd3f2ac..1ce5a25 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -166,9 +166,31 @@ else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y) KERNEL_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE) endif +define LINUX_CONFIGURE_INITRAMFS + $(if $(filter-out /%,$(call qstrip,$(BR2_LINUX_KERNEL_INITRAMFS_SOURCE))), + $(if $(filter-out ~%,$(call qstrip,$(BR2_LINUX_KERNEL_INITRAMFS_SOURCE))), + $(eval BR2_LINUX_KERNEL_INITRAMFS_SOURCE="${TOPDIR}/$(call qstrip,$(BR2_LINUX_KERNEL_INITRAMFS_SOURCE))"))) + + $(if $(BR2_LINUX_KERNEL_INITRAMFS_SOURCE), + $(call KCONFIG_ENABLE_OPT,CONFIG_BLK_DEV_INITRD,$(@D)/.config) + $(call KCONFIG_SET_OPT,CONFIG_INITRAMFS_SOURCE,\"$(BR2_LINUX_KERNEL_INITRAMFS_SOURCE)\",$(@D)/.config) + ) + + $(if $(BR2_LINUX_KERNEL_INITRAMFS_ROOT_UID), + $(call KCONFIG_SET_OPT,CONFIG_INITRAMFS_ROOT_UID,$(BR2_LINUX_KERNEL_INITRAMFS_ROOT_UID),$(@D)/.config) + ) + + $(if $(BR2_LINUX_KERNEL_INITRAMFS_ROOT_GID), + $(call KCONFIG_SET_OPT,CONFIG_INITRAMFS_ROOT_GID,$(BR2_LINUX_KERNEL_INITRAMFS_ROOT_GID),$(@D)/.config) + ) +endef + + define LINUX_CONFIGURE_CMDS $(INSTALL) -m 0644 $(KERNEL_SOURCE_CONFIG) $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) buildroot_defconfig + $(if $(BR2_LINUX_KERNEL_EXTERNAL_INITRAMFS), + $(call LINUX_CONFIGURE_INITRAMFS)) rm $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig $(if $(BR2_arm)$(BR2_armeb), $(call KCONFIG_ENABLE_OPT,CONFIG_AEABI,$(@D)/.config))
This patch allows building the kernel with an external initramfs. This is useful in cases where one wants to generate the kernel and rootfs with BR, and the kernel needs to contain an initramfs different to the rootfs. The path specified can be a cpio archive or a directory, just like the kernel config. The path is specified w.r.t. BRs topdir, and made absolute since the kernel builds in another directory. Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar> --- Hi, I'm marking this as RFC because I don't particularly like what I wrote to detect if the path is relative, but I found no other way. Any comment is appreciated, Thanks! Guido linux/Config.in | 32 ++++++++++++++++++++++++++++++++ linux/linux.mk | 22 ++++++++++++++++++++++ 2 files changed, 54 insertions(+)