[Qemu-devel] [PATCH] pc-bios: Use iPXE ROMs

[Qemu-devel] [PATCH] pc-bios: Use iPXE ROMs

[Alex Williamson]

Refresh PXE ROMs from the iPXE project (http://ipxe.org).  This
includes moving eepro100 to use standard naming and including a
script to easily refresh PXE ROMs from either a local git tree
or the upstream project.  We include the 'git describe' output
in the resulting product name, making it easy to identify and
reproduce.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Anthony - I've skipped actually including the binary diffs here to
be friendly to the mailing list for review.  You can reproduce them
with the included script.  Note that pc-bios/gpxe-eepro100-80861209.rom
is moved to pc-bios/pxe-eepro100.bin when doing the commit.  I'll be
happy to tar up the commit if you prefer.  I've tested these ROMs using
ipxe v1.0.0-492-g23daab1.

 Makefile                           |    2 -
 hw/eepro100.c                      |    2 -
 pc-bios/README                     |    9 ++
 pc-bios/gpxe-eepro100-80861209.rom |  Bin
 pc-bios/pxe-e1000.bin              |  Bin
 pc-bios/pxe-eepro100.bin           |  Bin
 pc-bios/pxe-ne2k_pci.bin           |  Bin
 pc-bios/pxe-pcnet.bin              |  Bin
 pc-bios/pxe-rtl8139.bin            |  Bin
 pc-bios/pxe-virtio.bin             |  Bin
 scripts/refresh-pxe-roms.sh        |  138 ++++++++++++++++++++++++++++++++++++
 11 files changed, 146 insertions(+), 5 deletions(-)
 delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom
 create mode 100644 pc-bios/pxe-eepro100.bin
 create mode 100755 scripts/refresh-pxe-roms.sh

diff --git a/Makefile b/Makefile
index eca4c76..55dee6f 100644
--- a/Makefile
+++ b/Makefile
@@ -207,7 +207,7 @@ ifdef INSTALL_BLOBS
 BLOBS=bios.bin vgabios.bin vgabios-cirrus.bin \
 vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
 ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
-gpxe-eepro100-80861209.rom \
+pxe-eepro100.bin \
 pxe-e1000.bin \
 pxe-ne2k_pci.bin pxe-pcnet.bin \
 pxe-rtl8139.bin pxe-virtio.bin \
diff --git a/hw/eepro100.c b/hw/eepro100.c
index edf48f6..ad63f50 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -2054,7 +2054,7 @@ static void eepro100_register_devices(void)
         PCIDeviceInfo *pci_dev = &e100_devices[i].pci;
         /* We use the same rom file for all device ids.
            QEMU fixes the device id during rom load. */
-        pci_dev->romfile = "gpxe-eepro100-80861209.rom";
+        pci_dev->romfile = "pxe-eepro100.bin";
         pci_dev->init = e100_nic_init;
         pci_dev->exit = pci_nic_uninit;
         pci_dev->qdev.props = e100_properties;
diff --git a/pc-bios/README b/pc-bios/README
index 3fc0944..27ad8fc 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -13,7 +13,12 @@
   The included image for PowerPC (for 32 and 64 bit PPC CPUs), Sparc32
   and Sparc64 are built from OpenBIOS SVN revision 1018.
 
-- The PXE roms come from Rom-o-Matic gPXE 0.9.9 with BANNER_TIMEOUT=0
+- The PXE roms come from iPXE.  Use scripts/refresh-pxe-roms.sh to
+  update, example:
+
+  [~/qemu.git]$ ./scripts/refresh-pxe-roms.sh [~/ipxe.git]
+
+  Currently builds the following ROMs:
 
   e1000 8086:100E
   eepro100 8086:1209 (also used for 8086:1229 and 8086:2449)
@@ -22,8 +27,6 @@
   rtl8139 10ec:8139
   virtio 1af4:1000
 
-  http://rom-o-matic.net/
-
 - The S390 zipl loader is an addition to the official IBM s390-tools
   package. That fork is maintained in its own git repository at:
   git://repo.or.cz/s390-tools.git
diff --git a/scripts/refresh-pxe-roms.sh b/scripts/refresh-pxe-roms.sh
new file mode 100755
index 0000000..3ed0916
--- /dev/null
+++ b/scripts/refresh-pxe-roms.sh
@@ -0,0 +1,138 @@
+#!/bin/sh
+
+# PXE ROM build script
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see <http://www.gnu.org/licenses/>.
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#   Authors: Alex Williamson <alex.williamson@redhat.com>
+#
+# Usage: Run from root of qemu tree
+# ./scripts/refresh-pxe-roms.sh [path to existing ipxe tree [build options]]
+
+QEMU_DIR=$PWD
+LOCAL_CONFIG="src/config/local/general.h"
+
+function cleanup ()
+{
+	if [ -n "$SAVED_CONFIG" ]; then
+		cp $SAVED_CONFIG $BUILD_DIR/$LOCAL_CONFIG
+		rm $SAVED_CONFIG
+	fi
+	if [ -n "$NEW_TREE" ]; then
+		rm -fr $NEW_TREE
+	fi
+	cd $QEMU_DIR
+}
+
+function usage ()
+{
+	echo "usage: $0 <path to ipxe git tree> [extra ipxe build options]"
+	cleanup
+	exit 1
+}
+
+function make_all ()
+{
+	cd $BUILD_DIR/src
+
+	BUILD_LOG=$(mktemp)
+
+	echo Building...
+
+	make $1 > $BUILD_LOG 2>&1
+
+	if [ $? -ne 0 ]; then
+		echo Build failed
+		tail --lines=100 $BUILD_LOG
+		rm $BUILD_LOG
+		cleanup
+		exit 1
+	fi
+	rm $BUILD_LOG
+
+	cd $QEMU_DIR
+}
+
+function make_rom ()
+{
+	cd $BUILD_DIR/src
+
+	BUILD_LOG=$(mktemp)
+
+	echo Building $2...
+	make bin/$1.rom $3 > $BUILD_LOG 2>&1
+
+	if [ $? -ne 0 ]; then
+		echo Build failed
+		tail --lines=100 $BUILD_LOG
+		rm $BUILD_LOG
+		cleanup
+		exit 1
+	fi
+	rm $BUILD_LOG
+
+	cp bin/$1.rom $QEMU_DIR/pc-bios/$2
+
+	cd $QEMU_DIR
+}
+
+if [ ! -d $QEMU_DIR/pc-bios ]; then
+	echo "error: can't find pc-bios directory," \
+             "run me from the root of the qemu tree"
+	usage
+fi
+
+if [ -z $1 ]; then
+	NEW_TREE=$(mktemp -d)
+	git clone git://git.ipxe.org/ipxe.git $NEW_TREE
+	if [ $? -ne 0 ]; then
+		echo "error: failed to clone ipxe git tree"
+		cleanup
+		exit 1
+	fi
+	BUILD_DIR=$NEW_TREE
+else
+	BUILD_DIR=$1
+fi
+
+if [ -e $BUILD_DIR/$LOCAL_CONFIG ]; then
+	SAVED_CONFIG=$(mktemp)
+	cp $BUILD_DIR/$LOCAL_CONFIG $SAVED_CONFIG
+fi
+
+echo "#undef BANNER_TIMEOUT" > $BUILD_DIR/$LOCAL_CONFIG
+echo "#define BANNER_TIMEOUT 0" >> $BUILD_DIR/$LOCAL_CONFIG
+
+IPXE_VERSION=$(cd $BUILD_DIR && git describe --tags)
+if [ -z $IPXE_VERSION ]; then
+	echo "error: unable to retrieve git version"
+	cleanup
+	exit 1
+fi
+
+echo "#undef PRODUCT_NAME" >> $BUILD_DIR/$LOCAL_CONFIG
+echo "#define PRODUCT_NAME \"iPXE $IPXE_VERSION\"" >> $BUILD_DIR/$LOCAL_CONFIG
+
+make_all $2
+
+make_rom 8086100e pxe-e1000.bin $2
+make_rom 80861209 pxe-eepro100.bin $2
+make_rom 10500940 pxe-ne2k_pci.bin $2
+make_rom 10222000 pxe-pcnet.bin $2
+make_rom 10ec8139 pxe-rtl8139.bin $2
+make_rom 1af41000 pxe-virtio.bin $2
+
+echo done
+cleanup

[Qemu-devel] Re: [PATCH] pc-bios: Use iPXE ROMs

[Anthony Liguori]

On 03/15/2011 12:00 PM, Alex Williamson wrote:
> Refresh PXE ROMs from the iPXE project (http://ipxe.org).  This
> includes moving eepro100 to use standard naming and including a
> script to easily refresh PXE ROMs from either a local git tree
> or the upstream project.  We include the 'git describe' output
> in the resulting product name, making it easy to identify and
> reproduce.
>
> Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> ---
>
> Anthony - I've skipped actually including the binary diffs here to
> be friendly to the mailing list for review.  You can reproduce them
> with the included script.  Note that pc-bios/gpxe-eepro100-80861209.rom
> is moved to pc-bios/pxe-eepro100.bin when doing the commit.  I'll be
> happy to tar up the commit if you prefer.  I've tested these ROMs using
> ipxe v1.0.0-492-g23daab1.
>
>   Makefile                           |    2 -
>   hw/eepro100.c                      |    2 -
>   pc-bios/README                     |    9 ++
>   pc-bios/gpxe-eepro100-80861209.rom |  Bin
>   pc-bios/pxe-e1000.bin              |  Bin
>   pc-bios/pxe-eepro100.bin           |  Bin
>   pc-bios/pxe-ne2k_pci.bin           |  Bin
>   pc-bios/pxe-pcnet.bin              |  Bin
>   pc-bios/pxe-rtl8139.bin            |  Bin
>   pc-bios/pxe-virtio.bin             |  Bin
>   scripts/refresh-pxe-roms.sh        |  138 ++++++++++++++++++++++++++++++++++++
>   11 files changed, 146 insertions(+), 5 deletions(-)
>   delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom
>   create mode 100644 pc-bios/pxe-eepro100.bin
>   create mode 100755 scripts/refresh-pxe-roms.sh

We should probably just put ipxe in roms/ as a git submodule and have 
the make infrastructure (optionally) build each rom as appropriate.

That way when we distribute releases, the ipxe code is part of the tarball.

Regards,

Anthony Liguori

> diff --git a/Makefile b/Makefile
> index eca4c76..55dee6f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -207,7 +207,7 @@ ifdef INSTALL_BLOBS
>   BLOBS=bios.bin vgabios.bin vgabios-cirrus.bin \
>   vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
>   ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
> -gpxe-eepro100-80861209.rom \
> +pxe-eepro100.bin \
>   pxe-e1000.bin \
>   pxe-ne2k_pci.bin pxe-pcnet.bin \
>   pxe-rtl8139.bin pxe-virtio.bin \
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index edf48f6..ad63f50 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -2054,7 +2054,7 @@ static void eepro100_register_devices(void)
>           PCIDeviceInfo *pci_dev =&e100_devices[i].pci;
>           /* We use the same rom file for all device ids.
>              QEMU fixes the device id during rom load. */
> -        pci_dev->romfile = "gpxe-eepro100-80861209.rom";
> +        pci_dev->romfile = "pxe-eepro100.bin";
>           pci_dev->init = e100_nic_init;
>           pci_dev->exit = pci_nic_uninit;
>           pci_dev->qdev.props = e100_properties;
> diff --git a/pc-bios/README b/pc-bios/README
> index 3fc0944..27ad8fc 100644
> --- a/pc-bios/README
> +++ b/pc-bios/README
> @@ -13,7 +13,12 @@
>     The included image for PowerPC (for 32 and 64 bit PPC CPUs), Sparc32
>     and Sparc64 are built from OpenBIOS SVN revision 1018.
>
> -- The PXE roms come from Rom-o-Matic gPXE 0.9.9 with BANNER_TIMEOUT=0
> +- The PXE roms come from iPXE.  Use scripts/refresh-pxe-roms.sh to
> +  update, example:
> +
> +  [~/qemu.git]$ ./scripts/refresh-pxe-roms.sh [~/ipxe.git]
> +
> +  Currently builds the following ROMs:
>
>     e1000 8086:100E
>     eepro100 8086:1209 (also used for 8086:1229 and 8086:2449)
> @@ -22,8 +27,6 @@
>     rtl8139 10ec:8139
>     virtio 1af4:1000
>
> -  http://rom-o-matic.net/
> -
>   - The S390 zipl loader is an addition to the official IBM s390-tools
>     package. That fork is maintained in its own git repository at:
>     git://repo.or.cz/s390-tools.git
> diff --git a/scripts/refresh-pxe-roms.sh b/scripts/refresh-pxe-roms.sh
> new file mode 100755
> index 0000000..3ed0916
> --- /dev/null
> +++ b/scripts/refresh-pxe-roms.sh
> @@ -0,0 +1,138 @@
> +#!/bin/sh
> +
> +# PXE ROM build script
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, see<http://www.gnu.org/licenses/>.
> +#
> +# Copyright (C) 2011 Red Hat, Inc.
> +#   Authors: Alex Williamson<alex.williamson@redhat.com>
> +#
> +# Usage: Run from root of qemu tree
> +# ./scripts/refresh-pxe-roms.sh [path to existing ipxe tree [build options]]
> +
> +QEMU_DIR=$PWD
> +LOCAL_CONFIG="src/config/local/general.h"
> +
> +function cleanup ()
> +{
> +	if [ -n "$SAVED_CONFIG" ]; then
> +		cp $SAVED_CONFIG $BUILD_DIR/$LOCAL_CONFIG
> +		rm $SAVED_CONFIG
> +	fi
> +	if [ -n "$NEW_TREE" ]; then
> +		rm -fr $NEW_TREE
> +	fi
> +	cd $QEMU_DIR
> +}
> +
> +function usage ()
> +{
> +	echo "usage: $0<path to ipxe git tree>  [extra ipxe build options]"
> +	cleanup
> +	exit 1
> +}
> +
> +function make_all ()
> +{
> +	cd $BUILD_DIR/src
> +
> +	BUILD_LOG=$(mktemp)
> +
> +	echo Building...
> +
> +	make $1>  $BUILD_LOG 2>&1
> +
> +	if [ $? -ne 0 ]; then
> +		echo Build failed
> +		tail --lines=100 $BUILD_LOG
> +		rm $BUILD_LOG
> +		cleanup
> +		exit 1
> +	fi
> +	rm $BUILD_LOG
> +
> +	cd $QEMU_DIR
> +}
> +
> +function make_rom ()
> +{
> +	cd $BUILD_DIR/src
> +
> +	BUILD_LOG=$(mktemp)
> +
> +	echo Building $2...
> +	make bin/$1.rom $3>  $BUILD_LOG 2>&1
> +
> +	if [ $? -ne 0 ]; then
> +		echo Build failed
> +		tail --lines=100 $BUILD_LOG
> +		rm $BUILD_LOG
> +		cleanup
> +		exit 1
> +	fi
> +	rm $BUILD_LOG
> +
> +	cp bin/$1.rom $QEMU_DIR/pc-bios/$2
> +
> +	cd $QEMU_DIR
> +}
> +
> +if [ ! -d $QEMU_DIR/pc-bios ]; then
> +	echo "error: can't find pc-bios directory," \
> +             "run me from the root of the qemu tree"
> +	usage
> +fi
> +
> +if [ -z $1 ]; then
> +	NEW_TREE=$(mktemp -d)
> +	git clone git://git.ipxe.org/ipxe.git $NEW_TREE
> +	if [ $? -ne 0 ]; then
> +		echo "error: failed to clone ipxe git tree"
> +		cleanup
> +		exit 1
> +	fi
> +	BUILD_DIR=$NEW_TREE
> +else
> +	BUILD_DIR=$1
> +fi
> +
> +if [ -e $BUILD_DIR/$LOCAL_CONFIG ]; then
> +	SAVED_CONFIG=$(mktemp)
> +	cp $BUILD_DIR/$LOCAL_CONFIG $SAVED_CONFIG
> +fi
> +
> +echo "#undef BANNER_TIMEOUT">  $BUILD_DIR/$LOCAL_CONFIG
> +echo "#define BANNER_TIMEOUT 0">>  $BUILD_DIR/$LOCAL_CONFIG
> +
> +IPXE_VERSION=$(cd $BUILD_DIR&&  git describe --tags)
> +if [ -z $IPXE_VERSION ]; then
> +	echo "error: unable to retrieve git version"
> +	cleanup
> +	exit 1
> +fi
> +
> +echo "#undef PRODUCT_NAME">>  $BUILD_DIR/$LOCAL_CONFIG
> +echo "#define PRODUCT_NAME \"iPXE $IPXE_VERSION\"">>  $BUILD_DIR/$LOCAL_CONFIG
> +
> +make_all $2
> +
> +make_rom 8086100e pxe-e1000.bin $2
> +make_rom 80861209 pxe-eepro100.bin $2
> +make_rom 10500940 pxe-ne2k_pci.bin $2
> +make_rom 10222000 pxe-pcnet.bin $2
> +make_rom 10ec8139 pxe-rtl8139.bin $2
> +make_rom 1af41000 pxe-virtio.bin $2
> +
> +echo done
> +cleanup
>

[Qemu-devel] Re: [PATCH] pc-bios: Use iPXE ROMs

[Alex Williamson]

On Tue, 2011-03-15 at 12:35 -0500, Anthony Liguori wrote:
> On 03/15/2011 12:00 PM, Alex Williamson wrote:
> > Refresh PXE ROMs from the iPXE project (http://ipxe.org).  This
> > includes moving eepro100 to use standard naming and including a
> > script to easily refresh PXE ROMs from either a local git tree
> > or the upstream project.  We include the 'git describe' output
> > in the resulting product name, making it easy to identify and
> > reproduce.
> >
> > Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> > ---
> >
> > Anthony - I've skipped actually including the binary diffs here to
> > be friendly to the mailing list for review.  You can reproduce them
> > with the included script.  Note that pc-bios/gpxe-eepro100-80861209.rom
> > is moved to pc-bios/pxe-eepro100.bin when doing the commit.  I'll be
> > happy to tar up the commit if you prefer.  I've tested these ROMs using
> > ipxe v1.0.0-492-g23daab1.
> >
> >   Makefile                           |    2 -
> >   hw/eepro100.c                      |    2 -
> >   pc-bios/README                     |    9 ++
> >   pc-bios/gpxe-eepro100-80861209.rom |  Bin
> >   pc-bios/pxe-e1000.bin              |  Bin
> >   pc-bios/pxe-eepro100.bin           |  Bin
> >   pc-bios/pxe-ne2k_pci.bin           |  Bin
> >   pc-bios/pxe-pcnet.bin              |  Bin
> >   pc-bios/pxe-rtl8139.bin            |  Bin
> >   pc-bios/pxe-virtio.bin             |  Bin
> >   scripts/refresh-pxe-roms.sh        |  138 ++++++++++++++++++++++++++++++++++++
> >   11 files changed, 146 insertions(+), 5 deletions(-)
> >   delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom
> >   create mode 100644 pc-bios/pxe-eepro100.bin
> >   create mode 100755 scripts/refresh-pxe-roms.sh
> 
> We should probably just put ipxe in roms/ as a git submodule and have 
> the make infrastructure (optionally) build each rom as appropriate.
> 
> That way when we distribute releases, the ipxe code is part of the tarball.

That sounds fine to me.  Do you want to add git://git.ipxe.org/ipxe.git
as a submodule in roms/ipxe and I'll follow-up with a patch to switch
over?  I'm not sure how to deal with adding submodules via patches.
Thanks,

Alex

Re: [Qemu-devel] [PATCH] pc-bios: Use iPXE ROMs

[Stefan Weil]

Am 15.03.2011 18:00, schrieb Alex Williamson:
> Refresh PXE ROMs from the iPXE project (http://ipxe.org). This
> includes moving eepro100 to use standard naming and including a
> script to easily refresh PXE ROMs from either a local git tree
> or the upstream project. We include the 'git describe' output
> in the resulting product name, making it easy to identify and
> reproduce.

The name used for eepro100 was the result of this discussion thread:
http://lists.nongnu.org/archive/html/qemu-devel/2010-01/msg00729.html

So you could as well change all other rom names.

iPXE default names like 80861209.rom are nice, too.

But there are more important problems than rom naming.

>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> Anthony - I've skipped actually including the binary diffs here to
> be friendly to the mailing list for review. You can reproduce them
> with the included script. Note that pc-bios/gpxe-eepro100-80861209.rom
> is moved to pc-bios/pxe-eepro100.bin when doing the commit. I'll be
> happy to tar up the commit if you prefer. I've tested these ROMs using
> ipxe v1.0.0-492-g23daab1.
>
> Makefile | 2 -
> hw/eepro100.c | 2 -
> pc-bios/README | 9 ++
> pc-bios/gpxe-eepro100-80861209.rom | Bin
> pc-bios/pxe-e1000.bin | Bin
> pc-bios/pxe-eepro100.bin | Bin
> pc-bios/pxe-ne2k_pci.bin | Bin
> pc-bios/pxe-pcnet.bin | Bin
> pc-bios/pxe-rtl8139.bin | Bin
> pc-bios/pxe-virtio.bin | Bin
> scripts/refresh-pxe-roms.sh | 138 ++++++++++++++++++++++++++++++++++++
> 11 files changed, 146 insertions(+), 5 deletions(-)
> delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom
> create mode 100644 pc-bios/pxe-eepro100.bin
> create mode 100755 scripts/refresh-pxe-roms.sh

Re: [Qemu-devel] [PATCH] pc-bios: Use iPXE ROMs

[Alex Williamson]

On Tue, 2011-03-15 at 23:24 +0100, Stefan Weil wrote:
> Am 15.03.2011 18:00, schrieb Alex Williamson:
> > Refresh PXE ROMs from the iPXE project (http://ipxe.org). This
> > includes moving eepro100 to use standard naming and including a
> > script to easily refresh PXE ROMs from either a local git tree
> > or the upstream project. We include the 'git describe' output
> > in the resulting product name, making it easy to identify and
> > reproduce.

Sorry for not cc'ing, I picked the wrong Stefan.

> The name used for eepro100 was the result of this discussion thread:
> http://lists.nongnu.org/archive/html/qemu-devel/2010-01/msg00729.html

Thanks for the pointer, here's the relevant text:

        But a modified name without the gpxe version like
        gpxe-80861209.rom would have some advantages:
        
        * gpxe* is better than pxe* because the files contain
           a gPXE boot ROM - not a proprietary PXE ROM.

A generic pxe- prefix to me doesn't imply a proprietary ROM, simply the
purpose of the ROM.  Naming it gpxe- just means here we'd need to rename
everything to ipxe- and eventually lpxe-, npxe-, etc...

        * The romfiles are ROM files, not undefined binaries,
           so *.rom looks better than *.bin.

I agree with this one, I'm happy to rename them as .rom files.

        * For drivers like eepro100.c which implement several devices,
           a naming rule based on PCI device and vendor id (80861209)
           is better than a rule based on device names:
           devices with same ids can share the same romfile.

eepro100 now only has one ROM for all devices, so I think this is not
currently an issue.  As Kevin pointed out in the original thread,
pxe-eepro100 is more meaningful to a user than pxe-80861209.

        * Transforming an etherboot romfile name to a qemu romfile name
           is simple when all you have to do is to remove the version.
           This would also simplify pc-bios/README.

The README already leaves something to be desired for reproducibility,
I'm hoping the subtree and build script clear that up.

> So you could as well change all other rom names.
> 
> iPXE default names like 80861209.rom are nice, too.

Yes, but confusing when we end up using that same rom for every eepro100
variant.  Thanks,

Alex

Re: [Qemu-devel] [PATCH] pc-bios: Use iPXE ROMs

[Stefan Hajnoczi]

On Tue, Mar 15, 2011 at 5:00 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> diff --git a/scripts/refresh-pxe-roms.sh b/scripts/refresh-pxe-roms.sh

Please enclose variable expansions in double-quotes so paths with
spaces in them work.  For example:
cp $BUILD_DIR/$LOCAL_CONFIG $SAVED_CONFIG

This will not do what you want if BUILD_DIR, LOCAL_CONFIG, or
SAVED_CONFIG have spaces.  Instead use:
cp "$BUILD_DIR/$LOCAL_CONFIG" "$SAVED_CONFIG"

> +function make_all ()
> +{
> +       cd $BUILD_DIR/src
> +
> +       BUILD_LOG=$(mktemp)
> +
> +       echo Building...
> +
> +       make $1 > $BUILD_LOG 2>&1
> +
> +       if [ $? -ne 0 ]; then
> +               echo Build failed
> +               tail --lines=100 $BUILD_LOG
> +               rm $BUILD_LOG
> +               cleanup
> +               exit 1
> +       fi
> +       rm $BUILD_LOG
> +
> +       cd $QEMU_DIR
> +}

What is the purpose of this function?  If you build one of the ROMs
all of iPXE gets compiled.  I'm not sure why we need to make all and
then make specific ROMs.

Stefan

Re: [Qemu-devel] [PATCH] pc-bios: Use iPXE ROMs

[Alex Williamson]

On Wed, 2011-03-16 at 18:48 +0000, Stefan Hajnoczi wrote:
> On Tue, Mar 15, 2011 at 5:00 PM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
> > diff --git a/scripts/refresh-pxe-roms.sh b/scripts/refresh-pxe-roms.sh
> 
> Please enclose variable expansions in double-quotes so paths with
> spaces in them work.  For example:
> cp $BUILD_DIR/$LOCAL_CONFIG $SAVED_CONFIG
> 
> This will not do what you want if BUILD_DIR, LOCAL_CONFIG, or
> SAVED_CONFIG have spaces.  Instead use:
> cp "$BUILD_DIR/$LOCAL_CONFIG" "$SAVED_CONFIG"

Thanks, I'll update.

> > +function make_all ()
> > +{
> > +       cd $BUILD_DIR/src
> > +
> > +       BUILD_LOG=$(mktemp)
> > +
> > +       echo Building...
> > +
> > +       make $1 > $BUILD_LOG 2>&1
> > +
> > +       if [ $? -ne 0 ]; then
> > +               echo Build failed
> > +               tail --lines=100 $BUILD_LOG
> > +               rm $BUILD_LOG
> > +               cleanup
> > +               exit 1
> > +       fi
> > +       rm $BUILD_LOG
> > +
> > +       cd $QEMU_DIR
> > +}
> 
> What is the purpose of this function?  If you build one of the ROMs
> all of iPXE gets compiled.  I'm not sure why we need to make all and
> then make specific ROMs.

I just figured it'd be easier to debug if I separated out the main build
from each rom.  Maybe it just confuses things instead.  Thanks,

Alex