On Mon, Aug 03, 2015 at 04:10:42PM +0300, Kirill Moizik wrote:
> From: Kirill Moizik <kmoizik@xxxxxxxxxx>
>
> Add redirecting property to UsbDeviceManager and redirecting field to GUdevCleint
"GUdevClient"
Please add more details as to when this property is supposed to be used
in the log, and in the property description ("It indicates when a
redirection operation is in progress on a device. It's set back to FALSE
once the device is fully redirected to the guest") or something like
that.
> ---
> src/map-file | 1 +
> src/usb-device-manager.c | 31 +++++++++++++++++++++++++++++--
> src/win-usb-dev.c | 13 +++++++++++++
> src/win-usb-dev.h | 1 +
> 4 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/src/map-file b/src/map-file
> index d5a073f..6d5a5ef 100644
> --- a/src/map-file
> +++ b/src/map-file
> @@ -117,6 +117,7 @@ spice_uri_to_string;
> spice_usb_device_get_description;
> spice_usb_device_get_libusb_device;
> spice_usb_device_get_type;
> +spice_g_udev_set_redirecting;
I don't think this needs to be exported
> spice_usb_device_manager_can_redirect_device;
> spice_usb_device_manager_connect_device_async;
> spice_usb_device_manager_connect_device_finish;
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index 5b8151f..adf7acc 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -93,6 +93,7 @@ enum {
> PROP_AUTO_CONNECT,
> PROP_AUTO_CONNECT_FILTER,
> PROP_REDIRECT_ON_CONNECT,
> + PROP_REDIRECTING,
> };
>
> enum
> @@ -130,6 +131,7 @@ struct _SpiceUsbDeviceManagerPrivate {
> SpiceWinUsbDriver *installer;
> #endif
> gboolean use_usbclerk;
> + gboolean redirecting;
> #endif
> GPtrArray *devices;
> GPtrArray *channels;
> @@ -421,13 +423,18 @@ static void spice_usb_device_manager_get_property(GObject *gobject,
> case PROP_REDIRECT_ON_CONNECT:
> g_value_set_string(value, priv->redirect_on_connect);
> break;
> +#ifdef USE_USBREDIR
> + case PROP_REDIRECTING:
> + g_value_set_boolean(value, priv->redirecting);
> + break;
> +#endif
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> break;
> }
> }
>
> -#ifdef G_OS_WIN32
> +#if defined(USE_USBREDIR) && defined(G_OS_WIN32)
Is this related to this patch ?
> static
> void _usbdk_autoredir_enable(SpiceUsbDeviceManager *manager);
> static
> @@ -448,7 +455,7 @@ static void spice_usb_device_manager_set_property(GObject *gobject,
> break;
> case PROP_AUTO_CONNECT:
> priv->auto_connect = g_value_get_boolean(value);
> -#ifdef G_OS_WIN32
> +#if defined(USE_USBREDIR) && defined(G_OS_WIN32)
ditto
> if (!priv->use_usbclerk) {
> if (priv->auto_connect) {
> _usbdk_autoredir_enable(self);
> @@ -504,6 +511,11 @@ static void spice_usb_device_manager_set_property(GObject *gobject,
> priv->redirect_on_connect = g_strdup(filter);
> break;
> }
> +#ifdef USE_USBREDIR
> + case PROP_REDIRECTING:
> + priv->redirecting = g_value_get_boolean(value);
> + break;
> +#endif
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> break;
> @@ -595,6 +607,21 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
> pspec);
>
> /**
> + * SpiceUsbDeviceManager:redirecting:
> + *
> + * Boolean variable specifying async usb redirection flow
> + *
> + * See #SpiceUsbDeviceManager:auto-connect-filter for the filter string
> + * format.
These last 2 lines should be removed, I'd like more details about the
exact meaning of the property in the comment.
> + */
> + pspec = g_param_spec_boolean("redirecting", "Redirecting",
> + "Usb redirection in progress",
> + FALSE,
> + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
> + g_object_class_install_property(gobject_class, PROP_REDIRECTING,
> + pspec);
> +
> + /**
> * SpiceUsbDeviceManager::device-added:
> * @manager: the #SpiceUsbDeviceManager that emitted the signal
> * @device: #SpiceUsbDevice boxed object corresponding to the added device
> diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> index 1e4b2d6..875ef89 100644
> --- a/src/win-usb-dev.c
> +++ b/src/win-usb-dev.c
> @@ -37,6 +37,7 @@ struct _GUdevClientPrivate {
> gssize udev_list_size;
> GList *udev_list;
> HWND hwnd;
> + gboolean redirecting;
> };
>
> #define G_UDEV_CLIENT_WINCLASS_NAME TEXT("G_UDEV_CLIENT")
> @@ -186,6 +187,7 @@ g_udev_client_initable_init(GInitable *initable, GCancellable *cancellable,
> self = G_UDEV_CLIENT(initable);
> priv = self->priv;
>
> + priv->redirecting = FALSE;
This is not needed, priv is set to 0 upon creation
> rc = libusb_init(&priv->ctx);
> if (rc < 0) {
> const char *errstr = spice_usbutil_libusb_strerror(rc);
> @@ -334,6 +336,17 @@ static gboolean gudev_devices_are_equal(GUdevDevice *a, GUdevDevice *b)
> return (same_pid && same_vid);
> }
>
> +static void handle_dev_change(GUdevClient *self);
> +
> +void spice_g_udev_set_redirecting(gboolean val)
> +{
> + GUdevClientPrivate *priv = singleton->priv;
> + gboolean redirecting_end;
> + redirecting_end = (priv->redirecting && !val);
> + priv->redirecting = val;
> + if (redirecting_end)
> + handle_dev_change(singleton);
> +}
I would make this method
win_gudev_client_set_redirecting(GUdevClient *client, gboolean val);
or something like that. However, seeing how this is used then, I suspect
things would be cleaner with a GUdevClient::redirecting gobject property rather
than having it in usb-device-manager.c where it needs to be kept in sync
with the GUdevClient state.
Christophe
Attachment:
pgpSCmGw2TC5W.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel