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. [PATCH 0/3 v2] Improve VPN property saving and property management
(Jussi Laakkonen)
2. [PATCH 2/3 v2] vpn-provider: Implement setting of multiple VPN properties
with one call
(Jussi Laakkonen)
3. [PATCH 3/3 v2] doc: Describe property array use with VPN connection
SetProperty D-Bus method
(Jussi Laakkonen)
----------------------------------------------------------------------
Date: Fri, 20 Dec 2019 11:55:07 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 0/3 v2] Improve VPN property saving and property
management
To: [email protected]
Message-ID: <[email protected]>
These changes improve the saving of VPN properties in order to avoid
losing information if connman-vpnd crashes. Also new feature to multiple
values in one D-Bus call is added.
The first patch in this set addresses the issue of not saving VPN
provider values immediately after the value has been changed over D-Bus.
To avoid unnecessary writes to disk the value of the property is
compared to the existing one. To do this for user routes, the routes are
read into a sorted list to make the comparing of them more efficient. As
a result each successful value change will be written immediately to
provider settings.
The second patch adds a new feature to be able to add and clear multiple
VPN provider properties using SetProperty D-Bus call. This is enabled
with set_properties() that accepts input in the same format as
GetProperties returns, a{sv}, wrapped inside a variant. With this change
the performance of changing multiple property values is improved because
the properties are written once to disk after the complete dict is
processed, if there is a change in at least one value. Empty property
values are allowed to be able to set and clear values in one D-Bus
method call. In case there are errorous properties these are reported
back as comma separated property name list at the end of the appropriate
error message.
The third patch adds documentation of the new feature in SetProperty
D-Bus method and documents all errors for SetProperty and ClearProperty
D-Bus methods.
Jussi Laakkonen (3):
vpn-provider: Save configuration only when a property value is changed
vpn-provider: Implement setting of multiple VPN properties with one
call
doc: Describe property array use with VPN connection SetProperty D-Bus
method
Changes since v2:
* Updated 2nd and 3rd patch summaries.
doc/vpn-connection-api.txt | 43 ++++-
vpn/vpn-provider.c | 327 +++++++++++++++++++++++++++++++++----
2 files changed, 330 insertions(+), 40 deletions(-)
--
2.20.1
------------------------------
Date: Fri, 20 Dec 2019 11:55:53 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 2/3 v2] vpn-provider: Implement setting of multiple
VPN properties with one call
To: [email protected]
Message-ID: <[email protected]>
Enhance SetProperty D-Bus method to accept an array of VPN properties in
order to enable setting of multiple properties with one D-Bus call. For
enabling this set_properties() method is implented, and is called when
an array is received that does not have name "UserRoutes". This improves
the performance when multiple (or all) properties are to be changed but
reduces the granularity of errors reported back to the caller. Properties
are accepted in the same format as with GetProperties D-Bus method sends
them, a{sv}, wrapped inside a variant value.
When an array of properties is sent each changed property value emits a
PropertyChanged signal. If there is at least one changed property
value then the VPN provider config is written to disk. Disk write is
done after all properties in the array are processed.
Empty strings as property values, or an empty array for "UserRoutes" are
allowed with this change when using SetProperty. In both cases the
effect is the same as calling ClearProperty on a property. Thus, this
change allows to clear multiple properties with one call when sending an
array of (empty) properties.
If there is at least one invalid property in the array then
InvalidProperty D-Bus error is sent as reply. Invalid properties >
immutable properties in case of error and there can be permission errors
caused by immutable properties when InvalidProperty error is sent. Only
when there are only permission errors then PermissionDenied D-Bus error
is sent. In both cases the D-Bus error message is amended with the
property names that caused the errors. If there are also properties that
had proper values and the values were changed a PropertyChange signal is
sent for each and VPN provider settings are written to disk.
Move setting of a single property into its own function, usable by both
set_property() and set_properties(). The set_vpn_property() retains the
functionality by changing either the routes or a single string value,
returns 0 when success, -EALREADY when there is no change on property
value and appropriate error (-EINVAL/-EPERM/-ENOMEM) otherwise.
---
Changes since v2:
* Removed SetProperties D-Bus method and moved functionality behind
SetProperty, which can be used to set an array of property values. If
an array is sent and it does not have name "UserRoutes" it is
processed as an array of properties.
* Changed to allow empty strings as values or empty arrays, both of
these will have the same effect as calling ClearProperty on a single
property.
* Updated the commit message and clarified issues, added mention of the
setting saving process.
* Changed commit title to be more appropriate.
vpn/vpn-provider.c | 193 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 161 insertions(+), 32 deletions(-)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index bf102536..8a002bd4 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -472,6 +472,159 @@ static bool compare_network_lists(GSList *a, GSList *b)
return true;
}
+static int set_provider_property(struct vpn_provider *provider,
+ const char *name, DBusMessageIter *value, int type)
+{
+ int err = 0;
+
+ DBG("provider %p", provider);
+
+ if (!provider || !name || !value)
+ return -EINVAL;
+
+ if (g_str_equal(name, "UserRoutes")) {
+ GSList *networks;
+
+ if (type != DBUS_TYPE_ARRAY)
+ return -EINVAL;
+
+ networks = get_user_networks(value);
+
+ if (compare_network_lists(provider->user_networks, networks)) {
+ g_slist_free_full(networks, free_route);
+ return -EALREADY;
+ }
+
+ del_routes(provider);
+ provider->user_networks = networks;
+ set_user_networks(provider, provider->user_networks);
+
+ if (!handle_routes)
+ send_routes(provider, provider->user_routes,
+ "UserRoutes");
+ } else {
+ const char *str;
+
+ if (type != DBUS_TYPE_STRING)
+ return -EINVAL;
+
+ dbus_message_iter_get_basic(value, &str);
+
+ DBG("property %s value %s", name, str);
+
+ /* Empty string clears the value, similar to ClearProperty. */
+ err = vpn_provider_set_string(provider, name,
+ *str ? str : NULL);
+ }
+
+ return err;
+}
+
+static GString *append_to_gstring(GString *str, const char *value)
+{
+ if (!str)
+ return g_string_new(value);
+
+ g_string_append_printf(str, ",%s", value);
+
+ return str;
+}
+
+static DBusMessage *set_properties(DBusMessageIter *iter, DBusMessage *msg,
+ void *data)
+{
+ struct vpn_provider *provider = data;
+ DBusMessageIter dict;
+ const char *key;
+ bool change = false;
+ GString *invalid = NULL;
+ GString *denied = NULL;
+ int type;
+ int err;
+
+ for (dbus_message_iter_recurse(iter, &dict);
+ dbus_message_iter_get_arg_type(&dict) ==
+ DBUS_TYPE_DICT_ENTRY;
+ dbus_message_iter_next(&dict)) {
+ DBusMessageIter entry, value;
+
+ dbus_message_iter_recurse(&dict, &entry);
+ /*
+ * Ignore invalid types in order to process all values in the
+ * dict. If there is an invalid type in between the dict there
+ * may already be changes on some values and breaking out here
+ * would have the provider in an inconsistent state, leaving
+ * the rest, potentially correct property values untouched.
+ */
+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+ continue;
+
+ dbus_message_iter_get_basic(&entry, &key);
+
+ DBG("key %s", key);
+
+ dbus_message_iter_next(&entry);
+ /* Ignore and report back all non variant types. */
+ if (dbus_message_iter_get_arg_type(&entry)
+ != DBUS_TYPE_VARIANT) {
+ invalid = append_to_gstring(invalid, key);
+ continue;
+ }
+
+ dbus_message_iter_recurse(&entry, &value);
+
+ type = dbus_message_iter_get_arg_type(&value);
+ /* Ignore and report back all invalid property types */
+ if (type != DBUS_TYPE_STRING && type != DBUS_TYPE_ARRAY) {
+ invalid = append_to_gstring(invalid, key);
+ continue;
+ }
+
+ err = set_provider_property(provider, key, &value, type);
+ switch (err) {
+ case 0:
+ change = true;
+ break;
+ case -EINVAL:
+ invalid = append_to_gstring(invalid, key);
+ break;
+ case -EPERM:
+ denied = append_to_gstring(denied, key);
+ break;
+ }
+ }
+
+ if (change)
+ vpn_provider_save(provider);
+
+ if (invalid || denied) {
+ DBusMessage *error;
+ char *invalid_str = g_string_free(invalid, FALSE);
+ char *denied_str = g_string_free(denied, FALSE);
+
+ /*
+ * If there are both invalid and denied properties report
+ * back invalid arguments. Add also the failed properties to
+ * the error message.
+ */
+ error = g_dbus_create_error(msg, (invalid ?
+ CONNMAN_ERROR_INTERFACE ".InvalidProperty" :
+ CONNMAN_ERROR_INTERFACE ".PermissionDenied"),
+ "%s %s%s%s", (invalid ? "Invalid properties" :
+ "Permission denied"),
+ (invalid ? invalid_str : ""),
+ (invalid && denied ? "," : ""),
+ (denied ? denied_str : ""));
+
+ g_free(invalid_str);
+ g_free(denied_str);
+
+ return error;
+ }
+
+ return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
void *data)
{
@@ -479,7 +632,7 @@ static DBusMessage *set_property(DBusConnection *conn,
DBusMessage *msg,
DBusMessageIter iter, value;
const char *name;
int type;
- int err = 0;
+ int err;
DBG("conn %p", conn);
@@ -501,38 +654,14 @@ 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"))
+ return set_properties(&value, msg, data);
- if (g_str_equal(name, "UserRoutes")) {
- GSList *networks;
-
- if (type != DBUS_TYPE_ARRAY)
- return __connman_error_invalid_arguments(msg);
-
- networks = get_user_networks(&value);
- if (!networks)
- return __connman_error_invalid_arguments(msg);
-
- if (compare_network_lists(provider->user_networks, networks))
- return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
-
- del_routes(provider);
- provider->user_networks = networks;
- set_user_networks(provider, provider->user_networks);
-
- if (!handle_routes)
- send_routes(provider, provider->user_routes,
- "UserRoutes");
- } else {
- const char *str;
-
- if (type != DBUS_TYPE_STRING)
- return __connman_error_invalid_arguments(msg);
-
- dbus_message_iter_get_basic(&value, &str);
-
- err = vpn_provider_set_string(provider, name, str);
- }
-
+ err = set_provider_property(provider, name, &value, type);
switch (err) {
case 0:
vpn_provider_save(provider);
--
2.20.1
------------------------------
Date: Fri, 20 Dec 2019 11:56:46 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 3/3 v2] doc: Describe property array use with VPN
connection SetProperty D-Bus method
To: [email protected]
Message-ID: <[email protected]>
Include documentation for the use of dict array with the SetProperty
D-Bus method in VPN connection API.
Add error PermissionDenied for SetProperty and ClearProperty into VPN
connection API. Add missing NotSupported error for SetProperty into VPN
connection API.
---
Changes since v2:
* Removed SetProperties D-Bus method description from API.
* Reformatted the SetProperties description under SetProperty
description in the API.
* Changed the commit title to be more appropriate.
doc/vpn-connection-api.txt | 43 ++++++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/doc/vpn-connection-api.txt b/doc/vpn-connection-api.txt
index 1fd3be26..80080ebe 100644
--- a/doc/vpn-connection-api.txt
+++ b/doc/vpn-connection-api.txt
@@ -14,13 +14,47 @@ Methods dict GetProperties() [experimental]
void SetProperty(string name, variant value) [experimental]
- Changes the value of the specified property. Only
- properties that are listed as read-write are
- changeable. On success a PropertyChanged signal
- will be emitted.
+ Changes the value of the specified property or the
+ 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
+ PropertyChanged signal will be emitted for the
+ specified property or for all changed properties
+ individually. If there is no change in property value
+ no PropertyChanged signal is sent. Configuration is
+ written to disk when one or more values are changed.
+ In case a dict of properties are given, configuration
+ write is done after all properties are processed.
+ Specifics in dict use in contrast to setting a single
+ property:
+ - Dict can contain values set as empty strings
+ or arrays. This causes the values to be
+ cleared as if using ClearProperty().
+ - If there are errors with the properties,
+ InvalidProperty or PermissionDenied error is
+ returned. InvalidProperty is sent when there
+ is at least one invalid property, in this
+ case there can be also properties that
+ cannot be changed (immutable properties).
+ If there are only immutable properties
+ PermissionDenied error is returned.
+ - The properties that are invalid or immutable
+ are reported back at the end of the error
+ message as a comma separated property name
+ list.
+ - One invalid/immutable property does not
+ cause the rest of the properties to be
+ ignored. If there are valid and invalid
+ properties, the valid properties emit
+ PropertyChanged signal and invalid are
+ reported back with an InvalidProperty
+ message.
Possible Errors: [connection].Error.InvalidArguments
[connection].Error.InvalidProperty
+ [connection].Error.PermissionDenied
+ [connection].Error.NotSupported
void ClearProperty(string name) [experimental]
@@ -28,6 +62,7 @@ Methods dict GetProperties() [experimental]
Possible Errors: [connection].Error.InvalidArguments
[connection].Error.InvalidProperty
+ [connection].Error.PermissionDenied
void Connect() [experimental]
--
2.20.1
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
End of connman Digest, Vol 50, Issue 10
***************************************
