Re: [PATCH] cdrom access patch

[Bean]

On Mon, Mar 17, 2008 at 12:31 PM, Pavel Roskin <address@hidden> wrote:
> On Sat, 2008-03-15 at 15:34 +0800, Bean wrote:
>
>  > Ok, the new patch add (cd0) alias for cdrom device, the corresponding
>  > (hdN) is also present.
>  ...
>
> >       (grub_biosdisk_call_hook): Only handle (hdN) device name.
>
>  I still don't understand this part.  Maybe you could split your patch
>  into smaller parts?
>
>  Suppose we hard drives at 0x80 and 0x82 and a CD at 0x90.  We can call
>  grub_biosdisk_get_cdinfo_int13_extension() and figure out that 0x90 is
>  the boot CD, so we call it (cd0).  Then we go through the drives and
>  call them (hd0), (hd1).
>
>  If it's OK to have an alias, why not suppress of hide the original name?
>
>  At what point do we need to use the _name_ to calculate the BIOS number?
>  Why cannot we simply store that number into some field?

If we use (cd0) as primary name, there are three places where the
cdrom has to been checked.

1. make_install_device in init.c.
Here, given the boot drive number grub_boot_drive, we need to figure
out the root device name. This is outside biosdisk, can't access
device or disk.

2. grub_biosdisk_iterate in biosdisk.c
We need to calculate all the disk names belong to this device, and
call the hook. No parameter is passed.

3. grub_biosdisk_open in biosdisk.c
Given a name, we need to figure out the bios number in order to
initialize the disk parameter.

4. Some boot loader
We need use the root variable to determine the bios number.

The call order is as follows, make_install_device will always be
called at init time, grub_biosdisk_open is called when we open a disk,
and grub_biosdisk_iterate is called when we iterate disk using ls or
search.

The tricky part is how to make them all consistent. In order to do so,
we have to detect cdrom and store the number as global variable
somewhere. If we put it the kernel, it's workable, but it seems odd
because this is clearly a biosdisk related function. If we put it in
biosdisk, then kernel and boot loader would have to depend on
biosdisk, then what if we want to use ata ?

Also some thoughts about generic alias handling, it's not easy to
implement. First, we need to decide where to create the alias. We
can't create it in grub_biosdisk_open, the mapping should be made
before calling grub_biosdisk_open. We can't create it in
grub_biosdisk_iterate, as it's not necessary called before
grub_biosdisk_open. It seems we can only do it in module
initialization. But then, we need to iterate disks in order to find
the cdrom, this somehow duplicate grub_biosdisk_iterate's job. Then,
We also need to consider when to release the alias.

-- 
Bean