[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFC: partman-auto-crypto

Hey David,

On Wed, Jun 21, 2006 at 09:42:17PM +0200, David Härdeman wrote:
> I've been working on partman-auto-crypto in the d-i SVN repo
> at /people/alphix-guest/partman-auto-crypto.

Thanks for working on this.. :-)

Last night I completed two test installs using the SVN 
version (r38313) and both worked flawlessly. One was using
the "All in one partition" scheme, the other the "/home, 
/usr, /var, /tmp" scheme. The system was a VMware instance
with one 2GB virtual SCSI disk attached. 

Only one thing surprised me: The /boot partition ended up 
with a size of ~250 MB when using the "All in one" scheme,
which seems very large. Maybe this is not specific to crypto
though, and it doesn't seem very important.

Have you had a chance to test on a system with multiple
disks attached yet?

> It is currently at the stage where it works for me (famous
> last words), so I'd like to get some review and feedback
> before moving the package into the main tree (especially as
> I've never written a debian package from scratch yet).

Looks good to me. Only a few minor points:

1) Some comments exceed 80 characters. This makes it a bit
ugly to read on normal terminals.

2) You can drop Standards-Version from debian/control. It 
does not apply to pure-udeb packages.

3) Please add some information about authorship of the
code to debian/copyright. I realize this is tricky as other
partman-* packages are not very clear on copyright. It would
already help to say which code -auto-crypto is derived from
and which parts you hold copyright on.

4) Some comments that originate from -auto-lvm no longer
match the code, which is a bit confusing. e.g. "This variable
will be used.. zero it" in automatically_partition/
some_device_crypto/do_option. It would help maintainability
if you could go through all comments and check whether they
still make sense.

5) Minor: There is "modprobe $foo 2>/dev/null 2>&1 || true" in
automatically_partition/.../do_option. This is not really
important, but it seems we either a) expect modprobe can fail -
in this case perhaps wrap it in log-output? Or we expect it
not to fail, which means the || true could be dropped. Also the
comment there, "Be sure modules are loaded" is not really true
if we don't check the return code of modprobe. </nitpick> 

> There are currently two items on the todo list:

> o Support loop-AES as well as dm-crypt (this mostly depends on 
>  partman-crypto getting encrypted-root-via-loop-AES support)

I'm hoping to work on this after my vacation (ie. in about two
weeks time). For now I actually think we can consider the non-
availability of loop-AES support an advantage, as we don't have
to think about how to present the choice of crypto system to
users in a non-confusing way. :-)

> None of these items seem to be a reason to postpone the use
> of partman-auto-crypto, and code sharing is hard to implement
> before both packages are in the source tree proper.

I agree and would like to add my support for moving the 
partman-auto-crypto package to trunk/. 


Reply to: