On Sat, Aug 19, 2006 at 09:25:18PM +0200, Frans Pop wrote:
On Saturday 19 August 2006 20:41, David Härdeman wrote:I'm hoping to get some feedback on whether this is a good idea before I commit anything.What is missing in the patch IMHO is a check if there is actually enough free memory to load the modules and run crypto before actually doing so.If you don't check, you still run the risk of crashing the installer.
On a more general note: is this something that you'd like to see in d-i before the Etch release?
This could be a bit tricky as swap may be active at the time of the check and AFAIK it will be disabled during the actual operation. Not sure if that needs to be taken into account; a plain check for sufficient free memory would be a great start (maybe implemented as a function in di-utils).
I think I'll add a naive check of the "MemFree" field in /proc/meminfo for now. If the memory seems low, it can warn the user that things may crash and let him choose to allow the installation to continue or to not use crypto.
Also, is there a possibility that the function will be called again later? In that case it could be good to prevent running the memory check and loading the modules a second time.
Ok, I'll add a check for that. I thought that anna wouldn't re-install the udeb if called a second time.
Finally, wasn't there some code that checked if everything was available before offering a method? Does that still work with this change?
The previous suggestion tried to load all methods as soon as crypto was chosen for any partition. This leads to additional downloads (since you will probably only want to use one method), memory usage and waiting.
Now it assumes that the method will be loadable and presents an error message if it fails.
I like the fact that you only load the modules needed for the particular method selected.Nitpick: I feel that the name of the function "crypto_set_defaults" kind of fails to cover the udeb loading. Maybe crypto_medthod_prepare or something would be better.
Oki Regards, David