Re: [PATCH] Remove root drive support

[Vladimir 'phcoder' Serbinenko]

On Fri, Jun 12, 2009 at 10:45 PM, Pavel Roskin<address@hidden> wrote:
> Root drive support is not actually used by the installer.  Root drive is
> superseded by grub_prefix with UUID, which is much more flexible, is
> already used by grub-setup for cross-drive installs and doesn't require
> a single byte in the boot sector.
I successfully tested this patch in qemu with both grub-mkrescue and
multiboot load grub2-by-grub2. As I see root drive was always set to
0xFF and so this field is useless and as it's in size-critical part.
However reading grub-install revealed that cross-drive install would
fail if target filesystem has no UUIDs or if UUIDs are unsupported by
grub. Currently it includes following filesystems:
affs,afs,cpio/tar,hfs,jfs,minix,sfs, udf, ufs
As you can see it includes even important filesystems like ufs. Now
the question to discuss is whether we want to implement UUIDs for this
fileystems, let it be like it is or have a cross-drive install w/o
UUIDs. Even if we choose the last option I think root field still has
to be removed and we can just use non-UUID drive name for prefix
>
> ChangeLog:
>        * boot/i386/pc/boot.S: Remove root_drive.  Ensure that
>        boot_drive_check is where boot.h expects it to be.  Don't save
>        %dx, we only need %dl and we never change it.
>        * boot/i386/pc/cdboot.S: Don't set the root drive.
>        * boot/i386/pc/pxeboot.S: Likewise.
>        * include/grub/i386/pc/boot.h: Remove
>        GRUB_BOOT_MACHINE_ROOT_DRIVE, adjust
>        GRUB_BOOT_MACHINE_DRIVE_CHECK.
>        * include/grub/i386/pc/kernel.h: Remove grub_root_drive.
>        * kern/i386/pc/init.c (make_install_device): Remove references
>        to grub_root_drive.
>        * kern/i386/pc/startup.S: Likewise.
>        * util/i386/pc/grub-setup.c (setup): Don't set root_drive.
> ---
>
>  boot/i386/pc/boot.S           |   11 +++--------
>  boot/i386/pc/cdboot.S         |    3 ---
>  boot/i386/pc/pxeboot.S        |    3 +--
>  include/grub/i386/pc/boot.h   |    5 +----
>  include/grub/i386/pc/kernel.h |    3 ---
>  kern/i386/pc/init.c           |   11 ++++-------
>  kern/i386/pc/startup.S        |    6 +-----
>  util/i386/pc/grub-setup.c     |    5 +----
>  8 files changed, 11 insertions(+), 36 deletions(-)
>
> diff --git a/boot/i386/pc/boot.S b/boot/i386/pc/boot.S
> index 2cd505e..8d8c27c 100644
> --- a/boot/i386/pc/boot.S
> +++ b/boot/i386/pc/boot.S
> @@ -102,8 +102,6 @@ kernel_sector:
>  boot_drive:
>        .byte 0xff      /* the disk to load kernel from */
>                        /* 0xff means use the boot drive */
> -root_drive:
> -        .byte 0xff
>
>  after_BPB:
>
> @@ -118,6 +116,7 @@ after_BPB:
>          * possible boot drive. If GRUB is installed into a floppy,
>          * this does nothing (only jump).
>          */
> +       . = _start + GRUB_BOOT_MACHINE_DRIVE_CHECK
>  boot_drive_check:
>         jmp     1f     /* grub-setup may overwrite this jump */
>         testb   $0x80, %dl
> @@ -151,14 +150,12 @@ real_start:
>        /*
>         *  Check if we have a forced disk reference here
>         */
> -        /* assign root_drive at the same time */
>  #ifdef APPLE_CC
>        boot_drive_abs = ABS (boot_drive)
> -       movw    boot_drive_abs, %ax
> +       movb    boot_drive_abs, %al
>  #else
> -       movw    ABS(boot_drive), %ax
> +       movb   ABS(boot_drive), %al
>  #endif
> -        movb    %ah, %dh
>        cmpb    $0xff, %al
>        je      1f
>        movb    %al, %dl
> @@ -343,7 +340,6 @@ setup_sectors:
>
>        /* restore %dl */
>        popw    %dx
> -        pushw   %dx
>
>        /* head start */
>        movb    %al, %dh
> @@ -399,7 +395,6 @@ copy_buffer:
>
>        popw    %ds
>        popa
> -        popw    %dx
>
>        /* boot kernel */
>  #ifdef APPLE_CC
> diff --git a/boot/i386/pc/cdboot.S b/boot/i386/pc/cdboot.S
> index 688b26c..efa65f5 100644
> --- a/boot/i386/pc/cdboot.S
> +++ b/boot/i386/pc/cdboot.S
> @@ -86,9 +86,6 @@ bi_reserved:
>
>        call    read_cdrom
>
> -        /* Root drive will default to boot drive */
> -        movb    $0xFF, %dh
> -
>        ljmp    $(DATA_ADDR >> 4), $0
>
>  /*
> diff --git a/boot/i386/pc/pxeboot.S b/boot/i386/pc/pxeboot.S
> index 1b51127..2fc53bc 100644
> --- a/boot/i386/pc/pxeboot.S
> +++ b/boot/i386/pc/pxeboot.S
> @@ -27,8 +27,7 @@
>  _start:
>  start:
>
> -        /* Root drive will default to boot drive */
> -        movb   $0xFF, %dh
> +       /* Use drive number 0x7F for PXE */
>         movb   $0x7F, %dl
>
>        /* Jump to the real world */
> diff --git a/include/grub/i386/pc/boot.h b/include/grub/i386/pc/boot.h
> index 3862214..f35cb3a 100644
> --- a/include/grub/i386/pc/boot.h
> +++ b/include/grub/i386/pc/boot.h
> @@ -34,9 +34,6 @@
>  /* The offset of BOOT_DRIVE.  */
>  #define GRUB_BOOT_MACHINE_BOOT_DRIVE   0x4c
>
> -/* The offset of ROOT_DRIVE.  */
> -#define GRUB_BOOT_MACHINE_ROOT_DRIVE   0x4d
> -
>  /* The offset of KERNEL_ADDRESS.  */
>  #define GRUB_BOOT_MACHINE_KERNEL_ADDRESS       0x40
>
> @@ -47,7 +44,7 @@
>  #define GRUB_BOOT_MACHINE_KERNEL_SEGMENT       0x42
>
>  /* The offset of BOOT_DRIVE_CHECK.  */
> -#define GRUB_BOOT_MACHINE_DRIVE_CHECK  0x4f
> +#define GRUB_BOOT_MACHINE_DRIVE_CHECK  0x4e
>
>  /* The offset of a magic number used by Windows NT.  */
>  #define GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC     0x1b8
> diff --git a/include/grub/i386/pc/kernel.h b/include/grub/i386/pc/kernel.h
> index 5acc883..5b9d8dc 100644
> --- a/include/grub/i386/pc/kernel.h
> +++ b/include/grub/i386/pc/kernel.h
> @@ -71,9 +71,6 @@ extern char grub_prefix[];
>  /* The boot BIOS drive number.  */
>  extern grub_uint8_t EXPORT_VAR(grub_boot_drive);
>
> -/* The root BIOS drive number.  */
> -extern grub_uint8_t grub_root_drive;
> -
>  #endif /* ! ASM_FILE */
>
>  #endif /* ! KERNEL_MACHINE_HEADER */
> diff --git a/kern/i386/pc/init.c b/kern/i386/pc/init.c
> index b533921..274ba15 100644
> --- a/kern/i386/pc/init.c
> +++ b/kern/i386/pc/init.c
> @@ -60,13 +60,10 @@ make_install_device (void)
>
>   if (grub_prefix[0] != '(')
>     {
> -      /* If the root drive is not set explicitly, assume that it is identical
> -         to the boot drive.  */
> -      if (grub_root_drive == 0xFF)
> -        grub_root_drive = grub_boot_drive;
> -
> -      grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
> -                    grub_root_drive & 0x7f);
> +      /* Need to make the root partition from the boot drive and the 
> partition
> +         number encoded at the install time.  */
> +      grub_sprintf (dev, "(%cd%u", (grub_boot_drive & 0x80) ? 'h' : 'f',
> +                    grub_boot_drive & 0x7f);
>
>       if (grub_install_dos_part >= 0)
>        grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 
> 1);
> diff --git a/kern/i386/pc/startup.S b/kern/i386/pc/startup.S
> index 0f80e8a..f77d7db 100644
> --- a/kern/i386/pc/startup.S
> +++ b/kern/i386/pc/startup.S
> @@ -195,9 +195,8 @@ codestart:
>
>        sti             /* we're safe again */
>
> -       /* save boot and root drive references */
> +       /* save the boot drive */
>        ADDR32  movb    %dl, EXT_C(grub_boot_drive)
> -       ADDR32  movb    %dh, EXT_C(grub_root_drive)
>
>        /* reset disk system (%ah = 0) */
>        int     $0x13
> @@ -300,9 +299,6 @@ codestart:
>  VARIABLE(grub_boot_drive)
>        .byte   0
>
> -VARIABLE(grub_root_drive)
> -       .byte   0
> -
>        .p2align        2       /* force 4-byte alignment */
>
>  /*
> diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c
> index bdf234c..5a51964 100644
> --- a/util/i386/pc/grub-setup.c
> +++ b/util/i386/pc/grub-setup.c
> @@ -94,7 +94,7 @@ setup (const char *dir,
>   grub_uint16_t core_sectors;
>   grub_device_t root_dev, dest_dev;
>   const char *dest_partmap;
> -  grub_uint8_t *boot_drive, *root_drive;
> +  grub_uint8_t *boot_drive;
>   grub_disk_addr_t *kernel_sector;
>   grub_uint16_t *boot_drive_check;
>   struct boot_blocklist *first_block, *block;
> @@ -207,7 +207,6 @@ setup (const char *dir,
>
>   /* Set the addresses of variables in the boot image.  */
>   boot_drive = (grub_uint8_t *) (boot_img + GRUB_BOOT_MACHINE_BOOT_DRIVE);
> -  root_drive = (grub_uint8_t *) (boot_img + GRUB_BOOT_MACHINE_ROOT_DRIVE);
>   kernel_sector = (grub_disk_addr_t *) (boot_img
>                                     + GRUB_BOOT_MACHINE_KERNEL_SECTOR);
>   boot_drive_check = (grub_uint16_t *) (boot_img
> @@ -379,7 +378,6 @@ setup (const char *dir,
>
>   /* FIXME: can this be skipped?  */
>   *boot_drive = 0xFF;
> -  *root_drive = 0xFF;
>
>   *kernel_sector = grub_cpu_to_le64 (embed_region.start);
>
> @@ -513,7 +511,6 @@ unable_to_embed:
>
>   /* FIXME: can this be skipped?  */
>   *boot_drive = 0xFF;
> -  *root_drive = 0xFF;
>
>   *install_dos_part = grub_cpu_to_le32 (dos_part);
>   *install_bsd_part = grub_cpu_to_le32 (bsd_part);
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko