[PATCH] libqmi-glib,client: new 'client-valid' property
Aleksander Morgado
aleksander at aleksander.es
Mon Nov 27 12:33:04 UTC 2017
On Mon, Nov 27, 2017 at 4:02 AM, Dan Williams <dcbw at redhat.com> wrote:
> On Sat, 2017-11-25 at 11:34 +0100, Aleksander Morgado wrote:
>> It may happen that the lifecycle of a QmiDevice and its allocated
>> QmiClients is managed in a different place of where the QmiClient
>> commands are run, which may keep a QmiClient reference of its own.
>>
>> In this setup, it may happen that the QmiDevice is closed and all
>> clients released while there are still full QmiClient references
>> around that the client may try to use. When this happens, the client
>> operation didn't fail with an error, instead it would just issue a
>> 'QMI_IS_DEVICE (self)' failed warning and go on.
>
> LGTM
>
Thanks for reviewing; I've pushed it to git master, plus a backport to
qmi-1-18 that doesn't introduce the new API (i.e. just adding the
logic that returns the error return).
>> ...
>> [/dev/cdc-wdm0] Releasing 'dms' client with flags 'release-
>> cid'...
>> [/dev/cdc-wdm0] Unregistered 'dms' client with ID '10'
>> [/dev/cdc-wdm0] sent message...
>> <<<<<< RAW:
>> <<<<<< length = 17
>> <<<<<< data =
>> 01:10:00:00:00:00:00:0C:23:00:05:00:01:02:00:02:0A
>> [/dev/cdc-wdm0] sent generic request (translated)...
>> <<<<<< QMUX:
>> <<<<<< length = 16
>> <<<<<< flags = 0x00
>> <<<<<< service = "ctl"
>> <<<<<< client = 0
>> <<<<<< QMI:
>> <<<<<< flags = "none"
>> <<<<<< transaction = 12
>> <<<<<< tlv_length = 5
>> <<<<<< message = "Release CID" (0x0023)
>> <<<<<< TLV:
>> <<<<<< type = "Release Info" (0x01)
>> <<<<<< length = 2
>> <<<<<< value = 02:0A
>> <<<<<< translated = [ service = 'dms' cid = '10' ]
>> qmi_device_command_full: assertion 'QMI_IS_DEVICE (self)' failed
>> <warn> [062790.772245] checking if connected failed: Transaction
>> timed out
>> <debug> [062793.774332] Couldn't refresh CDMA registration
>> status: 'QMI operation failed: Transaction timed out'
>> <debug> [062793.774398] Couldn't refresh signal quality:
>> 'Transaction timed out'
>> <debug> [062793.774414] Periodic signal checks not rescheduled:
>> disabled
>> qmi_device_command_full: assertion 'QMI_IS_DEVICE (self)' failed
>> <warn> [062795.773381] Reloading stats failed: QMI operation
>> failed: Transaction timed out
>> <warn> [062795.773425] checking if connected failed: Transaction
>> timed out
>> qmi_device_command_full: assertion 'QMI_IS_DEVICE (self)' failed
>> qmi_device_command_full: assertion 'QMI_IS_DEVICE (self)' failed
>> ...
>>
>> We avoid this situation by exposing a new 'client-valid' property
>> that
>> users may check before running any command.
>>
>> This same check is now also internally done by all commands so that
>> we
>> cleanly error out if we detect that the client isn't valid.
>> ---
>> build-aux/qmi-codegen/Client.py | 5 ++++
>> .../libqmi-glib/libqmi-glib-common.sections | 2 ++
>> src/libqmi-glib/qmi-client.c | 27
>> +++++++++++++++++++++
>> src/libqmi-glib/qmi-client.h | 28
>> ++++++++++++++++++++++
>> src/libqmi-glib/qmi-device.c | 2 +-
>> 5 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/build-aux/qmi-codegen/Client.py b/build-aux/qmi-
>> codegen/Client.py
>> index 445c862..be51197 100644
>> --- a/build-aux/qmi-codegen/Client.py
>> +++ b/build-aux/qmi-codegen/Client.py
>> @@ -477,6 +477,11 @@ class Client:
>> template += (
>> '\n'
>> ' task = g_task_new (self, cancellable, callback,
>> user_data);\n'
>> + ' if (!qmi_client_is_valid (QMI_CLIENT (self)))
>> {\n'
>> + ' g_task_return_new_error (task,
>> QMI_CORE_ERROR, QMI_CORE_ERROR_WRONG_STATE, "client invalid");\n'
>> + ' g_object_unref (task);\n'
>> + ' return;\n'
>> + ' }\n'
>> '\n'
>> ' transaction_id =
>> qmi_client_get_next_transaction_id (QMI_CLIENT (self));\n'
>> '\n'
>> diff --git a/docs/reference/libqmi-glib/libqmi-glib-common.sections
>> b/docs/reference/libqmi-glib/libqmi-glib-common.sections
>> index 6ce1e37..bc088e7 100644
>> --- a/docs/reference/libqmi-glib/libqmi-glib-common.sections
>> +++ b/docs/reference/libqmi-glib/libqmi-glib-common.sections
>> @@ -18,11 +18,13 @@ QMI_CLIENT_SERVICE
>> QMI_CLIENT_CID
>> QMI_CLIENT_VERSION_MAJOR
>> QMI_CLIENT_VERSION_MINOR
>> +QMI_CLIENT_VALID
>> QmiClient
>> qmi_client_get_device
>> qmi_client_peek_device
>> qmi_client_get_service
>> qmi_client_get_cid
>> +qmi_client_is_valid
>> qmi_client_get_version
>> qmi_client_check_version
>> qmi_client_get_next_transaction_id
>> diff --git a/src/libqmi-glib/qmi-client.c b/src/libqmi-glib/qmi-
>> client.c
>> index 4c0c5c3..5a9c3a3 100644
>> --- a/src/libqmi-glib/qmi-client.c
>> +++ b/src/libqmi-glib/qmi-client.c
>> @@ -37,6 +37,7 @@ enum {
>> PROP_CID,
>> PROP_VERSION_MAJOR,
>> PROP_VERSION_MINOR,
>> + PROP_VALID,
>> PROP_LAST
>> };
>>
>> @@ -92,6 +93,16 @@ qmi_client_get_cid (QmiClient *self)
>> return self->priv->cid;
>> }
>>
>> +gboolean
>> +qmi_client_is_valid (QmiClient *self)
>> +{
>> + g_return_val_if_fail (QMI_IS_CLIENT (self), FALSE);
>> +
>> + return (self->priv->service != QMI_SERVICE_UNKNOWN &&
>> + QMI_IS_DEVICE (self->priv->device) &&
>> + ((self->priv->cid != QMI_CID_NONE || self->priv->service
>> == QMI_SERVICE_CTL)));
>> +}
>> +
>> gboolean
>> qmi_client_get_version (QmiClient *self,
>> guint *major,
>> @@ -218,6 +229,9 @@ get_property (GObject *object,
>> case PROP_VERSION_MINOR:
>> g_value_set_uint (value, self->priv->version_minor);
>> break;
>> + case PROP_VALID:
>> + g_value_set_boolean (value, qmi_client_is_valid (self));
>> + break;
>> default:
>> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>> break;
>> @@ -320,4 +334,17 @@ qmi_client_class_init (QmiClientClass *klass)
>> 0,
>> G_PARAM_READWRITE);
>> g_object_class_install_property (object_class,
>> PROP_VERSION_MINOR, properties[PROP_VERSION_MINOR]);
>> +
>> + /**
>> + * QmiClient:client-valid:
>> + *
>> + * Since: 1.20
>> + */
>> + properties[PROP_VALID] =
>> + g_param_spec_boolean (QMI_CLIENT_VALID,
>> + "Valid",
>> + "Whether the client is valid and
>> usable",
>> + FALSE,
>> + G_PARAM_READABLE);
>> + g_object_class_install_property (object_class, PROP_VALID,
>> properties[PROP_VALID]);
>> }
>> diff --git a/src/libqmi-glib/qmi-client.h b/src/libqmi-glib/qmi-
>> client.h
>> index c8f81fd..52ef1c4 100644
>> --- a/src/libqmi-glib/qmi-client.h
>> +++ b/src/libqmi-glib/qmi-client.h
>> @@ -121,6 +121,15 @@ typedef struct _QmiClientPrivate
>> QmiClientPrivate;
>> */
>> #define QMI_CLIENT_VERSION_MINOR "client-version-minor"
>>
>> +/**
>> + * QMI_CLIENT_VALID:
>> + *
>> + * Symbol defining the #QmiClient:valid property.
>> + *
>> + * Since: 1.20
>> + */
>> +#define QMI_CLIENT_VALID "client-valid"
>> +
>> /**
>> * QmiClient:
>> *
>> @@ -195,6 +204,25 @@ QmiService qmi_client_get_service (QmiClient
>> *self);
>> */
>> guint8 qmi_client_get_cid (QmiClient *self);
>>
>> +/**
>> + * qmi_client_is_valid:
>> + * @self: a #QmiClient.
>> + *
>> + * Checks whether #QmiClient is a valid and usable client.
>> + *
>> + * The client is marked as invalid as soon as the client id is
>> released or when
>> + * the associated #QmiDevice is closed.
>> + *
>> + * This method may be used if the caller needs to ensure validity
>> before a
>> + * command is attempted, e.g. if the lifecycle of the object is
>> managed in some
>> + * other place and the caller just has a reference to the
>> #QmiClient.
>> + *
>> + * Returns: %TRUE if the client is valid, %FALSE otherwise.
>> + *
>> + * Since: 1.20
>> + */
>> +gboolean qmi_client_is_valid (QmiClient *self);
>> +
>> /**
>> * qmi_client_get_version:
>> * @self: A #QmiClient
>> diff --git a/src/libqmi-glib/qmi-device.c b/src/libqmi-glib/qmi-
>> device.c
>> index e98f3ca..2bf6a40 100644
>> --- a/src/libqmi-glib/qmi-device.c
>> +++ b/src/libqmi-glib/qmi-device.c
>> @@ -1219,7 +1219,7 @@ qmi_device_release_client (QmiDevice *self,
>> qmi_service_get_string (service),
>> cid);
>>
>> - /* Reset the contents of the client object, making it unusable
>> */
>> + /* Reset the contents of the client object, making it invalid */
>> g_object_set (client,
>> QMI_CLIENT_CID, QMI_CID_NONE,
>> QMI_CLIENT_SERVICE, QMI_SERVICE_UNKNOWN,
--
Aleksander
https://aleksander.es
More information about the libqmi-devel
mailing list