[Spice-devel] [spice-gtk v2 12/13] usb-redir: use persistent libusb_device under Windows also

2019-03-18 Thread Yuri Benditovich
Unify SpiceUsbDeviceInfo for Linux and Windows by using
persistent libusb_device. Probably there was some problem
doing it long time ago with winusb, but at least with
existing libusb and UsbDk the the libusb device can be
accessed as long as it is referenced.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 105 ---
 1 file changed, 105 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index a163501..621e303 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -136,11 +136,7 @@ typedef struct _SpiceUsbDeviceInfo {
 guint16 vid;
 guint16 pid;
 gboolean isochronous;
-#ifdef G_OS_WIN32
-guint8  state;
-#else
 libusb_device *libdev;
-#endif
 gintref;
 } SpiceUsbDeviceInfo;
 
@@ -758,13 +754,11 @@ gconstpointer
 spice_usb_device_get_libusb_device(const SpiceUsbDevice *device G_GNUC_UNUSED)
 {
 #ifdef USE_USBREDIR
-#ifndef G_OS_WIN32
 const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device;
 
 g_return_val_if_fail(info != NULL, FALSE);
 
 return info->libdev;
-#endif
 #endif
 return NULL;
 }
@@ -886,17 +880,6 @@ 
spice_usb_device_manager_device_match(SpiceUsbDeviceManager *self, SpiceUsbDevic
 spice_usb_device_get_devaddr(device) == address);
 }
 
-#ifdef G_OS_WIN32
-static gboolean
-spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, 
libusb_device *libdev,
-  const int bus, const int address)
-{
-/* match functions for Linux/UsbDk -- match by bus.addr */
-return (libusb_get_bus_number(libdev) == bus &&
-libusb_get_device_address(libdev) == address);
-}
-#endif
-
 static SpiceUsbDevice*
 spice_usb_device_manager_find_device(SpiceUsbDeviceManager *self,
  const int bus, const int address)
@@ -1150,10 +1133,6 @@ static void 
spice_usb_device_manager_check_redir_on_connect(
 continue;
 
 libdev = spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-if (libdev == NULL)
-continue;
-#endif
 if (usbredirhost_check_device_filter(
 priv->redirect_on_connect_rules,
 priv->redirect_on_connect_rules_count,
@@ -1258,10 +1237,6 @@ GPtrArray* 
spice_usb_device_manager_get_devices_with_filter(
 if (rules) {
 libusb_device *libdev =
 spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-if (libdev == NULL)
-continue;
-#endif
 if (usbredirhost_check_device_filter(rules, count, libdev, 0) != 0)
 continue;
 }
@@ -1353,24 +1328,6 @@ 
_spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
 continue; /* Skip already used channels */
 
 libdev = spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-if (libdev == NULL) {
-/* Most likely, the device was plugged out at driver installation
- * time, and its remove-device event was ignored.
- * So remove the device now
- */
-SPICE_DEBUG("libdev does not exist for %p -- removing", device);
-spice_usb_device_ref(device);
-g_ptr_array_remove(priv->devices, device);
-g_signal_emit(self, signals[DEVICE_REMOVED], 0, device);
-spice_usb_device_unref(device);
-g_task_return_new_error(task,
-SPICE_CLIENT_ERROR,
-SPICE_CLIENT_ERROR_FAILED,
-_("Device was not found"));
-goto done;
-}
-#endif
 spice_usbredir_channel_connect_device_async(channel,
  libdev,
  device,
@@ -1642,13 +1599,6 @@ 
spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
 libusb_device *libdev;
 
 libdev = spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-if (libdev == NULL) {
-g_set_error_literal(err, SPICE_CLIENT_ERROR, 
SPICE_CLIENT_ERROR_FAILED,
-_("Some USB devices were not found"));
-return FALSE;
-}
-#endif
 filter_ok = (usbredirhost_check_device_filter(
 guest_filter_rules, guest_filter_rules_count,
 libdev, 0) == 0);
@@ -1799,9 +1749,7 @@ static SpiceUsbDeviceInfo 
*spice_usb_device_new(libusb_device *libdev)
 info->pid = pid;
 info->ref = 1;
 info->isochronous = probe_isochronous_endpoint(libdev);
-#ifndef G_OS_WIN32
 info->libdev = libusb_ref_device(libdev);
-#endif
 
 return info;
 }
@@ -1937,14 +1885,11 @@ static void spice_usb_device_unref(SpiceUsbDevice 
*device)
 
 ref_count_is_0 = 

[Spice-devel] [spice-gtk v2 09/13] win-usb-dev: do not allocate memory for hub devices

2019-03-18 Thread Yuri Benditovich
When processing list of USB devices, avoid allocating memory
for devices which later will be skipped. Use existing libusb
struct first to check whether the device shall be excluded.

Signed-off-by: Yuri Benditovich 
---
 src/win-usb-dev.c | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index bb11e8b..b3b2ed8 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -97,7 +97,7 @@ static void g_udev_device_print_list(GList *l, const gchar 
*msg) {}
 #endif
 static void g_udev_device_print(GUdevDevice *udev, const gchar *msg);
 
-static gboolean g_udev_skip_search(GUdevDevice *udev);
+static gboolean g_udev_skip_search(libusb_device *dev);
 
 GQuark g_udev_client_error_quark(void)
 {
@@ -152,13 +152,12 @@ g_udev_client_list_devices(GUdevClient *self, GList 
**devs,
 
 n = 0;
 for (dev = lusb_list; *dev; dev++) {
+if (g_udev_skip_search(*dev)) {
+continue;
+}
 udevinfo = g_new0(GUdevDeviceInfo, 1);
 get_usb_dev_info(*dev, udevinfo);
 udevice = g_udev_device_new(udevinfo);
-if (g_udev_skip_search(udevice)) {
-g_object_unref(udevice);
-continue;
-}
 *devs = g_list_prepend(*devs, udevice);
 n++;
 }
@@ -549,19 +548,17 @@ static void g_udev_device_print(GUdevDevice *udev, const 
gchar *msg)
 udevinfo->vid, udevinfo->pid, udevinfo->class);
 }
 
-static gboolean g_udev_skip_search(GUdevDevice *udev)
+static gboolean g_udev_skip_search(libusb_device *dev)
 {
-GUdevDeviceInfo* udevinfo;
 gboolean skip;
+uint8_t addr  = libusb_get_device_address(dev);
+struct libusb_device_descriptor desc;
 
-g_return_val_if_fail(G_UDEV_DEVICE(udev), FALSE);
-
-udevinfo = udev->priv->udevinfo;
-g_return_val_if_fail(udevinfo != NULL, FALSE);
+libusb_get_device_descriptor(dev, );
 
-skip = ((udevinfo->addr == 0xff) ||  /* root hub (HCD) */
-(udevinfo->addr == 1) || /* root hub addr */
-(udevinfo->class == LIBUSB_CLASS_HUB) || /* hub*/
-(udevinfo->addr == 0)); /* bad address */
+skip = ((addr == 0xff) ||  /* root hub (HCD) */
+(addr == 1) || /* root hub addr */
+(desc.bDeviceClass == LIBUSB_CLASS_HUB) || /* hub*/
+(addr == 0)); /* bad address */
 return skip;
 }
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 07/13] usb-redir: discard cold-plug device list under Windows

2019-03-18 Thread Yuri Benditovich
Discard the optimization of initial device enumeration.
Just after connection to 'udev' signal request to report
all the devices  one by one as if they are inserted.
Further commits will remove device enumeration in
usb-dev-manager completely.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 19 +++
 src/win-usb-dev.c| 11 +++
 src/win-usb-dev.h|  2 +-
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index a9f19bf..c5e662c 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -104,7 +104,6 @@ struct _SpiceUsbDeviceManagerPrivate {
 int redirect_on_connect_rules_count;
 #ifdef G_OS_WIN32
 GUdevClient *udev;
-libusb_device **coldplug_list; /* Avoid needless reprobing during init */
 #else
 gboolean redirecting; /* Handled by GUdevClient in the gudev case */
 libusb_hotplug_callback_handle hp_handle;
@@ -307,15 +306,7 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 g_signal_connect(G_OBJECT(priv->udev), "uevent",
  G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
 /* Do coldplug (detection of already connected devices) */
-libusb_get_device_list(priv->context, >coldplug_list);
-list = g_udev_client_query_by_subsystem(priv->udev, "usb");
-for (it = g_list_first(list); it; it = g_list_next(it)) {
-spice_usb_device_manager_add_udev(self, it->data);
-g_object_unref(it->data);
-}
-g_list_free(list);
-libusb_free_device_list(priv->coldplug_list, 1);
-priv->coldplug_list = NULL;
+g_udev_client_report_devices(priv->udev);
 #else
 rc = libusb_hotplug_register_callback(priv->context,
 LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
@@ -1049,10 +1040,7 @@ static void 
spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
 return;
 }
 
-if (priv->coldplug_list)
-dev_list = priv->coldplug_list;
-else
-libusb_get_device_list(priv->context, _list);
+libusb_get_device_list(priv->context, _list);
 
 for (i = 0; dev_list && dev_list[i]; i++) {
 if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, 
address)) {
@@ -1067,8 +1055,7 @@ static void 
spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
 g_warning("Could not find USB device to add " DEV_ID_FMT,
   (guint) bus, (guint) address);
 
-if (!priv->coldplug_list)
-libusb_free_device_list(dev_list, 1);
+libusb_free_device_list(dev_list, 1);
 }
 
 static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager  *self,
diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index 6273cbe..50db4e5 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -247,11 +247,14 @@ static void 
g_udev_client_initable_iface_init(GInitableIface *iface)
 iface->init = g_udev_client_initable_init;
 }
 
-GList *g_udev_client_query_by_subsystem(GUdevClient *self, const gchar 
*subsystem)
+static void report_one_device(gpointer data, gpointer self)
 {
-GList *l = g_list_copy(self->priv->udev_list);
-g_list_foreach(l, (GFunc)g_object_ref, NULL);
-return l;
+g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", data);
+}
+
+void g_udev_client_report_devices(GUdevClient *self)
+{
+g_list_foreach(self->priv->udev_list, report_one_device, self);
 }
 
 static void g_udev_client_init(GUdevClient *self)
diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
index b7b7eda..bca8285 100644
--- a/src/win-usb-dev.h
+++ b/src/win-usb-dev.h
@@ -81,7 +81,7 @@ struct _GUdevClientClass
 GType g_udev_client_get_type(void) G_GNUC_CONST;
 GUdevClient *g_udev_client_new(void);
 libusb_context *g_udev_client_get_context(GUdevClient *client);
-GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar 
*subsystem);
+void g_udev_client_report_devices(GUdevClient *client);
 
 GType g_udev_device_get_type(void) G_GNUC_CONST;
 const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar 
*property);
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 03/13] usb-redir: reuse libusb context under Windows

2019-03-18 Thread Yuri Benditovich
Do not create own libusb context in usb-device-manager.
Reuse existing context created by win-usb-dev instead,
this will later allow usb-device-manager to use libusb
devices enumerated in win-usb-dev.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 11 +--
 src/win-usb-dev.c|  4 
 src/win-usb-dev.h|  1 +
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index 6435be8..debba4d 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -282,8 +282,9 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 SpiceUsbDeviceManagerPrivate *priv = self->priv;
 GList *list;
 GList *it;
-int rc;
 
+#ifndef G_OS_WIN32
+int rc;
 /* Initialize libusb */
 rc = libusb_init(>context);
 if (rc < 0) {
@@ -293,11 +294,6 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 "Error initializing USB support: %s [%i]", desc, rc);
 return FALSE;
 }
-
-#ifdef G_OS_WIN32
-#if LIBUSB_API_VERSION >= 0x01000106
-libusb_set_option(priv->context, LIBUSB_OPTION_USE_USBDK);
-#endif
 #endif
 
 /* Start listening for usb devices plug / unplug */
@@ -307,6 +303,7 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 g_warning("Error initializing GUdevClient");
 return FALSE;
 }
+priv->context = g_udev_client_get_context(priv->udev);
 g_signal_connect(G_OBJECT(priv->udev), "uevent",
  G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
 /* Do coldplug (detection of already connected devices) */
@@ -402,8 +399,10 @@ static void spice_usb_device_manager_finalize(GObject 
*gobject)
 g_clear_object(>udev);
 #endif
 g_return_if_fail(priv->event_thread == NULL);
+#ifndef G_OS_WIN32
 if (priv->context)
 libusb_exit(priv->context);
+#endif
 free(priv->auto_conn_filter_rules);
 free(priv->redirect_on_connect_rules);
 #ifdef G_OS_WIN32
diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index d3ff55a..b08bf02 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -113,6 +113,10 @@ GUdevClient *g_udev_client_new(void)
 return singleton;
 }
 
+libusb_context *g_udev_client_get_context(GUdevClient *client)
+{
+return client->priv->ctx;
+}
 
 /*
  * devs [in,out] an empty devs list in, full devs list out
diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
index 0f34a01..f3c7466 100644
--- a/src/win-usb-dev.h
+++ b/src/win-usb-dev.h
@@ -80,6 +80,7 @@ struct _GUdevClientClass
 
 GType g_udev_client_get_type(void) G_GNUC_CONST;
 GUdevClient *g_udev_client_new(void);
+libusb_context *g_udev_client_get_context(GUdevClient *client);
 GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar 
*subsystem);
 
 GType g_udev_device_get_type(void) G_GNUC_CONST;
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 08/13] usb-redir: change signal prototype of win-usb-dev

2019-03-18 Thread Yuri Benditovich
Changing signal definition from (boxed-boxed) to (pointer,int).
There is no need for additional referencing of GUdevDevice
object before signal callback. Second parameter (action) is
FALSE for device removal and TRUE for device addition.

Signed-off-by: Yuri Benditovich 
---
 src/spice-marshal.txt|  1 +
 src/usb-device-manager.c |  8 
 src/win-usb-dev.c| 18 +-
 src/win-usb-dev.h|  2 +-
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/src/spice-marshal.txt b/src/spice-marshal.txt
index cf35790..46be405 100644
--- a/src/spice-marshal.txt
+++ b/src/spice-marshal.txt
@@ -5,6 +5,7 @@ VOID:UINT,UINT,UINT,UINT
 VOID:INT,INT,INT,INT,POINTER
 VOID:INT,INT,INT,INT,INT,POINTER
 VOID:POINTER,INT
+VOID:POINTER,BOOLEAN
 BOOLEAN:POINTER,UINT
 BOOLEAN:UINT
 VOID:UINT,POINTER,UINT
diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index c5e662c..c99d359 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -153,8 +153,8 @@ static void channel_event(SpiceChannel *channel, 
SpiceChannelEvent event,
   gpointer user_data);
 #ifdef G_OS_WIN32
 static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
-   const gchar *action,
GUdevDevice *udevice,
+   gboolean add,
gpointer user_data);
 static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
   GUdevDevice*udev);
@@ -1070,15 +1070,15 @@ static void 
spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager  *self,
 }
 
 static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
-   const gchar *action,
GUdevDevice *udevice,
+   gboolean add,
gpointer user_data)
 {
 SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
 
-if (g_str_equal(action, "add"))
+if (add)
 spice_usb_device_manager_add_udev(self, udevice);
-else if (g_str_equal (action, "remove"))
+else
 spice_usb_device_manager_remove_udev(self, udevice);
 }
 #else
diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index 50db4e5..bb11e8b 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -249,7 +249,7 @@ static void 
g_udev_client_initable_iface_init(GInitableIface *iface)
 
 static void report_one_device(gpointer data, gpointer self)
 {
-g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", data);
+g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE);
 }
 
 void g_udev_client_report_devices(GUdevClient *self)
@@ -342,11 +342,11 @@ static void g_udev_client_class_init(GUdevClientClass 
*klass)
  G_SIGNAL_RUN_FIRST,
  G_STRUCT_OFFSET(GUdevClientClass, uevent),
  NULL, NULL,
- g_cclosure_user_marshal_VOID__BOXED_BOXED,
+ g_cclosure_user_marshal_VOID__POINTER_BOOLEAN,
  G_TYPE_NONE,
  2,
- G_TYPE_STRING,
- G_UDEV_TYPE_DEVICE);
+ G_TYPE_POINTER,
+ G_TYPE_BOOLEAN);
 
 /**
 * GUdevClient::redirecting:
@@ -408,15 +408,15 @@ static gint gudev_devices_differ(gconstpointer a, 
gconstpointer b)
 static void notify_dev_state_change(GUdevClient *self,
 GList *old_list,
 GList *new_list,
-const gchar *action)
+gboolean add)
 {
 GList *dev;
 
 for (dev = g_list_first(old_list); dev != NULL; dev = g_list_next(dev)) {
 if (g_list_find_custom(new_list, dev->data, gudev_devices_differ) == 
NULL) {
 /* Found a device that changed its state */
-g_udev_device_print(dev->data, action);
-g_signal_emit(self, signals[UEVENT_SIGNAL], 0, action, dev->data);
+g_udev_device_print(dev->data, add ? "add" : "remove");
+g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data, add);
 }
 }
 }
@@ -445,10 +445,10 @@ static void handle_dev_change(GUdevClient *self)
 g_udev_device_print_list(priv->udev_list, "handle_dev_change: previous 
list:");
 
 /* Unregister devices that are not present anymore */
-notify_dev_state_change(self, priv->udev_list, now_devs, "remove");
+notify_dev_state_change(self, priv->udev_list, now_devs, FALSE);
 
 /* Register newly inserted devices */
-notify_dev_state_change(self, now_devs, priv->udev_list, "add");
+notify_dev_state_change(self, now_devs, 

[Spice-devel] [spice-gtk v2 06/13] win-usb-dev: remove unused procedure

2019-03-18 Thread Yuri Benditovich
Remove unused g_udev_device_get_sysfs_attr.

Signed-off-by: Yuri Benditovich 
---
 src/win-usb-dev.c | 18 --
 src/win-usb-dev.h |  1 -
 2 files changed, 19 deletions(-)

diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index 52e61a8..6273cbe 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -521,24 +521,6 @@ const gchar *g_udev_device_get_property(GUdevDevice *udev, 
const gchar *property
 return NULL;
 }
 
-const gchar *g_udev_device_get_sysfs_attr(GUdevDevice *udev, const gchar *attr)
-{
-GUdevDeviceInfo* udevinfo;
-
-g_return_val_if_fail(G_UDEV_DEVICE(udev), NULL);
-g_return_val_if_fail(attr != NULL, NULL);
-
-udevinfo = udev->priv->udevinfo;
-g_return_val_if_fail(udevinfo != NULL, NULL);
-
-
-if (g_strcmp0(attr, "bDeviceClass") == 0) {
-return udevinfo->sclass;
-}
-g_warn_if_reached();
-return NULL;
-}
-
 #ifdef DEBUG_GUDEV_DEVICE_LISTS
 static void g_udev_device_print_list(GList *l, const gchar *msg)
 {
diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
index f3c7466..b7b7eda 100644
--- a/src/win-usb-dev.h
+++ b/src/win-usb-dev.h
@@ -85,7 +85,6 @@ GList *g_udev_client_query_by_subsystem(GUdevClient *client, 
const gchar *subsys
 
 GType g_udev_device_get_type(void) G_GNUC_CONST;
 const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar 
*property);
-const gchar *g_udev_device_get_sysfs_attr(GUdevDevice *udev, const gchar 
*attr);
 
 GQuark g_udev_client_error_quark(void);
 #define G_UDEV_CLIENT_ERROR g_udev_client_error_quark()
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 02/13] usb-redir: remove unused 'subsystem' parameter

2019-03-18 Thread Yuri Benditovich
Removing unused parameter for GUdevClient constructor.
This code is used only in build for Windows.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 5 +
 src/win-usb-dev.c| 2 +-
 src/win-usb-dev.h| 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index 6a36cfa..6435be8 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -283,9 +283,6 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 GList *list;
 GList *it;
 int rc;
-#ifdef G_OS_WIN32
-const gchar *const subsystems[] = {"usb", NULL};
-#endif
 
 /* Initialize libusb */
 rc = libusb_init(>context);
@@ -305,7 +302,7 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 
 /* Start listening for usb devices plug / unplug */
 #ifdef G_OS_WIN32
-priv->udev = g_udev_client_new(subsystems);
+priv->udev = g_udev_client_new();
 if (priv->udev == NULL) {
 g_warning("Error initializing GUdevClient");
 return FALSE;
diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index 964719c..d3ff55a 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -104,7 +104,7 @@ GQuark g_udev_client_error_quark(void)
 return g_quark_from_static_string("win-gudev-client-error-quark");
 }
 
-GUdevClient *g_udev_client_new(const gchar* const *subsystems)
+GUdevClient *g_udev_client_new(void)
 {
 if (singleton != NULL)
 return g_object_ref(singleton);
diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
index 7f40197..0f34a01 100644
--- a/src/win-usb-dev.h
+++ b/src/win-usb-dev.h
@@ -79,7 +79,7 @@ struct _GUdevClientClass
 };
 
 GType g_udev_client_get_type(void) G_GNUC_CONST;
-GUdevClient *g_udev_client_new(const gchar* const *subsystems);
+GUdevClient *g_udev_client_new(void);
 GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar 
*subsystem);
 
 GType g_udev_device_get_type(void) G_GNUC_CONST;
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 05/13] win-usb-dev: strict comparison of USB devices

2019-03-18 Thread Yuri Benditovich
If on device change the new device has the same bus:address
as existing device, win-usb-dev does not emit signal of
device change (for example, when due to some reason the
'redirecting' property is set for long time and during this
time one of devices is changed).
Make device comparison more strict: check not only bus:addr,
but also vid:pid.

Signed-off-by: Yuri Benditovich 
---
 src/win-usb-dev.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index b08bf02..52e61a8 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -385,20 +385,21 @@ static gboolean get_usb_dev_info(libusb_device *dev, 
GUdevDeviceInfo *udevinfo)
 return TRUE;
 }
 
-/* Only bus:addr are compared */
+/* comparing bus:addr and vid:pid */
 static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
 {
 GUdevDeviceInfo *ai, *bi;
-gboolean same_bus;
-gboolean same_addr;
+gboolean same_bus, same_addr, same_vid, same_pid;
 
 ai = G_UDEV_DEVICE(a)->priv->udevinfo;
 bi = G_UDEV_DEVICE(b)->priv->udevinfo;
 
 same_bus = (ai->bus == bi->bus);
 same_addr = (ai->addr == bi->addr);
+same_vid = (ai->vid == bi->vid);
+same_pid = (ai->pid == bi->pid);
 
-return (same_bus && same_addr) ? 0 : -1;
+return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1;
 }
 
 static void notify_dev_state_change(GUdevClient *self,
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 10/13] win-usb-dev: maintain list of libusb devices

2019-03-18 Thread Yuri Benditovich
Change internal device list to maintain libusb devices
instead of GUdevDevice objects. Create temporary
GUdevDevice object only for indication to usb-dev-manager.

Signed-off-by: Yuri Benditovich 
---
 src/win-usb-dev.c | 80 ++-
 1 file changed, 51 insertions(+), 29 deletions(-)

diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index b3b2ed8..0b87e75 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -95,7 +95,7 @@ static void g_udev_device_print_list(GList *l, const gchar 
*msg);
 #else
 static void g_udev_device_print_list(GList *l, const gchar *msg) {}
 #endif
-static void g_udev_device_print(GUdevDevice *udev, const gchar *msg);
+static void g_udev_device_print(libusb_device *dev, const gchar *msg);
 
 static gboolean g_udev_skip_search(libusb_device *dev);
 
@@ -129,8 +129,6 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
 gssize rc;
 libusb_device **lusb_list, **dev;
 GUdevClientPrivate *priv;
-GUdevDeviceInfo *udevinfo;
-GUdevDevice *udevice;
 ssize_t n;
 
 g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1);
@@ -155,10 +153,7 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
 if (g_udev_skip_search(*dev)) {
 continue;
 }
-udevinfo = g_new0(GUdevDeviceInfo, 1);
-get_usb_dev_info(*dev, udevinfo);
-udevice = g_udev_device_new(udevinfo);
-*devs = g_list_prepend(*devs, udevice);
+*devs = g_list_prepend(*devs, libusb_ref_device(*dev));
 n++;
 }
 libusb_free_device_list(lusb_list, 1);
@@ -166,11 +161,17 @@ g_udev_client_list_devices(GUdevClient *self, GList 
**devs,
 return n;
 }
 
+static void unreference_libusb_device(gpointer data)
+{
+libusb_unref_device((libusb_device *)data);
+}
+
 static void g_udev_client_free_device_list(GList **devs)
 {
 g_return_if_fail(devs != NULL);
 if (*devs) {
-g_list_free_full(*devs, g_object_unref);
+/* avoiding casting of imported procedure to GDestroyNotify */
+g_list_free_full(*devs, unreference_libusb_device);
 *devs = NULL;
 }
 }
@@ -246,9 +247,22 @@ static void 
g_udev_client_initable_iface_init(GInitableIface *iface)
 iface->init = g_udev_client_initable_init;
 }
 
+static void g_udev_notify_device(GUdevClient *self, libusb_device *dev, 
gboolean add)
+{
+GUdevDeviceInfo *udevinfo;
+GUdevDevice *udevice;
+udevinfo = g_new0(GUdevDeviceInfo, 1);
+if (get_usb_dev_info(dev, udevinfo)) {
+udevice = g_udev_device_new(udevinfo);
+g_signal_emit(self, signals[UEVENT_SIGNAL], 0, udevice, add);
+} else {
+g_free(udevinfo);
+}
+}
+
 static void report_one_device(gpointer data, gpointer self)
 {
-g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE);
+g_udev_notify_device(self, data, TRUE);
 }
 
 void g_udev_client_report_devices(GUdevClient *self)
@@ -388,18 +402,20 @@ static gboolean get_usb_dev_info(libusb_device *dev, 
GUdevDeviceInfo *udevinfo)
 }
 
 /* comparing bus:addr and vid:pid */
-static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
+static gint compare_libusb_devices(gconstpointer a, gconstpointer b)
 {
-GUdevDeviceInfo *ai, *bi;
+libusb_device *a_dev = (libusb_device *)a;
+libusb_device *b_dev = (libusb_device *)b;
+struct libusb_device_descriptor a_desc, b_desc;
 gboolean same_bus, same_addr, same_vid, same_pid;
 
-ai = G_UDEV_DEVICE(a)->priv->udevinfo;
-bi = G_UDEV_DEVICE(b)->priv->udevinfo;
+libusb_get_device_descriptor(a_dev, _desc);
+libusb_get_device_descriptor(b_dev, _desc);
 
-same_bus = (ai->bus == bi->bus);
-same_addr = (ai->addr == bi->addr);
-same_vid = (ai->vid == bi->vid);
-same_pid = (ai->pid == bi->pid);
+same_bus = (libusb_get_bus_number(a_dev) == libusb_get_bus_number(b_dev));
+same_addr = (libusb_get_device_address(a_dev) == 
libusb_get_device_address(b_dev));
+same_vid = (a_desc.idVendor == b_desc.idVendor);
+same_pid = (a_desc.idProduct == b_desc.idProduct);
 
 return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1;
 }
@@ -412,10 +428,17 @@ static void notify_dev_state_change(GUdevClient *self,
 GList *dev;
 
 for (dev = g_list_first(old_list); dev != NULL; dev = g_list_next(dev)) {
-if (g_list_find_custom(new_list, dev->data, gudev_devices_differ) == 
NULL) {
-/* Found a device that changed its state */
+GList *found = g_list_find_custom(new_list, dev->data, 
compare_libusb_devices);
+if (found == NULL) {
 g_udev_device_print(dev->data, add ? "add" : "remove");
-g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data, add);
+g_udev_notify_device(self, dev->data, add);
+} else if (add) {
+/* keep old existing libusb_device in the new list, when
+   usb-dev-manager will maintain its own list of libusb_device,
+   these lists 

[Spice-devel] [spice-gtk v2 00/13] use persistent libusb_device on Windows

2019-03-18 Thread Yuri Benditovich
This series unifies part of USB redirection code for Windows making
it the same as for Linux by using persistent libusb_device also on
Windows.

Changes from v1:
minor fixes per v1 review
rebase to latest master branch

Yuri Benditovich (13):
  usb-redir: replace USE_GUDEV with G_OS_WIN32
  usb-redir: remove unused 'subsystem' parameter
  usb-redir: reuse libusb context under Windows
  usb-redir: do not add device if one with the same bus:addr exists
  win-usb-dev: strict comparison of USB devices
  win-usb-dev: remove unused procedure
  usb-redir: discard cold-plug device list under Windows
  usb-redir: change signal prototype of win-usb-dev
  win-usb-dev: do not allocate memory for hub devices
  win-usb-dev: maintain list of libusb devices
  win-usb-dev: report libusb_device via signal
  usb-redir: use persistent libusb_device under Windows also
  win-usb-dev: remove unused code

 src/spice-marshal.txt|   1 +
 src/usb-device-manager.c | 267 ++-
 src/win-usb-dev.c| 254 ++---
 src/win-usb-dev.h|  35 +
 4 files changed, 110 insertions(+), 447 deletions(-)

-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 13/13] win-usb-dev: remove unused code

2019-03-18 Thread Yuri Benditovich
Remove unused code related to GUdevDevice.

Signed-off-by: Yuri Benditovich 
---
 src/win-usb-dev.c | 74 ---
 src/win-usb-dev.h | 27 -
 2 files changed, 101 deletions(-)

diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index 09bd031..7c21482 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -49,31 +49,6 @@ G_DEFINE_TYPE_WITH_CODE(GUdevClient, g_udev_client, 
G_TYPE_OBJECT,
 G_ADD_PRIVATE(GUdevClient)
 G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE, 
g_udev_client_initable_iface_init))
 
-
-typedef struct _GUdevDeviceInfo GUdevDeviceInfo;
-
-struct _GUdevDeviceInfo {
-guint16 bus;
-guint16 addr;
-guint16 vid;
-guint16 pid;
-guint16 class;
-gchar sclass[4];
-gchar sbus[4];
-gchar saddr[4];
-gchar svid[8];
-gchar spid[8];
-};
-
-struct _GUdevDevicePrivate
-{
-/* FixMe: move above fields to this structure and access them directly */
-GUdevDeviceInfo *udevinfo;
-};
-
-G_DEFINE_TYPE_WITH_PRIVATE(GUdevDevice, g_udev_device, G_TYPE_OBJECT)
-
-
 enum
 {
 UEVENT_SIGNAL,
@@ -447,55 +422,6 @@ static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, 
WPARAM wparam, LPARAM
 return DefWindowProc(hwnd, message, wparam, lparam);
 }
 
-/*** GUdevDevice ***/
-
-static void g_udev_device_finalize(GObject *object)
-{
-GUdevDevice *device =  G_UDEV_DEVICE(object);
-
-g_free(device->priv->udevinfo);
-if (G_OBJECT_CLASS(g_udev_device_parent_class)->finalize != NULL)
-(* G_OBJECT_CLASS(g_udev_device_parent_class)->finalize)(object);
-}
-
-static void g_udev_device_class_init(GUdevDeviceClass *klass)
-{
-GObjectClass *gobject_class = (GObjectClass *) klass;
-
-gobject_class->finalize = g_udev_device_finalize;
-}
-
-static void g_udev_device_init(GUdevDevice *device)
-{
-device->priv = g_udev_device_get_instance_private(device);
-}
-
-const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar 
*property)
-{
-GUdevDeviceInfo* udevinfo;
-
-g_return_val_if_fail(G_UDEV_DEVICE(udev), NULL);
-g_return_val_if_fail(property != NULL, NULL);
-
-udevinfo = udev->priv->udevinfo;
-g_return_val_if_fail(udevinfo != NULL, NULL);
-
-if (g_strcmp0(property, "BUSNUM") == 0) {
-return udevinfo->sbus;
-} else if (g_strcmp0(property, "DEVNUM") == 0) {
-return udevinfo->saddr;
-} else if (g_strcmp0(property, "DEVTYPE") == 0) {
-return "usb_device";
-} else if (g_strcmp0(property, "VID") == 0) {
-return udevinfo->svid;
-} else if (g_strcmp0(property, "PID") == 0) {
-return udevinfo->spid;
-}
-
-g_warn_if_reached();
-return NULL;
-}
-
 #ifdef DEBUG_GUDEV_DEVICE_LISTS
 static void g_udev_device_print_list(GList *l, const gchar *msg)
 {
diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
index b960dfc..b1c0e27 100644
--- a/src/win-usb-dev.h
+++ b/src/win-usb-dev.h
@@ -26,30 +26,6 @@
 
 G_BEGIN_DECLS
 
-/* GUdevDevice */
-
-#define G_UDEV_TYPE_DEVICE (g_udev_device_get_type())
-#define G_UDEV_DEVICE(o)   (G_TYPE_CHECK_INSTANCE_CAST((o), 
G_UDEV_TYPE_DEVICE, GUdevDevice))
-#define G_UDEV_DEVICE_CLASS(k) (G_TYPE_CHECK_CLASS_CAST((k), 
G_UDEV_TYPE_DEVICE, GUdevDeviceClass))
-#define G_UDEV_IS_DEVICE(o)(G_TYPE_CHECK_INSTANCE_TYPE ((o), 
G_UDEV_TYPE_DEVICE))
-#define G_UDEV_IS_DEVICE_CLASS(k)  (G_TYPE_CHECK_CLASS_TYPE((k), 
G_UDEV_TYPE_DEVICE))
-#define G_UDEV_DEVICE_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o), 
G_UDEV_TYPE_DEVICE, GUdevDeviceClass))
-
-typedef struct _GUdevDevice GUdevDevice;
-typedef struct _GUdevDeviceClass GUdevDeviceClass;
-typedef struct _GUdevDevicePrivate GUdevDevicePrivate;
-
-struct _GUdevDevice
-{
-  GObject parent;
-  GUdevDevicePrivate *priv;
-};
-
-struct _GUdevDeviceClass
-{
-  GObjectClass parent_class;
-};
-
 /* GUdevClient */
 
 #define G_UDEV_TYPE_CLIENT (g_udev_client_get_type())
@@ -83,9 +59,6 @@ GUdevClient *g_udev_client_new(void);
 libusb_context *g_udev_client_get_context(GUdevClient *client);
 void g_udev_client_report_devices(GUdevClient *client);
 
-GType g_udev_device_get_type(void) G_GNUC_CONST;
-const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar 
*property);
-
 GQuark g_udev_client_error_quark(void);
 #define G_UDEV_CLIENT_ERROR g_udev_client_error_quark()
 
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 01/13] usb-redir: replace USE_GUDEV with G_OS_WIN32

2019-03-18 Thread Yuri Benditovich
Replacing USE_GUDEV with G_OS_WIN32 anywhere. GUDEV simulation
is used only in Windows build.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 26 +++---
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index 2578350..6a36cfa 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -29,11 +29,7 @@
 
 #ifdef G_OS_WIN32
 #include "usbdk_api.h"
-#endif
-
-#if defined(G_OS_WIN32)
 #include "win-usb-dev.h"
-#define USE_GUDEV /* win-usb-dev.h provides a fake gudev interface */
 #endif
 
 #include "channel-usbredir-priv.h"
@@ -106,7 +102,7 @@ struct _SpiceUsbDeviceManagerPrivate {
 struct usbredirfilter_rule *redirect_on_connect_rules;
 int auto_conn_filter_rules_count;
 int redirect_on_connect_rules_count;
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 GUdevClient *udev;
 libusb_device **coldplug_list; /* Avoid needless reprobing during init */
 #else
@@ -156,7 +152,7 @@ static void channel_destroy(SpiceSession *session, 
SpiceChannel *channel,
 gpointer user_data);
 static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
   gpointer user_data);
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
const gchar *action,
GUdevDevice *udevice,
@@ -210,7 +206,7 @@ G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device,
 static void
 _set_redirecting(SpiceUsbDeviceManager *self, gboolean is_redirecting)
 {
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 g_object_set(self->priv->udev, "redirecting", is_redirecting, NULL);
 #else
 self->priv->redirecting = is_redirecting;
@@ -235,7 +231,7 @@ gboolean 
spice_usb_device_manager_is_redirecting(SpiceUsbDeviceManager *self)
 {
 #ifdef USE_USBREDIR
 
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 gboolean redirecting;
 g_object_get(self->priv->udev, "redirecting", , NULL);
 return redirecting;
@@ -287,7 +283,7 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 GList *list;
 GList *it;
 int rc;
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 const gchar *const subsystems[] = {"usb", NULL};
 #endif
 
@@ -308,7 +304,7 @@ static gboolean 
spice_usb_device_manager_initable_init(GInitable  *initable,
 #endif
 
 /* Start listening for usb devices plug / unplug */
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 priv->udev = g_udev_client_new(subsystems);
 if (priv->udev == NULL) {
 g_warning("Error initializing GUdevClient");
@@ -366,7 +362,7 @@ static void spice_usb_device_manager_dispose(GObject 
*gobject)
 SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(gobject);
 SpiceUsbDeviceManagerPrivate *priv = self->priv;
 
-#ifndef USE_GUDEV
+#ifndef G_OS_WIN32
 if (priv->hp_handle) {
 spice_usb_device_manager_stop_event_listening(self);
 if (g_atomic_int_get(>event_thread_run)) {
@@ -405,7 +401,7 @@ static void spice_usb_device_manager_finalize(GObject 
*gobject)
 g_ptr_array_unref(priv->devices);
 
 #ifdef USE_USBREDIR
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 g_clear_object(>udev);
 #endif
 g_return_if_fail(priv->event_thread == NULL);
@@ -737,7 +733,7 @@ static void 
spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
 /* -- */
 /* gudev / libusb Helper functions*/
 
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 static gboolean spice_usb_device_manager_get_udev_bus_n_address(
 SpiceUsbDeviceManager *manager, GUdevDevice *udev,
 int *bus, int *address)
@@ -927,7 +923,7 @@ spice_usb_device_manager_device_match(SpiceUsbDeviceManager 
*self, SpiceUsbDevic
 spice_usb_device_get_devaddr(device) == address);
 }
 
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 static gboolean
 spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, 
libusb_device *libdev,
   const int bus, const int address)
@@ -1027,7 +1023,7 @@ static void 
spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self,
 spice_usb_device_unref(device);
 }
 
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
   GUdevDevice*udev)
 {
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 04/13] usb-redir: do not add device if one with the same bus:addr exists

2019-03-18 Thread Yuri Benditovich
In initial device enumeration hotplug notification can be
called twice with the same libusb device. For details, see
http://libusb.sourceforge.net/api-1.0/group__libusb__hotplug.html#ga00e0c69ddf1fb1b6774dc918192e8dc7
Filter out devices that already present in the list.
Remove indentical call in spice_usb_device_manager_add_udev,
which add devices under Windows.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index debba4d..a9f19bf 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -962,6 +962,17 @@ static void 
spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
 if (desc.bDeviceClass == LIBUSB_CLASS_HUB)
 return;
 
+if (spice_usb_device_manager_find_device(self,
+libusb_get_bus_number(libdev),
+libusb_get_device_address(libdev))) {
+SPICE_DEBUG("device not added %d:%d %04x:%04x",
+libusb_get_bus_number(libdev),
+libusb_get_device_address(libdev),
+desc.idVendor,
+desc.idProduct);
+return;
+}
+
 device = (SpiceUsbDevice*)spice_usb_device_new(libdev);
 if (!device)
 return;
@@ -1025,7 +1036,6 @@ static void 
spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
 {
 SpiceUsbDeviceManagerPrivate *priv = self->priv;
 libusb_device *libdev = NULL, **dev_list = NULL;
-SpiceUsbDevice *device;
 const gchar *devtype;
 int i, bus, address;
 
@@ -1039,16 +1049,6 @@ static void 
spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
 return;
 }
 
-device = spice_usb_device_manager_find_device(self, bus, address);
-if (device) {
-SPICE_DEBUG("USB device 0x%04x:0x%04x at %d.%d already exists, 
ignored",
-spice_usb_device_get_vid(device),
-spice_usb_device_get_pid(device),
-spice_usb_device_get_busnum(device),
-spice_usb_device_get_devaddr(device));
-return;
-}
-
 if (priv->coldplug_list)
 dev_list = priv->coldplug_list;
 else
-- 
2.17.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice-gtk v2 11/13] win-usb-dev: report libusb_device via signal

2019-03-18 Thread Yuri Benditovich
Change the signal to report libusb_device instead of GUdevDevice
object. This simplifies the callback procedure in usb-dev-manager
and avoids additional reenumeration immediately after reenumeration
that already done by win-usb-dev.

Signed-off-by: Yuri Benditovich 
---
 src/usb-device-manager.c | 81 +++-
 src/win-usb-dev.c| 55 +--
 src/win-usb-dev.h|  2 +-
 3 files changed, 9 insertions(+), 129 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index c99d359..a163501 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -153,11 +153,9 @@ static void channel_event(SpiceChannel *channel, 
SpiceChannelEvent event,
   gpointer user_data);
 #ifdef G_OS_WIN32
 static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
-   GUdevDevice *udevice,
+   libusb_device   *udevice,
gboolean add,
gpointer user_data);
-static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
-  GUdevDevice*udev);
 #else
 static int spice_usb_device_manager_hotplug_cb(libusb_context   *ctx,
libusb_device*device,
@@ -720,28 +718,6 @@ static void 
spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
 /* -- */
 /* gudev / libusb Helper functions*/
 
-#ifdef G_OS_WIN32
-static gboolean spice_usb_device_manager_get_udev_bus_n_address(
-SpiceUsbDeviceManager *manager, GUdevDevice *udev,
-int *bus, int *address)
-{
-const gchar *bus_str, *address_str;
-
-*bus = *address = 0;
-
-   /* Linux or UsbDk backend on Windows*/
-bus_str = g_udev_device_get_property(udev, "BUSNUM");
-address_str = g_udev_device_get_property(udev, "DEVNUM");
-
-if (bus_str)
-*bus = atoi(bus_str);
-if (address_str)
-*address = atoi(address_str);
-
-return *bus && *address;
-}
-#endif
-
 static gboolean spice_usb_device_manager_get_device_descriptor(
 libusb_device *libdev,
 struct libusb_device_descriptor *desc)
@@ -1022,64 +998,19 @@ static void 
spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self,
 }
 
 #ifdef G_OS_WIN32
-static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
-  GUdevDevice*udev)
-{
-SpiceUsbDeviceManagerPrivate *priv = self->priv;
-libusb_device *libdev = NULL, **dev_list = NULL;
-const gchar *devtype;
-int i, bus, address;
-
-devtype = g_udev_device_get_property(udev, "DEVTYPE");
-/* Check if this is a usb device (and not an interface) */
-if (!devtype || strcmp(devtype, "usb_device"))
-return;
-
-if (!spice_usb_device_manager_get_udev_bus_n_address(self, udev, , 
)) {
-g_warning("USB device without bus number or device address");
-return;
-}
-
-libusb_get_device_list(priv->context, _list);
-
-for (i = 0; dev_list && dev_list[i]; i++) {
-if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, 
address)) {
-libdev = dev_list[i];
-break;
-}
-}
-
-if (libdev)
-spice_usb_device_manager_add_dev(self, libdev);
-else
-g_warning("Could not find USB device to add " DEV_ID_FMT,
-  (guint) bus, (guint) address);
-
-libusb_free_device_list(dev_list, 1);
-}
-
-static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager  *self,
- GUdevDevice*udev)
-{
-int bus, address;
-
-if (!spice_usb_device_manager_get_udev_bus_n_address(self, udev, , 
))
-return;
-
-spice_usb_device_manager_remove_dev(self, bus, address);
-}
-
 static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
-   GUdevDevice *udevice,
+   libusb_device   *dev,
gboolean add,
gpointer user_data)
 {
 SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
 
 if (add)
-spice_usb_device_manager_add_udev(self, udevice);
+spice_usb_device_manager_add_dev(self, dev);
 else
-spice_usb_device_manager_remove_udev(self, udevice);
+spice_usb_device_manager_remove_dev(self,
+libusb_get_bus_number(dev),
+libusb_get_device_address(dev));
 }
 #else
 struct 

Re: [Spice-devel] [spice-gtk v3] smartcard: Warn if multiple readers are detected

2019-03-18 Thread Marc-André Lureau
On Fri, Feb 22, 2019 at 11:40 AM Christophe Fergeau  wrote:
>
> spice-server does not deal properly with multiple smartcard readers,
> only the first one will be working. Add a warning when this happens to
> make it easier to diagnose such issues.
>
> Signed-off-by: Christophe Fergeau 

ack

> ---
>  src/smartcard-manager.c | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/src/smartcard-manager.c b/src/smartcard-manager.c
> index ceecfdc7..35bb2757 100644
> --- a/src/smartcard-manager.c
> +++ b/src/smartcard-manager.c
> @@ -389,6 +389,24 @@ typedef struct {
>  GError *err;
>  } SmartcardManagerInitArgs;
>
> +
> +static void smartcard_reader_free(gpointer data)
> +{
> +g_boxed_free(SPICE_TYPE_SMARTCARD_READER, data);
> +}
> +
> +/* spice-server only supports one smartcard reader being in use */
> +static void smartcard_check_reader_count(void)
> +{
> +GList *readers;
> +
> +readers = 
> spice_smartcard_manager_get_readers(spice_smartcard_manager_get());
> +if (g_list_length(readers) > 1) {
> +g_warning("Multiple smartcard readers are plugged in, only the first 
> one will be shared with the VM");
> +}
> +g_list_free_full(readers, smartcard_reader_free);
> +}
> +
>  static gboolean smartcard_manager_init(SmartcardManagerInitArgs *args)
>  {
>  gchar *emul_args = NULL;
> @@ -442,6 +460,7 @@ init:
>  "Failed to initialize smartcard");
>  goto end;
>  }
> +smartcard_check_reader_count();
>
>  retval = TRUE;
>
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v3] smartcard: Warn if multiple readers are detected

2019-03-18 Thread Christophe Fergeau
Ping? Or should I just go with one of the earlier versions?


On Fri, Feb 22, 2019 at 11:40:32AM +0100, Christophe Fergeau wrote:
> spice-server does not deal properly with multiple smartcard readers,
> only the first one will be working. Add a warning when this happens to
> make it easier to diagnose such issues.
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  src/smartcard-manager.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/smartcard-manager.c b/src/smartcard-manager.c
> index ceecfdc7..35bb2757 100644
> --- a/src/smartcard-manager.c
> +++ b/src/smartcard-manager.c
> @@ -389,6 +389,24 @@ typedef struct {
>  GError *err;
>  } SmartcardManagerInitArgs;
>  
> +
> +static void smartcard_reader_free(gpointer data)
> +{
> +g_boxed_free(SPICE_TYPE_SMARTCARD_READER, data);
> +}
> +
> +/* spice-server only supports one smartcard reader being in use */
> +static void smartcard_check_reader_count(void)
> +{
> +GList *readers;
> +
> +readers = 
> spice_smartcard_manager_get_readers(spice_smartcard_manager_get());
> +if (g_list_length(readers) > 1) {
> +g_warning("Multiple smartcard readers are plugged in, only the first 
> one will be shared with the VM");
> +}
> +g_list_free_full(readers, smartcard_reader_free);
> +}
> +
>  static gboolean smartcard_manager_init(SmartcardManagerInitArgs *args)
>  {
>  gchar *emul_args = NULL;
> @@ -442,6 +460,7 @@ init:
>  "Failed to initialize smartcard");
>  goto end;
>  }
> +smartcard_check_reader_count();
>  
>  retval = TRUE;
>  
> -- 
> 2.20.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk v2 2/2] win-usb-dev: remove ifdef for libusb on 1.0.13

2019-03-18 Thread Christophe Fergeau
On Fri, Feb 22, 2019 at 05:42:22PM +, Victor Toso wrote:
> From: Victor Toso 
> 
> We already require libusb 1.0.16 or above since 8269a5be62f4ce1
> (build-sys: drop support for libusb < 1.0.16)
> 
> Signed-off-by: Victor Toso 
> ---
>  src/win-usb-dev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> index 327976d..b614af0 100644
> --- a/src/win-usb-dev.c
> +++ b/src/win-usb-dev.c
> @@ -570,9 +570,7 @@ static gboolean g_udev_skip_search(GUdevDevice *udev)
>  g_return_val_if_fail(udevinfo != NULL, FALSE);
>  
>  skip = ((udevinfo->addr == 0xff) ||  /* root hub (HCD) */
> -#if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x01FF)
> -(udevinfo->addr == 1) || /* root hub addr for libusbx >= 1.0.13 
> */
> -#endif
> +(udevinfo->addr == 1) || /* root hub addr since libusbx 1.0.13 */

I'd drop "since libusbx 1.0.13" from the comment.
Apart from that, series looks good to me.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [spice-gtk 10/13] win-usb-dev: maintain list of libusb devices

2019-03-18 Thread Yuri Benditovich
On Tue, Mar 12, 2019 at 7:22 PM Christophe Fergeau
 wrote:>> On Sun, Mar 10, 2019 at 04:46:09PM
+0200, Yuri Benditovich wrote:
> > Change internal device list to maintain libusb devices
> > instead of GUdevDevice objects. Create temporary
> > GUdevDevice object only for indication to usb-dev-manager.
> >
> > Signed-off-by: Yuri Benditovich 
> > ---
> >  src/win-usb-dev.c | 80 ++-
> >  1 file changed, 51 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> > index 1ab704d..5d878ea 100644
> > --- a/src/win-usb-dev.c
> > +++ b/src/win-usb-dev.c
> > @@ -95,7 +95,7 @@ static void g_udev_device_print_list(GList *l, const 
> > gchar *msg);
> >  #else
> >  static void g_udev_device_print_list(GList *l, const gchar *msg) {}
> >  #endif
> > -static void g_udev_device_print(GUdevDevice *udev, const gchar *msg);
> > +static void g_udev_device_print(libusb_device *dev, const gchar *msg);
> >
> >  static gboolean g_udev_skip_search(libusb_device *dev);
> >
> > @@ -129,8 +129,6 @@ g_udev_client_list_devices(GUdevClient *self, GList 
> > **devs,
> >  gssize rc;
> >  libusb_device **lusb_list, **dev;
> >  GUdevClientPrivate *priv;
> > -GUdevDeviceInfo *udevinfo;
> > -GUdevDevice *udevice;
> >  ssize_t n;
> >
> >  g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1);
> > @@ -155,10 +153,7 @@ g_udev_client_list_devices(GUdevClient *self, GList 
> > **devs,
> >  if (g_udev_skip_search(*dev)) {
> >  continue;
> >  }
> > -udevinfo = g_new0(GUdevDeviceInfo, 1);
> > -get_usb_dev_info(*dev, udevinfo);
> > -udevice = g_udev_device_new(udevinfo);
> > -*devs = g_list_prepend(*devs, udevice);
> > +*devs = g_list_prepend(*devs, libusb_ref_device(*dev));
> >  n++;
> >  }
> >  libusb_free_device_list(lusb_list, 1);
> > @@ -166,11 +161,17 @@ g_udev_client_list_devices(GUdevClient *self, GList 
> > **devs,
> >  return n;
> >  }
> >
> > +static void unreference_libusb_device(gpointer data)
> > +{
> > +libusb_unref_device((libusb_device *)data);
> > +}
> > +
> >  static void g_udev_client_free_device_list(GList **devs)
> >  {
> >  g_return_if_fail(devs != NULL);
> >  if (*devs) {
> > -g_list_free_full(*devs, g_object_unref);
> > +/* avoiding casting of imported procedure to GDestroyNotify */
> > +g_list_free_full(*devs, unreference_libusb_device);
>
> Since all unreference_libusb_device is doing is blindly casting a void *
> to libusb_device *, I'd just use a cast to GDestroyNotify here rather
> than introduce an intermediate method. If you prefer to keep that
> intermediate helper, please remove the commit which I don't think is
> needed.
>
> >  *devs = NULL;
> >  }
> >  }
> > @@ -246,9 +247,22 @@ static void 
> > g_udev_client_initable_iface_init(GInitableIface *iface)
> >  iface->init = g_udev_client_initable_init;
> >  }
> >
> > +static void g_udev_notify_device(GUdevClient *self, libusb_device *dev, 
> > int add)
> > +{
> > +GUdevDeviceInfo *udevinfo;
> > +GUdevDevice *udevice;
> > +udevinfo = g_new0(GUdevDeviceInfo, 1);
> > +if (get_usb_dev_info(dev, udevinfo)) {
> > +udevice = g_udev_device_new(udevinfo);
> > +g_signal_emit(self, signals[UEVENT_SIGNAL], 0, udevice, add);
> > +} else {
> > +g_free(udevinfo);
> > +}
> > +}
> > +
> >  static void report_one_device(gpointer data, gpointer self)
> >  {
> > -g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE);
> > +g_udev_notify_device(self, data, TRUE);
> >  }
> >
> >  void g_udev_client_report_devices(GUdevClient *self)
> > @@ -388,18 +402,20 @@ static gboolean get_usb_dev_info(libusb_device *dev, 
> > GUdevDeviceInfo *udevinfo)
> >  }
> >
> >  /* comparing bus:addr and vid:pid */
> > -static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
> > +static gint compare_libusb_devices(gconstpointer a, gconstpointer b)
> >  {
> > -GUdevDeviceInfo *ai, *bi;
> > +libusb_device *a_dev = (libusb_device *)a;
> > +libusb_device *b_dev = (libusb_device *)b;
> > +struct libusb_device_descriptor a_desc, b_desc;
> >  gboolean same_bus, same_addr, same_vid, same_pid;
> >
> > -ai = G_UDEV_DEVICE(a)->priv->udevinfo;
> > -bi = G_UDEV_DEVICE(b)->priv->udevinfo;
> > +libusb_get_device_descriptor(a_dev, _desc);
> > +libusb_get_device_descriptor(b_dev, _desc);
> >
> > -same_bus = (ai->bus == bi->bus);
> > -same_addr = (ai->addr == bi->addr);
> > -same_vid = (ai->vid == bi->vid);
> > -same_pid = (ai->pid == bi->pid);
> > +same_bus = libusb_get_bus_number(a_dev) == 
> > libusb_get_bus_number(b_dev);
> > +same_addr = libusb_get_device_address(a_dev) == 
> > libusb_get_device_address(b_dev);
> > +same_vid = (a_desc.idVendor == b_desc.idVendor);
> > +same_pid = (a_desc.idProduct == b_desc.idProduct);
> >
> >  return (same_bus && 

Re: [Spice-devel] [PATCH spice-common v3 0/5] Generate C declarations automatically

2019-03-18 Thread Jonathon Jongsma
On Mon, 2019-03-18 at 09:53 -0400, Frediano Ziglio wrote:
> > On Mon, Mar 11, 2019 at 12:42:10PM -0400, Frediano Ziglio wrote:
> > > > 
> > > > Series looks good to me,
> > > > 
> > > > Reviewed-by: Christophe Fergeau 
> > > > 
> > > 
> > > Why not ack? Not good enough? Not tested? Missing something?
> > 
> > libvirt/qemu use of Reviewed-by/Acked-by confuses me, and to me
> > they are
> > more or less equivalent.
> > 
> > Christophe
> > 
> 
> As long as we are coherent is fine. But to me looks like some
> (Jonathon?)
> use Reviewed-by to mark as "the patch was fully reviewed but some
> thinks
> need updates".
> 
> Frediano

I usually use Reviewed-by to indicate that I looked at it, but am not
necessarily comfortable Acking it yet. But you're right that there's
not really any agreement on what this means. Would be good to be
consistent.

Jonathon

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 1/2] docs: Fix typo

2019-03-18 Thread Douglas Paul
On Mon, Mar 18, 2019 at 09:44:59AM -0400, Frediano Ziglio wrote:
> > > -Lots of code run in a single thread.
> > > +Lots of code runs in a single thread.
> > 
> > "Lots of" is plural, isn't it? Maybe "A lot of code runs .."
> > 
> > Christophe
> > 
> 
> You are right. Which one is better?

FWIW, actually 'lots of' and 'a lot of' are equivalent, and the verb
will be plural or singular depending on if the noun is countable (and
will be plural) or is uncountable (and will be singular).

So "Lots of people run" but "Lots of code runs" because code is
uncountable.

-- 
Douglas Paul


___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-common v3 0/5] Generate C declarations automatically

2019-03-18 Thread Frediano Ziglio
> On Mon, Mar 11, 2019 at 12:42:10PM -0400, Frediano Ziglio wrote:
> > > 
> > > Series looks good to me,
> > > 
> > > Reviewed-by: Christophe Fergeau 
> > > 
> > 
> > Why not ack? Not good enough? Not tested? Missing something?
> 
> libvirt/qemu use of Reviewed-by/Acked-by confuses me, and to me they are
> more or less equivalent.
> 
> Christophe
> 

As long as we are coherent is fine. But to me looks like some (Jonathon?)
use Reviewed-by to mark as "the patch was fully reviewed but some thinks
need updates".

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 1/2] docs: Fix typo

2019-03-18 Thread Frediano Ziglio
> 
> On Mon, Mar 11, 2019 at 02:03:32PM +, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  docs/spice_threading_model.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/docs/spice_threading_model.txt
> > b/docs/spice_threading_model.txt
> > index df4922168..9351141c8 100644
> > --- a/docs/spice_threading_model.txt
> > +++ b/docs/spice_threading_model.txt
> > @@ -1,7 +1,7 @@
> >  How does SPICE handle threading
> >  ---
> >  
> > -Lots of code run in a single thread.
> > +Lots of code runs in a single thread.
> 
> "Lots of" is plural, isn't it? Maybe "A lot of code runs .."
> 
> Christophe
> 

You are right. Which one is better?

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-protocol] protocol: Generate enums.h again to remove old protocol definitions

2019-03-18 Thread Frediano Ziglio
> 
> This is breaking spice-protocol API even if this should not impact
> current code. Do we want to advertise this somehow? Switch to
> spice-protocol 0.14.x?
> 

Fine for me both 0.14.0 or 0.12.16 (the future, current HEAD version)

> On Fri, Feb 22, 2019 at 12:16:52PM +, Frediano Ziglio wrote:
> > spice.proto was updated to remove really old definitions,
> > update enums.h accordingly.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  spice/enums.h | 57 ---
> >  1 file changed, 57 deletions(-)
> > 
> > diff --git a/spice/enums.h b/spice/enums.h
> > index 34e84c3..172cc4d 100644
> > --- a/spice/enums.h
> > +++ b/spice/enums.h
> > @@ -119,21 +119,6 @@ typedef enum SpiceMouseMode {
> >  SPICE_MOUSE_MODE_MASK = 0x3
> >  } SpiceMouseMode;
> >  
> > -typedef enum SpicePubkeyType {
> > -SPICE_PUBKEY_TYPE_INVALID,
> > -SPICE_PUBKEY_TYPE_RSA,
> > -SPICE_PUBKEY_TYPE_RSA2,
> > -SPICE_PUBKEY_TYPE_DSA,
> > -SPICE_PUBKEY_TYPE_DSA1,
> > -SPICE_PUBKEY_TYPE_DSA2,
> > -SPICE_PUBKEY_TYPE_DSA3,
> > -SPICE_PUBKEY_TYPE_DSA4,
> > -SPICE_PUBKEY_TYPE_DH,
> > -SPICE_PUBKEY_TYPE_EC,
> > -
> > -SPICE_PUBKEY_TYPE_ENUM_END
> > -} SpicePubkeyType;
> > -
> >  typedef enum SpiceDataCompressionType {
> >  SPICE_DATA_COMPRESSION_TYPE_NONE,
> >  SPICE_DATA_COMPRESSION_TYPE_LZ4,
> > @@ -397,21 +382,6 @@ typedef enum SpiceAudioFmt {
> >  SPICE_AUDIO_FMT_ENUM_END
> >  } SpiceAudioFmt;
> >  
> > -typedef enum SpiceTunnelServiceType {
> > -SPICE_TUNNEL_SERVICE_TYPE_INVALID,
> > -SPICE_TUNNEL_SERVICE_TYPE_GENERIC,
> > -SPICE_TUNNEL_SERVICE_TYPE_IPP,
> > -
> > -SPICE_TUNNEL_SERVICE_TYPE_ENUM_END
> > -} SpiceTunnelServiceType;
> > -
> > -typedef enum SpiceTunnelIpType {
> > -SPICE_TUNNEL_IP_TYPE_INVALID,
> > -SPICE_TUNNEL_IP_TYPE_IPv4,
> > -
> > -SPICE_TUNNEL_IP_TYPE_ENUM_END
> > -} SpiceTunnelIpType;
> > -
> >  typedef enum SpiceVscMessageType {
> >  SPICE_VSC_MESSAGE_TYPE_Init = 1,
> >  SPICE_VSC_MESSAGE_TYPE_Error,
> > @@ -613,33 +583,6 @@ enum {
> >  SPICE_MSGC_END_RECORD
> >  };
> >  
> > -enum {
> > -SPICE_MSG_TUNNEL_INIT = 101,
> > -SPICE_MSG_TUNNEL_SERVICE_IP_MAP,
> > -SPICE_MSG_TUNNEL_SOCKET_OPEN,
> > -SPICE_MSG_TUNNEL_SOCKET_FIN,
> > -SPICE_MSG_TUNNEL_SOCKET_CLOSE,
> > -SPICE_MSG_TUNNEL_SOCKET_DATA,
> > -SPICE_MSG_TUNNEL_SOCKET_CLOSED_ACK,
> > -SPICE_MSG_TUNNEL_SOCKET_TOKEN,
> > -
> > -SPICE_MSG_END_TUNNEL
> > -};
> > -
> > -enum {
> > -SPICE_MSGC_TUNNEL_SERVICE_ADD = 101,
> > -SPICE_MSGC_TUNNEL_SERVICE_REMOVE,
> > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_ACK,
> > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_NACK,
> > -SPICE_MSGC_TUNNEL_SOCKET_FIN,
> > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED,
> > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED_ACK,
> > -SPICE_MSGC_TUNNEL_SOCKET_DATA,
> > -SPICE_MSGC_TUNNEL_SOCKET_TOKEN,
> > -
> > -SPICE_MSGC_END_TUNNEL
> > -};
> > -
> >  enum {
> >  SPICE_MSG_SMARTCARD_DATA = 101,
> >  

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-common v3 0/5] Generate C declarations automatically

2019-03-18 Thread Christophe Fergeau
On Mon, Mar 11, 2019 at 12:42:10PM -0400, Frediano Ziglio wrote:
> > 
> > Series looks good to me,
> > 
> > Reviewed-by: Christophe Fergeau 
> > 
> 
> Why not ack? Not good enough? Not tested? Missing something?

libvirt/qemu use of Reviewed-by/Acked-by confuses me, and to me they are
more or less equivalent.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-protocol] protocol: Generate enums.h again to remove old protocol definitions

2019-03-18 Thread Christophe Fergeau
This is breaking spice-protocol API even if this should not impact
current code. Do we want to advertise this somehow? Switch to
spice-protocol 0.14.x?

On Fri, Feb 22, 2019 at 12:16:52PM +, Frediano Ziglio wrote:
> spice.proto was updated to remove really old definitions,
> update enums.h accordingly.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  spice/enums.h | 57 ---
>  1 file changed, 57 deletions(-)
> 
> diff --git a/spice/enums.h b/spice/enums.h
> index 34e84c3..172cc4d 100644
> --- a/spice/enums.h
> +++ b/spice/enums.h
> @@ -119,21 +119,6 @@ typedef enum SpiceMouseMode {
>  SPICE_MOUSE_MODE_MASK = 0x3
>  } SpiceMouseMode;
>  
> -typedef enum SpicePubkeyType {
> -SPICE_PUBKEY_TYPE_INVALID,
> -SPICE_PUBKEY_TYPE_RSA,
> -SPICE_PUBKEY_TYPE_RSA2,
> -SPICE_PUBKEY_TYPE_DSA,
> -SPICE_PUBKEY_TYPE_DSA1,
> -SPICE_PUBKEY_TYPE_DSA2,
> -SPICE_PUBKEY_TYPE_DSA3,
> -SPICE_PUBKEY_TYPE_DSA4,
> -SPICE_PUBKEY_TYPE_DH,
> -SPICE_PUBKEY_TYPE_EC,
> -
> -SPICE_PUBKEY_TYPE_ENUM_END
> -} SpicePubkeyType;
> -
>  typedef enum SpiceDataCompressionType {
>  SPICE_DATA_COMPRESSION_TYPE_NONE,
>  SPICE_DATA_COMPRESSION_TYPE_LZ4,
> @@ -397,21 +382,6 @@ typedef enum SpiceAudioFmt {
>  SPICE_AUDIO_FMT_ENUM_END
>  } SpiceAudioFmt;
>  
> -typedef enum SpiceTunnelServiceType {
> -SPICE_TUNNEL_SERVICE_TYPE_INVALID,
> -SPICE_TUNNEL_SERVICE_TYPE_GENERIC,
> -SPICE_TUNNEL_SERVICE_TYPE_IPP,
> -
> -SPICE_TUNNEL_SERVICE_TYPE_ENUM_END
> -} SpiceTunnelServiceType;
> -
> -typedef enum SpiceTunnelIpType {
> -SPICE_TUNNEL_IP_TYPE_INVALID,
> -SPICE_TUNNEL_IP_TYPE_IPv4,
> -
> -SPICE_TUNNEL_IP_TYPE_ENUM_END
> -} SpiceTunnelIpType;
> -
>  typedef enum SpiceVscMessageType {
>  SPICE_VSC_MESSAGE_TYPE_Init = 1,
>  SPICE_VSC_MESSAGE_TYPE_Error,
> @@ -613,33 +583,6 @@ enum {
>  SPICE_MSGC_END_RECORD
>  };
>  
> -enum {
> -SPICE_MSG_TUNNEL_INIT = 101,
> -SPICE_MSG_TUNNEL_SERVICE_IP_MAP,
> -SPICE_MSG_TUNNEL_SOCKET_OPEN,
> -SPICE_MSG_TUNNEL_SOCKET_FIN,
> -SPICE_MSG_TUNNEL_SOCKET_CLOSE,
> -SPICE_MSG_TUNNEL_SOCKET_DATA,
> -SPICE_MSG_TUNNEL_SOCKET_CLOSED_ACK,
> -SPICE_MSG_TUNNEL_SOCKET_TOKEN,
> -
> -SPICE_MSG_END_TUNNEL
> -};
> -
> -enum {
> -SPICE_MSGC_TUNNEL_SERVICE_ADD = 101,
> -SPICE_MSGC_TUNNEL_SERVICE_REMOVE,
> -SPICE_MSGC_TUNNEL_SOCKET_OPEN_ACK,
> -SPICE_MSGC_TUNNEL_SOCKET_OPEN_NACK,
> -SPICE_MSGC_TUNNEL_SOCKET_FIN,
> -SPICE_MSGC_TUNNEL_SOCKET_CLOSED,
> -SPICE_MSGC_TUNNEL_SOCKET_CLOSED_ACK,
> -SPICE_MSGC_TUNNEL_SOCKET_DATA,
> -SPICE_MSGC_TUNNEL_SOCKET_TOKEN,
> -
> -SPICE_MSGC_END_TUNNEL
> -};
> -
>  enum {
>  SPICE_MSG_SMARTCARD_DATA = 101,
>  
> -- 
> 2.20.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 2/2] docs: Add some notes on event scheduling and threading

2019-03-18 Thread Christophe Fergeau
On Mon, Mar 11, 2019 at 02:03:33PM +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  docs/spice_threading_model.txt | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt
> index 9351141c8..25a3a030c 100644
> --- a/docs/spice_threading_model.txt
> +++ b/docs/spice_threading_model.txt
> @@ -39,6 +39,14 @@ connect, disconnect and migrate. Connect and migrate are 
> asynchronous (the job
>  is done while the current thread is doing something else) while disconnect is
>  synchronous (the main thread will wait for termination).
>  
> +One aspect to take into consideration is the event scheduling. SPICE uses 
> some
> +`SpiceCoreInterface` to handle events. As the events will be handled from a
> +thread based on the core interface you have to use the correct core. Each
> +channel has an associated core interface which can be retrieved using
> +`red_channel_get_core_interface`. There's also a main core interface you can 
> get
> +using `reds_get_core_interface`. `reds_core_timer_*` and `reds_core_watch_*`
> +functions use the main core interface.

Do we need a few words as to when to use the main core interface?
Apart from this, looks good to me.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 1/2] docs: Fix typo

2019-03-18 Thread Christophe Fergeau
On Mon, Mar 11, 2019 at 02:03:32PM +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  docs/spice_threading_model.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt
> index df4922168..9351141c8 100644
> --- a/docs/spice_threading_model.txt
> +++ b/docs/spice_threading_model.txt
> @@ -1,7 +1,7 @@
>  How does SPICE handle threading
>  ---
>  
> -Lots of code run in a single thread.
> +Lots of code runs in a single thread.

"Lots of" is plural, isn't it? Maybe "A lot of code runs .."

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 1/2] Update spice-common submodule

2019-03-18 Thread Christophe Fergeau
For the series:
Acked-by: Christophe Fergeau 

On Fri, Mar 08, 2019 at 12:25:48PM +, Frediano Ziglio wrote:
> This brings in the following changes:
> 
> Frediano Ziglio (11):
>   codegen: Document ptr_array attribute
>   codegen: Use a better type for pointer converted to integer
>   codegen: Reduce indentation
>   codegen: Fix c_type result for TypeAlias
>   codegen: Check wrong attribute
>   codegen: Add a test for attribute combination
>   mem: Fix compile error if alignment-checks option is used
>   log: Remove useless includes
>   proto: Remove obsolete TunnelChannel
>   protocol: Add a dummy TunnelChannel
>   messages: Remove fields not used by the protocol
> 
> Marc-André Lureau (1):
>   docs: add spice URI scheme
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  subprojects/spice-common | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/subprojects/spice-common b/subprojects/spice-common
> index 02530a80d..92d5dfd4b 16
> --- a/subprojects/spice-common
> +++ b/subprojects/spice-common
> @@ -1 +1 @@
> -Subproject commit 02530a80dfa45c936215c47b8e3aa56720eb46b8
> +Subproject commit 92d5dfd4bfa7ae4857e96504a6f14c336ed85338
> -- 
> 2.20.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] syntax-check: Add missing contributors names to AUTHORS

2019-03-18 Thread Christophe Fergeau

Acked-by: Christophe Fergeau 

On Mon, Mar 18, 2019 at 10:57:41AM +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  AUTHORS | 1 +
>  1 file changed, 1 insertion(+)
> 
> This fixed "make syntax-check"
> 
> diff --git a/AUTHORS b/AUTHORS
> index 12158e60..a9b9ea79 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -66,5 +66,6 @@ Patches also contributed by
>  Wang Qiang 
>  Yann E. MORIN 
>  Zeeshan Ali (Khattak) 
> +Douglas Paul 
>  
> send patches to get your name here...
> -- 
> 2.20.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Build error (possible mirroring issue?)

2019-03-18 Thread Frediano Ziglio
> 
> Hey team,
> 
> 
> I get a build failure that looks like this:
> 
> char-device.c:906:52: error: too many arguments to function call, expected
> single argument 'm', have 2 arguments
> m2 = spice_marshaller_get_ptr_submarshaller(m, 0);
>  ~~^
> /Users/ddd/Work/spice/common/common/marshaller.h:56:1: note:
> 'spice_marshaller_get_ptr_submarshaller' declared here
> SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m);
> ^
> 1 error generated.
> 
> Indeed, the prototype was changed in spice-common by
> 
> commit 302e30ff43401d9b1e7043a5e5c4f186ca997f66
> Author: Frediano Ziglio 
> Date:   Wed Feb 27 08:55:56 2019 +
> 
> codegen: Remove support for --ptrsize
>  
> This is talking master for the server component from from
> https://gitlab.freedesktop.org/spice/spice.git.
> 
> Is this a git mirroring issue, or a missing commit on the server side?
> 
> 
> Thanks
> Christophe

I think the issue is that your spice-server is using the wrong spice-common
commit, just do a "git submodule update --init --recursive".

There's a patch pending to review to "fix" this issue.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] Build error (possible mirroring issue?)

2019-03-18 Thread Christophe de Dinechin
Hey team,


I get a build failure that looks like this:

char-device.c:906:52: error: too many arguments to function call, expected 
single argument 'm', have 2 arguments
m2 = spice_marshaller_get_ptr_submarshaller(m, 0);
 ~~^
/Users/ddd/Work/spice/common/common/marshaller.h:56:1: note: 
'spice_marshaller_get_ptr_submarshaller' declared here
SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m);
^
1 error generated.

Indeed, the prototype was changed in spice-common by

commit 302e30ff43401d9b1e7043a5e5c4f186ca997f66
Author: Frediano Ziglio 
Date:   Wed Feb 27 08:55:56 2019 +

codegen: Remove support for --ptrsize  
 
This is talking master for the server component from from 
https://gitlab.freedesktop.org/spice/spice.git.

Is this a git mirroring issue, or a missing commit on the server side?


Thanks
Christophe
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server] syntax-check: Add missing contributors names to AUTHORS

2019-03-18 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio 
---
 AUTHORS | 1 +
 1 file changed, 1 insertion(+)

This fixed "make syntax-check"

diff --git a/AUTHORS b/AUTHORS
index 12158e60..a9b9ea79 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -66,5 +66,6 @@ Patches also contributed by
 Wang Qiang 
 Yann E. MORIN 
 Zeeshan Ali (Khattak) 
+Douglas Paul 
 
send patches to get your name here...
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel

2019-03-18 Thread Frediano Ziglio
> On 3/18/19 12:12 PM, Frediano Ziglio wrote:
> >> Hi,
> >>
> >> On 3/15/19 11:27 AM, Frediano Ziglio wrote:
> >>> To avoid possibly regression check it only if extra checks are
> >>> enabled.
> >>
> >> Currently there's already a "channel duplication" warning upon connection
> >> but won't hurt to have this extra check and emit an error.
> >>
> > I cannot find it. This patch is for spice-server.
> 
> 
> Oh, sorry, "duplicate channel" on red_client_add_channel
> 

These functions are checking RedChannelClient, not RedChannel.
Basically they check if client is connecting twice to the same
channel.

> 
> >
> >>> This allowed to check previous "Move channel registration to constructed
> >>
> >> This allow/s? i don't get this sentence, it just avoids duplicate call.
> >> I think
> >> i'd not even mention the previous commit
> >>
> > No, it will also exit the program if this condition is met.
> 
> 
> Yes, i got it, i meant that such patch would have make sense also before
> the "Move channel reg.." commit,
> so not necessarily need to mention it.
> 

Yes, I suppose would have made sense even before.
But that's the reason why I added it and how I used it.

> 
> 
> >
> >> Snir.
> >>
> >>> vfunc" commit.
> >>>
> >>> Signed-off-by: Frediano Ziglio 
> >>> ---
> >>>server/reds.c | 5 +
> >>>1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/server/reds.c b/server/reds.c
> >>> index 429f8142..e182eba7 100644
> >>> --- a/server/reds.c
> >>> +++ b/server/reds.c
> >>> @@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds,
> >>> RedStatCounter *counter)
> >>>void reds_register_channel(RedsState *reds, RedChannel *channel)
> >>>{
> >>>spice_assert(reds);
> >>> +if (spice_extra_checks) {
> >>> +uint32_t this_type, this_id;
> >>> +g_object_get(channel, "channel-type", _type, "id",
> >>> _id,
> >>> NULL);
> >>> +spice_assert(reds_find_channel(reds, this_type, this_id) ==
> >>> NULL);
> >>> +}
> >>>reds->channels = g_list_prepend(reds->channels, channel);
> >>>// create new channel in the client if possible
> >>>main_channel_registered_new_channel(reds->main_channel, channel);
> > Frediano
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server v4 1/1] video-stream: prevent crash on stream reattach

2019-03-18 Thread Frediano Ziglio
> 
> I experienced some crashes with qemu 3.1.0 compiled with libspice-server
> 0.14.0 on Gentoo.
> 
> The problem reproduced reliably with a guest running Ubuntu 18.04.2 LTS.
> If I connect a viewer at system startup, I would get a crash just after
> the fade-in of the login prompt in GDM.
> 
> Interestingly, I usually was unable to reproduce the issue if I waited
> to connect until after the greeter was fully displayed.
> 
> The patch I used to correct the issue for me applies to the master
> branch cleanly, so I suspect the problem may still exist.
> 
> The only other references to this issue I could find were two abrt
> reports in CentOS:
> https://bugs.centos.org/view.php?id=15171
> https://bugs.centos.org/view.php?id=15441
> 
> I'm not sure if the agent->video_encoder is supposed to be guaranteed to
> exist when this function is called.
> 
> Signed-off-by: Douglas Paul 

Acked-by: Frediano Ziglio 

if the client does not support some encoding it sends back a specific
report and video_encoder is closed. This probably is causing the issue
you reported.

> ---
>  server/video-stream.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/server/video-stream.c b/server/video-stream.c
> index b624093e..19795098 100644
> --- a/server/video-stream.c
> +++ b/server/video-stream.c
> @@ -369,7 +369,9 @@ static void before_reattach_stream(DisplayChannel
> *display,
>  #ifdef STREAM_STATS
>  agent->stats.num_drops_pipe++;
>  #endif
> -
> agent->video_encoder->notify_server_frame_drop(agent->video_encoder);
> +if (agent->video_encoder) {
> +
> agent->video_encoder->notify_server_frame_drop(agent->video_encoder);
> +}
>  }
>  }
>  }

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel

2019-03-18 Thread Snir Sheriber


On 3/18/19 12:12 PM, Frediano Ziglio wrote:

Hi,

On 3/15/19 11:27 AM, Frediano Ziglio wrote:

To avoid possibly regression check it only if extra checks are
enabled.


Currently there's already a "channel duplication" warning upon connection
but won't hurt to have this extra check and emit an error.


I cannot find it. This patch is for spice-server.



Oh, sorry, "duplicate channel" on red_client_add_channel





This allowed to check previous "Move channel registration to constructed


This allow/s? i don't get this sentence, it just avoids duplicate call.
I think
i'd not even mention the previous commit


No, it will also exit the program if this condition is met.



Yes, i got it, i meant that such patch would have make sense also before 
the "Move channel reg.." commit,

so not necessarily need to mention it.






Snir.


vfunc" commit.

Signed-off-by: Frediano Ziglio 
---
   server/reds.c | 5 +
   1 file changed, 5 insertions(+)

diff --git a/server/reds.c b/server/reds.c
index 429f8142..e182eba7 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds,
RedStatCounter *counter)
   void reds_register_channel(RedsState *reds, RedChannel *channel)
   {
   spice_assert(reds);
+if (spice_extra_checks) {
+uint32_t this_type, this_id;
+g_object_get(channel, "channel-type", _type, "id", _id,
NULL);
+spice_assert(reds_find_channel(reds, this_type, this_id) == NULL);
+}
   reds->channels = g_list_prepend(reds->channels, channel);
   // create new channel in the client if possible
   main_channel_registered_new_channel(reds->main_channel, channel);

Frediano

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server 2/2] docs: Add some notes on event scheduling and threading

2019-03-18 Thread Frediano Ziglio
ping

> 
> Signed-off-by: Frediano Ziglio 
> ---
>  docs/spice_threading_model.txt | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt
> index 9351141c8..25a3a030c 100644
> --- a/docs/spice_threading_model.txt
> +++ b/docs/spice_threading_model.txt
> @@ -39,6 +39,14 @@ connect, disconnect and migrate. Connect and migrate are
> asynchronous (the job
>  is done while the current thread is doing something else) while disconnect
>  is
>  synchronous (the main thread will wait for termination).
>  
> +One aspect to take into consideration is the event scheduling. SPICE uses
> some
> +`SpiceCoreInterface` to handle events. As the events will be handled from a
> +thread based on the core interface you have to use the correct core. Each
> +channel has an associated core interface which can be retrieved using
> +`red_channel_get_core_interface`. There's also a main core interface you can
> get
> +using `reds_get_core_interface`. `reds_core_timer_*` and `reds_core_watch_*`
> +functions use the main core interface.
> +
>  Reference counting and ownership
>  
>  ->  pointer
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v2 spice-gtk] Adjust to window scaling

2019-03-18 Thread Frediano Ziglio
> 
> When GDK_SCALE is != 1 and egl is used, the image presented does not
> fit to the window (scale of 2 is often used with hidpi monitors).
> Usually this is not a problem since all components are adjusted by
> gdk/gtk but with egl, pixel-based data is not being scaled. In this
> case window's scale value can be used in order to determine whether
> to use a pixel resource with higher resolution data.
> 
> In order to reproduce the problem set spice with virgl/Intel-vGPU
> and run spice-gtk with GDK_SCALE=2

I still didn't try. So from what I understand the sizes and coordinated
from GTK when GDK_SCALE == 2 (for instance) are half of the pixel sizes,
right? So for instance width == 1024 means 2048 pixels.

> ---
> Changes from v1:
> -commit msg
> -replace var naming (ws with win_scale)
> 
> 
> This patch is kind of RFC, it fixes the issue, but it's a bit hacky
> and specific. I didn't come across other scale issues but it is likely
> that more of these exist and better and generic fix is needed.
> 
> ---
>  src/spice-widget-egl.c  | 15 +--
>  src/spice-widget-priv.h |  1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> index 43fccd7..600c87a 100644
> --- a/src/spice-widget-egl.c
> +++ b/src/spice-widget-egl.c
> @@ -326,6 +326,8 @@ static gboolean spice_widget_init_egl_win(SpiceDisplay
> *display, GdkWindow *win,
>  if (d->egl.surface)
>  return TRUE;
>  
> +d->egl.scale = gdk_window_get_scale_factor(win);
> +
>  #ifdef GDK_WINDOWING_X11
>  if (GDK_IS_X11_WINDOW(win)) {
>  native = (EGLNativeWindowType)GDK_WINDOW_XID(win);

This factor is only read on initialization, what would happen changing
resolution while the client is open or attaching a new HiDPI monitor?

Can't we keep the "win" and avoid caching it? It should not take long
to read the factor.

> @@ -431,15 +433,17 @@ void spice_egl_resize_display(SpiceDisplay *display,
> int w, int h)
>  {
>  SpiceDisplayPrivate *d = display->priv;
>  int prog;
> +gint win_scale;
>  
>  if (!gl_make_current(display, NULL))
>  return;
>  
> +win_scale = d->egl.scale;
>  glGetIntegerv(GL_CURRENT_PROGRAM, );
>  
>  glUseProgram(d->egl.prog);
> -apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1);
> -glViewport(0, 0, w, h);
> +apply_ortho(d->egl.mproj, 0, w * win_scale , 0, h * win_scale, -1, 1);
> +glViewport(0, 0, w * win_scale, h * win_scale);
>  

Why not multiplying w and h by d->egl.scale like in other function?

>  if (d->ready)
>  spice_egl_update_display(display);
> @@ -559,6 +563,13 @@ void spice_egl_update_display(SpiceDisplay *display)
>  
>  spice_display_get_scaling(display, , , , , );
>  
> +// Adjust to gdk scale
> +s *= d->egl.scale;
> +x *= d->egl.scale;
> +y *= d->egl.scale;
> +w *= d->egl.scale;
> +h *= d->egl.scale;
> +
>  glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
>  glClear(GL_COLOR_BUFFER_BIT);
>  
> diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
> index 65eb404..8f110ac 100644
> --- a/src/spice-widget-priv.h
> +++ b/src/spice-widget-priv.h
> @@ -149,6 +149,7 @@ struct _SpiceDisplayPrivate {
>  EGLImageKHR image;
>  gbooleancall_draw_done;
>  SpiceGlScanout  scanout;
> +gintscale;

Why in egl? It's not a property related to egl.

>  } egl;
>  #endif // HAVE_EGL
>  double scroll_delta_y;

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel

2019-03-18 Thread Frediano Ziglio
> 
> Hi,
> 
> On 3/15/19 11:27 AM, Frediano Ziglio wrote:
> > To avoid possibly regression check it only if extra checks are
> > enabled.
> 
> 
> Currently there's already a "channel duplication" warning upon connection
> but won't hurt to have this extra check and emit an error.
> 

I cannot find it. This patch is for spice-server.

> > This allowed to check previous "Move channel registration to constructed
> 
> 
> This allow/s? i don't get this sentence, it just avoids duplicate call.
> I think
> i'd not even mention the previous commit
> 

No, it will also exit the program if this condition is met.

> 
> Snir.
> 
> > vfunc" commit.
> >
> > Signed-off-by: Frediano Ziglio 
> > ---
> >   server/reds.c | 5 +
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/server/reds.c b/server/reds.c
> > index 429f8142..e182eba7 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds,
> > RedStatCounter *counter)
> >   void reds_register_channel(RedsState *reds, RedChannel *channel)
> >   {
> >   spice_assert(reds);
> > +if (spice_extra_checks) {
> > +uint32_t this_type, this_id;
> > +g_object_get(channel, "channel-type", _type, "id", _id,
> > NULL);
> > +spice_assert(reds_find_channel(reds, this_type, this_id) == NULL);
> > +}
> >   reds->channels = g_list_prepend(reds->channels, channel);
> >   // create new channel in the client if possible
> >   main_channel_registered_new_channel(reds->main_channel, channel);

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel

2019-03-18 Thread Snir Sheriber

Hi,

On 3/15/19 11:27 AM, Frediano Ziglio wrote:

To avoid possibly regression check it only if extra checks are
enabled.



Currently there's already a "channel duplication" warning upon connection
but won't hurt to have this extra check and emit an error.


This allowed to check previous "Move channel registration to constructed



This allow/s? i don't get this sentence, it just avoids duplicate call. 
I think

i'd not even mention the previous commit


Snir.


vfunc" commit.

Signed-off-by: Frediano Ziglio 
---
  server/reds.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/server/reds.c b/server/reds.c
index 429f8142..e182eba7 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds, RedStatCounter 
*counter)
  void reds_register_channel(RedsState *reds, RedChannel *channel)
  {
  spice_assert(reds);
+if (spice_extra_checks) {
+uint32_t this_type, this_id;
+g_object_get(channel, "channel-type", _type, "id", _id, 
NULL);
+spice_assert(reds_find_channel(reds, this_type, this_id) == NULL);
+}
  reds->channels = g_list_prepend(reds->channels, channel);
  // create new channel in the client if possible
  main_channel_registered_new_channel(reds->main_channel, channel);

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel