* [PATCH 0/1] A commit to extend the INITRD variable @ 2014-07-29 18:34 nitin.a.kamble 2014-07-29 18:34 ` [PATCH 1/1] INITRD var: make it a list of filesystem images nitin.a.kamble 0 siblings, 1 reply; 6+ messages in thread From: nitin.a.kamble @ 2014-07-29 18:34 UTC (permalink / raw) To: Openembedded-core, darren.hart, ross.burton, richard.purdie From: Nitin A Kamble <nitin.a.kamble@intel.com> This commit extends the INITRD variable, so that it can accommodate multiple filesystems as expected by the Linux kernel. Thanks to Ross Burton for this suggestion to simplify the early microcode loading support in the meta-intel Layer. Thanks, Nitin The following changes since commit 2d1660112e54653f7bb763939d0416472c49fe01: populate_sdk_base: Fix grep command usage on old hosts (2014-07-29 09:58:27 +0100) are available in the git repository at: git://git.yoctoproject.org/poky-contrib nitin/misc http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=nitin/misc Nitin A Kamble (1): INITRD var: make it a list of filesystem images documentation/ref-manual/ref-classes.xml | 4 ++-- documentation/ref-manual/ref-variables.xml | 2 +- meta/classes/boot-directdisk.bbclass | 13 ++++++++++--- meta/classes/bootimg.bbclass | 27 ++++++++++++++++++++++----- meta/classes/grub-efi.bbclass | 2 +- meta/classes/syslinux.bbclass | 2 +- meta/conf/documentation.conf | 2 +- 7 files changed, 38 insertions(+), 14 deletions(-) -- 1.8.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] INITRD var: make it a list of filesystem images 2014-07-29 18:34 [PATCH 0/1] A commit to extend the INITRD variable nitin.a.kamble @ 2014-07-29 18:34 ` nitin.a.kamble 2014-08-04 16:38 ` Hart, Darren 0 siblings, 1 reply; 6+ messages in thread From: nitin.a.kamble @ 2014-07-29 18:34 UTC (permalink / raw) To: Openembedded-core, darren.hart, ross.burton, richard.purdie From: Nitin A Kamble <nitin.a.kamble@intel.com> The initrd image used by the Linux kernel is list of file system images concatenated together and presented as a single initrd file at boot time. So far the initrd is a single filesystem image. But in cases like to support early microcode loading, the initrd image need to have multiple filesystem images concatenated together. This commit is extending the INITRD variable from a single filesystem image to a list of filesystem images to satisfy the need mentioned above. Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com> --- documentation/ref-manual/ref-classes.xml | 4 ++-- documentation/ref-manual/ref-variables.xml | 2 +- meta/classes/boot-directdisk.bbclass | 13 ++++++++++--- meta/classes/bootimg.bbclass | 27 ++++++++++++++++++++++----- meta/classes/grub-efi.bbclass | 2 +- meta/classes/syslinux.bbclass | 2 +- meta/conf/documentation.conf | 2 +- 7 files changed, 38 insertions(+), 14 deletions(-) diff --git a/documentation/ref-manual/ref-classes.xml b/documentation/ref-manual/ref-classes.xml index e7e9942..0bf3546 100644 --- a/documentation/ref-manual/ref-classes.xml +++ b/documentation/ref-manual/ref-classes.xml @@ -881,7 +881,7 @@ <itemizedlist> <listitem><para> <link linkend='var-INITRD'><filename>INITRD</filename></link>: - Indicates a filesystem image to use as an initrd (optional). + Indicates list of filesystem images to concatenate and use as an initrd (optional). </para></listitem> <listitem><para> <link linkend='var-ROOTFS'><filename>ROOTFS</filename></link>: @@ -2864,7 +2864,7 @@ The class supports the following variables: <itemizedlist> <listitem><para><emphasis><link linkend='var-INITRD'><filename>INITRD</filename></link>:</emphasis> - Indicates a filesystem image to use as an initial RAM disk + Indicates list of filesystem images to concatenate and use as an initial RAM disk (initrd). This variable is optional.</para></listitem> <listitem><para><emphasis><link linkend='var-ROOTFS'><filename>ROOTFS</filename></link>:</emphasis> diff --git a/documentation/ref-manual/ref-variables.xml b/documentation/ref-manual/ref-variables.xml index b4d6e71..16e3ed6 100644 --- a/documentation/ref-manual/ref-variables.xml +++ b/documentation/ref-manual/ref-variables.xml @@ -4020,7 +4020,7 @@ recipes-graphics/xorg-font/font-alias_1.0.3.bb:PR = "${INC_PR}.3" <glossentry id='var-INITRD'><glossterm>INITRD</glossterm> <glossdef> <para> - Indicates a filesystem image to use as an initial RAM + Indicates list of filesystem images to concatenate and use as an initial RAM disk (<filename>initrd</filename>). </para> diff --git a/meta/classes/boot-directdisk.bbclass b/meta/classes/boot-directdisk.bbclass index 0da9932..995d3e7 100644 --- a/meta/classes/boot-directdisk.bbclass +++ b/meta/classes/boot-directdisk.bbclass @@ -71,10 +71,17 @@ boot_direct_populate() { # Install bzImage, initrd, and rootfs.img in DEST for all loaders to use. install -m 0644 ${STAGING_KERNEL_DIR}/bzImage $dest/vmlinuz - if [ -n "${INITRD}" ] && [ -s "${INITRD}" ]; then - install -m 0644 ${INITRD} $dest/initrd + # initrd is made of concatenation of multiple filesystem images + if [ -n "${INITRD}" ]; then + rm -f $dest/initrd + for fs in ${INITRD} + do + if [ -n "${fs}" ] && [ -s "${fs}" ]; then + cat ${fs} >> $dest/initrd + fi + done + chmod 0644 $dest/initrd fi - } build_boot_dd() { diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass index d52aace..7b3ce65 100644 --- a/meta/classes/bootimg.bbclass +++ b/meta/classes/bootimg.bbclass @@ -18,7 +18,7 @@ # an hdd) # External variables (also used by syslinux.bbclass) -# ${INITRD} - indicates a filesystem image to use as an initrd (optional) +# ${INITRD} - indicates a list of filesystem images to concatenate and use as an initrd (optional) # ${COMPRESSISO} - Transparent compress ISO, reduce size ~40% if set to 1 # ${NOISO} - skip building the ISO image if set to 1 # ${NOHDD} - skip building the HDD image if set to 1 @@ -67,9 +67,17 @@ populate() { # Install bzImage, initrd, and rootfs.img in DEST for all loaders to use. install -m 0644 ${STAGING_KERNEL_DIR}/bzImage ${DEST}/vmlinuz - - if [ -n "${INITRD}" ] && [ -s "${INITRD}" ]; then - install -m 0644 ${INITRD} ${DEST}/initrd + + # initrd is made of concatenation of multiple filesystem images + if [ -n "${INITRD}" ]; then + rm -f ${DEST}/initrd + for fs in ${INITRD} + do + if [ -s "${fs}" ]; then + cat ${fs} >> ${DEST}/initrd + fi + done + chmod 0644 ${DEST}/initrd fi if [ -n "${ROOTFS}" ] && [ -s "${ROOTFS}" ]; then @@ -80,10 +88,19 @@ populate() { build_iso() { # Only create an ISO if we have an INITRD and NOISO was not set - if [ -z "${INITRD}" ] || [ ! -s "${INITRD}" ] || [ "${NOISO}" = "1" ]; then + if [ -z "${INITRD}" ] || [ "${NOISO}" = "1" ]; then bbnote "ISO image will not be created." return fi + # ${INITRD} is a list of multiple filesystem images + for fs in ${INITRD} + do + if [ ! -s "${fs}" ]; then + bbnote "ISO image will not be created. ${fs} is invalid." + return + fi + done + populate ${ISODIR} diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass index 505d032..47bd35e 100644 --- a/meta/classes/grub-efi.bbclass +++ b/meta/classes/grub-efi.bbclass @@ -7,7 +7,7 @@ # Provide grub-efi specific functions for building bootable images. # External variables -# ${INITRD} - indicates a filesystem image to use as an initrd (optional) +# ${INITRD} - indicates a list of filesystem images to concatenate and use as an initrd (optional) # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional) # ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu # ${LABELS} - a list of targets for the automatic config diff --git a/meta/classes/syslinux.bbclass b/meta/classes/syslinux.bbclass index b9701bf..d6498d9 100644 --- a/meta/classes/syslinux.bbclass +++ b/meta/classes/syslinux.bbclass @@ -5,7 +5,7 @@ # Provide syslinux specific functions for building bootable images. # External variables -# ${INITRD} - indicates a filesystem image to use as an initrd (optional) +# ${INITRD} - indicates a list of filesystem images to concatenate and use as an initrd (optional) # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional) # ${AUTO_SYSLINUXMENU} - set this to 1 to enable creating an automatic menu # ${LABELS} - a list of targets for the automatic config diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf index 7fa3f31..31fbd6c 100644 --- a/meta/conf/documentation.conf +++ b/meta/conf/documentation.conf @@ -225,7 +225,7 @@ INHIBIT_PACKAGE_STRIP[doc] = "If set to "1", causes the build to not strip binar INHERIT[doc] = "Causes the named class to be inherited at this point during parsing. The variable is only valid in configuration files." INHERIT_DISTRO[doc] = "Lists classes that will be inherited at the distribution level. It is unlikely that you want to edit this variable." INITRAMFS_FSTYPES[doc] = "Defines the format for the output image of an initial RAM disk (initramfs), which is used during boot." -INITRD[doc] = "Indicates a filesystem image to use as an initial RAM disk (initrd)." +INITRD[doc] = "Indicates list of filesystem images to concatenate and use as an initial RAM disk (initrd)." INITSCRIPT_NAME[doc] = "The filename of the initialization script as installed to ${sysconfdir}/init.d." INITSCRIPT_PACKAGES[doc] = "A list of the packages that contain initscripts. This variable is used in recipes when using update-rc.d.bbclass. The variable is optional and defaults to the PN variable." INITSCRIPT_PARAMS[doc] = "Specifies the options to pass to update-rc.d. The variable is mandatory and is used in recipes when using update-rc.d.bbclass." -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] INITRD var: make it a list of filesystem images 2014-07-29 18:34 ` [PATCH 1/1] INITRD var: make it a list of filesystem images nitin.a.kamble @ 2014-08-04 16:38 ` Hart, Darren 2014-08-05 4:33 ` Nitin A Kamble 0 siblings, 1 reply; 6+ messages in thread From: Hart, Darren @ 2014-08-04 16:38 UTC (permalink / raw) To: Kamble, Nitin A, Openembedded-core@lists.openembedded.org, Burton, Ross, richard.purdie@linuxfoundation.org On 7/29/14, 11:34, "Kamble, Nitin A" <nitin.a.kamble@intel.com> wrote: >From: Nitin A Kamble <nitin.a.kamble@intel.com> Hi Nitin, Generally speaking this looks like a good improvement. I don't have any major technical concerns, but we do need to address some grammatical issues in the commit and the docs below to make sure people can follow the intent here. > >The initrd image used by the Linux kernel is list of file system images >concatenated together and presented as a single initrd file at boot time. > >So far the initrd is a single filesystem image. But in cases like to >support >early microcode loading, the initrd image need to have multiple filesystem >images concatenated together. Consider: Currently, the INITRD variable is a single filesystem image. For optional early boot features, such as microcode loading, a modular approach would provide the most flexibility and is minimally invasive. Converting INITRD to a list of images to be concatenated accomplishes this. > >This commit is extending the INITRD variable from a single filesystem >image >to a list of filesystem images to satisfy the need mentioned above. Can now drop this paragraph. > >Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com> >--- > documentation/ref-manual/ref-classes.xml | 4 ++-- > documentation/ref-manual/ref-variables.xml | 2 +- > meta/classes/boot-directdisk.bbclass | 13 ++++++++++--- > meta/classes/bootimg.bbclass | 27 >++++++++++++++++++++++----- > meta/classes/grub-efi.bbclass | 2 +- > meta/classes/syslinux.bbclass | 2 +- > meta/conf/documentation.conf | 2 +- > 7 files changed, 38 insertions(+), 14 deletions(-) > >diff --git a/documentation/ref-manual/ref-classes.xml >b/documentation/ref-manual/ref-classes.xml >index e7e9942..0bf3546 100644 >--- a/documentation/ref-manual/ref-classes.xml >+++ b/documentation/ref-manual/ref-classes.xml >@@ -881,7 +881,7 @@ > <itemizedlist> > <listitem><para> > <link >linkend='var-INITRD'><filename>INITRD</filename></link>: >- Indicates a filesystem image to use as an initrd >(optional). >+ Indicates list of filesystem images to concatenate and >use as an initrd (optional). Missing article: ^ a > </para></listitem> > <listitem><para> > <link >linkend='var-ROOTFS'><filename>ROOTFS</filename></link>: >@@ -2864,7 +2864,7 @@ > The class supports the following variables: > <itemizedlist> > <listitem><para><emphasis><link >linkend='var-INITRD'><filename>INITRD</filename></link>:</emphasis> >- Indicates a filesystem image to use as an initial RAM >disk >+ Indicates list of filesystem images to concatenate and >use as an initial RAM disk Missing article: ^ a > (initrd). > This variable is optional.</para></listitem> > <listitem><para><emphasis><link >linkend='var-ROOTFS'><filename>ROOTFS</filename></link>:</emphasis> >diff --git a/documentation/ref-manual/ref-variables.xml >b/documentation/ref-manual/ref-variables.xml >index b4d6e71..16e3ed6 100644 >--- a/documentation/ref-manual/ref-variables.xml >+++ b/documentation/ref-manual/ref-variables.xml >@@ -4020,7 +4020,7 @@ recipes-graphics/xorg-font/font-alias_1.0.3.bb:PR = >"${INC_PR}.3" > <glossentry id='var-INITRD'><glossterm>INITRD</glossterm> > <glossdef> > <para> >- Indicates a filesystem image to use as an initial RAM >+ Indicates list of filesystem images to concatenate >and use as an initial RAM ditto > disk (<filename>initrd</filename>). > </para> > >diff --git a/meta/classes/boot-directdisk.bbclass >b/meta/classes/boot-directdisk.bbclass >index 0da9932..995d3e7 100644 >--- a/meta/classes/boot-directdisk.bbclass >+++ b/meta/classes/boot-directdisk.bbclass >@@ -71,10 +71,17 @@ boot_direct_populate() { > # Install bzImage, initrd, and rootfs.img in DEST for all loaders to >use. > install -m 0644 ${STAGING_KERNEL_DIR}/bzImage $dest/vmlinuz > >- if [ -n "${INITRD}" ] && [ -s "${INITRD}" ]; then >- install -m 0644 ${INITRD} $dest/initrd >+ # initrd is made of concatenation of multiple filesystem images # Assemble the initrd from the list of filesystem images >+ if [ -n "${INITRD}" ]; then >+ rm -f $dest/initrd >+ for fs in ${INITRD} >+ do >+ if [ -n "${fs}" ] && [ -s "${fs}" ]; then The -n test is unnecessary here. How would "for fs in ${INITRD}" result in an fs of "" ? >+ cat ${fs} >> $dest/ignited >+ fi Some kind of a warning at least is warranted if a file appears in the INITRD list but is either 0-size or non-existent. >+ done >+ chmod 0644 $dest/initrd > fi >- > } > > build_boot_dd() { >diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass >index d52aace..7b3ce65 100644 >--- a/meta/classes/bootimg.bbclass >+++ b/meta/classes/bootimg.bbclass >@@ -18,7 +18,7 @@ > # an hdd) > > # External variables (also used by syslinux.bbclass) >-# ${INITRD} - indicates a filesystem image to use as an initrd (optional) >+# ${INITRD} - indicates a list of filesystem images to concatenate and >use as an initrd (optional) > # ${COMPRESSISO} - Transparent compress ISO, reduce size ~40% if set to 1 > # ${NOISO} - skip building the ISO image if set to 1 > # ${NOHDD} - skip building the HDD image if set to 1 >@@ -67,9 +67,17 @@ populate() { > > # Install bzImage, initrd, and rootfs.img in DEST for all loaders to >use. > install -m 0644 ${STAGING_KERNEL_DIR}/bzImage ${DEST}/vmlinuz >- >- if [ -n "${INITRD}" ] && [ -s "${INITRD}" ]; then >- install -m 0644 ${INITRD} ${DEST}/initrd >+ >+ # initrd is made of concatenation of multiple filesystem images >+ if [ -n "${INITRD}" ]; then >+ rm -f ${DEST}/initrd >+ for fs in ${INITRD} >+ do >+ if [ -s "${fs}" ]; then >+ cat ${fs} >> ${DEST}/initrd >+ fi >+ done Same test commentary here. >+ chmod 0644 ${DEST}/initrd > fi > > if [ -n "${ROOTFS}" ] && [ -s "${ROOTFS}" ]; then >@@ -80,10 +88,19 @@ populate() { > > build_iso() { > # Only create an ISO if we have an INITRD and NOISO was not set >- if [ -z "${INITRD}" ] || [ ! -s "${INITRD}" ] || [ "${NOISO}" = "1" ]; >then >+ if [ -z "${INITRD}" ] || [ "${NOISO}" = "1" ]; then > bbnote "ISO image will not be created." > return > Fi Perhaps the -s test should be replaced with a -s of $dest/initrd? >+ # ${INITRD} is a list of multiple filesystem images >+ for fs in ${INITRD} >+ do >+ if [ ! -s "${fs}" ]; then >+ bbnote "ISO image will not be created. ${fs} is invalid." >+ return >+ fi >+ done This additional loop could be eliminated by including this test above. Right? Or am I missing something subtle here? >+ > > populate ${ISODIR} > >diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass >index 505d032..47bd35e 100644 >--- a/meta/classes/grub-efi.bbclass >+++ b/meta/classes/grub-efi.bbclass >@@ -7,7 +7,7 @@ > # Provide grub-efi specific functions for building bootable images. > > # External variables >-# ${INITRD} - indicates a filesystem image to use as an initrd (optional) >+# ${INITRD} - indicates a list of filesystem images to concatenate and >use as an initrd (optional) Used the "a" here, good :-) > # ${ROOTFS} - indicates a filesystem image to include as the root >filesystem (optional) > # ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the >boot menu > # ${LABELS} - a list of targets for the automatic config >diff --git a/meta/classes/syslinux.bbclass b/meta/classes/syslinux.bbclass >index b9701bf..d6498d9 100644 >--- a/meta/classes/syslinux.bbclass >+++ b/meta/classes/syslinux.bbclass >@@ -5,7 +5,7 @@ > # Provide syslinux specific functions for building bootable images. > > # External variables >-# ${INITRD} - indicates a filesystem image to use as an initrd (optional) >+# ${INITRD} - indicates a list of filesystem images to concatenate and >use as an initrd (optional) > # ${ROOTFS} - indicates a filesystem image to include as the root >filesystem (optional) > # ${AUTO_SYSLINUXMENU} - set this to 1 to enable creating an automatic >menu > # ${LABELS} - a list of targets for the automatic config >diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf >index 7fa3f31..31fbd6c 100644 >--- a/meta/conf/documentation.conf >+++ b/meta/conf/documentation.conf >@@ -225,7 +225,7 @@ INHIBIT_PACKAGE_STRIP[doc] = "If set to "1", causes >the build to not strip binar > INHERIT[doc] = "Causes the named class to be inherited at this point >during parsing. The variable is only valid in configuration files." > INHERIT_DISTRO[doc] = "Lists classes that will be inherited at the >distribution level. It is unlikely that you want to edit this variable." > INITRAMFS_FSTYPES[doc] = "Defines the format for the output image of an >initial RAM disk (initramfs), which is used during boot." >-INITRD[doc] = "Indicates a filesystem image to use as an initial RAM >disk (initrd)." >+INITRD[doc] = "Indicates list of filesystem images to concatenate and >use as an initial RAM disk (initrd)." "a list" > INITSCRIPT_NAME[doc] = "The filename of the initialization script as >installed to ${sysconfdir}/init.d." > INITSCRIPT_PACKAGES[doc] = "A list of the packages that contain >initscripts. This variable is used in recipes when using >update-rc.d.bbclass. The variable is optional and defaults to the PN >variable." > INITSCRIPT_PARAMS[doc] = "Specifies the options to pass to update-rc.d. >The variable is mandatory and is used in recipes when using >update-rc.d.bbclass." >-- >1.8.1.4 > > Thanks, -- Darren Hart Open Source Technology Center darren.hart@intel.com Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] INITRD var: make it a list of filesystem images 2014-08-04 16:38 ` Hart, Darren @ 2014-08-05 4:33 ` Nitin A Kamble 2014-08-05 16:45 ` Hart, Darren 0 siblings, 1 reply; 6+ messages in thread From: Nitin A Kamble @ 2014-08-05 4:33 UTC (permalink / raw) To: Hart, Darren, Openembedded-core@lists.openembedded.org, Burton, Ross, richard.purdie@linuxfoundation.org On 8/4/2014 9:38 AM, Hart, Darren wrote: > On 7/29/14, 11:34, "Kamble, Nitin A" <nitin.a.kamble@intel.com> wrote: > >> From: Nitin A Kamble <nitin.a.kamble@intel.com> > Hi Nitin, > > Generally speaking this looks like a good improvement. I don't have any > major technical concerns, but we do need to address some grammatical > issues in the commit and the docs below to make sure people can follow the > intent here. Thanks Darren for the detailed review. With such review the quality of my writtings will definitely improve. > >> The initrd image used by the Linux kernel is list of file system images >> concatenated together and presented as a single initrd file at boot time. >> >> So far the initrd is a single filesystem image. But in cases like to >> support >> early microcode loading, the initrd image need to have multiple filesystem >> images concatenated together. > Consider: > > Currently, the INITRD variable is a single filesystem image. For optional > early boot features, such as microcode loading, a modular approach would > provide the most flexibility and is minimally invasive. Converting INITRD > to a list of images to be concatenated accomplishes this. >> This commit is extending the INITRD variable from a single filesystem >> image >> to a list of filesystem images to satisfy the need mentioned above. > Can now drop this paragraph. I am fine with your wording. I would add further, that the Linux kernel can already handle initrd images which have multiple filesystem images concatenated together. >> Signed-off-by: Nitin A Kamble <nitin.a.kamble@intel.com> >> --- >> documentation/ref-manual/ref-classes.xml | 4 ++-- >> documentation/ref-manual/ref-variables.xml | 2 +- >> meta/classes/boot-directdisk.bbclass | 13 ++++++++++--- >> meta/classes/bootimg.bbclass | 27 >> ++++++++++++++++++++++----- >> meta/classes/grub-efi.bbclass | 2 +- >> meta/classes/syslinux.bbclass | 2 +- >> meta/conf/documentation.conf | 2 +- >> 7 files changed, 38 insertions(+), 14 deletions(-) >> >> diff --git a/documentation/ref-manual/ref-classes.xml >> b/documentation/ref-manual/ref-classes.xml >> index e7e9942..0bf3546 100644 >> --- a/documentation/ref-manual/ref-classes.xml >> +++ b/documentation/ref-manual/ref-classes.xml >> @@ -881,7 +881,7 @@ >> <itemizedlist> >> <listitem><para> >> <link >> linkend='var-INITRD'><filename>INITRD</filename></link>: >> - Indicates a filesystem image to use as an initrd >> (optional). >> + Indicates list of filesystem images to concatenate and >> use as an initrd (optional). > > Missing article: ^ a I hope I do not do these kind of article mistakes again. > >> </para></listitem> >> <listitem><para> >> <link >> linkend='var-ROOTFS'><filename>ROOTFS</filename></link>: >> @@ -2864,7 +2864,7 @@ >> The class supports the following variables: >> <itemizedlist> >> <listitem><para><emphasis><link >> linkend='var-INITRD'><filename>INITRD</filename></link>:</emphasis> >> - Indicates a filesystem image to use as an initial RAM >> disk >> + Indicates list of filesystem images to concatenate and >> use as an initial RAM disk > Missing article: ^ a > >> (initrd). >> This variable is optional.</para></listitem> >> <listitem><para><emphasis><link >> linkend='var-ROOTFS'><filename>ROOTFS</filename></link>:</emphasis> >> diff --git a/documentation/ref-manual/ref-variables.xml >> b/documentation/ref-manual/ref-variables.xml >> index b4d6e71..16e3ed6 100644 >> --- a/documentation/ref-manual/ref-variables.xml >> +++ b/documentation/ref-manual/ref-variables.xml >> @@ -4020,7 +4020,7 @@ recipes-graphics/xorg-font/font-alias_1.0.3.bb:PR = >> "${INC_PR}.3" >> <glossentry id='var-INITRD'><glossterm>INITRD</glossterm> >> <glossdef> >> <para> >> - Indicates a filesystem image to use as an initial RAM >> + Indicates list of filesystem images to concatenate >> and use as an initial RAM > ditto > >> disk (<filename>initrd</filename>). >> </para> >> >> diff --git a/meta/classes/boot-directdisk.bbclass >> b/meta/classes/boot-directdisk.bbclass >> index 0da9932..995d3e7 100644 >> --- a/meta/classes/boot-directdisk.bbclass >> +++ b/meta/classes/boot-directdisk.bbclass >> @@ -71,10 +71,17 @@ boot_direct_populate() { >> # Install bzImage, initrd, and rootfs.img in DEST for all loaders to >> use. >> install -m 0644 ${STAGING_KERNEL_DIR}/bzImage $dest/vmlinuz >> >> - if [ -n "${INITRD}" ] && [ -s "${INITRD}" ]; then >> - install -m 0644 ${INITRD} $dest/initrd >> + # initrd is made of concatenation of multiple filesystem images > # Assemble the initrd from the list of filesystem images > >> + if [ -n "${INITRD}" ]; then >> + rm -f $dest/initrd >> + for fs in ${INITRD} >> + do >> + if [ -n "${fs}" ] && [ -s "${fs}" ]; then > > The -n test is unnecessary here. How would "for fs in ${INITRD}" result in > an fs of "" ? The -n test is needed here, it checks whether the file exist or not. > >> + cat ${fs} >> $dest/ignited >> + fi > Some kind of a warning at least is warranted if a file appears in the > INITRD list but is either 0-size or non-existent. I tried to keep the original style of the code. But it makes sense to add a warning or even an error here. >> + done >> + chmod 0644 $dest/initrd >> fi >> - >> } >> >> build_boot_dd() { >> diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass >> index d52aace..7b3ce65 100644 >> --- a/meta/classes/bootimg.bbclass >> +++ b/meta/classes/bootimg.bbclass >> @@ -18,7 +18,7 @@ >> # an hdd) >> >> # External variables (also used by syslinux.bbclass) >> -# ${INITRD} - indicates a filesystem image to use as an initrd (optional) >> +# ${INITRD} - indicates a list of filesystem images to concatenate and >> use as an initrd (optional) >> # ${COMPRESSISO} - Transparent compress ISO, reduce size ~40% if set to 1 >> # ${NOISO} - skip building the ISO image if set to 1 >> # ${NOHDD} - skip building the HDD image if set to 1 >> @@ -67,9 +67,17 @@ populate() { >> >> # Install bzImage, initrd, and rootfs.img in DEST for all loaders to >> use. >> install -m 0644 ${STAGING_KERNEL_DIR}/bzImage ${DEST}/vmlinuz >> - >> - if [ -n "${INITRD}" ] && [ -s "${INITRD}" ]; then >> - install -m 0644 ${INITRD} ${DEST}/initrd >> + >> + # initrd is made of concatenation of multiple filesystem images >> + if [ -n "${INITRD}" ]; then >> + rm -f ${DEST}/initrd >> + for fs in ${INITRD} >> + do >> + if [ -s "${fs}" ]; then >> + cat ${fs} >> ${DEST}/initrd >> + fi >> + done > Same test commentary here. > >> + chmod 0644 ${DEST}/initrd >> fi >> >> if [ -n "${ROOTFS}" ] && [ -s "${ROOTFS}" ]; then >> @@ -80,10 +88,19 @@ populate() { >> >> build_iso() { >> # Only create an ISO if we have an INITRD and NOISO was not set >> - if [ -z "${INITRD}" ] || [ ! -s "${INITRD}" ] || [ "${NOISO}" = "1" ]; >> then >> + if [ -z "${INITRD}" ] || [ "${NOISO}" = "1" ]; then >> bbnote "ISO image will not be created." >> return >> Fi > Perhaps the -s test should be replaced with a -s of $dest/initrd? The -s test is replaced by the loop few lines below. >> + # ${INITRD} is a list of multiple filesystem images >> + for fs in ${INITRD} >> + do >> + if [ ! -s "${fs}" ]; then >> + bbnote "ISO image will not be created. ${fs} is invalid." >> + return >> + fi >> + done > This additional loop could be eliminated by including this test above. > Right? Or am I missing something subtle here? That approach will leave a hole where, the function will continue when one of the filesystem image is invalid. So the loop is better here as it does not leave any hole. >> + >> >> populate ${ISODIR} >> >> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass >> index 505d032..47bd35e 100644 >> --- a/meta/classes/grub-efi.bbclass >> +++ b/meta/classes/grub-efi.bbclass >> @@ -7,7 +7,7 @@ >> # Provide grub-efi specific functions for building bootable images. >> >> # External variables >> -# ${INITRD} - indicates a filesystem image to use as an initrd (optional) >> +# ${INITRD} - indicates a list of filesystem images to concatenate and >> use as an initrd (optional) > Used the "a" here, good :-) That was not a typo :) >> # ${ROOTFS} - indicates a filesystem image to include as the root >> filesystem (optional) >> # ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the >> boot menu >> # ${LABELS} - a list of targets for the automatic config >> diff --git a/meta/classes/syslinux.bbclass b/meta/classes/syslinux.bbclass >> index b9701bf..d6498d9 100644 >> --- a/meta/classes/syslinux.bbclass >> +++ b/meta/classes/syslinux.bbclass >> @@ -5,7 +5,7 @@ >> # Provide syslinux specific functions for building bootable images. >> >> # External variables >> -# ${INITRD} - indicates a filesystem image to use as an initrd (optional) >> +# ${INITRD} - indicates a list of filesystem images to concatenate and >> use as an initrd (optional) >> # ${ROOTFS} - indicates a filesystem image to include as the root >> filesystem (optional) >> # ${AUTO_SYSLINUXMENU} - set this to 1 to enable creating an automatic >> menu >> # ${LABELS} - a list of targets for the automatic config >> diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf >> index 7fa3f31..31fbd6c 100644 >> --- a/meta/conf/documentation.conf >> +++ b/meta/conf/documentation.conf >> @@ -225,7 +225,7 @@ INHIBIT_PACKAGE_STRIP[doc] = "If set to "1", causes >> the build to not strip binar >> INHERIT[doc] = "Causes the named class to be inherited at this point >> during parsing. The variable is only valid in configuration files." >> INHERIT_DISTRO[doc] = "Lists classes that will be inherited at the >> distribution level. It is unlikely that you want to edit this variable." >> INITRAMFS_FSTYPES[doc] = "Defines the format for the output image of an >> initial RAM disk (initramfs), which is used during boot." >> -INITRD[doc] = "Indicates a filesystem image to use as an initial RAM >> disk (initrd)." >> +INITRD[doc] = "Indicates list of filesystem images to concatenate and >> use as an initial RAM disk (initrd)." > "a list" > >> INITSCRIPT_NAME[doc] = "The filename of the initialization script as >> installed to ${sysconfdir}/init.d." >> INITSCRIPT_PACKAGES[doc] = "A list of the packages that contain >> initscripts. This variable is used in recipes when using >> update-rc.d.bbclass. The variable is optional and defaults to the PN >> variable." >> INITSCRIPT_PARAMS[doc] = "Specifies the options to pass to update-rc.d. >> The variable is mandatory and is used in recipes when using >> update-rc.d.bbclass." >> -- >> 1.8.1.4 >> >> > Thanks, > Thanks Darren for the review. I think I can improve myself with it. Nitin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] INITRD var: make it a list of filesystem images 2014-08-05 4:33 ` Nitin A Kamble @ 2014-08-05 16:45 ` Hart, Darren 2014-08-05 17:05 ` Nitin A Kamble 0 siblings, 1 reply; 6+ messages in thread From: Hart, Darren @ 2014-08-05 16:45 UTC (permalink / raw) To: Kamble, Nitin A, Openembedded-core@lists.openembedded.org, Burton, Ross, richard.purdie@linuxfoundation.org On 8/4/14, 21:33, "Kamble, Nitin A" <nitin.a.kamble@intel.com> wrote: > >On 8/4/2014 9:38 AM, Hart, Darren wrote: >> On 7/29/14, 11:34, "Kamble, Nitin A" <nitin.a.kamble@intel.com> wrote: ... >> >>> + if [ -n "${INITRD}" ]; then >>> + rm -f $dest/initrd >>> + for fs in ${INITRD} >>> + do >>> + if [ -n "${fs}" ] && [ -s "${fs}" ]; then >> >> The -n test is unnecessary here. How would "for fs in ${INITRD}" result >>in >> an fs of "" ? >The -n test is needed here, it checks whether the file exist or not. Nope, -n tests if the string length is non-zero. See the bash manual section "CONDITIONAL EXPRESSIONS". -s tests if the file exists and has a size > 0. > >> >>> + cat ${fs} >> $dest/ignited >>> + fi >> Some kind of a warning at least is warranted if a file appears in the >> INITRD list but is either 0-size or non-existent. > >I tried to keep the original style of the code. But it makes sense to >add a warning or even an error here. Style is fine, but error checking is a functional thing. If it was missing before, it was a bug. >>> >>> build_iso() { >>> # Only create an ISO if we have an INITRD and NOISO was not set >>> - if [ -z "${INITRD}" ] || [ ! -s "${INITRD}" ] || [ "${NOISO}" = "1" >>>]; >>> then >>> + if [ -z "${INITRD}" ] || [ "${NOISO}" = "1" ]; then >>> bbnote "ISO image will not be created." >>> return >>> Fi >> Perhaps the -s test should be replaced with a -s of $dest/initrd? >The -s test is replaced by the loop few lines below. >>> + # ${INITRD} is a list of multiple filesystem images >>> + for fs in ${INITRD} >>> + do >>> + if [ ! -s "${fs}" ]; then >>> + bbnote "ISO image will not be created. ${fs} is invalid." >>> + return >>> + fi >>> + done >> This additional loop could be eliminated by including this test above. >> Right? Or am I missing something subtle here? >That approach will leave a hole where, the function will continue when >one of the filesystem image is invalid. >So the loop is better here as it does not leave any hole. But you've already built it right? So you have already tested for -s ${fs} previously. The only thing that matters now is that the assembled image is valid. $dest/initrd. Right? -- Darren Hart Open Source Technology Center darren.hart@intel.com Intel Corporation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] INITRD var: make it a list of filesystem images 2014-08-05 16:45 ` Hart, Darren @ 2014-08-05 17:05 ` Nitin A Kamble 0 siblings, 0 replies; 6+ messages in thread From: Nitin A Kamble @ 2014-08-05 17:05 UTC (permalink / raw) To: Hart, Darren, Openembedded-core@lists.openembedded.org, Burton, Ross, richard.purdie@linuxfoundation.org On 8/5/2014 9:45 AM, Hart, Darren wrote: > On 8/4/14, 21:33, "Kamble, Nitin A" <nitin.a.kamble@intel.com> wrote: > >> On 8/4/2014 9:38 AM, Hart, Darren wrote: >>> On 7/29/14, 11:34, "Kamble, Nitin A" <nitin.a.kamble@intel.com> wrote: > ... > >>>> + if [ -n "${INITRD}" ]; then >>>> + rm -f $dest/initrd >>>> + for fs in ${INITRD} >>>> + do >>>> + if [ -n "${fs}" ] && [ -s "${fs}" ]; then >>> The -n test is unnecessary here. How would "for fs in ${INITRD}" result >>> in >>> an fs of "" ? >> The -n test is needed here, it checks whether the file exist or not. > > Nope, -n tests if the string length is non-zero. See the bash manual > section "CONDITIONAL EXPRESSIONS". -s tests if the file exists and has a > size > 0. You are right. it is "-N" which checks for presence of the file. > >>>> + cat ${fs} >> $dest/ignited >>>> + fi >>> Some kind of a warning at least is warranted if a file appears in the >>> INITRD list but is either 0-size or non-existent. >> I tried to keep the original style of the code. But it makes sense to >> add a warning or even an error here. > > Style is fine, but error checking is a functional thing. If it was missing > before, it was a bug. Noted. >>>> build_iso() { >>>> # Only create an ISO if we have an INITRD and NOISO was not set >>>> - if [ -z "${INITRD}" ] || [ ! -s "${INITRD}" ] || [ "${NOISO}" = "1" >>>> ]; >>>> then >>>> + if [ -z "${INITRD}" ] || [ "${NOISO}" = "1" ]; then >>>> bbnote "ISO image will not be created." >>>> return >>>> Fi >>> Perhaps the -s test should be replaced with a -s of $dest/initrd? >> The -s test is replaced by the loop few lines below. >>>> + # ${INITRD} is a list of multiple filesystem images >>>> + for fs in ${INITRD} >>>> + do >>>> + if [ ! -s "${fs}" ]; then >>>> + bbnote "ISO image will not be created. ${fs} is invalid." >>>> + return >>>> + fi >>>> + done >>> This additional loop could be eliminated by including this test above. >>> Right? Or am I missing something subtle here? >> That approach will leave a hole where, the function will continue when >> one of the filesystem image is invalid. >> So the loop is better here as it does not leave any hole. > > But you've already built it right? So you have already tested for -s ${fs} > previously. The only thing that matters now is that the assembled image is > valid. $dest/initrd. Right? No, the dest/initrd is not built at this point. It will be built in the populate function which is called later. so that check will always fail wrongly. I also notice that RP has pulled in part of the commit already, hence I will be making another commit to address the feedback. Thanks, Nitin ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-05 17:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-29 18:34 [PATCH 0/1] A commit to extend the INITRD variable nitin.a.kamble 2014-07-29 18:34 ` [PATCH 1/1] INITRD var: make it a list of filesystem images nitin.a.kamble 2014-08-04 16:38 ` Hart, Darren 2014-08-05 4:33 ` Nitin A Kamble 2014-08-05 16:45 ` Hart, Darren 2014-08-05 17:05 ` Nitin A Kamble
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.