Review Request: add support for network zones

[Lukáš Tinkl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------

Review request for Network Management and Lamarque Vieira Souza.


Description
-------

NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].

This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.

[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701

(on behalf of Jiri Popelka <jpopelka at redhat.com>)


Diffs
-----

backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157

Diff: http://git.reviewboard.kde.org/r/105324/diff/


Testing
-------


Thanks,

Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/08d3d857/attachment.html>

[Sebastian Kügler]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14991
-----------------------------------------------------------


Could you post a screenshot of the UI change, before & after?

- Sebastian K?gler

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/29c9e9af/attachment.html>

[Jiri Popelka]

Post by Lukáš Tinkl

Post by Sebastian Kügler
Could you post a screenshot of the UI change, before & after?

Loading Image...
Loading Image...


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14991
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/b18f7ec4/attachment-0001.html>

[Lamarque Vieira Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------



backends/NetworkManager/connectiondbus.cpp
<http://git.reviewboard.kde.org/r/105324/#comment11791>

Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.

I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.



libs/ui/connection.ui
<http://git.reviewboard.kde.org/r/105324/#comment11796>

If possible could you keep the order of items in increase order? This is row 2, it should go after row 1, not before row 0.



libs/ui/connection.ui
<http://git.reviewboard.kde.org/r/105324/#comment11797>

You should use KComboBox instead of QComboBox.



libs/ui/connection.ui
<http://git.reviewboard.kde.org/r/105324/#comment11799>

If this is a system connection and the user does not have permission to edit it this should be changed to false in the source code, no?



libs/ui/connectionwidget.cpp
<http://git.reviewboard.kde.org/r/105324/#comment11792>

It is not advisabled to include entire Qt modules into the source code. Please add only the classes you need here.



libs/ui/connectionwidget.cpp
<http://git.reviewboard.kde.org/r/105324/#comment11798>

Code style, you should use { } like this:

if (...) {
...
} else {
...
}

There are other places that needs the same code style change.


- Lamarque Vieira Souza

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/11de49cb/attachment.html>

[Jiri Popelka]

Post by Lukáš Tinkl

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/a4c6e7c6/attachment-0001.html>

[Lamarque Vieira Souza]

Post by Jiri Popelka

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html

Anyway, Plasma NM needs to support older versions of NetworkManager (distributions are not that eager to use the latest NM version), so I think it is better not add NM_SETTING_CONNECTION_ZONE to the map if it is empty.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Jiri Popelka
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/66a3d0b4/attachment.html>

[Lamarque Vieira Souza]

Post by Jiri Popelka

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html
Anyway, Plasma NM needs to support older versions of NetworkManager (distributions are not that eager to use the latest NM version), so I think it is better not add NM_SETTING_CONNECTION_ZONE to the map if it is empty.
We need to be able to add empty NM_SETTING_CONNECTION_ZONE to the map. That's how we specify 'Default' zone.

Your fix in the link above is going to be added into which NetworkManager version? Is there any released NetworkManager with that bug? If so then we need to add a warning somewhere that the user must use NetworkManager x.y.z.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Jiri Popelka
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/d8042d69/attachment.html>

[Lamarque Vieira Souza]

libs/ui/connection.ui, line 135
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70136#file70136line135>
If this is a system connection and the user does not have permission to edit it this should be changed to false in the source code, no?

Well, I must admit I don't know how this 'System connection' works.

Hmmm thinking about this again I think you do not need to change this part. NetworkManager is going to ask for the root password anyway when updating the system connection.

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html
Anyway, Plasma NM needs to support older versions of NetworkManager (distributions are not that eager to use the latest NM version), so I think it is better not add NM_SETTING_CONNECTION_ZONE to the map if it is empty.
We need to be able to add empty NM_SETTING_CONNECTION_ZONE to the map. That's how we specify 'Default' zone.
Your fix in the link above is going to be added into which NetworkManager version? Is there any released NetworkManager with that bug? If so then we need to add a warning somewhere that the user must use NetworkManager x.y.z.
NetworkManager-0.9.4 has this bug. I hope it's going to be fixed in next version, so 0.9.6 I guess. From my point of view it's not a big deal because to use these 'network zones' in NetworkManager you also need to have FirewallD running and Fedora is at the moment the only distro, which uses it. I think it'll take some time before other distributions notice FirewallD. And regarding Fedora: NetworkManager package in Fedora is done from git snapshots so it'll probably fixed quite soon there.

Does firewalld support pkgconfig? If so we can add a check in the main CMakeLists.txt and print a warning if it is found and NetworkManager version <= 0.9.4. Just for the distribution packagers be aware of the version we require to run properly.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/48e01e17/attachment-0001.html>

[Jiri Popelka]

Post by Jiri Popelka

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html
Anyway, Plasma NM needs to support older versions of NetworkManager (distributions are not that eager to use the latest NM version), so I think it is better not add NM_SETTING_CONNECTION_ZONE to the map if it is empty.

We need to be able to add empty NM_SETTING_CONNECTION_ZONE to the map. That's how we specify 'Default' zone.


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Jiri Popelka
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/f180d618/attachment-0001.html>

[Jiri Popelka]

Post by Jiri Popelka

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html
Anyway, Plasma NM needs to support older versions of NetworkManager (distributions are not that eager to use the latest NM version), so I think it is better not add NM_SETTING_CONNECTION_ZONE to the map if it is empty.
We need to be able to add empty NM_SETTING_CONNECTION_ZONE to the map. That's how we specify 'Default' zone.
Your fix in the link above is going to be added into which NetworkManager version? Is there any released NetworkManager with that bug? If so then we need to add a warning somewhere that the user must use NetworkManager x.y.z.

NetworkManager-0.9.4 has this bug. I hope it's going to be fixed in next version, so 0.9.6 I guess. From my point of view it's not a big deal because to use these 'network zones' in NetworkManager you also need to have FirewallD running and Fedora is at the moment the only distro, which uses it. I think it'll take some time before other distributions notice FirewallD. And regarding Fedora: NetworkManager package in Fedora is done from git snapshots so it'll probably fixed quite soon there.


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Jiri Popelka
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/0c2aa0a4/attachment-0001.html>

[Jiri Popelka]

Post by Lukáš Tinkl

libs/ui/connection.ui, line 135
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70136#file70136line135>
If this is a system connection and the user does not have permission to edit it this should be changed to false in the source code, no?

Well, I must admit I don't know how this 'System connection' works.


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120622/2c80560c/attachment-0001.html>

[Jiri Popelka]

Post by Lukáš Tinkl

libs/ui/connection.ui, line 14
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70136#file70136line14>
If possible could you keep the order of items in increase order? This is row 2, it should go after row 1, not before row 0.

I left what had Qt Designer created. However I've fixed it now and Lukas will update the patch soon (I don't seem to be able to update it myself. BTW: this has been my first Qt/KDE contribution).


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120625/dfad3a4c/attachment-0001.html>

[Jiri Popelka]

Post by Jiri Popelka

backends/NetworkManager/connectiondbus.cpp, line 195
<http://git.reviewboard.kde.org/r/105324/diff/1/?file=70133#file70133line195>
Some NetworkManager settings when empty strings must not be added to the dbus map or NetworkManager invalidates the connection. Is this the case for NM_SETTING_CONNECTION_ZONE? If so then you must check if it is empty and do not add it here if it is empty.
I cannot answer if that is the case for NM_SETTING_CONNECTION_ZONE because that seems to be a implicit rule not described in the NetworkManager specification. You must look into NetworkManager source code to check if it the case for NM_SETTING_CONNECTION_ZONE.

Actually yes, current NetworkManager invalidates the connection when NM_SETTING_CONNECTION_ZONE is empty. But it's a bug (caused by me) and I already sent patch to https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00087.html
Anyway, Plasma NM needs to support older versions of NetworkManager (distributions are not that eager to use the latest NM version), so I think it is better not add NM_SETTING_CONNECTION_ZONE to the map if it is empty.
We need to be able to add empty NM_SETTING_CONNECTION_ZONE to the map. That's how we specify 'Default' zone.
Your fix in the link above is going to be added into which NetworkManager version? Is there any released NetworkManager with that bug? If so then we need to add a warning somewhere that the user must use NetworkManager x.y.z.
NetworkManager-0.9.4 has this bug. I hope it's going to be fixed in next version, so 0.9.6 I guess. From my point of view it's not a big deal because to use these 'network zones' in NetworkManager you also need to have FirewallD running and Fedora is at the moment the only distro, which uses it. I think it'll take some time before other distributions notice FirewallD. And regarding Fedora: NetworkManager package in Fedora is done from git snapshots so it'll probably fixed quite soon there.
Does firewalld support pkgconfig? If so we can add a check in the main CMakeLists.txt and print a warning if it is found and NetworkManager version <= 0.9.4. Just for the distribution packagers be aware of the version we require to run properly.

No, it doesn't AFAIK.


- Jiri


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review14997
-----------------------------------------------------------

Post by Jiri Popelka
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 22, 2012, 9:54 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120625/5c17fa99/attachment-0001.html>

[Lukáš Tinkl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------

(Updated June 25, 2012, 10:41 a.m.)


Review request for Network Management and Lamarque Vieira Souza.


Changes
-------

Updated patch


Description
-------

NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].

This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.

[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701

(on behalf of Jiri Popelka <jpopelka at redhat.com>)


Diffs (updated)
-----

backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157

Diff: http://git.reviewboard.kde.org/r/105324/diff/


Testing
-------


Thanks,

Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120625/dac8e1ed/attachment.html>

[Lamarque Vieira Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15110
-----------------------------------------------------------

Ship it!


Ship It!

- Lamarque Vieira Souza

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120625/1fc94b00/attachment.html>

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15165
-----------------------------------------------------------


This review has been submitted with commit 71cff79a2128b6e62e61555cc38383196bbeb540 by Lukas Tinkl to branch nm09.

- Commit Hook

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120626/ce7aa2e3/attachment.html>

[Lamarque V. Souza]

Post by Lukáš Tinkl

Post by Commit Hook
This review has been submitted with commit
71cff79a2128b6e62e61555cc38383196bbeb540 by Lukas Tinkl to branch
nm09.

This commit caused build failure. I committed a fix, please review if the
UI patch is correct.

I think you have not pushed the commit.

- Christoph
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/#review15165
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3]
http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0
aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

[Christoph Feck]

Post by Lamarque V. Souza

Post by Lukáš Tinkl

Post by Commit Hook
This review has been submitted with commit
71cff79a2128b6e62e61555cc38383196bbeb540 by Lukas Tinkl to
branch nm09.

This commit caused build failure. I committed a fix, please
review if the UI patch is correct.

I think you have not pushed the commit.

I had pushed it to nm09 branch, because at that time, the review
commit was not in master. Luk?? also corrected the patch before
pushing to master, but it is now different in nm09 branch, likely
because my commit was wrong.

Luk??, can you please check and correct it?

Post by Lamarque V. Souza

- Christoph
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/#review15165
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira
Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network
Zones [2] since [3].
This patch adds one combo box that enables to change the
network zone. This box is visible only when FirewallD is
running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3]
http://cgit.freedesktop.org/NetworkManager/NetworkManager/commi
t/?id=1c0 aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

[Christoph Feck]

Post by Lukáš Tinkl

Post by Commit Hook
This review has been submitted with commit 71cff79a2128b6e62e61555cc38383196bbeb540 by Lukas Tinkl to branch nm09.

This commit caused build failure. I committed a fix, please review if the UI patch is correct.


- Christoph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15165
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120626/96b04286/attachment-0001.html>

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15171
-----------------------------------------------------------


This review has been submitted with commit af729e8244848c19be99eebc7250820ab830b2cf by Lukas Tinkl to branch master.

- Commit Hook

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120626/c0cabdaf/attachment.html>

[Kai Uwe Broulik]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15550
-----------------------------------------------------------


Could you check if your ifdefs are properly working?
On my machine, NM happily compiles but then refuses to create any connection (dropped me back in my workplan an entire week). When I removed that patch, it worked fine again.

- Kai Uwe Broulik

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120708/75a17975/attachment.html>

[Lamarque Vieira Souza]

Post by Lukáš Tinkl

Post by Kai Uwe Broulik
Could you check if your ifdefs are properly working?
On my machine, NM happily compiles but then refuses to create any connection (dropped me back in my workplan an entire week). When I removed that patch, it worked fine again.

Do you have firewalld installed? The patch should not affect anyone without firewalld installed (like me).


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15550
-----------------------------------------------------------

Post by Lukáš Tinkl
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120714/6a073823/attachment.html>

[Lamarque Vieira Souza]

Post by Lamarque Vieira Souza

Post by Kai Uwe Broulik
Could you check if your ifdefs are properly working?
On my machine, NM happily compiles but then refuses to create any connection (dropped me back in my workplan an entire week). When I removed that patch, it worked fine again.

Do you have firewalld installed? The patch should not affect anyone without firewalld installed (like me).

I have just pushed a fix for the problem. Please test and check if it works now.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15550
-----------------------------------------------------------

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120714/33914138/attachment.html>

[Raymond Wooninck]

Post by Lamarque Vieira Souza

Post by Kai Uwe Broulik
Could you check if your ifdefs are properly working?
On my machine, NM happily compiles but then refuses to create any connection (dropped me back in my workplan an entire week). When I removed that patch, it worked fine again.

Do you have firewalld installed? The patch should not affect anyone without firewalld installed (like me).
I have just pushed a fix for the problem. Please test and check if it works now.

Hi Lamarque,

I am running PNM on openSUSE Factory (12.2) and am 100% sure that I do not have firewalld installed. However even with the current latest snapshot from GIT, I can not add any new connections. Even trying to add a wired connection through the Network Settings KCM fails with the error message "error adding connection". Existing connections can be used, but new ones cannot be saved.


- Raymond


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15550
-----------------------------------------------------------

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120719/39341e9b/attachment.html>

[Lamarque Vieira Souza]

Post by Lamarque Vieira Souza

Post by Kai Uwe Broulik
Could you check if your ifdefs are properly working?
On my machine, NM happily compiles but then refuses to create any connection (dropped me back in my workplan an entire week). When I removed that patch, it worked fine again.

Do you have firewalld installed? The patch should not affect anyone without firewalld installed (like me).
I have just pushed a fix for the problem. Please test and check if it works now.
Hi Lamarque,
I am running PNM on openSUSE Factory (12.2) and am 100% sure that I do not have firewalld installed. However even with the current latest snapshot from GIT, I can not add any new connections. Even trying to add a wired connection through the Network Settings KCM fails with the error message "error adding connection". Existing connections can be used, but new ones cannot be saved.

I have just pushed a new fix, git pull and try again. In case of master you also must update QtNetworkManager (former libnm-qt).


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15550
-----------------------------------------------------------

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120719/615c91e7/attachment.html>

[Raymond Wooninck]

Post by Lamarque Vieira Souza

Post by Kai Uwe Broulik
Could you check if your ifdefs are properly working?
On my machine, NM happily compiles but then refuses to create any connection (dropped me back in my workplan an entire week). When I removed that patch, it worked fine again.

Do you have firewalld installed? The patch should not affect anyone without firewalld installed (like me).
I have just pushed a fix for the problem. Please test and check if it works now.
Hi Lamarque,
I am running PNM on openSUSE Factory (12.2) and am 100% sure that I do not have firewalld installed. However even with the current latest snapshot from GIT, I can not add any new connections. Even trying to add a wired connection through the Network Settings KCM fails with the error message "error adding connection". Existing connections can be used, but new ones cannot be saved.
I have just pushed a new fix, git pull and try again. In case of master you also must update QtNetworkManager (former libnm-qt).

Hi Lamarque,

After this latest update I am able to create new connections again. So I guess we can mark this little bugger as fixed. Thanks for your support.


- Raymond


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105324/#review15550
-----------------------------------------------------------

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/105324/
-----------------------------------------------------------
(Updated June 25, 2012, 10:41 a.m.)
Review request for Network Management and Lamarque Vieira Souza.
Description
-------
NetworkManager has had support for FirewallD [1] and Network Zones [2]
since [3].
This patch adds one combo box that enables to change the network zone.
This box is visible only when FirewallD is running.
[1] https://fedorahosted.org/firewalld/
[2] https://fedoraproject.org/wiki/Features/network-zones
[3] http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=1c0aeb457546ac5e485ca21dfe23f1a009912701
(on behalf of Jiri Popelka <jpopelka at redhat.com>)
Diffs
-----
backends/NetworkManager/connectiondbus.cpp 4649a19
libs/internals/connection.h 485b810
libs/internals/connection.cpp 67f31bc
libs/ui/connection.ui 7b9f873
libs/ui/connectionwidget.h aac7050
libs/ui/connectionwidget.cpp f67f157
Diff: http://git.reviewboard.kde.org/r/105324/diff/
Testing
-------
Thanks,
Luk?? Tinkl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20120722/eca1796d/attachment.html>