[Spice-devel] [PATCH spice-server] Save running property in QXLState

2019-03-21 Thread Jonathon Jongsma
From: Frediano Ziglio 

This is a preparatory patch that states the running property in QXLState
and provides accessor functions that allows us to check whether the QXL
device is running from different threads.

Signed-off-by: Jonathon Jongsma 
---
Alternate proposal for patch 7/10. I also rebased the rest of Frediano's branch 
that follows this
commit, which can be found in the dispatcher_red_channel branch at
https://gitlab.freedesktop.org/jjongsma/spice.git

 server/display-channel-private.h |  1 +
 server/red-qxl.c | 19 +++
 server/red-qxl.h |  2 ++
 server/red-worker.c  | 23 +++
 4 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 58179531d..067c64188 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -89,6 +89,7 @@ struct DisplayChannelPrivate
 uint32_t renderer;
 int enable_jpeg;
 int enable_zlib_glz_wrap;
+bool running;
 
 /* A ring of pending drawables for this DisplayChannel, regardless of which
  * surface they're associated with. This list is mainly used to flush older
diff --git a/server/red-qxl.c b/server/red-qxl.c
index 8274be567..152e9f068 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -59,6 +59,9 @@ struct QXLState {
 uint32_t device_display_ids[MAX_MONITORS_COUNT];
 size_t monitors_count;  // length of ^^^
 
+bool running;
+pthread_mutex_t running_mutex;
+
 pthread_mutex_t scanout_mutex;
 SpiceMsgDisplayGlScanoutUnix scanout;
 uint64_t gl_draw_cookie;
@@ -66,6 +69,21 @@ struct QXLState {
 
 #define GL_DRAW_COOKIE_INVALID (~((uint64_t) 0))
 
+bool red_qxl_is_running(QXLInstance *qxl) {
+bool running;
+pthread_mutex_lock(&qxl->st->running_mutex);
+running = qxl->st->running;
+pthread_mutex_unlock(&qxl->st->running_mutex);
+return running;
+}
+
+void red_qxl_set_running(QXLInstance *qxl, bool running) {
+pthread_mutex_lock(&qxl->st->running_mutex);
+qxl->st->running = running;
+pthread_mutex_unlock(&qxl->st->running_mutex);
+}
+
+
 int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int minor)
 {
 int qxl_major = qxl_get_interface(qxl)->base.major_version;
@@ -825,6 +843,7 @@ void red_qxl_init(RedsState *reds, QXLInstance *qxl)
 qxl_state->reds = reds;
 qxl_state->qxl = qxl;
 pthread_mutex_init(&qxl_state->scanout_mutex, NULL);
+pthread_mutex_init(&qxl_state->running_mutex, NULL);
 qxl_state->scanout.drm_dma_buf_fd = -1;
 qxl_state->gl_draw_cookie = GL_DRAW_COOKIE_INVALID;
 qxl_state->dispatcher = dispatcher_new(RED_WORKER_MESSAGE_COUNT);
diff --git a/server/red-qxl.h b/server/red-qxl.h
index 947539482..97d28d67c 100644
--- a/server/red-qxl.h
+++ b/server/red-qxl.h
@@ -60,5 +60,7 @@ void red_qxl_update_area_complete(QXLInstance *qxl, uint32_t 
surface_id,
 void red_qxl_set_client_capabilities(QXLInstance *qxl,
  uint8_t client_present,
  uint8_t caps[SPICE_CAPABILITIES_SIZE]);
+void red_qxl_set_running(QXLInstance *qxl, bool running);
+bool red_qxl_is_running(QXLInstance *qxl);
 
 #endif /* RED_QXL_H_ */
diff --git a/server/red-worker.c b/server/red-worker.c
index 27fe04ccb..b80fefabe 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -63,7 +63,6 @@ struct RedWorker {
 pthread_t thread;
 QXLInstance *qxl;
 SpiceWatch *dispatch_watch;
-int running;
 SpiceCoreInterfaceInternal core;
 
 unsigned int event_timeout;
@@ -115,7 +114,7 @@ static int red_process_cursor(RedWorker *worker, int 
*ring_is_empty)
 QXLCommandExt ext_cmd;
 int n = 0;
 
-if (!worker->running) {
+if (!red_qxl_is_running(worker->qxl)) {
 *ring_is_empty = TRUE;
 return n;
 }
@@ -173,7 +172,7 @@ static int red_process_display(RedWorker *worker, int 
*ring_is_empty)
 int n = 0;
 uint64_t start = spice_get_monotonic_time_ns();
 
-if (!worker->running) {
+if (!red_qxl_is_running(worker->qxl)) {
 *ring_is_empty = TRUE;
 return n;
 }
@@ -378,7 +377,7 @@ static void guest_set_client_capabilities(RedWorker *worker)
 #define CLEAR_CAP(a,c)  \
 ((a)[(c) / 8] &= ~(1 << ((c) % 8)))
 
-if (!worker->running) {
+if (!red_qxl_is_running(worker->qxl)) {
 return;
 }
 if ((worker->display_channel == NULL) ||
@@ -406,7 +405,7 @@ static void handle_dev_update_async(void *opaque, void 
*payload)
 QXLRect *qxl_dirty_rects = NULL;
 uint32_t num_dirty_rects = 0;
 
-spice_return_if_fail(worker->running);
+spice_return_if_fail(red_qxl_is_running(worker->qxl));
 spice_return_if_fail(qxl_get_interface(worker->qxl)->update_area_complete);
 
 flush_display_commands(worker);
@@ -426,7 +425,7 @@ static void handle_dev_update(void *opaque, void *payload)

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

2019-03-21 Thread Yuri Benditovich
On Thu, Mar 21, 2019 at 3:19 PM Christophe Fergeau  wrote:
>
> On Tue, Mar 19, 2019 at 07:56:05AM +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 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 */
>
> Maybe make this very explicit with something like this?
>
>   /* the unreference_libusb_device method is required as
>* libusb_unref_device calling convention differs from glib's
>* see 558c967ec
>*/
>

From my point of view this is obvious and existing comment is enough.
Anyway, please feel free to change the comment as you want.

>
>
> > +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, &a_desc);
> > +libusb_get_device_descriptor(b_dev, &b_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_des

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

2019-03-21 Thread Yuri Benditovich
On Thu, Mar 21, 2019 at 3:34 PM Christophe Fergeau  wrote:
>
> On Thu, Mar 21, 2019 at 02:15:41PM +0100, Christophe Fergeau wrote:
> > Hey,
> >
> > Overall series looks good to me, you seem to have ignored most of the
> > suggestions to change the commit logs/comments, I dunno if that's
> > intentional?

As I suggested earlier, please feel free to change commit logs and
even commit messages as you want.
I'm not able to understand your reasons of changing them anyway.

>
> Actually, while doing the review, I adjusted the commit logs
> https://gitlab.freedesktop.org/teuf/spice-gtk/commits/daynix-cd-sharing-v4
> But we can do more adjustments, including going back to the initial logs
> :)
>
> Christophe
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2019-03-21 Thread Christophe Fergeau
On Thu, Mar 21, 2019 at 02:15:41PM +0100, Christophe Fergeau wrote:
> Hey,
> 
> Overall series looks good to me, you seem to have ignored most of the
> suggestions to change the commit logs/comments, I dunno if that's
> intentional?

Actually, while doing the review, I adjusted the commit logs
https://gitlab.freedesktop.org/teuf/spice-gtk/commits/daynix-cd-sharing-v4
But we can do more adjustments, including going back to the initial logs
:)

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 v2 10/13] win-usb-dev: maintain list of libusb devices

2019-03-21 Thread Christophe Fergeau
On Tue, Mar 19, 2019 at 07:56:05AM +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 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 */

Maybe make this very explicit with something like this?

  /* the unreference_libusb_device method is required as
   * libusb_unref_device calling convention differs from glib's
   * see 558c967ec
   */



> +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, &a_desc);
> +libusb_get_device_descriptor(b_dev, &b_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 chang

Re: [Spice-devel] [spice] Use display_channel_surface_has_canvas() instead of free coding it

2019-03-21 Thread Frediano Ziglio
> 
> Signed-off-by: Francois Gouget 
> ---
>  server/dcc.c |  2 +-
>  server/display-channel.c | 16 
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index ae7b4380f..a05b6e59e 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -579,7 +579,7 @@ void dcc_start(DisplayChannelClient *dcc)
>  return;
>  
>  red_channel_client_ack_zero_messages_window(rcc);
> -if (display->priv->surfaces[0].context.canvas) {
> +if (display_channel_surface_has_canvas(display, 0)) {
>  display_channel_current_flush(display, 0);
>  red_channel_client_pipe_add_type(rcc,
>  RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
>  dcc_create_surface(dcc, 0);
> diff --git a/server/display-channel.c b/server/display-channel.c
> index e179abfd3..cf12e99bf 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -108,7 +108,7 @@ display_channel_finalize(GObject *object)
>  spice_assert(ring_is_empty(&self->priv->streams));
>  
>  for (count = 0; count < NUM_SURFACES; ++count) {
> -spice_assert(self->priv->surfaces[count].context.canvas ==
> NULL);
> +spice_assert(!display_channel_surface_has_canvas(self, count));
>  }
>  }
>  
> @@ -1498,7 +1498,7 @@ void display_channel_flush_all_surfaces(DisplayChannel
> *display)
>  int x;
>  
>  for (x = 0; x < NUM_SURFACES; ++x) {
> -if (display->priv->surfaces[x].context.canvas) {
> +if (display_channel_surface_has_canvas(display, x)) {
>  display_channel_current_flush(display, x);
>  }
>  }
> @@ -2072,7 +2072,7 @@ void
> display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
>  {
>  if (!display_channel_validate_surface(display, surface_id))
>  return;
> -if (!display->priv->surfaces[surface_id].context.canvas)
> +if (!display_channel_surface_has_canvas(display, surface_id))
>  return;
>  
>  draw_depend_on_me(display, surface_id);
> @@ -2092,12 +2092,12 @@ void display_channel_destroy_surfaces(DisplayChannel
> *display)
>  spice_debug("trace");
>  //to handle better
>  for (i = 0; i < NUM_SURFACES; ++i) {
> -if (display->priv->surfaces[i].context.canvas) {
> +if (display_channel_surface_has_canvas(display, i)) {
>  display_channel_destroy_surface_wait(display, i);
> -if (display->priv->surfaces[i].context.canvas) {
> +if (display_channel_surface_has_canvas(display, i)) {
>  display_channel_surface_unref(display, i);
>  }
> -spice_assert(!display->priv->surfaces[i].context.canvas);
> +spice_assert(!display_channel_surface_has_canvas(display, i));
>  }
>  }
>  spice_warn_if_fail(ring_is_empty(&display->priv->streams));
> @@ -2424,7 +2424,7 @@ gboolean
> display_channel_validate_surface(DisplayChannel *display, uint32_t surf
>  spice_warning("invalid surface_id %u", surface_id);
>  return FALSE;
>  }
> -if (!display->priv->surfaces[surface_id].context.canvas) {
> +if (!display_channel_surface_has_canvas(display, surface_id)) {
>  spice_warning("canvas address is %p for %d (and is NULL)",
> &(display->priv->surfaces[surface_id].context.canvas),
> surface_id);
>  spice_warning("failed on %d", surface_id);
> @@ -2462,7 +2462,7 @@ void
> display_channel_set_monitors_config_to_primary(DisplayChannel *display)
>  QXLHead head = { 0, };
>  uint16_t old_max = 1;
>  
> -spice_return_if_fail(display->priv->surfaces[0].context.canvas);
> +spice_return_if_fail(display_channel_surface_has_canvas(display, 0));
>  
>  if (display->priv->monitors_config) {
>  old_max = display->priv->monitors_config->max_allowed;

My paranoia level is saying that this function should be inlined, on the other
hand is also used in red-worker.c where can't be inlined.

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

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

2019-03-21 Thread Christophe Fergeau
Hey,

Overall series looks good to me, you seem to have ignored most of the
suggestions to change the commit logs/comments, I dunno if that's
intentional? Still a few comments on "win-usb-dev: maintain list of
libusb devices"

So apart from this patch,

Acked-by: Christophe Fergeau 

Christophe

On Tue, Mar 19, 2019 at 07:55:55AM +0200, Yuri Benditovich wrote:
> 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


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

[Spice-devel] [PATCH spice-gtk 2/2] clipboard: do not release between remote grabs

2019-03-21 Thread marcandre . lureau
From: Marc-André Lureau 

Delay the release events for 0.5 sec. If no further grab comes in,
then release the grab. Otherwise, let's skip the release. This avoids
some races with clipboard managers.

Related to:
https://gitlab.freedesktop.org/spice/spice-gtk/issues/82

Signed-off-by: Marc-André Lureau 
---
 src/spice-gtk-session.c | 80 -
 1 file changed, 72 insertions(+), 8 deletions(-)

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 0e748b6..d73a44b 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -59,6 +59,7 @@ struct _SpiceGtkSessionPrivate {
 gbooleanclip_hasdata[CLIPBOARD_LAST];
 gbooleanclip_grabbed[CLIPBOARD_LAST];
 gbooleanclipboard_by_guest[CLIPBOARD_LAST];
+guint   clipboard_release_delay[CLIPBOARD_LAST];
 /* auto-usbredir related */
 gbooleanauto_usbredir_enable;
 int auto_usbredir_reqs;
@@ -95,6 +96,7 @@ struct _SpiceGtkSessionPrivate {
 
 /* -- */
 /* Prototypes for private functions */
+static void clipboard_release(SpiceGtkSession *self, guint selection);
 static void clipboard_owner_change(GtkClipboard *clipboard,
GdkEventOwnerChange *event,
gpointer user_data);
@@ -247,6 +249,23 @@ static void spice_gtk_session_dispose(GObject *gobject)
 G_OBJECT_CLASS(spice_gtk_session_parent_class)->dispose(gobject);
 }
 
+static void clipboard_release_delay_remove(SpiceGtkSession *self, guint 
selection,
+   gboolean release_if_delayed)
+{
+SpiceGtkSessionPrivate *s = self->priv;
+
+if (!s->clipboard_release_delay[selection])
+return;
+
+if (release_if_delayed) {
+SPICE_DEBUG("delayed clipboard release, sel:%u", selection);
+clipboard_release(self, selection);
+}
+
+g_source_remove(s->clipboard_release_delay[selection]);
+s->clipboard_release_delay[selection] = 0;
+}
+
 static void spice_gtk_session_finalize(GObject *gobject)
 {
 SpiceGtkSession *self = SPICE_GTK_SESSION(gobject);
@@ -256,6 +275,7 @@ static void spice_gtk_session_finalize(GObject *gobject)
 /* release stuff */
 for (i = 0; i < CLIPBOARD_LAST; ++i) {
 g_clear_pointer(&s->clip_targets[i], g_free);
+clipboard_release_delay_remove(self, i, true);
 }
 
 /* Chain up to the parent class */
@@ -815,6 +835,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main, 
guint selection,
 int m, n;
 int num_targets = 0;
 
+clipboard_release_delay_remove(self, selection, false);
+
 cb = get_clipboard_from_selection(s, selection);
 g_return_val_if_fail(cb != NULL, FALSE);
 
@@ -1044,17 +1066,12 @@ static gboolean clipboard_request(SpiceMainChannel 
*main, guint selection,
 return TRUE;
 }
 
-static void clipboard_release(SpiceMainChannel *main, guint selection,
-  gpointer user_data)
+static void clipboard_release(SpiceGtkSession *self, guint selection)
 {
-g_return_if_fail(SPICE_IS_GTK_SESSION(user_data));
-
-SpiceGtkSession *self = user_data;
 SpiceGtkSessionPrivate *s = self->priv;
 GtkClipboard* clipboard = get_clipboard_from_selection(s, selection);
 
-if (!clipboard)
-return;
+g_return_if_fail(clipboard != NULL);
 
 s->nclip_targets[selection] = 0;
 
@@ -1064,6 +1081,53 @@ static void clipboard_release(SpiceMainChannel *main, 
guint selection,
 s->clipboard_by_guest[selection] = FALSE;
 }
 
+typedef struct SpiceGtkClipboardRelease {
+SpiceGtkSession *self;
+guint selection;
+} SpiceGtkClipboardRelease;
+
+static gboolean clipboard_release_timeout(gpointer user_data)
+{
+SpiceGtkClipboardRelease *rel = user_data;
+
+clipboard_release_delay_remove(rel->self, rel->selection, true);
+
+return G_SOURCE_REMOVE;
+}
+
+/*
+ * The agents send release between two grabs. This may trigger
+ * clipboard managers trying to grab the clipboard. We end up with two
+ * sides, client and remote, racing for the clipboard grab, and
+ * believing each other is the owner.
+ *
+ * Workaround this problem by delaying the release event by 0.5 sec.
+ * FIXME: protocol change to solve the conflict and set client priority.
+ */
+#define CLIPBOARD_RELEASE_DELAY 500 /* ms */
+
+static void clipboard_release_delay(SpiceMainChannel *main, guint selection,
+gpointer user_data)
+{
+SpiceGtkSession *self = SPICE_GTK_SESSION(user_data);
+SpiceGtkSessionPrivate *s = self->priv;
+GtkClipboard* clipboard = get_clipboard_from_selection(s, selection);
+SpiceGtkClipboardRelease *rel;
+
+if (!clipboard)
+return;
+
+clipboard_release_delay_remove(self, selection, true);
+
+rel = g_new0(SpiceGtkClipboardRelease, 1);
+rel->self = self;
+ 

[Spice-devel] [PATCH spice-gtk 0/2] clipboard: skip release between grabs

2019-03-21 Thread marcandre . lureau
From: Marc-André Lureau 

Hi,

There has been several reports of clipboard issues that seem related
to clipboard managers interactions. Although it's not crystal clear
what are the problems, we realized the spice protocol does't handle
well conflicting grabs coming simultaneously from client and remote.
Each other believe the other side is the owner. This situation
is rare, but with a clipboard manager that reacts to release events,
it may be more easily reproducible.

Both agent & client send release between grabs today. This series
propose to stop releasing between grabs on client side, and adds a
0.5s delay for the release event coming from the agent side, to filter
out release between grabs.

Further protocol improvements should remove the need for a delay, and
solve the conflicting situation in all cases. This will come in a
future series.

Marc-André Lureau (2):
  clipboard: do not release between client grabs
  clipboard: do not release between remote grabs

 src/spice-gtk-session.c | 101 +++-
 1 file changed, 80 insertions(+), 21 deletions(-)

-- 
2.21.0.4.g36eb1cb9cf

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

[Spice-devel] [PATCH spice-gtk 1/2] clipboard: do not release between client grabs

2019-03-21 Thread marcandre . lureau
From: Marc-André Lureau 

On the client side, whenever the grab owner changes (and the clipboard
was previously grabbed), spice-gtk sends a clipboard release followed
immediately by a new grab. But some clipboard managers on the remote
side react to clipboard release events by taking a clipboard grab,
presumably to avoid empty clipboards.

The two grabs, coming from the client and from the remote sides, will
race in both directions, which may confuse the client & remote side,
as both believe the other side is the current grab owner, and thus
further clipboard data requests are likely to fail.

Let's avoid sending a release event when re-grabing.

The race described above may still happen in other rare circunstances,
and will require a protocol change. To avoid the conflict, a discussed
solution could use a clipboard serial number.

Tested with current linux & windows vdagent. Looking at earlier
version of the code, it doesn't seem like subsequent grabs will be
treated as an error.

Signed-off-by: Marc-André Lureau 
---
 src/spice-gtk-session.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index b48f92a..0e748b6 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -569,8 +569,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
 g_return_if_fail(selection != -1);
 
 if (s->clip_grabbed[selection]) {
-SPICE_DEBUG("Clipboard is already grabbed, ignoring %d atoms", 
n_atoms);
-return;
+SPICE_DEBUG("Clipboard is already grabbed, re-grab: %d atoms", 
n_atoms);
 }
 
 /* Set all Atoms that matches our current protocol implementation */
@@ -652,18 +651,14 @@ static void clipboard_owner_change(GtkClipboard
*clipboard,
 return;
 }
 
-/* In case we sent a grab to the agent, we need to release it now as
- * previous clipboard data should not be reachable anymore */
-if (s->clip_grabbed[selection]) {
-s->clip_grabbed[selection] = FALSE;
-if (spice_main_channel_agent_test_capability(s->main, 
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) {
-spice_main_channel_clipboard_selection_release(s->main, selection);
-}
-}
-
-/* We are mostly interested when owner has changed in which case
- * we would like to let agent know about new clipboard data. */
 if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) {
+if (s->clip_grabbed[selection]) {
+/* grab was sent to the agent, so release it */
+s->clip_grabbed[selection] = FALSE;
+if (spice_main_channel_agent_test_capability(s->main, 
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) {
+spice_main_channel_clipboard_selection_release(s->main, 
selection);
+}
+}
 s->clip_hasdata[selection] = FALSE;
 return;
 }
-- 
2.21.0.4.g36eb1cb9cf

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

Re: [Spice-devel] [spice] Consistently check if drm_dma_buf_fd is greater or lower than zero

2019-03-21 Thread Frediano Ziglio
> 
> Based on a patch by Frediano Ziglio.
> 

I don't remember

> Signed-off-by: Francois Gouget 

Acked

> ---
>  server/red-qxl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 0dd26b22c..789817f69 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -791,7 +791,7 @@ void spice_qxl_gl_scanout(QXLInstance *qxl,
>  
>  pthread_mutex_lock(&qxl_state->scanout_mutex);
>  
> -if (qxl_state->scanout.drm_dma_buf_fd != -1) {
> +if (qxl_state->scanout.drm_dma_buf_fd >= 0) {
>  close(qxl_state->scanout.drm_dma_buf_fd);
>  }
>  
> @@ -832,7 +832,7 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl,
>  
>  spice_return_if_fail(qxl != NULL);
>  qxl_state = qxl->st;
> -if (qxl_state->scanout.drm_dma_buf_fd == -1) {
> +if (qxl_state->scanout.drm_dma_buf_fd < 0) {
>  spice_warning("called spice_qxl_gl_draw_async without a buffer");
>  red_qxl_async_complete(qxl, cookie);
>  return;

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

Re: [Spice-devel] [spice] dcc: Remove a redundant NULL pointer check in dcc_create_surface()

2019-03-21 Thread Frediano Ziglio
> 
> dcc_create_surface() already returns immediately if dcc is NULL.

Yes, and even this check is useless.

> 
> Signed-off-by: Francois Gouget 

Acked.

> ---
>  server/dcc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index a05b6e59e..b6cd6b3f9 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -303,8 +303,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, int
> surface_id)
>  flags = is_primary_surface(display, surface_id) ?
>  SPICE_SURFACE_FLAGS_PRIMARY : 0;
>  
>  /* don't send redundant create surface commands to client */
> -if (!dcc ||
> -
> common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> ||
> +if
> (common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> ||
>  dcc->priv->surface_client_created[surface_id]) {
>  return;
>  }
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice] Consistently check if drm_dma_buf_fd is greater or lower than zero

2019-03-21 Thread Francois Gouget
Based on a patch by Frediano Ziglio.

Signed-off-by: Francois Gouget 
---
 server/red-qxl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/server/red-qxl.c b/server/red-qxl.c
index 0dd26b22c..789817f69 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -791,7 +791,7 @@ void spice_qxl_gl_scanout(QXLInstance *qxl,
 
 pthread_mutex_lock(&qxl_state->scanout_mutex);
 
-if (qxl_state->scanout.drm_dma_buf_fd != -1) {
+if (qxl_state->scanout.drm_dma_buf_fd >= 0) {
 close(qxl_state->scanout.drm_dma_buf_fd);
 }
 
@@ -832,7 +832,7 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl,
 
 spice_return_if_fail(qxl != NULL);
 qxl_state = qxl->st;
-if (qxl_state->scanout.drm_dma_buf_fd == -1) {
+if (qxl_state->scanout.drm_dma_buf_fd < 0) {
 spice_warning("called spice_qxl_gl_draw_async without a buffer");
 red_qxl_async_complete(qxl, cookie);
 return;
-- 
2.20.1

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

[Spice-devel] [spice] dcc: Remove a redundant NULL pointer check in dcc_create_surface()

2019-03-21 Thread Francois Gouget
dcc_create_surface() already returns immediately if dcc is NULL.

Signed-off-by: Francois Gouget 
---
 server/dcc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index a05b6e59e..b6cd6b3f9 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -303,8 +303,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, int 
surface_id)
 flags = is_primary_surface(display, surface_id) ? 
SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
 /* don't send redundant create surface commands to client */
-if (!dcc ||
-
common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
 ||
+if 
(common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
 ||
 dcc->priv->surface_client_created[surface_id]) {
 return;
 }
-- 
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [spice] Use display_channel_surface_has_canvas() instead of free coding it

2019-03-21 Thread Francois Gouget
Signed-off-by: Francois Gouget 
---
 server/dcc.c |  2 +-
 server/display-channel.c | 16 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index ae7b4380f..a05b6e59e 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -579,7 +579,7 @@ void dcc_start(DisplayChannelClient *dcc)
 return;
 
 red_channel_client_ack_zero_messages_window(rcc);
-if (display->priv->surfaces[0].context.canvas) {
+if (display_channel_surface_has_canvas(display, 0)) {
 display_channel_current_flush(display, 0);
 red_channel_client_pipe_add_type(rcc, 
RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
 dcc_create_surface(dcc, 0);
diff --git a/server/display-channel.c b/server/display-channel.c
index e179abfd3..cf12e99bf 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -108,7 +108,7 @@ display_channel_finalize(GObject *object)
 spice_assert(ring_is_empty(&self->priv->streams));
 
 for (count = 0; count < NUM_SURFACES; ++count) {
-spice_assert(self->priv->surfaces[count].context.canvas == NULL);
+spice_assert(!display_channel_surface_has_canvas(self, count));
 }
 }
 
@@ -1498,7 +1498,7 @@ void display_channel_flush_all_surfaces(DisplayChannel 
*display)
 int x;
 
 for (x = 0; x < NUM_SURFACES; ++x) {
-if (display->priv->surfaces[x].context.canvas) {
+if (display_channel_surface_has_canvas(display, x)) {
 display_channel_current_flush(display, x);
 }
 }
@@ -2072,7 +2072,7 @@ void display_channel_destroy_surface_wait(DisplayChannel 
*display, uint32_t surf
 {
 if (!display_channel_validate_surface(display, surface_id))
 return;
-if (!display->priv->surfaces[surface_id].context.canvas)
+if (!display_channel_surface_has_canvas(display, surface_id))
 return;
 
 draw_depend_on_me(display, surface_id);
@@ -2092,12 +2092,12 @@ void display_channel_destroy_surfaces(DisplayChannel 
*display)
 spice_debug("trace");
 //to handle better
 for (i = 0; i < NUM_SURFACES; ++i) {
-if (display->priv->surfaces[i].context.canvas) {
+if (display_channel_surface_has_canvas(display, i)) {
 display_channel_destroy_surface_wait(display, i);
-if (display->priv->surfaces[i].context.canvas) {
+if (display_channel_surface_has_canvas(display, i)) {
 display_channel_surface_unref(display, i);
 }
-spice_assert(!display->priv->surfaces[i].context.canvas);
+spice_assert(!display_channel_surface_has_canvas(display, i));
 }
 }
 spice_warn_if_fail(ring_is_empty(&display->priv->streams));
@@ -2424,7 +2424,7 @@ gboolean display_channel_validate_surface(DisplayChannel 
*display, uint32_t surf
 spice_warning("invalid surface_id %u", surface_id);
 return FALSE;
 }
-if (!display->priv->surfaces[surface_id].context.canvas) {
+if (!display_channel_surface_has_canvas(display, surface_id)) {
 spice_warning("canvas address is %p for %d (and is NULL)",
&(display->priv->surfaces[surface_id].context.canvas), 
surface_id);
 spice_warning("failed on %d", surface_id);
@@ -2462,7 +2462,7 @@ void 
display_channel_set_monitors_config_to_primary(DisplayChannel *display)
 QXLHead head = { 0, };
 uint16_t old_max = 1;
 
-spice_return_if_fail(display->priv->surfaces[0].context.canvas);
+spice_return_if_fail(display_channel_surface_has_canvas(display, 0));
 
 if (display->priv->monitors_config) {
 old_max = display->priv->monitors_config->max_allowed;
-- 
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 v2 2/2] worker: Fix potential sprintf overflow

2019-03-21 Thread Christophe Fergeau
On Wed, Mar 20, 2019 at 03:57:46PM +, Frediano Ziglio wrote:
> From: Christophe Fergeau 
> 
> If worker->qxl->id is bigger than 0x7ff (in other words, it's a
> negative signed int) then
> printf(worker_str, "display[%d]", worker->qxl->id);
> will need:
> 
> "display[]" -> 9 bytes
> %d -> 11 bytes
> 
> The trailing \0 will thus overflow our 20 bytes destination.
> As QXLInstance::id should be an unsigned int, this commit changes the
> format string to use %u. This also switches to snprintf.
> ---
>  server/red-worker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 8051d1e4..50612aca 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -1291,7 +1291,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
>  worker->zlib_glz_state = reds_get_zlib_glz_state(reds);
>  worker->driver_cap_monitors_config = 0;
>  char worker_str[SPICE_STAT_NODE_NAME_MAX];
> -sprintf(worker_str, "display[%d]", worker->qxl->id);
> +snprintf(worker_str, sizeof(worker_str), "display[%u]", (unsigned 
> int)worker->qxl->id);

I'd still add a &0xff at the end to make it explicit that we expect a
uint8_t. It's a patch I wrote, so no further comments ;)

Christophe

>  stat_init_node(&worker->stat, reds, NULL, worker_str, TRUE);
>  stat_init_counter(&worker->wakeup_counter, reds, &worker->stat, 
> "wakeups", TRUE);
>  stat_init_counter(&worker->command_counter, reds, &worker->stat, 
> "commands", 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] [PATCH spice-server 07/10] Propagate running property from RedWorker to DisplayChannel

2019-03-21 Thread Frediano Ziglio
> 
> Somehow this information doesn't really seem to belong to
> DisplayChannel, but I can't put my finger on exactly why it feels out
> of place...
> 
> 
> 
> On Wed, 2019-03-20 at 09:59 +, Frediano Ziglio wrote:
> > This is a preparatory patch to allows DisplayChannel to check
> > if device is running.
> > 
> > Signed-off-by: Frediano Ziglio 
> > ---
> >  server/display-channel-private.h | 1 +
> >  server/display-channel.c | 8 
> >  server/display-channel.h | 2 ++
> >  server/red-worker.c  | 2 ++
> >  4 files changed, 13 insertions(+)
> > 
> > diff --git a/server/display-channel-private.h b/server/display-
> > channel-private.h
> > index 58179531..067c6418 100644
> > --- a/server/display-channel-private.h
> > +++ b/server/display-channel-private.h
> > @@ -89,6 +89,7 @@ struct DisplayChannelPrivate
> >  uint32_t renderer;
> >  int enable_jpeg;
> >  int enable_zlib_glz_wrap;
> > +bool running;
> 
> At minimum, this seems like way too generic of a name. I will probably
> look at this later and wonder what it means for a DisplayChannel to be
> 'running'. But in reality this variable indicates that the QXL device
> is started? Actually, after looking briefly at this handle_dev_start()
> function, I can't even figure out what causes this function to be
> called...
> 

Yes, all these dispatching can be confusing.
Basically the VM is running or not (paused for different reasons).
When the VM is paused you don't want to send events to it or change
its status.
Maybe "qxl_active" ?

> >  
> >  /* A ring of pending drawables for this DisplayChannel,
> > regardless of which
> >   * surface they're associated with. This list is mainly used to
> > flush older
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index cb052bfc..49a2bd48 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -2540,3 +2540,11 @@ void display_channel_debug_oom(DisplayChannel
> > *display, const char *msg)
> >  ring_get_length(&display->priv->current_list),
> >  red_channel_sum_pipes_size(channel));
> >  }
> > +
> > +void display_channel_set_running(DisplayChannel *display, bool
> > running)
> > +{
> > +if (running == display->priv->running) {
> > +return;
> > +}
> > +display->priv->running = running;
> > +}
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index f64da0bd..bdf2f059 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -158,6 +158,8 @@ void
> > display_channel_reset_image_cache(DisplayChannel *self);
> >  
> >  void display_channel_debug_oom(DisplayChannel *display, const char
> > *msg);
> >  
> > +void display_channel_set_running(DisplayChannel *display, bool
> > running);
> > +
> >  G_END_DECLS
> >  
> >  #endif /* DISPLAY_CHANNEL_H_ */
> > diff --git a/server/red-worker.c b/server/red-worker.c
> > index 07329b17..c9c47133 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -588,6 +588,7 @@ static void handle_dev_stop(void *opaque, void
> > *payload)
> >  spice_assert(worker->running);
> >  
> >  worker->running = FALSE;
> > +display_channel_set_running(worker->display_channel, false);
> >  
> >  display_channel_free_glz_drawables(worker->display_channel);
> >  display_channel_flush_all_surfaces(worker->display_channel);
> > @@ -616,6 +617,7 @@ static void handle_dev_start(void *opaque, void
> > *payload)
> >  display_channel_wait_for_migrate_data(worker-
> > >display_channel);
> >  }
> >  worker->running = TRUE;
> > +display_channel_set_running(worker->display_channel, true);
> >  worker->event_timeout = 0;
> >  guest_set_client_capabilities(worker);
> >  } 

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