Attachment 111372 Details for Bug 84994 – Patch 3/3

[patch] Patch 3/3

0003-mbim-if-not-opened-error-is-found-mark-the-modem-as-.patch (text/plain), 16.91 KB, created by Aleksander Morgado on 2014-12-26 13:21:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Aleksander Morgado

Created: 2014-12-26 13:21:36 UTC

Size: 16.91 KB

patch

obsolete

>From 2f06e26362e4cff8ffb94e566293b22303f1ef64 Mon Sep 17 00:00:00 2001
>From: Aleksander Morgado <aleksander@aleksander.es>
>Date: Fri, 26 Dec 2014 14:10:43 +0100
>Subject: [PATCH 3/3] mbim: if not-opened error is found, mark the modem as
> invalid
>
>Some setups will end up powering off the modem during a suspend, but not
>reporting the cdc-wdm port removals in the kernel. Therefore, it may happen that
>ModemManager thinks that the MBIM ports are open and ready to be used while in
>reality they have been implicitly closed.
>
>Try to detect this looking at the generic "not-opened" errors reported by the
>device, which can happen in any kind of command sent.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=84994
>---
> src/mm-bearer-mbim.c          | 28 +++++++++++++++++
> src/mm-broadband-modem-mbim.c | 73 +++++++++++++++++++++++++++++++++----------
> src/mm-sim-mbim.c             | 55 ++++++++++++++++++++++++++------
> src/mm-sms-mbim.c             | 25 +++++++++++++++
> 4 files changed, 154 insertions(+), 27 deletions(-)
>
>diff --git a/src/mm-bearer-mbim.c b/src/mm-bearer-mbim.c
>index c554c52..c174344 100644
>--- a/src/mm-bearer-mbim.c
>+++ b/src/mm-bearer-mbim.c
>@@ -107,6 +107,29 @@ peek_ports (gpointer self,
> }
> 
> /*****************************************************************************/
>+/* Common check to see if we got not-opened errors.
>+ * When the port is detected not-opened, we'll mark the modem as fully invalid. */
>+
>+static void
>+bearer_ensure_opened (MMBearerMbim *self,
>+                      const GError *error)
>+{
>+    if (g_error_matches (error, MBIM_PROTOCOL_ERROR, MBIM_PROTOCOL_ERROR_NOT_OPENED)) {
>+        MMBaseModem *modem = NULL;
>+
>+        g_object_get (G_OBJECT (self),
>+                      MM_BASE_BEARER_MODEM, &modem,
>+                      NULL);
>+
>+        if (modem) {
>+            mm_warn ("MBIM device is not opened, reporting invalid modem...");
>+            mm_base_modem_set_invalid_idle (MM_BASE_MODEM (modem));
>+            g_object_unref (modem);
>+        }
>+    }
>+}
>+
>+/*****************************************************************************/
> /* Connect */
> 
> typedef enum {
>@@ -435,6 +458,7 @@ ip_configuration_query_ready (MbimDevice *device,
>         mbim_message_unref (response);
> 
>     if (error) {
>+        bearer_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>         connect_context_complete_and_free (ctx);
>         return;
>@@ -496,6 +520,7 @@ connect_set_ready (MbimDevice *device,
>         mbim_message_unref (response);
> 
>     if (error) {
>+        bearer_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>         connect_context_complete_and_free (ctx);
>         return;
>@@ -544,6 +569,7 @@ provisioned_contexts_query_ready (MbimDevice *device,
> 
>         mbim_provisioned_context_element_array_free (provisioned_contexts);
>     } else {
>+        bearer_ensure_opened (ctx->self, error);
>         mm_dbg ("Error listing provisioned contexts: %s", error->message);
>         g_error_free (error);
>     }
>@@ -618,6 +644,7 @@ packet_service_set_ready (MbimDevice *device,
>             g_error_free (error);
>         } else {
>             /* All other errors are fatal */
>+            bearer_ensure_opened (ctx->self, error);
>             g_simple_async_result_take_error (ctx->result, error);
>             connect_context_complete_and_free (ctx);
>             return;
>@@ -1022,6 +1049,7 @@ disconnect_set_ready (MbimDevice *device,
>         mbim_message_unref (response);
> 
>     if (error) {
>+        bearer_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>         disconnect_context_complete_and_free (ctx);
>         return;
>diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c
>index f689127..c6475a9 100644
>--- a/src/mm-broadband-modem-mbim.c
>+++ b/src/mm-broadband-modem-mbim.c
>@@ -106,6 +106,21 @@ peek_device (gpointer self,
> }
> 
> /*****************************************************************************/
>+/* Common check to see if we got not-opened errors.
>+ * When the port is detected not-opened, we'll mark the modem as fully invalid. */
>+
>+static void
>+modem_ensure_opened (MMBroadbandModemMbim *self,
>+                     const GError         *error)
>+{
>+    if (g_error_matches (error, MBIM_PROTOCOL_ERROR, MBIM_PROTOCOL_ERROR_NOT_OPENED)) {
>+        /* Report invalid modem */
>+        mm_warn ("MBIM device is not opened, reporting invalid modem...");
>+        mm_base_modem_set_invalid_idle (MM_BASE_MODEM (self));
>+    }
>+}
>+
>+/*****************************************************************************/
> 
> typedef struct {
>     MMBroadbandModemMbim *self;
>@@ -196,8 +211,10 @@ device_caps_query_ready (MbimDevice *device,
>         g_simple_async_result_set_op_res_gpointer (ctx->result,
>                                                    GUINT_TO_POINTER (mask),
>                                                    NULL);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -575,8 +592,10 @@ pin_query_ready (MbimDevice *device,
>                                                    GUINT_TO_POINTER (MBIM_PIN_TYPE_PIN1),
>                                                    NULL);
>     }
>-    else
>+    else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -629,6 +648,7 @@ unlock_required_subscriber_ready_state_ready (MbimDevice *device,
> 
>     /* Fatal errors are reported right away */
>     if (error) {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>         load_unlock_required_context_complete_and_free (ctx);
>     }
>@@ -762,8 +782,10 @@ pin_query_unlock_retries_ready (MbimDevice *device,
>                                    remaining_attempts);
>         }
>         g_simple_async_result_set_op_res_gpointer (ctx->result, retries, g_object_unref);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -834,8 +856,10 @@ own_numbers_subscriber_ready_state_ready (MbimDevice *device,
>             &telephone_numbers,
>             &error)) {
>         g_simple_async_result_set_op_res_gpointer (ctx->result, telephone_numbers, NULL);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -911,8 +935,10 @@ radio_state_query_ready (MbimDevice *device,
>         g_simple_async_result_set_op_res_gpointer (ctx->result,
>                                                    GUINT_TO_POINTER (state),
>                                                    NULL);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -969,9 +995,10 @@ radio_state_set_down_ready (MbimDevice *device,
>     if (response)
>         mbim_message_command_done_get_result (response, &error);
> 
>-    if (error)
>+    if (error) {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>-    else
>+    } else
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> 
>     if (response)
>@@ -1005,9 +1032,10 @@ radio_state_set_up_ready (MbimDevice *device,
>             g_warn_if_reached ();
>     }
> 
>-    if (error)
>+    if (error) {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>-    else
>+    } else
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> 
>     if (response)
>@@ -1492,8 +1520,10 @@ pin_list_query_ready (MbimDevice *device,
>         g_simple_async_result_set_op_res_gpointer (ctx->result,
>                                                    GUINT_TO_POINTER (mask),
>                                                    NULL);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -1852,6 +1882,7 @@ alert_sms_read_query_ready (MbimDevice *device,
>     }
> 
>     if (error) {
>+        modem_ensure_opened (self, error);
>         mm_dbg ("Flash message reading failed: %s", error->message);
>         g_error_free (error);
>     }
>@@ -2085,9 +2116,10 @@ subscribe_list_set_ready_cb (MbimDevice *device,
>     if (response)
>         mbim_message_command_done_get_result (response, &error);
> 
>-    if (error)
>+    if (error) {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>-    else
>+    } else
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> 
>     if (response)
>@@ -2361,8 +2393,10 @@ register_state_query_ready (MbimDevice *device,
>                                   provider_id,
>                                   provider_name);
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -2437,9 +2471,10 @@ register_state_set_ready (MbimDevice *device,
>             error = mm_mobile_equipment_error_from_mbim_nw_error (nw_error);
>     }
> 
>-    if (error)
>+    if (error) {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>-    else
>+    } else
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
> 
>     if (response)
>@@ -2527,8 +2562,10 @@ visible_providers_query_ready (MbimDevice *device,
>     response = mbim_device_command_finish (device, res, &error);
>     if (response)
>         g_simple_async_result_set_op_res_gpointer (ctx->result, response, (GDestroyNotify)mbim_message_unref);
>-    else
>+    else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     simple_context_complete_and_free (ctx);
> }
>@@ -2699,8 +2736,10 @@ sms_read_query_ready (MbimDevice *device,
>         for (i = 0; i < messages_count; i++)
>             add_sms_part (ctx->self, pdu_messages[i]);
>         mbim_sms_pdu_read_record_array_free (pdu_messages);
>-    } else
>+    } else {
>+        modem_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>diff --git a/src/mm-sim-mbim.c b/src/mm-sim-mbim.c
>index e01a3ba..c820dd5 100644
>--- a/src/mm-sim-mbim.c
>+++ b/src/mm-sim-mbim.c
>@@ -64,6 +64,29 @@ peek_device (gpointer self,
> }
> 
> /*****************************************************************************/
>+/* Common check to see if we got not-opened errors.
>+ * When the port is detected not-opened, we'll mark the modem as fully invalid. */
>+
>+static void
>+sim_ensure_opened (MMSimMbim    *self,
>+                   const GError *error)
>+{
>+    if (g_error_matches (error, MBIM_PROTOCOL_ERROR, MBIM_PROTOCOL_ERROR_NOT_OPENED)) {
>+        MMBaseModem *modem = NULL;
>+
>+        g_object_get (G_OBJECT (self),
>+                      MM_BASE_SIM_MODEM, &modem,
>+                      NULL);
>+
>+        if (modem) {
>+            mm_warn ("MBIM device is not opened, reporting invalid modem...");
>+            mm_base_modem_set_invalid_idle (MM_BASE_MODEM (modem));
>+            g_object_unref (modem);
>+        }
>+    }
>+}
>+
>+/*****************************************************************************/
> 
> typedef struct {
>     MMSimMbim *self;
>@@ -128,8 +151,10 @@ simid_subscriber_ready_state_ready (MbimDevice *device,
>             NULL, /* telephone_numbers */
>             &error))
>         g_simple_async_result_set_op_res_gpointer (ctx->result, sim_iccid, (GDestroyNotify)g_free);
>-    else
>+    else {
>+        sim_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -195,8 +220,10 @@ imsi_subscriber_ready_state_ready (MbimDevice *device,
>             NULL, /* telephone_numbers */
>             &error))
>         g_simple_async_result_set_op_res_gpointer (ctx->result, subscriber_id, (GDestroyNotify)g_free);
>-    else
>+    else {
>+        sim_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -261,8 +288,10 @@ load_operator_identifier_ready (MbimDevice *device,
>             &provider,
>             &error))
>         g_simple_async_result_set_op_res_gpointer (ctx->result, provider, (GDestroyNotify)mbim_provider_free);
>-    else
>+    else {
>+        sim_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -327,8 +356,10 @@ load_operator_name_ready (MbimDevice *device,
>             &provider,
>             &error))
>         g_simple_async_result_set_op_res_gpointer (ctx->result, provider, (GDestroyNotify)mbim_provider_free);
>-    else
>+    else {
>+        sim_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>+    }
> 
>     if (response)
>         mbim_message_unref (response);
>@@ -404,9 +435,10 @@ pin_set_enter_ready (MbimDevice *device,
>     if (response)
>         mbim_message_unref (response);
> 
>-    if (error)
>+    if (error) {
>+        sim_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>-    else
>+    } else
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>     simple_context_complete_and_free (ctx);
> }
>@@ -494,9 +526,10 @@ puk_set_enter_ready (MbimDevice *device,
>     if (response)
>         mbim_message_unref (response);
> 
>-    if (error)
>+    if (error) {
>+        sim_ensure_opened (ctx->self, error);
>         g_simple_async_result_take_error (ctx->result, error);
>-    else
>+    } else
>         g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
>     simple_context_complete_and_free (ctx);
> }
>@@ -570,7 +603,8 @@ pin_set_enable_ready (MbimDevice *device,
>             error = g_error_new (MM_CORE_ERROR,
>                                  MM_CORE_ERROR_WRONG_STATE,
>                                  "Need to be unlocked to allow enabling/disabling PIN");
>-        }
>+        } else
>+            sim_ensure_opened (ctx->self, error);
> 
>         g_simple_async_result_take_error (ctx->result, error);
>     } else
>@@ -647,7 +681,8 @@ pin_set_change_ready (MbimDevice *device,
>             error = g_error_new (MM_CORE_ERROR,
>                                  MM_CORE_ERROR_WRONG_STATE,
>                                  "Need to be unlocked to allow changing PIN");
>-        }
>+        } else
>+            sim_ensure_opened (ctx->self, error);
> 
>         g_simple_async_result_take_error (ctx->result, error);
>     } else
>diff --git a/src/mm-sms-mbim.c b/src/mm-sms-mbim.c
>index 3bdd68a..bb87734 100644
>--- a/src/mm-sms-mbim.c
>+++ b/src/mm-sms-mbim.c
>@@ -71,6 +71,29 @@ peek_device (gpointer self,
> }
> 
> /*****************************************************************************/
>+/* Common check to see if we got not-opened errors.
>+ * When the port is detected not-opened, we'll mark the modem as fully invalid. */
>+
>+static void
>+sms_ensure_opened (MMSmsMbim    *self,
>+                   const GError *error)
>+{
>+    if (g_error_matches (error, MBIM_PROTOCOL_ERROR, MBIM_PROTOCOL_ERROR_NOT_OPENED)) {
>+        MMBaseModem *modem = NULL;
>+
>+        g_object_get (G_OBJECT (self),
>+                      MM_BASE_SMS_MODEM, &modem,
>+                      NULL);
>+
>+        if (modem) {
>+            mm_warn ("MBIM device is not opened, reporting invalid modem...");
>+            mm_base_modem_set_invalid_idle (MM_BASE_MODEM (modem));
>+            g_object_unref (modem);
>+        }
>+    }
>+}
>+
>+/*****************************************************************************/
> /* Send the SMS */
> 
> typedef struct {
>@@ -126,6 +149,7 @@ sms_send_set_ready (MbimDevice *device,
>         mbim_message_unref (response);
> 
>     if (error) {
>+        sms_ensure_opened (MM_SMS_MBIM (ctx->self), error);
>         g_prefix_error (&error, "Couldn't send SMS part: ");
>         g_simple_async_result_take_error (ctx->result, error);
>         sms_send_context_complete_and_free (ctx);
>@@ -255,6 +279,7 @@ sms_delete_set_ready (MbimDevice *device,
>         mbim_message_unref (response);
> 
>     if (error) {
>+        sms_ensure_opened (MM_SMS_MBIM (ctx->self), error);
>         ctx->n_failed++;
>         mm_dbg ("Couldn't delete SMS part with index %u: '%s'",
>                 mm_sms_part_get_index ((MMSmsPart *)ctx->current->data),
>-- 
>2.2.1
>

Actions: View | Splinter Review

Attachments on bug 84994: 111370 | 111371 | 111372 | 111380 | 111396 | 111422 | 111423 | 111473 | 111474 | 111476 | 111477 | 111478 | 112386 | 112387 | 112388

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.