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
)
-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"
Index: partman-basicmethods/debian/changelog
===================================================================
--- 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
Reply to: