Re: [PATCH] Reimplementing the legacy "map" command

["Vesa Jääskeläinen" <chaac@nic.fi>]

Javier Martín wrote:
> Hi again everybody. After some hacking with the GRUB internals, I've
> completed a reimplementation of the legacy "map" command that allowed
> the BIOS drives to have their numbers reassigned, in order to boot OSes
> whose boot code requires their installation to be located in the first
> HD (BIOS drive 0x80).
> 
> This implementation gets next to the status of the command in GRUB
> Legacy, with the main changes being:
> 
> * Command: now called "drivemap", attending to suggestions in this list
> * UI: the less complete part of the job. The syntax is now as follows:
> 	drivemap (hd1) 0x80 # Makes (hd1) appear as the first BIOS drive
>       i.e. the second argument is now any BIOS drive number instead
>       of another device. This change is in part due to my lack of time,
>       but also because others here pointed that it would be useful to
>       allow arbitrary mappings. If required, old syntax could be
>       reimplemented in short time.
> * Backend: the command is now a module, but there is also code in the
>            several files like the machine-specific loader.S, the loader
>            header file and the makefiles.
> * Backend: a preboot hook system has been added to the loader backend,
>            allowing modules to register functions to be run just before
>            the grub_X_real_boot function is called. The drivemap module
>            uses this new functionality to get its install_int13_handler
>            function to be called before booting the target OS without
>            creating chaos in GRUB itself.
> * Functionality: the new module is independent of chainloader, so its
>                  functions can be applied with any target loader. What
>                  effects will this have, I don't know.
> 
> This patch is just a preliminary version that has been tested only in
> QEMU and Bochs, not on real hardware because I'm cut off from x86
> computers from the time being (I'm running on a PPC mac now). I will be
> back to town (and to my olde amd64 box) this weekend, but I wanted to
> submit this so others could scrutinize it for obvious mistakes, and
> maybe test it.
> 
> I've tried to respect the style rules I've seen, but please note that
> this is my first patch. Cheers!

Did you forgot something from this patch :) ?

Anyway... Couple of comments. No chatting on the code. No commented out 
code unless there is a really good reason for it. Try to move int13h 
handle to module not to kernel. We do not want to put anything more to 
kernel unless there is a really good reason to do so.

+/* This is NOT a typo: we just want this "void" var in order to take its
+ * address with &var - used for relative addressing within the int13 
handler */
+extern void EXPORT_VAR(grub_drivemap_int13_handler_base);

Create new type for it. Or use variable type that eats that space. This 
way you do not need this comment to confuse people.

Please do not include .mk files on next patch.

Abort on error?... Ok... do you go to deinit everything before that was 
successfully installed or just give warning or ?