Re: [PATCH] PXE support for grub2

[Robert Millan <rmh@aybabtu.com>]

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."