[Spice-devel] [PATCH v3 02/13] Add redirecting state
Dmitry Fleytman
dmitry at daynix.com
Sun Aug 16 04:36:49 PDT 2015
> On Aug 11, 2015, at 13:50 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
>
> On Mon, Aug 03, 2015 at 04:10:42PM +0300, Kirill Moizik wrote:
>> From: Kirill Moizik <kmoizik at redhat.com <mailto:kmoizik at redhat.com>>
>>
>> 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.
Done.
>
>> ---
>> 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
Fixed in v4.
>
>> 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 ?
You are right, moving to a separate 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
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.
Fixed.
>
>> + */
>> + 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
Fixed.
>
>> 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.
Thanks for the idea, Christophe, indeed Udev property is better.
Changed in v4.
>
> Christophe
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150816/aa00e4b0/attachment-0001.html>
More information about the Spice-devel
mailing list