[arch-projects] [MKINITCPIO][PATCH 1/2] Fix save build directory with presets
Current behaviour of -s option of mkinitcpio is bad when presets are used. mkinitcpio saves main process build directory which is always empty when preset option is used. mkinitcpio doesn't save preset sub processes build directories which is the expected behaviour. This patch fix the two previous points by: - inherit -s parameter inside preset sub processes. - force main process (in case of preset) to clean build directory Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- mkinitcpio | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/mkinitcpio b/mkinitcpio index 49dabbd..53254d7 100755 --- a/mkinitcpio +++ b/mkinitcpio @@ -64,7 +64,7 @@ cleanup () { if [[ $TMPDIR ]]; then if (( $SAVELIST )); then - msg "build directory saved in %s" "$TMPDIR" + msg "Build directory saved in %s" "$TMPDIR" else rm -rf "$TMPDIR" fi @@ -198,6 +198,7 @@ if [[ $PRESET ]]; then # Use -b, -m and -v options specified earlier declare -a preset_mkopts preset_cmd [[ $BASEDIR ]] && preset_mkopts+=(-b "$BASEDIR") + (( SAVELIST )) && preset_mkopts+=(-s) (( QUIET )) || preset_mkopts+=(-v) # Build all images . "$PRESET" @@ -237,7 +238,8 @@ if [[ $PRESET ]]; then msg2 "${preset_cmd[*]}" "$0" "${preset_cmd[@]}" done - cleanup 0 + # Main process never need to keep builded directory + SAVELIST=0 cleanup 0 else die "Preset not found: \`%s'" "$PRESET" fi -- Sebastien "Seblu" Luttringer
This allow initcpio to boot vm which use virtio as network/disk backend. Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')" +} + +help () +{ +cat<<HELPEOF + This hook loads the necessary modules for a virtio root device. + Detection will take place at runtime. To minimize the modules + in the image, add the autodetect hook too. +HELPEOF +} + +# vim: set ft=sh ts=4 sw=4 et: -- Sebastien "Seblu" Luttringer
On Fri, Sep 16, 2011 at 02:39:44PM +0200, Sebastien Luttringer wrote:
This allow initcpio to boot vm which use virtio as network/disk backend.
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio
diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')"
checked_modules can take multiple parameters: MODULES=$(checked_modules virtio_{blk,pci})
+} + +help () +{ +cat<<HELPEOF + This hook loads the necessary modules for a virtio root device. + Detection will take place at runtime. To minimize the modules + in the image, add the autodetect hook too. +HELPEOF +} + +# vim: set ft=sh ts=4 sw=4 et:
I'd appreciate it if you could follow the newer style shown in the autodetect or base hook. I've been making an effort to clean these all up to match the remainder of the codebase: https://github.com/falconindy/mkinitcpio/commit/d0e2a180 d
On Fri, Sep 16, 2011 at 2:50 PM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 16, 2011 at 02:39:44PM +0200, Sebastien Luttringer wrote:
This allow initcpio to boot vm which use virtio as network/disk backend.
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio
diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')"
checked_modules can take multiple parameters:
MODULES=$(checked_modules virtio_{blk,pci})
This doesn't work but something like "virtio_\(blk\|pci\)" do the job.
+} + +help () +{ +cat<<HELPEOF + This hook loads the necessary modules for a virtio root device. + Detection will take place at runtime. To minimize the modules + in the image, add the autodetect hook too. +HELPEOF +} + +# vim: set ft=sh ts=4 sw=4 et:
I'd appreciate it if you could follow the newer style shown in the autodetect or base hook. I've been making an effort to clean these all up to match the remainder of the codebase:
https://github.com/falconindy/mkinitcpio/commit/d0e2a180
d
sure -- Sébastien Luttringer www.seblu.net
On Fri, Sep 16, 2011 at 9:31 AM, Seblu <seblu@seblu.net> wrote:
On Fri, Sep 16, 2011 at 2:50 PM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 16, 2011 at 02:39:44PM +0200, Sebastien Luttringer wrote:
This allow initcpio to boot vm which use virtio as network/disk backend.
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio
diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')"
checked_modules can take multiple parameters:
MODULES=$(checked_modules virtio_{blk,pci})
This doesn't work but something like "virtio_\(blk\|pci\)" do the job. Define "doesn't work"? Because your solution doesn't work:
$ echo checked_modules 'virtio_\(blk\|pci\)' checked_modules virtio_\(blk\|pci\) $ echo checked_modules virtio_{blk,pci} checked_modules virtio_blk virtio_pci -Dan
On Fri, Sep 16, 2011 at 09:42:21AM -0400, Dan McGee wrote:
On Fri, Sep 16, 2011 at 9:31 AM, Seblu <seblu@seblu.net> wrote:
On Fri, Sep 16, 2011 at 2:50 PM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 16, 2011 at 02:39:44PM +0200, Sebastien Luttringer wrote:
This allow initcpio to boot vm which use virtio as network/disk backend.
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio
diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')"
checked_modules can take multiple parameters:
MODULES=$(checked_modules virtio_{blk,pci})
This doesn't work but something like "virtio_\(blk\|pci\)" do the job. Define "doesn't work"? Because your solution doesn't work:
$ echo checked_modules 'virtio_\(blk\|pci\)' checked_modules virtio_\(blk\|pci\) $ echo checked_modules virtio_{blk,pci} checked_modules virtio_blk virtio_pci
checked_modules() invokes all_modules() which calls grep(1) to filter modules, so using a regular expression here should work (although it isn't perfect).
On Fri, Sep 16, 2011 at 03:48:22PM +0200, Lukas Fleischer wrote:
On Fri, Sep 16, 2011 at 09:42:21AM -0400, Dan McGee wrote:
On Fri, Sep 16, 2011 at 9:31 AM, Seblu <seblu@seblu.net> wrote:
On Fri, Sep 16, 2011 at 2:50 PM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 16, 2011 at 02:39:44PM +0200, Sebastien Luttringer wrote:
This allow initcpio to boot vm which use virtio as network/disk backend.
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio
diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')"
checked_modules can take multiple parameters:
MODULES=$(checked_modules virtio_{blk,pci})
This doesn't work but something like "virtio_\(blk\|pci\)" do the job. Define "doesn't work"? Because your solution doesn't work:
$ echo checked_modules 'virtio_\(blk\|pci\)' checked_modules virtio_\(blk\|pci\) $ echo checked_modules virtio_{blk,pci} checked_modules virtio_blk virtio_pci
checked_modules() invokes all_modules() which calls grep(1) to filter modules, so using a regular expression here should work (although it isn't perfect).
Yup, and we use this elsewhere. My mistake in reading over the function. d
On Fri, Sep 16, 2011 at 3:42 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Fri, Sep 16, 2011 at 9:31 AM, Seblu <seblu@seblu.net> wrote:
On Fri, Sep 16, 2011 at 2:50 PM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 16, 2011 at 02:39:44PM +0200, Sebastien Luttringer wrote:
This allow initcpio to boot vm which use virtio as network/disk backend.
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- install/virtio | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 install/virtio
diff --git a/install/virtio b/install/virtio new file mode 100644 index 0000000..12503e9 --- /dev/null +++ b/install/virtio @@ -0,0 +1,15 @@ +build() { + MODULES="$(checked_modules 'virtio_blk')" + MODULES+=" $(checked_modules 'virtio_pci')"
checked_modules can take multiple parameters:
MODULES=$(checked_modules virtio_{blk,pci})
This doesn't work but something like "virtio_\(blk\|pci\)" do the job. Define "doesn't work"? Because your solution doesn't work: I tested carrefuly before posting.
$ echo checked_modules 'virtio_\(blk\|pci\)' checked_modules virtio_\(blk\|pci\) $ echo checked_modules virtio_{blk,pci} checked_modules virtio_blk virtio_pci
If we use bash exp, 2 args will be passed to check modules, which will pass 2 args to all_modules, which will pass 2 args to grep and fail. ==> Building image from preset: 'default' -> -k /boot/vmlinuz-linux -c /etc/mkinitcpio.conf -g /boot/initramfs-linux.img ==> Starting build: 3.0-ARCH -> Parsing hook: [base] -> Parsing hook: [udev] -> Parsing hook: [autodetect] -> Parsing hook: [pata] -> Parsing hook: [scsi] -> Parsing hook: [sata] -> Parsing hook: [virtio] grep: virtio_pci: No such file or directory -> Parsing hook: [filesystems] -> Parsing hook: [usbinput] ==> Generating module dependencies Lukas, do you have concrete suggestions to improve it? -- Sébastien Luttringer www.seblu.net
On Fri, Sep 16, 2011 at 02:39:43PM +0200, Sebastien Luttringer wrote:
Current behaviour of -s option of mkinitcpio is bad when presets are used.
mkinitcpio saves main process build directory which is always empty when preset option is used. mkinitcpio doesn't save preset sub processes build directories which is the expected behaviour.
I'm not really sure I see the point of this. Is there a use case other than debugging where this is wanted? d
This patch fix the two previous points by: - inherit -s parameter inside preset sub processes. - force main process (in case of preset) to clean build directory
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- mkinitcpio | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/mkinitcpio b/mkinitcpio index 49dabbd..53254d7 100755 --- a/mkinitcpio +++ b/mkinitcpio @@ -64,7 +64,7 @@ cleanup () { if [[ $TMPDIR ]]; then if (( $SAVELIST )); then - msg "build directory saved in %s" "$TMPDIR" + msg "Build directory saved in %s" "$TMPDIR" else rm -rf "$TMPDIR" fi @@ -198,6 +198,7 @@ if [[ $PRESET ]]; then # Use -b, -m and -v options specified earlier declare -a preset_mkopts preset_cmd [[ $BASEDIR ]] && preset_mkopts+=(-b "$BASEDIR") + (( SAVELIST )) && preset_mkopts+=(-s) (( QUIET )) || preset_mkopts+=(-v) # Build all images . "$PRESET" @@ -237,7 +238,8 @@ if [[ $PRESET ]]; then msg2 "${preset_cmd[*]}" "$0" "${preset_cmd[@]}" done - cleanup 0 + # Main process never need to keep builded directory
s/need/needs/ s/builded/build/
+ SAVELIST=0 cleanup 0 else die "Preset not found: \`%s'" "$PRESET" fi -- Sebastien "Seblu" Luttringer
On Fri, Sep 16, 2011 at 2:57 PM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Sep 16, 2011 at 02:39:43PM +0200, Sebastien Luttringer wrote:
Current behaviour of -s option of mkinitcpio is bad when presets are used.
mkinitcpio saves main process build directory which is always empty when preset option is used. mkinitcpio doesn't save preset sub processes build directories which is the expected behaviour.
I'm not really sure I see the point of this. Is there a use case other than debugging where this is wanted? Only debug when you use preset.
d
This patch fix the two previous points by: - inherit -s parameter inside preset sub processes. - force main process (in case of preset) to clean build directory
Signed-off-by: Sebastien Luttringer <seblu@seblu.net> --- mkinitcpio | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/mkinitcpio b/mkinitcpio index 49dabbd..53254d7 100755 --- a/mkinitcpio +++ b/mkinitcpio @@ -64,7 +64,7 @@ cleanup () { if [[ $TMPDIR ]]; then if (( $SAVELIST )); then - msg "build directory saved in %s" "$TMPDIR" + msg "Build directory saved in %s" "$TMPDIR" else rm -rf "$TMPDIR" fi @@ -198,6 +198,7 @@ if [[ $PRESET ]]; then # Use -b, -m and -v options specified earlier declare -a preset_mkopts preset_cmd [[ $BASEDIR ]] && preset_mkopts+=(-b "$BASEDIR") + (( SAVELIST )) && preset_mkopts+=(-s) (( QUIET )) || preset_mkopts+=(-v) # Build all images . "$PRESET" @@ -237,7 +238,8 @@ if [[ $PRESET ]]; then msg2 "${preset_cmd[*]}" "$0" "${preset_cmd[@]}" done - cleanup 0 + # Main process never need to keep builded directory
s/need/needs/ s/builded/build/
+ SAVELIST=0 cleanup 0 else die "Preset not found: \`%s'" "$PRESET" fi -- Sebastien "Seblu" Luttringer
-- Sébastien Luttringer www.seblu.net
participants (5)
-
Dan McGee
-
Dave Reisner
-
Lukas Fleischer
-
Sebastien Luttringer
-
Seblu