From 4418f59e32858d7badcc8e9b23dd6c26b26c42f0 Mon Sep 17 00:00:00 2001 From: Dieter Plaetinck Date: Sun, 18 Apr 2010 11:44:00 +0200 Subject: implement consistent/generic, more stable {en,de}coding of fs_params and fs_opts. this should fix some bugs with filesystem creating failing when options were specified --- examples/fancy-install-on-sda | 2 +- src/core/libs/lib-blockdevices-filesystems.sh | 62 +++++++++++----------- src/core/libs/lib-ui-interactive.sh | 37 ++++++------- .../automatic-dmcrypt-lvm-install-sda/profile | 2 +- .../automatic-lvm-dmcrypt-install-sda/profile | 2 +- unofficial/dieter-desktop-a7n8x.automaticprofile | 2 +- 6 files changed, 50 insertions(+), 57 deletions(-) diff --git a/examples/fancy-install-on-sda b/examples/fancy-install-on-sda index 86ba343..b5b316c 100644 --- a/examples/fancy-install-on-sda +++ b/examples/fancy-install-on-sda @@ -28,7 +28,7 @@ worker_configure_system () { GRUB_DEVICE=/dev/sda PARTITIONS='/dev/sda 100:ext2:+ *:ext4' BLOCKDATA='/dev/sda1 raw no_label ext2;yes;/boot;target;no_opts;no_label;no_params -/dev/sda2 raw no_label dm_crypt;yes;no_mountpoint;target;no_opts;sda2crypt;-c_aes-xts-plain_-y_-s_512 +/dev/sda2 raw no_label dm_crypt;yes;no_mountpoint;target;no_opts;sda2crypt;-c__aes-xts-plain__-y__-s__512 /dev/mapper/sda2crypt dm_crypt no_label lvm-pv;yes;no_mountpoint;target;no_opts;no_label;no_params /dev/mapper/sda2crypt+ lvm-pv no_label lvm-vg;yes;no_mountpoint;target;no_opts;cryptpool;/dev/mapper/sda2crypt /dev/mapper/cryptpool lvm-vg cryptpool lvm-lv;yes;no_mountpoint;target;no_opts;cryptswap;1G|lvm-lv;yes;no_mountpoint;target;no_opts;cryptroot;5G|lvm-lv;yes;no_mountpoint;target;no_opts;crypthome;5G diff --git a/src/core/libs/lib-blockdevices-filesystems.sh b/src/core/libs/lib-blockdevices-filesystems.sh index 0022128..3ebdc75 100644 --- a/src/core/libs/lib-blockdevices-filesystems.sh +++ b/src/core/libs/lib-blockdevices-filesystems.sh @@ -17,10 +17,11 @@ # NOTE: filesystems that span multiple underlying filesystems/devices (eg lvm VG) should specify those in params, separated by colons. \ # the in the beginning doesn't matter much, it can be pretty much any device, or not existent, i think. But it's probably best to make it one of the devices listed in params # no '+' characters allowed for devices in $fs_params (eg use the real names) - +# NOTE: if you want spaces in params or opts, you must "encode" them by replacing them with 2 underscores. # -- ADDITIONAL INTERNAL FORMATS -- # $TMP_FILESYSTEMS: each filesystem on a separate line, so block devices can appear multiple times be on multiple lines (eg LVM volumegroups with more lvm LV's) +# uses spaces for separation (so you can easily do 'while.. read' loops. so fs_params and fs_opts are encoded here also # part part_type part_label fs_type fs_create fs_mountpoint fs_mount fs_opts fs_label fs_params @@ -410,6 +411,7 @@ process_disk () } # $1 fs_string +# $2 decode yes/no parse_filesystem_string () { fs="$1" @@ -420,6 +422,16 @@ parse_filesystem_string () fs_opts=` cut -d ';' -f 5 <<< $fs` fs_label=` cut -d ';' -f 6 <<< $fs` fs_params=` cut -d ';' -f 7 <<< $fs` + if [ "$2" == 'yes' ]; then + fs_opts="${fs_opts//__/ }" + fs_params="${fs_params//__/ }" + [ "$fs_type" = no_type ] && fs_type= + [ "$fs_mountpoint" = no_mountpoint ] && fs_mountpoint= + [ "$fs_mount" = no_mount ] && fs_mount= + [ "$fs_opts" = no_opts ] && fs_opts= + [ "$fs_label" = no_label ] && fs_label= + [ "$fs_params" = no_params ] && fs_params= + fi } @@ -477,7 +489,7 @@ process_filesystems () debug 'FS' "$fs_id ->Still need to do it: Making the filesystem on a non-pv volume" inform "Making $fs_type filesystem on $part" disks process_filesystem $part $fs_type $fs_create $fs_mountpoint no_mount $fs_opts $fs_label $fs_params && done_filesystems+=("$fs_id") || returncode=1 - elif [ "$part_type" = lvm-pv ] && pvdisplay ${fs_params//:/ } >/dev/null # $part is a lvm PV. all needed lvm pv's exist. note that pvdisplay exits 5 as long as one of the args doesn't exist + elif [ "$part_type" = lvm-pv ] && pvdisplay ${fs_params//__/ } >/dev/null # $part is a lvm PV. all needed lvm pv's exist. note that pvdisplay exits 5 as long as one of the args doesn't exist then debug 'FS' "$fs_id ->Still need to do it: Making the filesystem on a pv volume" inform "Making $fs_type filesystem on $part" disks @@ -681,6 +693,7 @@ rollback_filesystems () # make a filesystem on a blockdevice and mount if needed. +# parameters: (all encoded) # $1 partition # $2 fs_type # $3 fs_create (optional. defaults to yes) @@ -696,20 +709,8 @@ process_filesystem () [ -z "$2" ] && die_error "process_filesystem needs a filesystem type as \$2" debug 'FS' "process_filesystem $@" local ret=0 - - part=$1 - fs_type=$2 - fs_create=${3:-yes} - fs_mountpoint=${4:-no_mountpoint} - fs_mount=${5:-no_mount} - fs_opts=${6:-no_opts} - fs_label=${7:-no_label} - fs_params=${8:-no_params} - [ "$fs_mountpoint" = no_mountpoint ] && fs_mountpoint= - [ "$fs_mount" = no_mount ] && fs_mount= - [ "$fs_opts" = no_opts ] && fs_opts= - [ "$fs_label" = no_label ] && fs_label= - [ "$fs_params" = no_params ] && fs_params= + part=$1 + parse_filesystem_string "$2;$3;$4;$5;$6;$7;$8" yes # Create the FS if [ "$fs_create" = yes ] @@ -722,31 +723,30 @@ process_filesystem () [ -z "$fs_label" ] && [ "$fs_type" = lvm-vg -o "$fs_type" = lvm-pv ] && fs_label=default #TODO. implement the incrementing numbers label for lvm vg's and lv's #TODO: health checks on $fs_params etc - case ${fs_type} in #TODO implement options (opts, fs_opts, whatever) - xfs) mkfs.xfs -f $part $opts >$LOG 2>&1; ret=$? ;; - jfs) yes | mkfs.jfs $part $opts >$LOG 2>&1; ret=$? ;; - reiserfs) yes | mkreiserfs $part $opts >$LOG 2>&1; ret=$? ;; - ext2) mke2fs "$part" $opts >$LOG 2>&1; ret=$? ;; - ext3) mke2fs -j $part $opts >$LOG 2>&1; ret=$? ;; - ext4) mkfs.ext4 $part $opts >$LOG 2>&1; ret=$? ;; #TODO: installer.git uses mke2fs -t ext4 -O dir_index,extent,uninit_bg , which is best? - vfat) mkfs.vfat $part $opts >$LOG 2>&1; ret=$? ;; + case ${fs_type} in + xfs) mkfs.xfs -f $part $fs_opts >$LOG 2>&1; ret=$? ;; + jfs) yes | mkfs.jfs $part $fs_opts >$LOG 2>&1; ret=$? ;; + reiserfs) yes | mkreiserfs $part $fs_opts >$LOG 2>&1; ret=$? ;; + ext2) mke2fs "$part" $fs_opts >$LOG 2>&1; ret=$? ;; + ext3) mke2fs -j $part $fs_opts >$LOG 2>&1; ret=$? ;; + ext4) mkfs.ext4 $part $fs_opts >$LOG 2>&1; ret=$? ;; #TODO: installer.git uses mke2fs -t ext4 -O dir_index,extent,uninit_bg , which is best? + vfat) mkfs.vfat $part $fs_opts >$LOG 2>&1; ret=$? ;; swap) if [ -z "$fs_label" ]; then - mkswap $part $opts >$LOG 2>&1; ret=$? + mkswap $part $fs_opts >$LOG 2>&1; ret=$? else - mkswap -L $fs_label $part $opts >$LOG 2>&1; ret=$? + mkswap -L $fs_label $part $fs_opts >$LOG 2>&1; ret=$? fi ;; dm_crypt) [ -z "$fs_params" ] && fs_params='-c aes-xts-plain -y -s 512'; - fs_params=${fs_params//_/ } inform "Please enter your passphrase to encrypt the device (with confirmation)" - cryptsetup $fs_params $opts luksFormat -q $part >$LOG 2>&1 < /dev/tty ; ret=$? #hack to give cryptsetup the approriate stdin. keep in mind we're in a loop (see process_filesystems where something else is on stdin) + cryptsetup $fs_params $fs_opts luksFormat -q $part >$LOG 2>&1 < /dev/tty ; ret=$? #hack to give cryptsetup the approriate stdin. keep in mind we're in a loop (see process_filesystems where something else is on stdin) inform "Please enter your passphrase to unlock the device" cryptsetup luksOpen $part $fs_label >$LOG 2>&1 < /dev/tty; ret=$? || ( show_warning 'cryptsetup' "Error luksOpening $part on /dev/mapper/$fs_label" ) ;; lvm-pv) pvcreate $fs_opts $part >$LOG 2>&1; ret=$? ;; - lvm-vg) # $fs_params: ':'-separated list of PV's - vgcreate $fs_opts $fs_label ${fs_params//:/ } >$LOG 2>&1; ret=$? ;; + lvm-vg) # $fs_params: list of PV's + vgcreate $fs_opts $fs_label $fs_params >$LOG 2>&1; ret=$? ;; lvm-lv) # $fs_params = size string (eg '5G') - lvcreate -L $fs_params $fs_opts -n $fs_label `sed 's#/dev/mapper/##' <<< $part` >$LOG 2>&1; ret=$? ;; #$opts is usually something like -L 10G # Strip '/dev/mapper/' part because device file may not exist. TODO: do i need to activate them? + lvcreate -L $fs_params $fs_opts -n $fs_label `sed 's#/dev/mapper/##' <<< $part` >$LOG 2>&1; ret=$? ;; #$fs_opts is usually something like -L 10G # Strip '/dev/mapper/' part because device file may not exist. TODO: do i need to activate them? # don't handle anything else here, we will error later esac # The udevadm settle is a workaround for a bug/racecondition in cryptsetup. See: diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh index 15a752a..50fee6d 100644 --- a/src/core/libs/lib-ui-interactive.sh +++ b/src/core/libs/lib-ui-interactive.sh @@ -357,6 +357,7 @@ interactive_partition() { # but I think it's better to go through them all and by default always show the previous choice. interactive_filesystem () { + # note: fs_mount: we dont need to ask this to the user. this is always 'target' for 99.99% of the users local part=$1 # must be given and (scheduled to become) a valid device -> don't do [ -b "$1" ] because the device might not exist *yet* local part_type=$2 # a part should always have a type local part_label=$3 # can be empty @@ -371,26 +372,14 @@ interactive_filesystem () NEW_FILESYSTEM= if [ -n "$fs_string" ] then - fs_type=` cut -d ';' -f 1 <<< $fs_string` - fs_create=` cut -d ';' -f 2 <<< $fs_string` #not asked for to the user. this is always 'yes' for now - fs_mountpoint=` cut -d ';' -f 3 <<< $fs_string` - fs_mount=` cut -d ';' -f 4 <<< $fs_string` #we dont need to ask this to the user. this is always 'target' for 99.99% of the users - fs_opts=` cut -d ';' -f 5 <<< $fs_string` - fs_label=` cut -d ';' -f 6 <<< $fs_string` - fs_params=` cut -d ';' -f 7 <<< $fs_string` - [ "$fs_type" = no_type ] && fs_type= - [ "$fs_mountpoint" = no_mountpoint ] && fs_mountpoint= - [ "$fs_mount" = no_mount ] && fs_mount= - [ "$fs_opts" = no_opts ] && fs_opts= - [ "$fs_label" = no_label ] && fs_label= - [ "$fs_params" = no_params ] && fs_params= + parse_filesystem_string "$fs_string" yes local old_fs_type=$fs_type local old_fs_create=$fs_create local old_fs_mountpoint=$fs_mountpoint local old_fs_mount=$fs_mount - local old_fs_opts=$fs_opts + local old_fs_opts="$fs_opts" local old_fs_label=$fs_label - local old_fs_params=$fs_params + local old_fs_params="$fs_params" ask_option edit "Change $fs_type filesystem settings on $part ?" \ "Change $fs_type filesystem settings (create:$fs_create, label:$fs_label, mountpoint:$fs_mountpoint) on $part (type:$part_type, label:$part_label) ?" required \ @@ -488,9 +477,13 @@ interactive_filesystem () then # add $part to $fs_params if it's not in there because the user wants this enabled by default. TODO: we should find something out so you can't disable $part. (would be weird to have a vg listed on $part and not have $part it fs_params) pv=${part/+/} - grep -q ":$pv:" <<< $fs_params || grep -q ":$pv\$" <<< $fs_params || fs_params="$fs_params:$pv" + if ! egrep -q "$pv(\$| )" <<< "$fs_params"; then + [ -n "$fs_params" ] && fs_params="$fs_params " + fs_params="$fs_params$pv" + fi + list= - for pv in `sed 's/:/ /' <<< $fs_params` + for pv in $fs_params do list="$list $pv ^ ON" done @@ -505,7 +498,7 @@ interactive_filesystem () fs_params=${list2[0]} else ask_checklist "Which lvm PV's must this volume group span?" $list || return 1 - fs_params="$(sed 's/ /:/' <<< "$ANSWER_CHECKLIST")" #replace spaces by colon's, we cannot have spaces anywhere in any string + fs_params="$ANSWER_CHECKLIST" fi fi if [ "$fs_create" == yes ] && [ "$fs_type" = lvm-lv ] @@ -519,9 +512,9 @@ interactive_filesystem () if [ "$fs_create" == yes ] && [ "$fs_type" = dm_crypt ] then [ -z "$fs_params" ] && default='-c aes-xts-plain -y -s 512' - [ -n "$fs_params" ] && default="${fs_params//_/ }" + [ -n "$fs_params" ] && default="$fs_params" ask_string "Enter the options for this $fs_type on $part" "$default" || return 1 - fs_params="${ANSWER_STRING// /_}" + fs_params="$ANSWER_STRING" fi # ask opts @@ -531,7 +524,7 @@ interactive_filesystem () [ -n "$fs_opts" ] && default="$fs_opts" program=`get_filesystem_program $fs_type` ask_string "Enter any additional opts for $program" "$default" 0 - fs_opts=$(sed 's/ /_/g' <<< "$ANSWER_STRING") #TODO: clean up all whitespace (tabs and shit) + fs_opts="$ANSWER_STRING" fi [ -z "$fs_type" ] && fs_type=no_type @@ -539,7 +532,7 @@ interactive_filesystem () [ -z "$fs_opts" ] && fs_opts=no_opts [ -z "$fs_label" ] && fs_label=no_label [ -z "$fs_params" ] && fs_params=no_params - NEW_FILESYSTEM="$fs_type;$fs_create;$fs_mountpoint;target;$fs_opts;$fs_label;$fs_params" + NEW_FILESYSTEM="$fs_type;$fs_create;$fs_mountpoint;target;${fs_opts// /__};$fs_label;${fs_params// /__}" # add new theoretical blockdevice, if relevant new_device= diff --git a/tests/runtime/automatic-dmcrypt-lvm-install-sda/profile b/tests/runtime/automatic-dmcrypt-lvm-install-sda/profile index f0c37c7..8828cf3 100644 --- a/tests/runtime/automatic-dmcrypt-lvm-install-sda/profile +++ b/tests/runtime/automatic-dmcrypt-lvm-install-sda/profile @@ -18,5 +18,5 @@ BLOCKDATA='/dev/sda1 raw no_label ext2;yes;/boot;target;no_opts;no_label;no_para /dev/sda2 raw lvm-pv;yes;no_mountpoint;target;no_opts;no_label;no_params /dev/sda2+ lvm-pv no_label lvm-vg;yes;no_mountpoint;target;no_opts;mypool;/dev/sda2 /dev/mapper/mypool lvm-vg mypool lvm-lv;yes;no_mountpoint;target;no_opts;root;800M -/dev/mapper/mypool-root lvm-lv no_label dm_crypt;yes;no_mountpoint;target;no_opts;mypool-rootcrypt;-c_aes-xts-plain_-y_-s_512 +/dev/mapper/mypool-root lvm-lv no_label dm_crypt;yes;no_mountpoint;target;no_opts;mypool-rootcrypt;-c__aes-xts-plain__-y__-s__512 /dev/mapper/mypool-rootcrypt dm_crypt no_label xfs;yes;/;target;no_opts;no_label;no_params' \ No newline at end of file diff --git a/tests/runtime/automatic-lvm-dmcrypt-install-sda/profile b/tests/runtime/automatic-lvm-dmcrypt-install-sda/profile index b936576..c90b1ec 100644 --- a/tests/runtime/automatic-lvm-dmcrypt-install-sda/profile +++ b/tests/runtime/automatic-lvm-dmcrypt-install-sda/profile @@ -15,7 +15,7 @@ TARGET_PACKAGES='openssh aif' GRUB_DEVICE=/dev/sda PARTITIONS='/dev/sda 40:ext2:+ *:ext4' BLOCKDATA='/dev/sda1 raw no_label ext2;yes;/boot;target;no_opts;no_label;no_params -/dev/sda2 raw no_label dm_crypt;yes;no_mountpoint;target;no_opts;sda2crypt;-c_aes-xts-plain_-y_-s_512 +/dev/sda2 raw no_label dm_crypt;yes;no_mountpoint;target;no_opts;sda2crypt;-c__aes-xts-plain__-y__-s__512 /dev/mapper/sda2crypt dm_crypt no_label lvm-pv;yes;no_mountpoint;target;no_opts;no_label;no_params /dev/mapper/sda2crypt+ lvm-pv no_label lvm-vg;yes;no_mountpoint;target;no_opts;cryptpool;/dev/mapper/sda2crypt /dev/mapper/cryptpool lvm-vg cryptpool lvm-lv;yes;no_mountpoint;target;no_opts;cryptswap;20M|lvm-lv;yes;no_mountpoint;target;no_opts;cryptroot;800M|lvm-lv;yes;no_mountpoint;target;no_opts;crypthome;50M diff --git a/unofficial/dieter-desktop-a7n8x.automaticprofile b/unofficial/dieter-desktop-a7n8x.automaticprofile index 38762fc..73d1a36 100644 --- a/unofficial/dieter-desktop-a7n8x.automaticprofile +++ b/unofficial/dieter-desktop-a7n8x.automaticprofile @@ -22,7 +22,7 @@ SVN_BASE=https://192.168.1.2/svn/repos var_PARTITIONS='/dev/sda 100:ext2:+ 2048:swap *:xfs' var_BLOCKDATA='/dev/sda1 raw no_label ext2;yes;/boot;target;no_opts;no_label;no_params /dev/sda2 raw no_label swap;yes;no_mountpoint;target;no_opts;no_label;no_params -/dev/sda3 raw no_label dm_crypt;yes;no_mountpoint;target;no_opts;sda3crypt;-c_aes-xts-plain_-y_-s_512 +/dev/sda3 raw no_label dm_crypt;yes;no_mountpoint;target;no_opts;sda3crypt;-c__aes-xts-plain__-y__-s__512 /dev/mapper/sda3crypt dm_crypt no_label lvm-pv;yes;no_mountpoint;target;no_opts;no_label;no_params /dev/mapper/sda3crypt+ lvm-pv no_label lvm-vg;yes;no_mountpoint;target;no_opts;cryptpool;/dev/mapper/sda3crypt /dev/mapper/cryptpool lvm-vg cryptpool lvm-lv;yes;no_mountpoint;target;no_opts;cryptroot;5G|lvm-lv;yes;no_mountpoint;target;no_opts;crypthome;5G -- cgit v1.2.3-54-g00ecf