Re: [RFC PATCH 2/4] gprs: add automatic context settings provisioning

2011-01-11 Thread Marcel Holtmann
Hi Jukka,

 diff --git a/src/gprs.c b/src/gprs.c
 index 58166f8..7d188a3 100644
 --- a/src/gprs.c
 +++ b/src/gprs.c
 @@ -43,6 +43,7 @@
  #include common.h
  #include storage.h
  #include idmap.h
 +#include gprs-provision.h
  
  #define GPRS_FLAG_ATTACHING 0x1
  #define GPRS_FLAG_RECHECK 0x2
 @@ -2454,13 +2455,15 @@ remove:
   storage_sync(imsi, SETTINGS_STORE, gprs-settings);
  }
  
 -void ofono_gprs_register(struct ofono_gprs *gprs)
 +static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
  {
   DBusConnection *conn = ofono_dbus_get_connection();
   struct ofono_modem *modem = __ofono_atom_get_modem(gprs-atom);
   const char *path = __ofono_atom_get_path(gprs-atom);
   struct ofono_atom *netreg_atom;
 - struct ofono_atom *sim_atom;
 +
 + if (gprs-contexts == NULL) /* Automatic provisioning failed */
 + add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);

so I see a potential race here between the UI trying to provision and us
trying to provision.

We don't wanna end up with duplicated contexts here. Any ideas on how we
could prevent that nicely? Or should we just fail any attempts to create
new contexts if provisioning is still ongoing?

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [RFC PATCH 2/4] gprs: add automatic context settings provisioning

2011-01-11 Thread Jukka Saunamaki
Hello Marcel,

On Tue, 2011-01-11 at 22:51 -0800, ext Marcel Holtmann wrote:
   
  -void ofono_gprs_register(struct ofono_gprs *gprs)
  +static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
   {
  DBusConnection *conn = ofono_dbus_get_connection();
  struct ofono_modem *modem = __ofono_atom_get_modem(gprs-atom);
  const char *path = __ofono_atom_get_path(gprs-atom);
  struct ofono_atom *netreg_atom;
  -   struct ofono_atom *sim_atom;
  +
  +   if (gprs-contexts == NULL) /* Automatic provisioning failed */
  +   add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
 
 so I see a potential race here between the UI trying to provision and us
 trying to provision.
 
 We don't wanna end up with duplicated contexts here. Any ideas on how we
 could prevent that nicely? Or should we just fail any attempts to create
 new contexts if provisioning is still ongoing?

Code continues there with:
if (!g_dbus_register_interface(conn, path,
OFONO_CONNECTION_MANAGER_INTERFACE,
manager_methods, manager_signals, NULL,
gprs, NULL)) {
ofono_error(Could not create %s interface,
OFONO_CONNECTION_MANAGER_INTERFACE);

return;
}

ofono_modem_add_interface(modem,
OFONO_CONNECTION_MANAGER_INTERFACE);

Patch rearranges the initial context creation and interface
registration, so there is no DBUS interface for UI to use until
provisioning is ready. Or have misunderstood something?

--Jukka


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[RFC PATCH 2/4] gprs: add automatic context settings provisioning

2011-01-10 Thread Jukka Saunamaki
---
 src/gprs.c |  116 ---
 1 files changed, 102 insertions(+), 14 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index 58166f8..7d188a3 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -43,6 +43,7 @@
 #include common.h
 #include storage.h
 #include idmap.h
+#include gprs-provision.h
 
 #define GPRS_FLAG_ATTACHING 0x1
 #define GPRS_FLAG_RECHECK 0x2
@@ -2454,13 +2455,15 @@ remove:
storage_sync(imsi, SETTINGS_STORE, gprs-settings);
 }
 
-void ofono_gprs_register(struct ofono_gprs *gprs)
+static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
 {
DBusConnection *conn = ofono_dbus_get_connection();
struct ofono_modem *modem = __ofono_atom_get_modem(gprs-atom);
const char *path = __ofono_atom_get_path(gprs-atom);
struct ofono_atom *netreg_atom;
-   struct ofono_atom *sim_atom;
+
+   if (gprs-contexts == NULL) /* Automatic provisioning failed */
+   add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
 
if (!g_dbus_register_interface(conn, path,
OFONO_CONNECTION_MANAGER_INTERFACE,
@@ -2475,18 +2478,6 @@ void ofono_gprs_register(struct ofono_gprs *gprs)
ofono_modem_add_interface(modem,
OFONO_CONNECTION_MANAGER_INTERFACE);
 
-   sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
-
-   if (sim_atom) {
-   struct ofono_sim *sim = __ofono_atom_get_data(sim_atom);
-   const char *imsi = ofono_sim_get_imsi(sim);
-
-   gprs_load_settings(gprs, imsi);
-   }
-
-   if (gprs-contexts == NULL)
-   add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
-
gprs-netreg_watch = __ofono_modem_add_atom_watch(modem,
OFONO_ATOM_TYPE_NETREG,
netreg_watch, gprs, NULL);
@@ -2500,6 +2491,103 @@ void ofono_gprs_register(struct ofono_gprs *gprs)
__ofono_atom_register(gprs-atom, gprs_unregister);
 }
 
+static void provision_context(gpointer data, gpointer user_data)
+{
+   struct ofono_gprs_provisioning_data *ap = data;
+   struct ofono_gprs *gprs = user_data;
+
+   unsigned int id;
+   struct pri_context *context = NULL;
+
+   /* Sanity check */
+   if (ap == NULL || ap-name == NULL)
+   return;
+
+   if (gprs-last_context_id)
+   id = idmap_alloc_next(gprs-pid_map, gprs-last_context_id);
+   else
+   id = idmap_alloc(gprs-pid_map);
+
+   if (id  idmap_get_max(gprs-pid_map))
+   return;
+
+   context = pri_context_create(gprs, ap-name, ap-type);
+   DBG(%s context%d '%s' %s, gprs_context_default_name(ap-type),
+   id, ap-name, context ? created : creation failed);
+
+   if (context == NULL)
+   return;
+
+   context-id = id;
+
+   if (ap-username != NULL)
+   strncpy(context-context.username, ap-username,
+   OFONO_GPRS_MAX_USERNAME_LENGTH);
+
+   if (ap-password != NULL)
+   strncpy(context-context.password, ap-password,
+   OFONO_GPRS_MAX_PASSWORD_LENGTH);
+
+   if (ap-apn != NULL)
+   strncpy(context-context.apn, ap-apn,
+   OFONO_GPRS_MAX_APN_LENGTH);
+
+   context-context.proto = ap-proto;
+
+   if (ap-type == OFONO_GPRS_CONTEXT_TYPE_MMS 
+   ap-message_proxy != NULL)
+   strncpy(context-message_proxy, ap-message_proxy,
+   MAX_MESSAGE_PROXY_LENGTH);
+
+   if (ap-type == OFONO_GPRS_CONTEXT_TYPE_MMS 
+   ap-message_center != NULL)
+   strncpy(context-message_center, ap-message_center,
+   MAX_MESSAGE_CENTER_LENGTH);
+
+   if (context_dbus_register(context) == TRUE) {
+   gprs-last_context_id = id;
+   gprs-contexts = g_slist_append(gprs-contexts,
+   context);
+   write_context_settings(gprs, context);
+   }
+}
+
+
+static void provision_cb(GSList *settings, void *userdata)
+{
+   struct ofono_gprs *gprs = userdata;
+
+   g_slist_foreach(settings, provision_context, gprs);
+   g_slist_foreach(settings, (GFunc) ofono_gprs_provisioning_data_free,
+   NULL);
+   g_slist_free(settings);
+
+   ofono_gprs_finish_register(gprs);
+}
+
+void ofono_gprs_register(struct ofono_gprs *gprs)
+{
+   struct ofono_modem *modem = __ofono_atom_get_modem(gprs-atom);
+   struct ofono_atom *sim_atom;
+   struct ofono_sim *sim = NULL;
+
+   sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
+
+   if (sim_atom != NULL) {
+   const char *imsi = ofono_sim_get_imsi(sim);
+   sim = __ofono_atom_get_data(sim_atom);
+