Re: [PATCH] PXE support for grub2

[Robert Millan]

On Thu, Jul 31, 2008 at 12:50:22AM +0800, Bean wrote:
> +#if 1
> +#define GRUB_PXE_BOOTP_DHCPVEND      1024    /* DHCP extended vendor field 
> size */
> +#else
> +#define GRUB_PXE_BOOTP_DHCPVEND      312     /* DHCP standard vendor field 
> size */
> +#endif

A comment would be helpful to explain the 'if 1' option.

> --- a/kern/i386/pc/init.c
> +++ b/kern/i386/pc/init.c
> @@ -71,14 +71,19 @@ make_install_device (void)
>        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);
> +      if (grub_root_drive == 0x7F)
> +        grub_strcpy (dev, "(pxe");
> +      else
> +        {
> +          grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
> +                        grub_root_drive & 0x7f);
>        
> -      if (grub_install_dos_part >= 0)
> -     grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 
> 1);
> +          if (grub_install_dos_part >= 0)
> +         grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part 
> + 1);
>        
> -      if (grub_install_bsd_part >= 0)
> -     grub_sprintf (dev + grub_strlen (dev), ",%c", grub_install_bsd_part + 
> 'a');
> +          if (grub_install_bsd_part >= 0)
> +         grub_sprintf (dev + grub_strlen (dev), ",%c", grub_install_bsd_part 
> + 'a');
> +     }
>        
>        grub_sprintf (dev + grub_strlen (dev), ")%s", grub_prefix);
>        grub_strcpy (grub_prefix, dev);

Can we avoid this?  I wish make_install_device() was reduced and eventually
disappeared.

Since we already know at install time that the boot drive will be 0x7f, why not
just set the drive via grub-mkimage --prefix?

> --- a/kern/i386/pc/startup.S
> +++ b/kern/i386/pc/startup.S
> @@ -2053,3 +2053,102 @@ FUNCTION(grub_vbe_bios_set_palette_data)
>       popl    %ebx
>       popl    %ebp
>       ret
> +
> +
> +pxe_rm_entry:
> +     .long   0
> +
> +/*
> + * struct grub_pxenv *grub_pxe_scan (void);
> + */
> +FUNCTION(grub_pxe_scan)

Could these go in a module?  If they're only used by disk/i386/pc/pxe.c,
maybe they could be merged with that?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."