On Mon, 2011-07-25 at 18:12 -0400, Mathieu Trudel-Lapierre wrote: > Hi, > > We don't really need to wait before both IPv4 and IPv6 are established before > applying all the settings to the device. Instead, we can apply each separately > when they are ready, which will bring up the interface sooner. > > Patch is attached; since it's relatively large.
Hah, my first impression was that it was pretty contained and small :) Which is good. A few comments: 1) Just return a gboolean from update_ip6_config_with_dhcp(); the semantics of returning something that you've passed in are kinda weird, and makes it unclear whether what's returned is new, got referenced, etc. Also add an "NMDeviceStateReason *reason" argument which gets populated with the right error when things fail. 2) you don't want to call nm_device_state_changed() in update_ip6_config_with_dhcp() since the functions that call update_ip6_config_with_dhcp() do that depending on the return value; the callers better know what to do when this function fails. 3) Also, I don't think we need to care about LINK_LOCAL in update_ip6_config_with_dhcp() since with LINK_LOCAL you'll never be running DHCP at all; it's link-local. The only time you run DHCP are AUTO (when the RA tells you to) or DHCP, so just skip the check for LINK_LOCAL there. 4) also, just move merge_dhcp_config_to_master() above update_ip6_config_with_dhcp() and kill the forward declaration, looks kinda icky IMHO; don't worry about moving code around 5) You probably want the g_object_notify (G_OBJECT (device), NM_DEVICE_INTERFACE_DHCP6_CONFIG); statement inside the success block for nm_device_set_ip6_config () right after the 'dhcp6-change' dispatcher event gets fired, otherwise the notify gets sent out even if things fail which isnt' quite right. Also, what's the reason for killing nm_device_activate_schedule_stage5_ip_config_commit()? Looking good so far, lets do a respin. Thanks! Dan > --- > src/nm-device.c | 207 ++++++++++++++++++++++++++++-------------------------- > 1 files changed, 107 insertions(+), 100 deletions(-) > > -- > Mathieu Trudel-Lapierre <[email protected]> > Freenode: cyphermox, Jabber: [email protected] > 4096R/EE018C93 1967 8F7D 03A1 8F38 732E FF82 C126 33E1 EE01 8C93 > _______________________________________________ > networkmanager-list mailing list > [email protected] > http://mail.gnome.org/mailman/listinfo/networkmanager-list _______________________________________________ networkmanager-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/networkmanager-list
