Re: [Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs

[Stefan Weil <weil@mail.berlios.de>]

Am 11.04.2011 21:35, schrieb Alex Williamson:
> This series replaces our current gPXE based PXE ROMs with iPXE
> versions from the iPXE project (http://ipxe.org). This version
> adds ipxe to our submodules so it can be easily included in
> releases. I'm still including a script for updating these,
> perhaps someone better with Makefiles can eventually adopt this
> to a build target.
>
> This email series is mainly for reference, there's too much
> renaming and replacing binary files to send out to the mailing
> list. I'll strip out the binaries here so the rest can be
> reviewed. For the real code, please pull:
>
> git://github.com/awilliam/qemu.git (ipxe branch)
>
> Thanks to Anthony for already setting up an ipxe mirror.
> Thanks,
>
> Alex
>
> ---
>
> Alex Williamson (3):
> PXE: Refresh all PXE ROMs from the ipxe submodule
> PXE: Use consistent naming for PXE ROMs
> Add ipxe submodule
>
>
> .gitmodules | 3 +
> Makefile | 16 +++---
> hw/e1000.c | 2 -
> hw/eepro100.c | 2 -
> hw/ne2000.c | 2 -
> hw/pcnet-pci.c | 2 -
> hw/rtl8139.c | 2 -
> hw/virtio-pci.c | 2 -
> pc-bios/README | 19 +++----
> pc-bios/gpxe-eepro100-80861209.rom | Bin
> pc-bios/pxe-e1000.bin | Bin
> pc-bios/pxe-e1000.rom | Bin
> pc-bios/pxe-eepro100.rom | Bin
> pc-bios/pxe-ne2k_pci.bin | Bin
> pc-bios/pxe-ne2k_pci.rom | Bin
> pc-bios/pxe-pcnet.bin | Bin
> pc-bios/pxe-pcnet.rom | Bin
> pc-bios/pxe-rtl8139.bin | Bin
> pc-bios/pxe-rtl8139.rom | Bin
> pc-bios/pxe-virtio.bin | Bin
> pc-bios/pxe-virtio.rom | Bin
> roms/ipxe | 1
> scripts/refresh-pxe-roms.sh | 99 ++++++++++++++++++++++++++++++++++++
> 23 files changed, 126 insertions(+), 24 deletions(-)
> delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom
> delete mode 100644 pc-bios/pxe-e1000.bin
> create mode 100644 pc-bios/pxe-e1000.rom
> create mode 100644 pc-bios/pxe-eepro100.rom
> delete mode 100644 pc-bios/pxe-ne2k_pci.bin
> create mode 100644 pc-bios/pxe-ne2k_pci.rom
> delete mode 100644 pc-bios/pxe-pcnet.bin
> create mode 100644 pc-bios/pxe-pcnet.rom
> delete mode 100644 pc-bios/pxe-rtl8139.bin
> create mode 100644 pc-bios/pxe-rtl8139.rom
> delete mode 100644 pc-bios/pxe-virtio.bin
> create mode 100644 pc-bios/pxe-virtio.rom
> create mode 160000 roms/ipxe
> create mode 100755 scripts/refresh-pxe-roms.sh
>

Hi,

thanks for your patches. I have two small remarks:

$ scripts/refresh-pxe-roms.sh
scripts/refresh-pxe-roms.sh: 29: Syntax error: "(" unexpected

Obviously this script depends on features only supported by
more advanced shells like bash. It fails with /bin/sh -> dash.

My second remark is about lists like those in Makefile:
they should be sorted alphabetically.
This may seem a cosmetic change, but it improves readability,
avoids errors like duplicate or missing entries and also can
avoid merge conflicts.

Regards,
Stefan Weil