Re: [PATCH] Fix manual IP config

[Helmut Schaa <hschaa suse de>]

Am Dienstag, 18. März 2008 19:48:28 schrieb Dan Williams:
> On Tue, 2008-03-18 at 12:46 -0600, Tambet Ingo wrote:
> > On Tue, Mar 18, 2008 at 12:36 PM, Dan Williams <dcbw redhat com> wrote:
> > > On Tue, 2008-03-18 at 11:37 +0100, Helmut Schaa wrote:
> > >  > Nevertheless I want to give you an hypothetical scenario why I would
> > >  > prefer it to be set in real_act_stage3_ip_config_start:
> > >  >
> > >  > As far as I can see nm_device_set_use_dhcp is defined in nm_device.h
> > >  > and could therefore be used from some places outside nm_device.cpp.
> > >  > It would be possible to call nm_device_set_use_dhcp even if the
> > >  > device is down. Afterwards starting a non-dhcp connection on that
> > >  > device would lead to the initial problem. Setting it in
> > >  > real_act_stage3_ip_config_start would avoid such issues.
> > >
> > >  Seems kind of odd; I can't think why we'd want to set it when the
> > > device is down?  The use-dhcp parameter should _only_ be set during
> > > activation, based on the 'method' parameter of the activating
> > > connection's ip4-config setting.  When the activation cancels, or the
> > > device is deactivated, the use-dhcp setting should be cleared, because
> > > the next time the device activates, it might have a different
> > > connection, and therefore have a different 'method' parameter in the
> > > ip4-config setting, and therefore use autoipv4 or static addressing
> > > instead of DHCP.
> >
> > Well, nm_device_set_use_dhcp() shouldn't really be public, it's
> > declaration should be in nm-device-private.h instead. The only allowed
> > callers should be NMDevice and it's subclasses. Or maybe not even
> > subclasses and the function should be static.
>
> Probably the last one; I can't think why the subclasses would need to
> touch the settings, since the decisions about IP4 config method (autoip,
> dhcp, manual) are all made by the NMDevice superclass in
> real_act_stage3_ip_config_start() and real_act_stage4_get_ip4_config().
>
> Note that as of now, the decision to do DHCP or not is currently made in
> stage3, and torn down in deactivate_quickly().
>
> So I think this issue is fixed?

If nm_device_set_use_dhcp is private I have no more objections :)

Helmut