On 02/07/2011 06:57 PM, Matthew Gyurgyik wrote:
On 02/07/2011 04:32 PM, Dieter Plaetinck wrote:
On Sun, 30 Jan 2011 10:24:46 -0500
[email protected] wrote:

The function now stores parameters into var kernel_parameters. The
new function does not set the kernel name (vmlinuz). This must now be
done in the bootloader configuration code.
---
src/core/libs/lib-ui-interactive.sh | 135 +++++++++++++++++++---------------
  1 files changed, 75 insertions(+), 60 deletions(-)

diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh
index 829556a..359a016 100644
--- a/src/core/libs/lib-ui-interactive.sh
+++ b/src/core/libs/lib-ui-interactive.sh
@@ -1045,79 +1045,30 @@ generate_grub_menulst() {
          fi
      fi
      # Now that we have our grub-legacy root device (grubdev).
-    # keep the file from being completely bogus
+    # keep the file from being completely bogus
      [ "$grubdev" = "DEVICE NOT FOUND" ]&&  grubdev=
      if [ -z "$grubdev" ]; then
- notify "Your root boot device could not be autodetected by setup. Ensure you adjust the 'root (hd0,0)' line in your GRUB config accordingly."
-                grubdev="(hd0,0)"
-            fi
- # remove default entries by truncating file at our little tag (#-*)
-            sed -i -e '/#-\*/q' $grubmenu
+ notify "Your root boot device could not be autodetected by setup. Ensure you adjust the 'root (hd0,0)' line in your GRUB config accordingly."
+        grubdev="(hd0,0)"
+    fi

-    # find label or uuid of the root partition
-    case $PART_ACCESS in
-        label)
-            local _label="$(getlabel $_rootpart)"
-            if [ -n "${_label}" ]; then
-                _rootpart="/dev/disk/by-label/${_label}"
-            fi
-            ;;
-        uuid)
-            local _uuid="$(getuuid $_rootpart)"
-            if [ -n "${_uuid}" ]; then
-                _rootpart="/dev/disk/by-uuid/${_uuid}"
-            fi
-            ;;
-    esac
+ # remove default entries by truncating file at our little tag (#-*)
+    sed -i -e '/#-\*/q' $grubmenu

- # handle dmraid/mdadm,lvm,dm_crypt etc. replace entries where needed automatically
-        kernel="kernel $subdir/vmlinuz26 root=${_rootpart} ro"
-        if get_anchestors_mount ';/;'
-        then
- if echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$'&& echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'raw$'
-            then
- debug 'FS' 'Grub kernel line? Found / on dm_crypt on raw' - raw_device=`echo "$ANSWER_DEVICES" | sed -n '2p' | cut -d ' ' -f1` - crypt_device=`echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1` - kernel="kernel $subdir/vmlinuz26 root=$crypt_device cryptdevice=$raw_device:`basename $crypt_device` ro" - elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'lvm-lv$'&& echo "$ANSWER_DEVICES" | sed -n '4p' | grep -q 'dm_crypt$'&& echo "$ANSWER_DEVICES" | sed -n '5p' | grep -q 'raw$'
-            then
- debug 'FS' 'Grub kernel line? Found / on lvm on dm_crypt on raw' - lv_device=`echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1` - crypt_device=`echo "$ANSWER_DEVICES" | sed -n '4p' | cut -d ' ' -f1` - raw_device=`echo "$ANSWER_DEVICES" | sed -n '5p' | cut -d ' ' -f1` - kernel="kernel $subdir/vmlinuz26 root=$lv_device cryptdevice=$raw_device:`basename $crypt_device` ro" - elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$'&& echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'lvm-lv$'&& echo "$ANSWER_DEVICES" | sed -n '5p' | grep -q 'raw$'
-            then
- debug 'FS' 'Grub kernel line? Found / on dm_crypt on lvm on raw' - crypt_device=`echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1` - lv_device=`echo "$ANSWER_DEVICES" | sed -n '2p' | cut -d ' ' -f1` - kernel="kernel $subdir/vmlinuz26 root=$crypt_device cryptdevice=$lv_device:`basename $crypt_device` ro"
-            elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'raw$'
-            then
-                debug 'FS' 'Grub kernel line? Found / on raw'
- elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'lvm-lv$'&& echo "$ANSWER_DEVICES" | sed -n '4p' | grep -q 'raw$'
-            then
-                debug 'FS' 'Grub kernel line? Found / on lvm on raw'
-            else
- debug 'FS' 'Grub kernel line? Could not figure this one out' - show_warning "Disk setup detection" "Are you using some really fancy dm_crypt/lvm/softraid setup?\nI could not figure it out, so the kernel line will be the default: you'll probably need to edit it.\nPlease file a bug for this and supply all files from /tmp/aif/"
-            fi
-        else
- show_warning "Disk setup detection" "Could not find out where your / is. Did you setup filesystems/blockdevices? manual/autoprepare? If yes, please file a bug and tell us about this"
-        fi
-            cat>>$grubmenu<<EOF
+    get_kernel_parameters || return $?
+
+    cat>>$grubmenu<<EOF

  # (0) Arch Linux
  title  Arch Linux
  root   $grubdev
-$kernel
+kernel $subdir/vmlinuz26 $kernel_parameters
  initrd $subdir/kernel26.img

  # (1) Arch Linux
  title  Arch Linux Fallback
  root   $grubdev
-$kernel
+kernel $subdir/vmlinuz26 $kernel_parameters
  initrd $subdir/kernel26-fallback.img

  # (2) Windows
@@ -1179,6 +1130,70 @@ EOF
      fi
  }

+get_kernel_parameters()
+{
+    get_device_with_mount '/' || return 1
+    local _rootpart="$ANSWER_DEVICE"
+
+    case "$PART_ACCESS" in
+        label)
+            local _label="$(getlabel $_rootpart)"
+            if [[ -n "${_label}" ]]; then
why the [[ ]]?
It is recommended to use [[ ]] in bash when we are not trying to right a POSIX compliant script. However, [[ ]] implies -n by default. Therefore, I shortened the code as so: [[ "${_label}" ]] && _rootpart="/dev/disk/by-label/${_label}"
+                _rootpart="/dev/disk/by-label/${_label}"
+            fi
+        ;;
+        uuid)
+            local _uuid="$(getuuid $_rootpart)"
+            if [[ -n "${_uuid}" ]]; then
same here.
+                _rootpart="/dev/disk/by-uuid/${_uuid}"
+            fi
+        ;;
+    esac
+
+    kernel_parameters="root=$_rootpart ro"
+
+    # No seperate /boot partition
+    if [[ -z "$bootdev" ]]; then
+        subdir='/boot'
+    fi
this branch seems useless in this function. the variable $subdir is not used here?

I was thinking that we could do this assignment here and use it for both the grub and Syslinux menu generation code. However, now looking at it, $subdir would have to be global variable. Since the check is simple and can be condensed to one line of code [[ -z "$bootdev" ]] && local subdir="/boot" it would make more sense to add the assigment in generate_grub_menulst and generate_syslinux_menu. That code isn't very likely to change or get modified.


-----------------------------------

    local raw_device crypt_device lv_device

What do you think of making these variables local? As far as I can tell they are not used anywhere else in the code.
+
+    if get_anchestors_mount ';/;'
+    then
+ if echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$'&& echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'raw$'
messed up indentation here.
Woops, fixed. IIRC that is the way it was originally.


I will reply to this email, with a patch of these changes so you can see the code that has changed.

My github commit: https://github.com/pyther/aif/commit/9a3da9565c1308aa708ce917de48cccdda1f8182 I rebased and forced a push to github, therefore the above commit has the updated code.

------

Use local variables for raw_device, crypt_device, lv_device

Fixed indentation and moved subdir assigment to generate_grub_menulst
---
src/core/libs/lib-ui-interactive.sh | 80 ++++++++++++++++------------------
 1 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh
index 359a016..e0ad6c9 100644
--- a/src/core/libs/lib-ui-interactive.sh
+++ b/src/core/libs/lib-ui-interactive.sh
@@ -1057,6 +1057,9 @@ generate_grub_menulst() {

     get_kernel_parameters || return $?

+    # No seperate /boot partition
+    [[ -z "$bootdev" ]] && local subdir="/boot"
+
     cat >>$grubmenu <<EOF

 # (0) Arch Linux
@@ -1132,62 +1135,55 @@ EOF

 get_kernel_parameters()
 {
+
     get_device_with_mount '/' || return 1
     local _rootpart="$ANSWER_DEVICE"

     case "$PART_ACCESS" in
         label)
             local _label="$(getlabel $_rootpart)"
-            if [[ -n "${_label}" ]]; then
-                _rootpart="/dev/disk/by-label/${_label}"
-            fi
+            [[ "${_label}" ]] && _rootpart="/dev/disk/by-label/${_label}"
         ;;
         uuid)
             local _uuid="$(getuuid $_rootpart)"
-            if [[ -n "${_uuid}" ]]; then
-                _rootpart="/dev/disk/by-uuid/${_uuid}"
-            fi
+            [[ "${_uuid}" ]] &&    _rootpart="/dev/disk/by-uuid/${_uuid}"
         ;;
     esac

     kernel_parameters="root=$_rootpart ro"

-    # No seperate /boot partition
-    if [[ -z "$bootdev" ]]; then
-        subdir='/boot'
-    fi
-
-    if get_anchestors_mount ';/;'
-    then
- if echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$' && echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'raw$'
-        then
-            debug 'FS' 'get_kernel_parameters - Found / on dm_crypt on raw'
- raw_device="$(echo "$ANSWER_DEVICES" | sed -n '2p' | cut -d ' ' -f1)" - crypt_device="$(echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1)" - kernel_parameters="root=$crypt_device cryptdevice=$raw_device:`basename $crypt_device` ro" - elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'lvm-lv$' && echo "$ANSWER_DEVICES" | sed -n '4p' | grep -q 'dm_crypt$' && echo "$ANSWER_DEVICES" | sed -n '5p' | grep -q 'raw$'
-        then
- debug 'FS' 'get_kernel_parameters - Found / on lvm on dm_crypt on raw' - lv_device="$(echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1)" - crypt_device="$(echo "$ANSWER_DEVICES" | sed -n '4p' | cut -d ' ' -f1)" - raw_device="$(echo "$ANSWER_DEVICES" | sed -n '5p' | cut -d ' ' -f1)" - kernel_parameters="root=$lv_device cryptdevice=$raw_device:`basename $crypt_device` ro" - elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$' && echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'lvm-lv$' && echo "$ANSWER_DEVICES" | sed -n '5p' | grep -q 'raw$'
-        then
- debug 'FS' 'get_kernel_parameters - Found / on dm_crypt on lvm on raw' - crypt_device="$(echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1)" - lv_device="$(echo "$ANSWER_DEVICES" | sed -n '2p' | cut -d ' ' -f1)" - kernel_parameters="root=$crypt_device cryptdevice=$lv_device:`basename $crypt_device` ro"
-        elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'raw$'
-        then
-            debug 'FS' 'get_kernel_parameters - Found / on raw'
- elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'lvm-lv$' && echo "$ANSWER_DEVICES" | sed -n '4p' | grep -q 'raw$'
-        then
-            debug 'FS' 'get_kernel_parameters - Found / on lvm on raw'
-        else
- debug 'FS' 'get_kernel_parameters - Could not figure this one out' - show_warning "Disk setup detection" "Are you using some really fancy dm_crypt/lvm/softraid setup?\nI could not figure it out, so the kernel line will be the default: you'll probably need to edit it.\nPlease file a bug for this and supply all files from /tmp/aif/"
-    fi
+    local raw_device crypt_device lv_device
+
+    if get_anchestors_mount ';/;'; then
+ if echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$' && echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'raw$'
+            then
+ debug 'FS' 'get_kernel_parameters - Found / on dm_crypt on raw' + raw_device="$(echo "$ANSWER_DEVICES" | sed -n '2p' | cut -d ' ' -f1)" + crypt_device="$(echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1)" + kernel_parameters="root=$crypt_device cryptdevice=$raw_device:`basename $crypt_device` ro" + elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'lvm-lv$' && echo "$ANSWER_DEVICES" | sed -n '4p' | grep -q 'dm_crypt$' && echo "$ANSWER_DEVICES" | sed -n '5p' | grep -q 'raw$'
+            then
+ debug 'FS' 'get_kernel_parameters - Found / on lvm on dm_crypt on raw' + lv_device="$(echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1)" + crypt_device="$(echo "$ANSWER_DEVICES" | sed -n '4p' | cut -d ' ' -f1)" + raw_device="$(echo "$ANSWER_DEVICES" | sed -n '5p' | cut -d ' ' -f1)" + kernel_parameters="root=$lv_device cryptdevice=$raw_device:`basename $crypt_device` ro" + elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'dm_crypt$' && echo "$ANSWER_DEVICES" | sed -n '2p' | grep -q 'lvm-lv$' && echo "$ANSWER_DEVICES" | sed -n '5p' | grep -q 'raw$'
+            then
+ debug 'FS' 'get_kernel_parameters - Found / on dm_crypt on lvm on raw' + crypt_device="$(echo "$ANSWER_DEVICES" | sed -n '1p' | cut -d ' ' -f1)" + lv_device="$(echo "$ANSWER_DEVICES" | sed -n '2p' | cut -d ' ' -f1)" + kernel_parameters="root=$crypt_device cryptdevice=$lv_device:`basename $crypt_device` ro"
+            elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'raw$'
+            then
+                debug 'FS' 'get_kernel_parameters - Found / on raw'
+ elif echo "$ANSWER_DEVICES" | sed -n '1p' | grep -q 'lvm-lv$' && echo "$ANSWER_DEVICES" | sed -n '4p' | grep -q 'raw$'
+            then
+                debug 'FS' 'get_kernel_parameters - Found / on lvm on raw'
+            else
+ debug 'FS' 'get_kernel_parameters - Could not figure this one out' + show_warning "Disk setup detection" "Are you using some really fancy dm_crypt/lvm/softraid setup?\nI could not figure it out, so the kernel line will be the default: you'll probably need to edit it.\nPlease file a bug for this and supply all files from /tmp/aif/"
+        fi
     else
show_warning "Disk setup detection" "Could not find out where your / is. Did you setup filesystems/blockdevices? manual/autoprepare? If yes, please file a bug and tell us about this"
         return 1
--
1.7.4

Reply via email to