[gPXE-devel] [PATCH] Fix autoboot for multiple NIC's on different networks

[Gianni Tedesco]

On Wed, 2010-07-07 at 17:21 +0100, Joshua Oreman wrote:
> On Wed, Jul 7, 2010 at 8:35 AM, Gianni Tedesco
> <gianni.tedesco at citrix.com> wrote:
> > Hi,
> >
> > Without this patch retrieving boot image from the second "next-server"
> > and "filename" actually ends up using the results obtained from DHCP on
> > the first NIC after boot either fails or continues.
> 
> I'm not convinced that this patch is correct. I think you want to
> clear the netX/filename and netX/root-path settings after a boot from
> netX fails, but still allow global filename and root-path settings to
> override them (which is the current behavior).

Oh I see. How about unregistering the entire DHCP and PROXYDHCP blocks
after the boot fails? Would seem to be the correct approach since that
maintains global settings but prevents any DHCP options gathered on one
NIC overriding those on the next.

> Also, the extra netdev_close() is almost certainly not correct,
> because boot_netdev can be NULL at that point.

Yes it's also redundant...

How about something like this?

diff --git a/src/core/settings.c b/src/core/settings.c
index 7d83101..0ef9fa2 100644
--- a/src/core/settings.c
+++ b/src/core/settings.c
@@ -237,7 +237,7 @@ struct generic_settings generic_settings_root = {
  * @v name             Name within this parent
  * @ret settings       Settings block, or NULL
  */
-static struct settings * find_child_settings ( struct settings *parent,
+struct settings * find_child_settings ( struct settings *parent,
                                               const char *name ) {
        struct settings *settings;
 
diff --git a/src/include/gpxe/settings.h b/src/include/gpxe/settings.h
index efefe73..6414ddd 100644
--- a/src/include/gpxe/settings.h
+++ b/src/include/gpxe/settings.h
@@ -209,6 +209,8 @@ extern int fetch_uuid_setting ( struct settings *settings,
 extern void clear_settings ( struct settings *settings );
 extern int setting_cmp ( struct setting *a, struct setting *b );
 
+extern struct settings * find_child_settings ( struct settings *parent,
+                                              const char *name );
 extern struct settings * find_settings ( const char *name );
 
 extern int storef_setting ( struct settings *settings,
diff --git a/src/usr/autoboot.c b/src/usr/autoboot.c
index d76751b..587f862 100644
--- a/src/usr/autoboot.c
+++ b/src/usr/autoboot.c
@@ -174,9 +174,18 @@ static int netboot ( struct net_device *netdev ) {
        fetch_ipv4_setting ( NULL, &next_server_setting, &next_server );
        fetch_string_setting ( NULL, &filename_setting, buf, sizeof ( buf ) );
        if ( buf[0] ) {
+               struct settings *dhcpset;
                printf ( "Booting from filename \"%s\"\n", buf );
-               if ( ( rc = boot_next_server_and_filename ( next_server,
-                                                           buf ) ) != 0 ) {
+               rc = boot_next_server_and_filename ( next_server, buf );
+               if ( ( dhcpset = find_child_settings (
+                               netdev_settings(netdev),
+                               DHCP_SETTINGS_NAME) ))
+                       unregister_settings ( dhcpset );
+               if ( ( dhcpset = find_child_settings (
+                               netdev_settings(netdev),
+                               PROXYDHCP_SETTINGS_NAME) ))
+                       unregister_settings ( dhcpset );
+               if ( rc ) {
                        printf ( "Could not boot from filename \"%s\": %s\n",
                                 buf, strerror ( rc ) );
                        return rc;