Re: [PATCH v2] Re: Change the state of iface from disconnected to connected using nmcli

[Dan Williams <dcbw redhat com>]

On Thu, 2012-05-17 at 17:25 +0200, Jirka Klimes wrote:
> On Wednesday 16 of May 2012 18:02:12 Dan Williams wrote:
> > On Mon, 2012-05-14 at 17:36 +0200, Jirka Klimes wrote:
> > > On Thursday 22 of March 2012 11:10:56 Dan Williams wrote:
> > > > On Sun, 2012-03-18 at 23:43 +0530, Abhijeet Rastogi wrote:
> > > > > Hi,
> > > > > 
> > > > > 
> > > > > This is my first post here. I have tried googling and looking at the
> > > > > manpage but no help.
> > > > > 
> > > > > 
> > > > > We can use
> > > > > 
> > > > > 
> > > > > $nmcli dev disconnect iface eth0
> > > > > 
> > > > > 
> > > > > to change the state of eth0 to disconnect. Is there a command to
> > > > > change it's state back to connected? I tried looking at the manpage &
> > > > > I can't seem to find  any option for that.
> > > > 
> > > > At the moment, you need to tell NM to reconnect to something, eg 'nmcli
> > > > con up uuid <uuid>'.  Disconnect places the device into manual mode
> > > > which requires user action to return to automatic mode.  I suppose we
> > > > can add an 'autoconnect' property to each device, which Disconnect()
> > > > would set to false, but which could be changed via the D-Bus properties
> > > > interface (authenticated of course) back to TRUE.  This property would
> > > > follow the internal 'autoconnect_inhibit' device member variable and the
> > > > NMPolicy would listen for changes to this variable and retrigger an auto
> > > > connection check if it changes back to TRUE.  Patches accepted for that.
> > > > 
> > > > Dan
> > > 
> > > Attached are patches adding 'Autoconnect' property to NMDevice for core NM
> > > and libnm-glib as well.
> > 
> > For the API docs, I'd say:
> > 
> > "If TRUE, indicates the device is allowed to autoconnect.  If FALSE,
> > manual intervention is required before the device will automatically
> > connect to a known network, such as activating a connection using the
> > device, or setting this property to TRUE."
> > 
> Changed the text in xml.
> 
> > There's a redundant NM_DEVICE_GET_PRIVATE() in
> > nm_device_get_autoconnect().
> > 
> Removed.
> 
> > The other thing we need to do is authenticate the property write call,
> > which we've got some code for in nm-manager.c::prop_filter().  It's
> > going to get a bit complicated since that function only works on the
> > manager object, so we'll have to have something that's a bit more
> > involved than just strcmp(propiface, NM_DBUS_IFACE).  That said, it
> > shouldn't be too bad.  We basically want to enforce the same permissions
> > as manager_device_disconnect_request() does.
> > 
> After some tries it seems to me that the easiest way is to tweak the 
> prop_filter() and use it.
> I have added a check for .Device interface and find the right NMDevice object
> according to object path extracted from the D-Bus message.
> NM_AUTH_PERMISSION_NETWORK_CONTROL permission is used.
> Does it look right?
> 
> I'm sending authorization changes as a separate patch, so that it is easier to
> see what changed.
> The changes were made to the previous "core" patch.
> "libnm-glib" didn't change.
> 
> > The rest of it looks good!
> > 
> > Dan
> > 
> > And while we're at it, to reduce confusion, we should just rename
> > "autoconnect_inhibit" to autoconnect.  Something like the attached patch
> > applied on top of yours?
> Changed to "autoconnect" variable.

One last thing with the permissions code; if we can't find the device
when we're about to call g_object_set(), we should return
NM_MANAGER_ERROR_UNKNOWN_DEVICE.  Other than that they both look good.

THanks!
Dan