"enable networking" patch, bug #238325

[Jirka Klimes]

Em Tuesday 14 September 2010, Rex Dieter escreveu:
> > On 09/14/2010 11:37 AM, Sebastian Kügler wrote:
> > > Hey Rex,
> > > 
> > > On Tuesday, September 14, 2010 18:38:03 Rex Dieter wrote:
> > >> Can I get some comment and feedback on the patch attached to bug
> > >> #238325, "Adding "Enable networking" button to knetworkmanager""
> > >> 
> > >> It seems to work as advertised in testing, includes adding a bit to
> > >> solid as  well.  OK to commit?
> > > 
> > > knetworkmanager isn't even build (and it won't with newer versions of
> > > dbusmenu), what's likely to happen is that I'll delete is because it's
> > > bitrotting as we speak. Our focus is fully on the Plasmoid (which 
doesn't
> > > have the problem to begin with).
> > > 
> > > Best is probably to ship it as a local patch for those that need a fix
> > > for that particular problem.
> > 
> > To be clear, thepatch fixes both monolithic and plasma applet, and adds
> > a solid method for enable/disable.  So, it's more than just monolithic
> > knm being affected here.

>         As sebas said Plasma NM does not have this particular problem. If 
> NetworkManager is sleeping and you click on "Enable Networking" checkbox 
> (maybe you need to do it twice), it will wake up NetworkManager and 
everything 
> works. The problem I guess is that most people does not realize that is the 
> way to wake NetworkManager with Plasma NM.

>        I got you point about adding the "Enable" method to NM. It is in NM 
0.8.1 
> specification so probably we will need to add it in the future and change the 
> "Enable Networking" checkbox in Plasma NM to use it instead of the > 
sleep/wake 
> methods, which is what your patches do. By what I read in NM's git 
repository 
> sleep/wake is not going to change the file 
>/var/lib/NetworkManager/NetworkManager.state anymore, only the "Enable" 
method 
> is going to do it. If we want the "Enable Networking" checkbox to work 
during 
> reboots probably we will need to use this new "Enable" method instead of 
> sleep/wake or it will not work, although I think few people is going to make 
> use of this scenario.

There were many problems due to using a single knob for two different things: 
i.e. sleep/wake used both for suspend/resume and user enable/disable stuff.

That's why now the states are separated and sleep/wake is dedicated just for 
suspend cases and Enable is meant for user enablement/disablement usage.
Moreover, Sleep is now protected via PolicyKit and is available just for root 
by default.
See commit: 
http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=65818d517e386c03daca08db4be7b53aff94359a

That means that without the patches "Enable networking" checkbox in the 
plasmoid won't work for NM after 30.5.2010. Of course, there is a workaround 
to enable the sleep for normal users:
Set allow_active to yes for org.freedesktop.NetworkManager.sleep-wake action
in /usr/share/polkit-1/actions/org.freedesktop.NetworkManager.policy
However, that would go against the intended functionality and applying patches 
is much better to keep track of upstream NM, IMO.

>        Please change this slot to use the parameter enabled instead of doing 
a 
> solid call, the parameter is there for that reason:

> +void NMPopup::managerNetworkingEnabledChanged(bool enabled)
> +{
> +    kDebug() << "NM daemon changed networking enable state" << enabled;
> +    m_networkingCheckBox-> 
setChecked(Solid::Control::NetworkManager::isNetworkingEnabled());
> +}
> +

Updated patch included in the kde #238325, thanks.

Jirka