On Fri, May 18, 2012 at 6:22 PM, Matthew Monaco <dgbaley27@0x01b.net> wrote:
With the exception of some special values, convert the comma separated list of options to pass directly to cryptsetup. This means we don't have to explicitly support all of cryptsetups options.
If at all possible, I think it is better to explicitly handle the things we want to support. Passing on the options blindly means that we are tightly coupled to the underlying tool (which is for instance what made the transition from net-tools to iproute so painful). Maybe one solution could be to handle the things we want to support, and for the sake of backwards compatibility we do your catch-all matching at the end, but print a warning "Passing unrecognized options to cryptsetup"?
'size' and 'device-size' are needed because (yes) systemd and Debian use size for cryptsetup's --key-size option. "cryptsetup --size create" limits the size of a device.
'swap' means run mkswap after mounting, this is standard as far as the Debian and systemd format goes.
'luks' and 'plain' are also used by Debian and systemd, but they seem like superfluous configuration because "cryptsetup isLuks" gives this information.
'noauto' will work with the default $FILTER (-O) of !noauto
'tmp' is supported by Debian and systemd. This will come in a later commit.
There is also list of options which are supported by Debian that we'll ignore.
It should be pointed out that in this context "Debian+systemd" essentially means "everyone else". Also, we don't "need" to deal with these options, but I absolutely agree that we should (even if all we do is warn that the option is not supported). I think we should aim for: 1) a crypttab file copied from another distro to Arch should do something sensible (hopefully work, but at least give reasonable error messages) and 2) a crypttab file that works without warnings on Arch should work just fine when copied to another distro.
--- cryptmount.sh | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
diff --git a/cryptmount.sh b/cryptmount.sh index 55a6944..7e89bbc 100755 --- a/cryptmount.sh +++ b/cryptmount.sh @@ -149,6 +149,17 @@ ct_main() { info "udevd not running, or unable to detect it: waiting for devices disabled" fi
+ # Pre-parse OPTIONS, a little ugly, but it works + preparse() { + local args swap + if ! ct_parse_options $OPTIONS; then + error "Invalid options string: $OPTIONS" + exit 1 + fi + OPTIONS="$args" + } + preparse + if [ -z "$action" -o "$action" = "list" ]; then
if [ $# -ne 0 ]; then @@ -359,6 +370,64 @@ ct_resolve_device() { return 1 fi } + +ct_parse_options() { + + local IFS=',' optlst="$*" opt key val + + for opt in $optlst; do + + # separate key and value + unset key val + case "$opt" in + "") + continue;; + *=*) + key="${opt%%=*}" + val="${opt#*=}" + [ "$key" = "$val" ] && unset val + ;; + *) + key="$opt";; + esac + + case "$key" in + swap) + # set external variable + swap=1 + ;; + luks|plain) + warn "Ignoring option $key, LUKS volumes are automatically detected" + ;; + noauto|%*) + : + ;; + skip|precheck|check|checkargs|noearly|loud|keyscript) + warn "Ignoring Debian specific option '$key'" + ;; + tmp) + warn "The tmp= option is not supported" + ;; + size) + args="$args --key-size $val" + ;; + device-size) + args="$args --size $val" + ;; + *) + if [ ${#key} -eq 1 ]; then + args="$args -$key $val" + else + args="$args --$key $val" + fi + ;; + esac + + done + + return 0 +} + # # # ---------------------------------------------------------------------------- #
-- 1.7.10.2