Re: [RFC] Allow block device providers to veto file systems

Hi Frans,

On Fri, Nov 30, 2007 at 06:39:50PM +0100, Frans Pop wrote:
> On Friday 30 November 2007, Max Vozeler wrote:
> > > Why pipe them and not just pass them as a parameter?
> > > Call the script as '$i $dev $id "$filesystems"' and in the script have
> > > 'filesystems="$3"'.
> >
> > That's what I tried first.
> >
> > I changed to piping because otherwise we'd have to do
> > comparably complex list comparisons. E.g. either:
>
> Hmm. I don't get this. You could still echo back the valid options, same as
> you do now. You just pass them _in_ as a parameter which avoids the (IMO)
> ugly 'cat' commands in the veto script.
> AFAICT my suggestion does not fundamentally change your solution.

Ah, yes, you are right. I though into a different direction. This is indeed easier and should be more robust.

> > I personally prefer the style I originally used because
> > it saves one level of (to me) not meaningful indentation,
> > but that's a matter of taste. I'm happy to change it :-)
>
> No, it does not cost a level of indentation because the conditions are
> indented by only 4 spaces so their code is still only indented by a single
> tab. Indenting the conditions by spaces has the advantage that the total
> case-esac block is better recognizable.
>
> This exception to using only tabs for indentation is documented in the D-I
> coding style document.

OK, fair enough. I've changed it and updated the patch to include the other suggestions. (Attached)

        Max

Index: partman-basicmethods/choose_method/filesystem/choices

• partman-basicmethods/choose_method/filesystem/choices (Revision 50282) +++ partman-basicmethods/choose_method/filesystem/choices (Arbeitskopie) @@ -13,7 +13,13 @@ done )

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.debian.org Links debian-announce debian-apache debian-arm-changes debian-beowulf debian-boot debian-cd debian-cd-vendors debian-changes debian-changes-digest debian-isp debian-kde debian-laptop debian-mirrors-announce debian-news debian-security debian-security-announce debian-testing debian-user debian-user-digest Request more information about Pantek

-for fs in $filesystems; do

+allowed=$filesystems
+for i in /lib/partman/test_valid_filesystems/*; do
+    [ -x $i ] || continue
+    allowed=$($i $dev $id "$allowed")
+done
+
+for fs in $allowed; do
     db_metaget partman/filesystem_long/$fs description || RET=''
     RET=${RET:-$fs}
     printf "${fs}\t${RET}\n"

• partman-basicmethods/debian/changelog (Revision 50282) +++ partman-basicmethods/debian/changelog (Arbeitskopie) @@ -7,8 +7,13 @@ [ Colin Watson ]
  • Use 'mkdir -p' rather than more awkward test-then-create constructions.
    • -- Frans Pop <fjp@debian.org> Sun, 13 May 2007 04:05:35 +0200 + [ Max Vozeler ] + * choose_method/filesystem/choices: Allow scripts in + test_valid_filesystems to prevent certain filesystems + from being offered.

+ -- Max Vozeler <xam@debian.org> Fri, 30 Nov 2007 14:10:02 +0000 +
 partman-basicmethods (35) unstable; urgency=low  

• Move sanity-checking scripts from finish.d to check.d. Requires Index: partman-crypto/debian/changelog

  ---

  • partman-crypto/debian/changelog (Revision 50283) +++ partman-crypto/debian/changelog (Arbeitskopie) @@ -6,6 +6,10 @@ [ Max Vozeler ]
• Correct dependencies in base64/Makefile; Thanks to Robert Millan for report + patch. Closes: #452830 + + * Use test_valid_filesystems to allow only ext2 on crypto + devices with random keys. Closes: #414638. This is only + effective with partman-basicmethods 36 or later.
• Drop use of the obsolete /dev/loop/ directory

Index: partman-crypto/debian/rules

• partman-crypto/debian/rules (Revision 50282) +++ partman-crypto/debian/rules (Arbeitskopie) @@ -48,6 +48,7 @@ dh_install base64/base64 bin/ dh_install blockdev-keygen bin/ dh_install blockdev-wipe/blockdev-wipe bin/ + dh_install test_valid_filesystems lib/partman rm -rf `find debian/$(PACKAGE) -name .svn`

 binary-indep: install-indep
Index: partman-crypto/test_valid_filesystems/crypto

• partman-crypto/test_valid_filesystems/crypto (Revision 0) +++ partman-crypto/test_valid_filesystems/crypto (Revision 0) @@ -0,0 +1,39 @@ +#!/bin/sh +# Veto filesystems unsuitable for certain crypto setups + +dev=$1 +id=$2 +filesystems="$3" + +filesystems_veto () +{ + [ -f $dev/crypt_realdev ] || return 1 + + # Get to the underlying crypto device directory + r=$(cat $dev/crypt_realdev) + cryptodev=${r##*:} + + [ -f $cryptodev/method ] || return 1 + method=$(cat $cryptodev/method) + + if [ $method = crypto ]; then + [ -f $cryptodev/keytype ] || return 1 + keytype=$(cat $cryptodev/keytype) + + if [ $keytype = random ]; then + # Veto anything but ext2 + for fs in $filesystems; do + case fs in + ext2) echo $fs ;; + esac + done + return 0 + fi + fi + + return 1 +} + +filesystems_veto || echo $filesystems + +exit 0

-- 
To UNSUBSCRIBE, email to debian-boot-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org