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

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2011-04-11 at 23:13 +0200, Stefan Weil wrote:
> 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.

Ok, switched to bash, hopefully that works better for you.  Thanks for
finding that.

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

Simple enough.  I fixed both of these in the git tree, please verify.
Thanks,

Alex