Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via email, send a message with subject or
body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH 2/3 v2] vpn-provider: Implement setting of multiple VPN
properties with one call
(Jussi Laakkonen)
2. Re: [PATCH 3/3 v2] doc: Describe property array use with VPN connection
SetProperty D-Bus method
(Jussi Laakkonen)
3. [PATCH 0/3] Explicitly define "Properties" as name for dict of properties
(Jussi Laakkonen)
4. [PATCH 2/3] doc: Mention "Properties" as SetProperty prop name in VPN
connection API
(Jussi Laakkonen)
5. [PATCH 3/3] error: Handle EPERM as PermissionDenied D-Bus error
(Jussi Laakkonen)
6. [PATCH 1/3] vpn-provider: Use property name "Properties" to indicate a
dict
(Jussi Laakkonen)
7. Re: [PATCH 0/3] Explicitly define "Properties" as name for dict of
properties
(Daniel Wagner)
----------------------------------------------------------------------
Date: Tue, 31 Dec 2019 11:27:44 +0200
From: Jussi Laakkonen <[email protected]>
Subject: Re: [PATCH 2/3 v2] vpn-provider: Implement setting of
multiple VPN properties with one call
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 12/30/19 4:10 PM, Daniel Wagner wrote:
> On 30.12.19 13:56, Jussi Laakkonen wrote:
>> On 12/21/19 1:53 PM, Daniel Wagner wrote:
>>> The missing concept in the Property API for changing multiple settings
>>> at once leads to a bit strange things. Obvioulsy, it would be more
>>> naturual to have single value set/get/clear/changed and the same thing
>>> for multiple properties at once. Well, since this doesn't exist
>>> neither for ConnMan nor for the freedesktop API let's go with this.
>>>
>>> Maybe we should bring this up at freedesktop and see what the
>>> maintainers of the Property API think about it.
>>>
>>
>> Having the methods for multiple properties would offer more
>> flexibility, for the use of the method and in the underlying
>> implementation. So yes, I'd agree that could be proposed in
>> freedesktop as well.
>>
>> Specification [1] does, however, state, that there is a
>> PropertiesChanged signal already in the D-Bus Properties interface
>> (since version 0.14 in 2010) that can be used to indicate which
>> properties were changed and which properties were invalidated (array
>> of strings). So, the error reporting for the need of setting multiple
>> properties at once (and clearing etc.) is already speficied. This may
>> be good to implement in connman as well, when service.c would support
>> setting multiple properties in one call.
>
> The only thing we need to be careful, is not breaking existing users.
> That means we still need to fire individual property changed signals.
Yes, individual property changed signals should be kept as is. But I did
get an idea from that approach for this multiple property setting. Would
it sound reasonable to send the PropertiesChanged as well when multiple
properties are set with one call, as the specification already supports
a list of invalidated properties?
So instead of packing them at the end of the error message that signal
could be used to indicate the client about the change. The D-Bus method
call return just would contain then a simple error, where invalid
property > permission denied, in case any of the property values could
not be changed.
I know, that would create duplicate content sent to clients and more
noise in D-Bus but it would be in line with the specs. Any thoughts?
>
>> From the looks of it, PropertiesChanged is used only by the network
>> manager compatiblity plugin (plugins/nmcompat.c) and gsupplicant.
>> PropertiesChanged would not fit to all situations but to the ones
>> where multiple properties are changed in any way.
>
> IIRC, we needed it to emulate the NM interfaces. The 1.0 release of
> ConnMan shipped without this API. In hindsight, it would have been
> better to go with a PropertiesChanged...
>
Ah, some legacy content then. I'll nevermind that :)
Cheers,
Jussi
------------------------------
Date: Tue, 31 Dec 2019 13:19:24 +0200
From: Jussi Laakkonen <[email protected]>
Subject: Re: [PATCH 3/3 v2] doc: Describe property array use with VPN
connection SetProperty D-Bus method
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 12/30/19 4:04 PM, Daniel Wagner wrote:
> "Properties" is probably the best name for it. We call it so in the
> documentation already, e.g. service-api.txt. This would be consistent
> and as you say, we can simplify the parser.
I think I did promise too much with that parser simplification. I
fiddled with the code for hours and all I came up with was a even more
complex solution. So that simplification is left for further work.
I think at this point it is best to do a simpler change of including
"Properties" as name in the code and doc. I'll send a small patch later
today.
Cheers,
Jussi
------------------------------
Date: Tue, 31 Dec 2019 16:54:49 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 0/3] Explicitly define "Properties" as name for dict
of properties
To: [email protected]
Message-ID: <[email protected]>
Use property name "Properties" to indicate that multiple properties are
to be set with a dict. Document this also in the VPN connection API. To
conform with the specification in the VPN connection API handle also
EPERM in src/error.c as PermissionDenied D-Bus error.
Jussi Laakkonen (3):
vpn-provider: Use property name "Properties" to indicate a dict
doc: Mention "Properties" as SetProperty prop name in VPN connection
API
error: Handle EPERM as PermissionDenied D-Bus error
doc/vpn-connection-api.txt | 3 ++-
src/error.c | 1 +
vpn/vpn-provider.c | 6 +-----
3 files changed, 4 insertions(+), 6 deletions(-)
--
2.20.1
------------------------------
Date: Tue, 31 Dec 2019 16:54:51 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 2/3] doc: Mention "Properties" as SetProperty prop
name in VPN connection API
To: [email protected]
Message-ID: <[email protected]>
Mention that "Properties" property name is to be used for a dict when
multiple properties are to be set using SetProperty.
---
doc/vpn-connection-api.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/vpn-connection-api.txt b/doc/vpn-connection-api.txt
index 80080ebe..cb5de69b 100644
--- a/doc/vpn-connection-api.txt
+++ b/doc/vpn-connection-api.txt
@@ -18,7 +18,8 @@ Methods dict GetProperties() [experimental]
properties defined as a dict passed as variant, where
the format is equal to the dict returned by
GetProperties(). Only properties that are listed as
- read-write are changeable. On success a
+ read-write are changeable. Property name "Properties"
+ indicates a dict of properties. On success a
PropertyChanged signal will be emitted for the
specified property or for all changed properties
individually. If there is no change in property value
--
2.20.1
------------------------------
Date: Tue, 31 Dec 2019 16:54:52 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 3/3] error: Handle EPERM as PermissionDenied D-Bus
error
To: [email protected]
Message-ID: <[email protected]>
PermissionDenied error must be sent also when EPERM is the errorcode.
This is used in vpn-provider.c to indicate that property to be changed
or cleared is immutable and cannot be changed. This change ensures that
the replied error message corresponds to the situation.
---
src/error.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/error.c b/src/error.c
index 660e57d3..a7a8a1de 100644
--- a/src/error.c
+++ b/src/error.c
@@ -39,6 +39,7 @@ DBusMessage *__connman_error_failed(DBusMessage *msg, int
errnum)
return __connman_error_not_registered(msg);
case ENXIO:
return __connman_error_not_found(msg);
+ case EPERM:
case EACCES:
return __connman_error_permission_denied(msg);
case EEXIST:
--
2.20.1
------------------------------
Date: Tue, 31 Dec 2019 16:54:50 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 1/3] vpn-provider: Use property name "Properties" to
indicate a dict
To: [email protected]
Message-ID: <[email protected]>
Property name "Properties" is to be used when multiple properties are to
be set with one D-Bus call. It is better to have the property name
explicitly defined to avoid confusion.
---
vpn/vpn-provider.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 8a002bd4..d93f5b47 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -654,11 +654,7 @@ static DBusMessage *set_property(DBusConnection *conn,
DBusMessage *msg,
dbus_message_iter_recurse(&iter, &value);
type = dbus_message_iter_get_arg_type(&value);
- /*
- * If message name other than UserRoutes array process it as an array
- * of multiple properties.
- */
- if (type == DBUS_TYPE_ARRAY && !g_str_equal(name, "UserRoutes"))
+ if (type == DBUS_TYPE_ARRAY && g_str_equal(name, "Properties"))
return set_properties(&value, msg, data);
err = set_provider_property(provider, name, &value, type);
--
2.20.1
------------------------------
Date: Tue, 31 Dec 2019 16:28:27 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 0/3] Explicitly define "Properties" as name for
dict of properties
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Tue, Dec 31, 2019 at 04:54:49PM +0200, Jussi Laakkonen wrote:
> Use property name "Properties" to indicate that multiple properties are
> to be set with a dict. Document this also in the VPN connection API. To
> conform with the specification in the VPN connection API handle also
> EPERM in src/error.c as PermissionDenied D-Bus error.
All patches applied. Thanks!
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
End of connman Digest, Vol 50, Issue 14
***************************************
