[PATCH] qmi-firmware-update: new optional runtime check to see if MM running

Dan Williams dcbw at redhat.com
Tue Feb 21 16:01:56 UTC 2017


On Wed, 2017-02-15 at 15:17 +0100, Aleksander Morgado wrote:
> Enabled by default, may be disabled using --without-mm-runtime-check
> during configure.
> ---
> 
> Hey Dan and everyone,
> 
> I've already got reports from a user that tried to use qmi-firmware-
> update while ModemManager was running, which resulted in several
> different types of errors, including CRC errors during the download
> process.
> 
> In order to avoid that, I've added a runtime check looking for
> ModemManager in the system bus; if it's found the process will be
> halted. Of course the real solution is to make sure there is nothing
> using the modem (not MM, not anything).
> 
> The check would be enabled by default (e.g. for common builds in the
> different distributions), but can also be disabled via configure
> switch for systems that don't require it (e.g. if MM isn't used).
> 
> What do you think?

LGTM.

> ---
>  configure.ac                        | 49 ++++++++++++++++++++++++++-
> ----------
>  src/qmi-firmware-update/qfu-main.c  | 40
> +++++++++++++++++++++++++++++-
>  src/qmi-firmware-update/qfu-utils.c | 46
> ++++++++++++++++++++++++++++++++++
>  src/qmi-firmware-update/qfu-utils.h |  9 +++++++
>  4 files changed, 129 insertions(+), 15 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index dd1937c..7ea2084 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -80,17 +80,29 @@ AC_SUBST(GLIB_LIBS)
>  GLIB_MKENUMS=`$PKG_CONFIG --variable=glib_mkenums glib-2.0`
>  AC_SUBST(GLIB_MKENUMS)
> 
> +dnl qmi-firmware-update is optional, enabled by default
> +AC_ARG_ENABLE([firmware-update],
> +              AS_HELP_STRING([--enable-firmware-update],
> +                             [enable compilation of `qmi-firmware-
> update' [default=yes]]),
> +              [build_firmware_update=$enableval],
> +              [build_firmware_update=yes])
> +AM_CONDITIONAL([BUILD_FIRMWARE_UPDATE], [test
> "x$build_firmware_update" = "xyes"])
> +
>  dnl udev support is optional, enabled by default
>  AC_ARG_WITH(udev, AS_HELP_STRING([--without-udev], [Build without
> udev support]), [], [with_udev=yes])
>  case $with_udev in
>      yes)
> -        PKG_CHECK_MODULES(GUDEV, [gudev-1.0 >= $GUDEV_VERSION],
> [have_gudev=yes],[have_gudev=no])
> -        if test "x$have_gudev" = "xno"; then
> -            AC_MSG_ERROR([Couldn't find gudev >= $GUDEV_VERSION.
> Install it, or otherwise configure using --without-udev to disable
> udev support.])
> +        if test "x$build_firmware_update" = "xyes"; then
> +            PKG_CHECK_MODULES(GUDEV, [gudev-1.0 >= $GUDEV_VERSION],
> [have_gudev=yes],[have_gudev=no])
> +            if test "x$have_gudev" = "xno"; then
> +                AC_MSG_ERROR([Couldn't find gudev >= $GUDEV_VERSION.
> Install it, or otherwise configure using --without-udev to disable
> udev support.])
> +            else
> +                AC_DEFINE(WITH_UDEV, 1, [Define if you want udev
> support])
> +                AC_SUBST(GUDEV_CFLAGS)
> +                AC_SUBST(GUDEV_LIBS)
> +            fi
>          else
> -            AC_DEFINE(WITH_UDEV, 1, [Define if you want udev
> support])
> -            AC_SUBST(GUDEV_CFLAGS)
> -            AC_SUBST(GUDEV_LIBS)
> +            with_udev="n/a"
>          fi
>          ;;
>      *)
> @@ -98,13 +110,20 @@ case $with_udev in
>          ;;
>  esac
> 
> -dnl qmi-firmware-update is optional, enabled by default
> -AC_ARG_ENABLE([firmware-update],
> -              AS_HELP_STRING([--enable-firmware-update],
> -                             [enable compilation of `qmi-firmware-
> update' [default=yes]]),
> -              [build_firmware_update=$enableval],
> -              [build_firmware_update=yes])
> -AM_CONDITIONAL([BUILD_FIRMWARE_UPDATE], [test
> "x$build_firmware_update" = "xyes"])
> +dnl runtime MM check is optional, enabled by default
> +AC_ARG_WITH(mm-runtime-check, AS_HELP_STRING([--without-mm-runtime-
> check], [Build without ModemManager runtime check]), [],
> [with_mm_runtime_check=yes])
> +case $with_mm_runtime_check in
> +    yes)
> +        if test "x$build_firmware_update" = "xyes"; then
> +            AC_DEFINE(WITH_MM_RUNTIME_CHECK, 1, [Define if you want
> ModemManager runtime check])
> +        else
> +            with_mm_runtime_check="n/a"
> +        fi
> +        ;;
> +    *)
> +        with_mm_runtime_check=no
> +        ;;
> +esac
> 
>  dnl Documentation
>  GTK_DOC_CHECK(1.0)
> @@ -208,5 +227,7 @@ echo "
>      Built items:
>        libqmi-glib:         yes
>        qmicli:              yes
> -      qmi-firmware-update: ${build_firmware_update} (with udev:
> ${with_udev})
> +      qmi-firmware-update: ${build_firmware_update}
> +          with udev:             ${with_udev}
> +          with MM runtime check: ${with_mm_runtime_check}
>  "
> diff --git a/src/qmi-firmware-update/qfu-main.c b/src/qmi-firmware-
> update/qfu-main.c
> index 7d53cc9..2ecdca6 100644
> --- a/src/qmi-firmware-update/qfu-main.c
> +++ b/src/qmi-firmware-update/qfu-main.c
> @@ -33,6 +33,7 @@
>  #include "qfu-operation.h"
>  #include "qfu-device-selection.h"
>  #include "qfu-udev-helpers.h"
> +#include "qfu-utils.h"
> 
>  #define PROGRAM_NAME    "qmi-firmware-update"
>  #define PROGRAM_VERSION PACKAGE_VERSION
> @@ -50,6 +51,10 @@ static guint16 pid;
>  static gchar *cdc_wdm_str;
>  static gchar *tty_str;
> 
> +#if defined WITH_MM_RUNTIME_CHECK
> +static gboolean ignore_mm_runtime_check_flag;
> +#endif
> +
>  #if defined WITH_UDEV
>  /* Update */
>  static gboolean   action_update_flag;
> @@ -287,6 +292,12 @@ static GOptionEntry context_main_entries[] = {
>        "Open a cdc-wdm device in either QMI or MBIM mode (default)",
>        NULL
>      },
> +#if defined WITH_MM_RUNTIME_CHECK
> +    { "ignore-mm-runtime-check", 0, 0, G_OPTION_ARG_NONE,
> &ignore_mm_runtime_check_flag,
> +      "Ignore ModemManager runtime check",
> +      NULL
> +    },
> +#endif
>      { "verbose", 'v', 0, G_OPTION_ARG_NONE, &stdout_verbose_flag,
>        "Run action with verbose messages in standard output,
> including the debug ones.",
>        NULL
> @@ -595,7 +606,6 @@ int main (int argc, char **argv)
>      /* Initialize logging */
>      qfu_log_init (stdout_verbose_flag, stdout_silent_flag,
> verbose_log_str);
> 
> -
>      /* Total actions */
>      n_actions = (action_verify_flag + action_update_qdl_flag +
> action_reset_flag);
>      /* Actions that require images specified */
> @@ -640,6 +650,34 @@ int main (int argc, char **argv)
>              g_error_free (error);
>              goto out;
>          }
> +
> +#if defined WITH_MM_RUNTIME_CHECK
> +        /* For all those actions that require a device, we will be
> doing MM runtime checks */
> +        if (!ignore_mm_runtime_check_flag) {
> +            gboolean mm_running;
> +            gboolean abort_needed = FALSE;
> +
> +            if (!qfu_utils_modemmanager_running (&mm_running,
> &error)) {
> +                g_printerr ("error: couldn't check if ModemManager
> running: %s\n", error->message);
> +                g_error_free (error);
> +                abort_needed = TRUE;
> +            } else if (mm_running) {
> +                g_printerr ("error: ModemManager is running: cannot
> perform firmware upgrade operation at the same time\n");
> +                abort_needed = TRUE;
> +            }
> +
> +            if (abort_needed) {
> +                g_printerr ("\n"
> +                            "Note: in order to do a clean firmware
> upgrade, ModemManager must NOT be running;\n"
> +                            "please stop it and retry. On a
> 'systemd' based system this may be done as follows:\n"
> +                            " # systemctl stop ModemManager\n"
> +                            "\n"
> +                            "You can also ignore this runtime check
> (at your own risk) using --ignore-mm-runtime-check.\n"
> +                            "\n");
> +                goto out;
> +            }
> +        }
> +#endif
>      }
> 
>      /* Validate device open flags */
> diff --git a/src/qmi-firmware-update/qfu-utils.c b/src/qmi-firmware-
> update/qfu-utils.c
> index 2667fbf..9109987 100644
> --- a/src/qmi-firmware-update/qfu-utils.c
> +++ b/src/qmi-firmware-update/qfu-utils.c
> @@ -714,3 +714,49 @@ qfu_utils_power_cycle
> (QmiClientDms         *qmi_client,
> 
>      power_cycle_step (task);
>  }
> +
> +/*******************************************************************
> ***********/
> +
> +#if defined WITH_MM_RUNTIME_CHECK
> +
> +gboolean
> +qfu_utils_modemmanager_running (gboolean  *mm_running,
> +                                GError   **error)
> +{
> +    GDBusConnection *connection;
> +    GVariant        *response;
> +    GError          *inner_error = NULL;
> +
> +    g_assert (mm_running);
> +
> +    connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, error);
> +    if (!connection) {
> +        g_prefix_error (error, "Couldn't get system bus: ");
> +        return FALSE;
> +    }
> +
> +    response = g_dbus_connection_call_sync (connection,
> +                                            "org.freedesktop.ModemMa
> nager1",
> +                                            "/org/freedesktop/ModemM
> anager1",
> +                                            "org.freedesktop.DBus.Pe
> er",
> +                                            "Ping",
> +                                            NULL,
> +                                            NULL,
> +                                            G_DBUS_CALL_FLAGS_NO_AUT
> O_START,
> +                                            -1,
> +                                            NULL,
> +                                            &inner_error);
> +    if (!response) {
> +        g_debug ("[qfu-utils] couldn't ping ModemManager: %s",
> inner_error->message);
> +        g_error_free (inner_error);
> +        *mm_running = FALSE;
> +    } else {
> +        *mm_running = TRUE;
> +        g_variant_unref (response);
> +    }
> +
> +    g_object_unref (connection);
> +    return TRUE;
> +}
> +
> +#endif
> diff --git a/src/qmi-firmware-update/qfu-utils.h b/src/qmi-firmware-
> update/qfu-utils.h
> index 1490053..3444db1 100644
> --- a/src/qmi-firmware-update/qfu-utils.h
> +++ b/src/qmi-firmware-update/qfu-utils.h
> @@ -23,6 +23,8 @@
>  #ifndef QFU_UTILS_H
>  #define QFU_UTILS_H
> 
> +#include "config.h"
> +
>  #include <glib.h>
>  #include <gio/gio.h>
>  #include <libqmi-glib.h>
> @@ -70,6 +72,13 @@ gboolean qfu_utils_power_cycle_finish
> (QmiClientDms         *qmi_client,
>                                         GAsyncResult         *res,
>                                         GError              **error);
> 
> +#if defined WITH_MM_RUNTIME_CHECK
> +
> +gboolean qfu_utils_modemmanager_running (gboolean  *mm_running,
> +                                         GError   **error);
> +
> +#endif
> +
>  G_END_DECLS
> 
>  #endif /* QFU_UTILS_H */
> --
> 2.11.1


More information about the libqmi-devel mailing list