I've spent about 6 hours yesterday looking at #396023 and #425829, the issues introduced after implementation of support for erasing encrypted volumes. I've looked at both the original patch and the changes proposed by Jérémy.
The main issues are that the original patch:
- relies on dmsetup to be loaded basically for all installs
- introduces a lot of code specific to crypto support in partman-auto
- introduces new functions with names that can easily be confused with
existing functions in partman-crypto
The patch proposed by Jérémy has some good ideas and does improve things to
some extend, but the end result is still a spaghetti of interrelated
functions in weird places.
For example, it moves quite a few lvm/crypto specific functions into
definitions.sh in partman-base. As definitions.sh is sourced by almost
every script in partman, I do not think this is desirable.
I also found that I had real problems following the patches and that
stacking major corrections on top of David's original changes is not a good
idea.
I therefore suggest reverting David's changes (which luckily is quite
straightforward) and then first do some refactoring of existing code as
preparation for a reimplementation of support for erasing encrypted
volumes.
For refactoring, I propose the following changes.
1) Rename current "wipe" functions
Both partman-auto and partman-crypto have wipe functions. In the first case
it is basically erasing existing data on a disk by creating a new disk
label; in the second case it is actually wiping the data on a device.
Using "wipe" for both is confusing.
I suggest we rename the functions in partman-auto to "erase" or "init"
instead of "wipe" to better match what actually happens.
For partman-crypto I have a patch that renames the existing functions to
include the crypto namespace:
- wipe -> crypto_do_wipe
- dev_wipe -> crypto_wipe_device
2) Reorder function libraries
We currently have various function libraries: definitions.sh, resize.sh,
recipes.sh, *_tools.sh.
- definitions.sh since long contains more than just definitions
- the various "tools" libraries don't really contain tools
- in come cases they are starting to contain a somewhat random mix of
functions and scripts that source them often use only a small subset
- they all sit in /lib/partman together with the hook dirs and are only
recognizable because of the .sh extention
I suggest we move all function libraries into /lib/partman/lib/ [1].
definitions.sh could be renamed to just base.sh.
The various *_tools.sh files could be renamed to lvm-base.sh, lvm-*.sh,
auto-base, etc.
This would also lower the barrier to introduce additional new function
libraries when that is warranted.
For definitions.sh I plan to temporarily include a symlink to the current
location to ease the transition. For others that is probably not necessary
if uploads are coordinated.
3) Further reduce memory consumption for LVM and crypto
Both partman-lvm and partman-crypto have huge sets of templates and load
additional modules and library/utility udebs. As they currently are loaded
by default for normal installs, they contribute heavily to D-I's memory
footprint.
I propose we make partman-(auto-){lvm,crypto} optional and create a new
init.d script that only loads them if there is sufficient memory available.
This script should probably just be part of partman-base.
If this is implemented, I also do not have any real problems with depending
on dmsetup-udeb for both lvm and crypto as it would no longer increase the
memory usage for systems that cannot afford it. Still, it would be better
if requiring dmsetup could be avoided for partman-lvm.
4) Possibly create a partman-dm-support udeb
From the current patches to support erasing encrypted volumes, it looks like
partman-lvm and partman-crypto may need to share some code. Moving that
code into partman-base or partman-auto is IMO undesirable, so the best
solution seems to be to create a new udeb for it which both depend on.
Cheers,
FJP
[1] I also considered /usr/share/partman, but it seems better to keep things
together under /lib/partman/. Other idea was to rename all to functions-*.sh
but moving them to a subdir seems clearer. An alternative could be to use
/lib/partman/functions/ instead of /lib/partman/lib/.
signature.asc
Description: This is a digitally signed message part.
