Re: [Spice-devel] [spice-gtk v1] file-xfer: differentiate error from host/guest and client

2016-12-22 Thread Pavel Grunt
Hi,

it works. But do we need a new public symbol ? It is an internal stuff
for channel-main.

Pavel

On Wed, 2016-12-21 at 18:55 +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> During file transfer, we can have error and cancellation in client,
> host or guest side.
> 
> If error happens in the client side, we must send a message to the
> guest, VD_AGENT_FILE_XFER_STATUS, with either _STATUS_CANCELLED or
> _STATUS_ERROR.
> 
> But the current code is also sending a message to the guest when
> error/cancellation does not come from client, leading to unexpected
> messages to be received in host/guest.
> 
> SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED is introduced in this patch
> to
> mark a SpiceFileTransferTask to be failed due host or guest
> problems.
> 
> The generic error code SPICE_CLIENT_ERROR_FAILED is still used in
> spice_main_file_copy_async() API.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=99170
> Signed-off-by: Victor Toso 
> Reported-by: Pavel Grunt 
> ---
>  src/channel-main.c | 11 +++
>  src/spice-client.h |  1 +
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/src/channel-main.c b/src/channel-main.c
> index ed5d611..e92d363 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -1862,18 +1862,18 @@ static void
> main_agent_handle_xfer_status(SpiceMainChannel *channel,
>  spice_file_transfer_task_read_async(xfer_task,
> file_xfer_read_async_cb, xfer_op);
>  return;
>  case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FAILED,
> +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
>  _("The spice agent cancelled
> the file transfer"));
>  break;
>  case VD_AGENT_FILE_XFER_STATUS_ERROR:
> -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FAILED,
> +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
>  _("The spice agent reported an
> error during the file transfer"));
>  break;
>  case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
>  break;
>  default:
>  g_warn_if_reached();
> -error = g_error_new(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FAILED,
> +error = g_error_new(SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
>  "unhandled status type: %u", msg-
> >result);
>  break;
>  }
> @@ -2960,7 +2960,10 @@ static void
> file_transfer_operation_task_finished(SpiceFileTransferTask *xfer_ta
>  task_id = spice_file_transfer_task_get_id(xfer_task);
>  g_return_if_fail(task_id != 0);
>  
> -if (error) {
> +if (g_error_matches(error, SPICE_CLIENT_ERROR,
> +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED)) {
> +spice_debug("xfer task: %u failed due cancel/error on
> server or agent", task_id);
> +} else if (error) {
>  VDAgentFileXferStatusMessage msg;
>  msg.id = task_id;
>  if (g_error_matches(error, G_IO_ERROR,
> G_IO_ERROR_CANCELLED)) {
> diff --git a/src/spice-client.h b/src/spice-client.h
> index 32b79ea..8297537 100644
> --- a/src/spice-client.h
> +++ b/src/spice-client.h
> @@ -82,6 +82,7 @@ typedef enum
>  SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>  SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>  SPICE_CLIENT_ERROR_USB_SERVICE,
> +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
>  } SpiceClientError;
>  
>  #ifndef SPICE_DISABLE_DEPRECATED
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v1] file-xfer: differentiate error from host/guest and client

2016-12-22 Thread Victor Toso
Hi,

On Thu, Dec 22, 2016 at 11:35:13AM +0100, Pavel Grunt wrote:
> Hi,
>
> it works. But do we need a new public symbol ? It is an internal stuff
> for channel-main.
>
> Pavel

Yeah, I was also thinking if this is really necessary.

Application can track SpiceFileTransferTask "finished" signal too, so I
thought this distinction in GError would make sense even though the main
API function still uses SPICE_CLIENT_ERROR_FAILED error code in that
case.

The changes in the code are quite minor this way too, but this is not
really important.

Pinging Jonathon for hints/ideas/opinions :)

  toso

>
> On Wed, 2016-12-21 at 18:55 +0100, Victor Toso wrote:
> > From: Victor Toso 
> > 
> > During file transfer, we can have error and cancellation in client,
> > host or guest side.
> > 
> > If error happens in the client side, we must send a message to the
> > guest, VD_AGENT_FILE_XFER_STATUS, with either _STATUS_CANCELLED or
> > _STATUS_ERROR.
> > 
> > But the current code is also sending a message to the guest when
> > error/cancellation does not come from client, leading to unexpected
> > messages to be received in host/guest.
> > 
> > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED is introduced in this patch
> > to
> > mark a SpiceFileTransferTask to be failed due host or guest
> > problems.
> > 
> > The generic error code SPICE_CLIENT_ERROR_FAILED is still used in
> > spice_main_file_copy_async() API.
> > 
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=99170
> > Signed-off-by: Victor Toso 
> > Reported-by: Pavel Grunt 
> > ---
> >  src/channel-main.c | 11 +++
> >  src/spice-client.h |  1 +
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index ed5d611..e92d363 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -1862,18 +1862,18 @@ static void
> > main_agent_handle_xfer_status(SpiceMainChannel *channel,
> >  spice_file_transfer_task_read_async(xfer_task,
> > file_xfer_read_async_cb, xfer_op);
> >  return;
> >  case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> > -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FAILED,
> > +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> >  _("The spice agent cancelled
> > the file transfer"));
> >  break;
> >  case VD_AGENT_FILE_XFER_STATUS_ERROR:
> > -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FAILED,
> > +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> >  _("The spice agent reported an
> > error during the file transfer"));
> >  break;
> >  case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
> >  break;
> >  default:
> >  g_warn_if_reached();
> > -error = g_error_new(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FAILED,
> > +error = g_error_new(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> >  "unhandled status type: %u", msg-
> > >result);
> >  break;
> >  }
> > @@ -2960,7 +2960,10 @@ static void
> > file_transfer_operation_task_finished(SpiceFileTransferTask *xfer_ta
> >  task_id = spice_file_transfer_task_get_id(xfer_task);
> >  g_return_if_fail(task_id != 0);
> >  
> > -if (error) {
> > +if (g_error_matches(error, SPICE_CLIENT_ERROR,
> > +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED)) {
> > +spice_debug("xfer task: %u failed due cancel/error on
> > server or agent", task_id);
> > +} else if (error) {
> >  VDAgentFileXferStatusMessage msg;
> >  msg.id = task_id;
> >  if (g_error_matches(error, G_IO_ERROR,
> > G_IO_ERROR_CANCELLED)) {
> > diff --git a/src/spice-client.h b/src/spice-client.h
> > index 32b79ea..8297537 100644
> > --- a/src/spice-client.h
> > +++ b/src/spice-client.h
> > @@ -82,6 +82,7 @@ typedef enum
> >  SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> >  SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
> >  SPICE_CLIENT_ERROR_USB_SERVICE,
> > +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> >  } SpiceClientError;
> >  
> >  #ifndef SPICE_DISABLE_DEPRECATED


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] win-usb: remove usbclerk

2016-12-22 Thread Victor Toso
Let me know if this one needs more tweaks!

On Mon, Dec 19, 2016 at 06:50:02PM +0100, Victor Toso wrote:
> From: Victor Toso 
> 
> As we have UsbDk integration now which is well maintained upstream.
> 
> Signed-off-by: Victor Toso 
> ---
>  doc/reference/Makefile.am|   2 -
>  src/Makefile.am  |   3 -
>  src/usb-device-manager.c | 308 +++
>  src/win-usb-clerk.h  |  36 
>  src/win-usb-driver-install.c | 421 
> ---
>  src/win-usb-driver-install.h | 106 ---
>  6 files changed, 23 insertions(+), 853 deletions(-)
>  delete mode 100644 src/win-usb-clerk.h
>  delete mode 100644 src/win-usb-driver-install.c
>  delete mode 100644 src/win-usb-driver-install.h
> 
> diff --git a/doc/reference/Makefile.am b/doc/reference/Makefile.am
> index eaaf98c..999c1dc 100644
> --- a/doc/reference/Makefile.am
> +++ b/doc/reference/Makefile.am
> @@ -58,9 +58,7 @@ IGNORE_HFILES=  \
>   usbutil.h   \
>   vmcstream.h \
>   vncdisplaykeymap.h  \
> - win-usb-clerk.h \
>   win-usb-dev.h   \
> - win-usb-driver-install.h\
>   wocky-http-proxy.h  \
>   $(NULL)
>  
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 78b215f..e43cee0 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -371,9 +371,6 @@ endif
>  WIN_USB_FILES= \
>   win-usb-dev.h   \
>   win-usb-dev.c   \
> - win-usb-clerk.h \
> - win-usb-driver-install.h\
> - win-usb-driver-install.c\
>   usbdk_api.h \
>   usbdk_api.c \
>   $(NULL)
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index 6d10daa..7f0b263 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -35,7 +35,6 @@
>  #include 
>  #elif defined(G_OS_WIN32)
>  #include "win-usb-dev.h"
> -#include "win-usb-driver-install.h"
>  #define USE_GUDEV /* win-usb-dev.h provides a fake gudev interface */
>  #elif !defined USE_LIBUSB_HOTPLUG
>  #error "Expecting one of USE_GUDEV or USE_LIBUSB_HOTPLUG to be defined"
> @@ -127,9 +126,7 @@ struct _SpiceUsbDeviceManagerPrivate {
>  #ifdef G_OS_WIN32
>  usbdk_api_wrapper *usbdk_api;
>  HANDLE usbdk_hider_handle;
> -SpiceWinUsbDriver *installer;
>  #endif
> -gboolean   use_usbclerk;
>  #endif
>  GPtrArray *devices;
>  GPtrArray *channels;
> @@ -188,9 +185,6 @@ static SpiceUsbDevice 
> *spice_usb_device_ref(SpiceUsbDevice *device);
>  static void spice_usb_device_unref(SpiceUsbDevice *device);
>  
>  #ifdef G_OS_WIN32
> -static guint8 spice_usb_device_get_state(SpiceUsbDevice *device);
> -static void  spice_usb_device_set_state(SpiceUsbDevice *device, guint8 s);
> -
>  static void _usbdk_hider_update(SpiceUsbDeviceManager *manager);
>  static void _usbdk_hider_clear(SpiceUsbDeviceManager *manager);
>  #endif
> @@ -274,8 +268,11 @@ static void 
> spice_usb_device_manager_init(SpiceUsbDeviceManager *self)
>  self->priv = priv;
>  
>  #if defined(G_OS_WIN32) && defined(USE_USBREDIR)
> -priv->use_usbclerk = !usbdk_is_driver_installed() ||
> - !(priv->usbdk_api = usbdk_api_load());
> +if (usbdk_is_driver_installed()) {
> +priv->usbdk_api = usbdk_api_load();
> +} else {
> +spice_debug("UsbDk driver is not installed");
> +}
>  #endif
>  priv->channels = g_ptr_array_new();
>  #ifdef USE_USBREDIR
> @@ -298,16 +295,6 @@ static gboolean 
> spice_usb_device_manager_initable_init(GInitable  *initable,
>  const gchar *const subsystems[] = {"usb", NULL};
>  #endif
>  
> -#ifdef G_OS_WIN32
> -if (priv->use_usbclerk) {
> -priv->installer = spice_win_usb_driver_new(err);
> -if (!priv->installer) {
> -SPICE_DEBUG("failed to initialize winusb driver");
> -return FALSE;
> -}
> -}
> -#endif
> -
>  /* Initialize libusb */
>  rc = libusb_init(&priv->context);
>  if (rc < 0) {
> @@ -425,14 +412,8 @@ static void spice_usb_device_manager_finalize(GObject 
> *gobject)
>  free(priv->auto_conn_filter_rules);
>  free(priv->redirect_on_connect_rules);
>  #ifdef G_OS_WIN32
> -if (priv->installer) {
> -g_warn_if_fail(priv->use_usbclerk);
> -g_object_unref(priv->installer);
> -}
> -if (!priv->use_usbclerk) {
> -_usbdk_hider_clear(self);
> -usbdk_api_unload(priv->usbdk_api);
> -}
> +_usbdk_hider_clear(self);
> +usbdk_api_unload(priv->usbdk_api);
>  #endif
>  #endif
>  
> @@ -505,9 +486,7 @@ static void spice_usb_device_manager_set_property(GObject 
>   *gobject,
>  case PROP_AUTO_CONNECT:
>  priv->auto_connect = g_value_get_boolean(value

Re: [Spice-devel] [spice-gtk v2] win-usb: remove usbclerk

2016-12-22 Thread Pavel Grunt
Hi,
lgtm
considering that you tested and it works - Ack

Pavel

On Thu, 2016-12-22 at 14:18 +0100, Victor Toso wrote:
> Let me know if this one needs more tweaks!
> 
> On Mon, Dec 19, 2016 at 06:50:02PM +0100, Victor Toso wrote:
> > From: Victor Toso 
> > 
> > As we have UsbDk integration now which is well maintained
> > upstream.
> > 
> > Signed-off-by: Victor Toso 
> > ---
> >  doc/reference/Makefile.am|   2 -
> >  src/Makefile.am  |   3 -
> >  src/usb-device-manager.c | 308 +++---
> > -
> >  src/win-usb-clerk.h  |  36 
> >  src/win-usb-driver-install.c | 421 --
> > -
> >  src/win-usb-driver-install.h | 106 ---
> >  6 files changed, 23 insertions(+), 853 deletions(-)
> >  delete mode 100644 src/win-usb-clerk.h
> >  delete mode 100644 src/win-usb-driver-install.c
> >  delete mode 100644 src/win-usb-driver-install.h
> > 
> > diff --git a/doc/reference/Makefile.am b/doc/reference/Makefile.am
> > index eaaf98c..999c1dc 100644
> > --- a/doc/reference/Makefile.am
> > +++ b/doc/reference/Makefile.am
> > @@ -58,9 +58,7 @@ IGNORE_HFILES=
> > \
> >     usbutil.h   \
> >     vmcstream.h \
> >     vncdisplaykeymap.h  \
> > -   win-usb-clerk.h \
> >     win-usb-dev.h   \
> > -   win-usb-driver-install.h\
> >     wocky-http-proxy.h  \
> >     $(NULL)
> >  
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 78b215f..e43cee0 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -371,9 +371,6 @@ endif
> >  WIN_USB_FILES= \
> >     win-usb-dev.h   \
> >     win-usb-dev.c   \
> > -   win-usb-clerk.h \
> > -   win-usb-driver-install.h\
> > -   win-usb-driver-install.c\
> >     usbdk_api.h \
> >     usbdk_api.c \
> >     $(NULL)
> > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> > index 6d10daa..7f0b263 100644
> > --- a/src/usb-device-manager.c
> > +++ b/src/usb-device-manager.c
> > @@ -35,7 +35,6 @@
> >  #include 
> >  #elif defined(G_OS_WIN32)
> >  #include "win-usb-dev.h"
> > -#include "win-usb-driver-install.h"
> >  #define USE_GUDEV /* win-usb-dev.h provides a fake gudev
> > interface */
> >  #elif !defined USE_LIBUSB_HOTPLUG
> >  #error "Expecting one of USE_GUDEV or USE_LIBUSB_HOTPLUG to be
> > defined"
> > @@ -127,9 +126,7 @@ struct _SpiceUsbDeviceManagerPrivate {
> >  #ifdef G_OS_WIN32
> >  usbdk_api_wrapper *usbdk_api;
> >  HANDLE usbdk_hider_handle;
> > -SpiceWinUsbDriver *installer;
> >  #endif
> > -gboolean   use_usbclerk;
> >  #endif
> >  GPtrArray *devices;
> >  GPtrArray *channels;
> > @@ -188,9 +185,6 @@ static SpiceUsbDevice
> > *spice_usb_device_ref(SpiceUsbDevice *device);
> >  static void spice_usb_device_unref(SpiceUsbDevice *device);
> >  
> >  #ifdef G_OS_WIN32
> > -static guint8 spice_usb_device_get_state(SpiceUsbDevice *device);
> > -static void  spice_usb_device_set_state(SpiceUsbDevice *device,
> > guint8 s);
> > -
> >  static void _usbdk_hider_update(SpiceUsbDeviceManager *manager);
> >  static void _usbdk_hider_clear(SpiceUsbDeviceManager *manager);
> >  #endif
> > @@ -274,8 +268,11 @@ static void
> > spice_usb_device_manager_init(SpiceUsbDeviceManager *self)
> >  self->priv = priv;
> >  
> >  #if defined(G_OS_WIN32) && defined(USE_USBREDIR)
> > -priv->use_usbclerk = !usbdk_is_driver_installed() ||
> > - !(priv->usbdk_api = usbdk_api_load());
> > +if (usbdk_is_driver_installed()) {
> > +priv->usbdk_api = usbdk_api_load();
> > +} else {
> > +spice_debug("UsbDk driver is not installed");
> > +}
> >  #endif
> >  priv->channels = g_ptr_array_new();
> >  #ifdef USE_USBREDIR
> > @@ -298,16 +295,6 @@ static gboolean
> > spice_usb_device_manager_initable_init(GInitable  *initable,
> >  const gchar *const subsystems[] = {"usb", NULL};
> >  #endif
> >  
> > -#ifdef G_OS_WIN32
> > -if (priv->use_usbclerk) {
> > -priv->installer = spice_win_usb_driver_new(err);
> > -if (!priv->installer) {
> > -SPICE_DEBUG("failed to initialize winusb driver");
> > -return FALSE;
> > -}
> > -}
> > -#endif
> > -
> >  /* Initialize libusb */
> >  rc = libusb_init(&priv->context);
> >  if (rc < 0) {
> > @@ -425,14 +412,8 @@ static void
> > spice_usb_device_manager_finalize(GObject *gobject)
> >  free(priv->auto_conn_filter_rules);
> >  free(priv->redirect_on_connect_rules);
> >  #ifdef G_OS_WIN32
> > -if (priv->installer) {
> > -g_warn_if_fail(priv->use_usbclerk);
> > -g_object_unref(priv->installer);
> > -}
> > -if (!priv->use_usbclerk) {
> > -_usbdk_hi

[Spice-devel] [vdagent-win PATCH] vdservice: drop "RHEV" from service name

2016-12-22 Thread Uri Lublin
---
 vdservice/vdservice.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index 1892b72..dc49ec5 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -25,7 +25,7 @@
 
 //#define DEBUG_VDSERVICE
 
-#define VD_SERVICE_DISPLAY_NAME TEXT("RHEV Spice Agent")
+#define VD_SERVICE_DISPLAY_NAME TEXT("Spice Agent")
 #define VD_SERVICE_NAME TEXT("vdservice")
 #define VD_SERVICE_DESCRIPTION  TEXT("Enables Spice event injection and 
display configuration.")
 #define VD_SERVICE_LOG_PATH TEXT("%svdservice.log")
-- 
2.9.3

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


[Spice-devel] [spice-gtk 2/3] ssl: Rework our custom BIO type

2016-12-22 Thread Christophe Fergeau
From: Sebastian Andrzej Siewior 

This commit changes to an actual new BIO method rather than reusing an
existing BIO method, and overriding only the fields that we need.
The approach before this commit would be causing issues with OpenSSL
1.1.0 as some of the fields we access have become opaque.
---
 src/bio-gio.c | 60 +++
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/src/bio-gio.c b/src/bio-gio.c
index b310c97..20ee57a 100644
--- a/src/bio-gio.c
+++ b/src/bio-gio.c
@@ -23,21 +23,29 @@
 #include "spice-util.h"
 #include "bio-gio.h"
 
-typedef struct bio_gsocket_method {
-BIO_METHOD method;
-GIOStream *stream;
-} bio_gsocket_method;
+static long bio_gio_ctrl(BIO *b, int cmd, long num, void *ptr)
+{
+long ret;
 
-#define BIO_GET_GSOCKET(bio)  (((bio_gsocket_method*)bio->method)->gsocket)
-#define BIO_GET_ISTREAM(bio)  
(g_io_stream_get_input_stream(((bio_gsocket_method*)bio->method)->stream))
-#define BIO_GET_OSTREAM(bio)  
(g_io_stream_get_output_stream(((bio_gsocket_method*)bio->method)->stream))
+switch (cmd) {
+case BIO_CTRL_FLUSH:
+ret = 1;
+break;
+default:
+ret = 0;
+break;
+}
+return ret;
+}
 
 static int bio_gio_write(BIO *bio, const char *in, int inl)
 {
+GOutputStream *stream;
 gssize ret;
 GError *error = NULL;
 
-ret = 
g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(BIO_GET_OSTREAM(bio)),
+stream = g_io_stream_get_output_stream(bio->ptr);
+ret = 
g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(stream),
  in, inl, NULL, &error);
 BIO_clear_retry_flags(bio);
 
@@ -53,10 +61,12 @@ static int bio_gio_write(BIO *bio, const char *in, int inl)
 
 static int bio_gio_read(BIO *bio, char *out, int outl)
 {
+GInputStream *stream;
 gssize ret;
 GError *error = NULL;
 
-ret = 
g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(BIO_GET_ISTREAM(bio)),
+stream = g_io_stream_get_input_stream(bio->ptr);
+ret = 
g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(stream),
out, outl, NULL, &error);
 BIO_clear_retry_flags(bio);
 
@@ -70,17 +80,6 @@ static int bio_gio_read(BIO *bio, char *out, int outl)
 return ret;
 }
 
-static int bio_gio_destroy(BIO *bio)
-{
-if (bio == NULL || bio->method == NULL)
-return 0;
-
-SPICE_DEBUG("bio gsocket destroy");
-g_clear_pointer(&bio->method, g_free);
-
-return 1;
-}
-
 static int bio_gio_puts(BIO *bio, const char *str)
 {
 int n, ret;
@@ -94,20 +93,25 @@ static int bio_gio_puts(BIO *bio, const char *str)
 G_GNUC_INTERNAL
 BIO* bio_new_giostream(GIOStream *stream)
 {
-// TODO: make an actual new BIO type, or just switch to GTls already...
-BIO *bio = BIO_new_socket(-1, BIO_NOCLOSE);
+BIO *bio;
+static BIO_METHOD bio_gio_method;
 
-bio_gsocket_method *bio_method = g_new(bio_gsocket_method, 1);
-bio_method->method = *bio->method;
-bio_method->stream = stream;
+if (bio_gio_method.name == NULL) {
+bio_gio_method.type = 128 | BIO_TYPE_SOURCE_SINK;
+bio_gio_method.name = "gio stream";
+}
 
-bio->method->destroy(bio);
-bio->method = (BIO_METHOD*)bio_method;
+bio = BIO_new(&bio_gio_method);
+if (!bio)
+return NULL;
 
 bio->method->bwrite = bio_gio_write;
 bio->method->bread = bio_gio_read;
 bio->method->bputs = bio_gio_puts;
-bio->method->destroy = bio_gio_destroy;
+bio->method->ctrl = bio_gio_ctrl;
+
+bio->init = 1;
+bio->ptr = stream;
 
 return bio;
 }
-- 
2.9.3

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


[Spice-devel] [spice-gtk 0/3] ssl: Add support for OpenSSL 1.1.0

2016-12-22 Thread Christophe Fergeau
Hey,

Sebastian sent these patches privately a while ago, I've run some tests on them
and helped split them. They add support for OpenSSL 1.1.0 which makes some of 
the
structures we were directly accessing private. This also keeps support with 
older
OpenSSL releases by adding some compat helpers.

These patches have been tested against a RHEV instance, and against manually 
configured
QEMU+SPICE+TLS.

Christophe

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


[Spice-devel] [spice-gtk 1/3] ssl: Stop creating our own X509_LOOKUP_METHOD

2016-12-22 Thread Christophe Fergeau
From: Sebastian Andrzej Siewior 

OpenSSL 1.1.0 does not seem to provide API to do that anymore.

There is no need to create a custom lookup to begin with. This method
here has no callbacks implemented and is doing nothing. The way I
understand it, it is used to retrieve a `lookup' object which provides a
certificate store.  The SSL ctx provides also such a store.
---
 src/spice-channel.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 95662f3..6a911a6 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -2352,17 +2352,12 @@ static gboolean spice_channel_delayed_unref(gpointer 
data)
 return FALSE;
 }
 
-static X509_LOOKUP_METHOD spice_x509_mem_lookup = {
-"spice_x509_mem_lookup",
-0
-};
-
 static int spice_channel_load_ca(SpiceChannel *channel)
 {
 SpiceChannelPrivate *c = channel->priv;
 STACK_OF(X509_INFO) *inf;
 X509_INFO *itmp;
-X509_LOOKUP *lookup;
+X509_STORE *store;
 BIO *in;
 int i, count = 0;
 guint8 *ca;
@@ -2372,13 +2367,13 @@ static int spice_channel_load_ca(SpiceChannel *channel)
 
 g_return_val_if_fail(c->ctx != NULL, 0);
 
-lookup = X509_STORE_add_lookup(c->ctx->cert_store, &spice_x509_mem_lookup);
 ca_file = spice_session_get_ca_file(c->session);
 spice_session_get_ca(c->session, &ca, &size);
 
 CHANNEL_DEBUG(channel, "Load CA, file: %s, data: %p", ca_file, ca);
 
 if (ca != NULL) {
+store = SSL_CTX_get_cert_store(c->ctx);
 in = BIO_new_mem_buf(ca, size);
 inf = PEM_X509_INFO_read_bio(in, NULL, NULL, NULL);
 BIO_free(in);
@@ -2386,11 +2381,11 @@ static int spice_channel_load_ca(SpiceChannel *channel)
 for (i = 0; i < sk_X509_INFO_num(inf); i++) {
 itmp = sk_X509_INFO_value(inf, i);
 if (itmp->x509) {
-X509_STORE_add_cert(lookup->store_ctx, itmp->x509);
+X509_STORE_add_cert(store, itmp->x509);
 count++;
 }
 if (itmp->crl) {
-X509_STORE_add_crl(lookup->store_ctx, itmp->crl);
+X509_STORE_add_crl(store, itmp->crl);
 count++;
 }
 }
-- 
2.9.3

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


[Spice-devel] [spice-gtk 3/3] ssl: Use accessors rather than direct struct access

2016-12-22 Thread Christophe Fergeau
From: Sebastian Andrzej Siewior 

In OpenSSL 1.1.0, the struct fields are private so we can no longer
directly access them.

The accessors are not available in previous OpenSSL releases, so we need
to add compat helpers.
---
 src/bio-gio.c   | 103 
 src/spice-channel.c |  11 +-
 2 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/src/bio-gio.c b/src/bio-gio.c
index 20ee57a..701df93 100644
--- a/src/bio-gio.c
+++ b/src/bio-gio.c
@@ -23,6 +23,72 @@
 #include "spice-util.h"
 #include "bio-gio.h"
 
+#if OPENSSL_VERSION_NUMBER < 0x1010
+static BIO_METHOD one_static_bio;
+
+static int BIO_meth_set_read(BIO_METHOD *biom,
+ int (*bread) (BIO *, char *, int))
+{
+biom->bread = bread;
+return 1;
+}
+
+static int BIO_meth_set_write(BIO_METHOD *biom,
+  int (*bwrite) (BIO *, const char *, int))
+{
+biom->bwrite = bwrite;
+return 1;
+}
+
+static int BIO_meth_set_puts(BIO_METHOD *biom,
+ int (*bputs) (BIO *, const char *))
+{
+biom->bputs = bputs;
+return 1;
+}
+
+static int BIO_meth_set_ctrl(BIO_METHOD *biom,
+ long (*ctrl) (BIO *, int, long, void *))
+{
+biom->ctrl = ctrl;
+return 1;
+}
+
+static int BIO_get_new_index(void)
+{
+return 128;
+}
+
+static void BIO_set_init(BIO *a, int init)
+{
+   a->init = init;
+}
+
+static void BIO_set_data(BIO *a, void *ptr)
+{
+a->ptr = ptr;
+}
+
+static void *BIO_get_data(BIO *a)
+{
+return a->ptr;
+}
+
+static BIO_METHOD *BIO_meth_new(int type, const char *name)
+{
+BIO_METHOD *biom = &one_static_bio;
+
+biom->type = type;
+biom->name = name;
+return biom;
+}
+
+static void BIO_meth_free(BIO_METHOD *biom)
+{
+}
+
+#endif
+
 static long bio_gio_ctrl(BIO *b, int cmd, long num, void *ptr)
 {
 long ret;
@@ -44,7 +110,7 @@ static int bio_gio_write(BIO *bio, const char *in, int inl)
 gssize ret;
 GError *error = NULL;
 
-stream = g_io_stream_get_output_stream(bio->ptr);
+stream = g_io_stream_get_output_stream(BIO_get_data(bio));
 ret = 
g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(stream),
  in, inl, NULL, &error);
 BIO_clear_retry_flags(bio);
@@ -65,7 +131,7 @@ static int bio_gio_read(BIO *bio, char *out, int outl)
 gssize ret;
 GError *error = NULL;
 
-stream = g_io_stream_get_input_stream(bio->ptr);
+stream = g_io_stream_get_input_stream(BIO_get_data(bio));
 ret = 
g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(stream),
out, outl, NULL, &error);
 BIO_clear_retry_flags(bio);
@@ -90,28 +156,35 @@ static int bio_gio_puts(BIO *bio, const char *str)
 return ret;
 }
 
+static BIO_METHOD *bio_gio_method;
+
 G_GNUC_INTERNAL
 BIO* bio_new_giostream(GIOStream *stream)
 {
 BIO *bio;
-static BIO_METHOD bio_gio_method;
 
-if (bio_gio_method.name == NULL) {
-bio_gio_method.type = 128 | BIO_TYPE_SOURCE_SINK;
-bio_gio_method.name = "gio stream";
+if (!bio_gio_method) {
+bio_gio_method = BIO_meth_new(BIO_get_new_index() |
+  BIO_TYPE_SOURCE_SINK,
+  "gio stream");
+if (!bio_gio_method)
+return NULL;
+
+if (!BIO_meth_set_write(bio_gio_method, bio_gio_write)
+|| !BIO_meth_set_read(bio_gio_method, bio_gio_read)
+|| !BIO_meth_set_puts(bio_gio_method, bio_gio_puts)
+|| !BIO_meth_set_ctrl(bio_gio_method, bio_gio_ctrl)) {
+BIO_meth_free(bio_gio_method);
+bio_gio_method = NULL;
+return NULL;
+}
 }
 
-bio = BIO_new(&bio_gio_method);
+bio = BIO_new(bio_gio_method);
 if (!bio)
 return NULL;
 
-bio->method->bwrite = bio_gio_write;
-bio->method->bread = bio_gio_read;
-bio->method->bputs = bio_gio_puts;
-bio->method->ctrl = bio_gio_ctrl;
-
-bio->init = 1;
-bio->ptr = stream;
-
+BIO_set_init(bio, 1);
+BIO_set_data(bio, stream);
 return bio;
 }
diff --git a/src/spice-channel.c b/src/spice-channel.c
index 6a911a6..6556db3 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -55,6 +55,15 @@ static void spice_channel_reset_capabilities(SpiceChannel 
*channel);
 static void spice_channel_send_migration_handshake(SpiceChannel *channel);
 static gboolean channel_connect(SpiceChannel *channel, gboolean tls);
 
+#if OPENSSL_VERSION_NUMBER < 0x1010
+static RSA *EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
+{
+if (pkey->type != EVP_PKEY_RSA) {
+return NULL;
+}
+return pkey->pkey.rsa;
+}
+#endif
 /**
  * SECTION:spice-channel
  * @short_description: the base channel class
@@ -1161,7 +1170,7 @@ static SpiceChannelEvent 
spice_channel_send_spice_ticket(SpiceChannel *channel)
 pubkey 

Re: [Spice-devel] [vdagent-win PATCH] vdservice: drop "RHEV" from service name

2016-12-22 Thread Victor Toso
Hi,

On Thu, Dec 22, 2016 at 06:02:00PM +0200, Uri Lublin wrote:
> ---
>  vdservice/vdservice.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
> index 1892b72..dc49ec5 100644
> --- a/vdservice/vdservice.cpp
> +++ b/vdservice/vdservice.cpp
> @@ -25,7 +25,7 @@
>  
>  //#define DEBUG_VDSERVICE
>  
> -#define VD_SERVICE_DISPLAY_NAME TEXT("RHEV Spice Agent")

Acked-by: Victor Toso 

> +#define VD_SERVICE_DISPLAY_NAME TEXT("Spice Agent")
>  #define VD_SERVICE_NAME TEXT("vdservice")
>  #define VD_SERVICE_DESCRIPTION  TEXT("Enables Spice event injection and 
> display configuration.")
>  #define VD_SERVICE_LOG_PATH TEXT("%svdservice.log")
> -- 
> 2.9.3
> 
> ___
> 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 v9 03/11] Make RedChannelClient::incoming private

2016-12-22 Thread Jonathon Jongsma
I'd like additional comments in the commit log explaining why we can do
this (because we refactored the sound stuff to use the RedChannelClient
infrastructure, etc)

The code looks fine though

Acked-by: Jonathon Jongsma 


On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/red-channel-client-private.h | 11 +++
>  server/red-channel-client.c | 12 ++--
>  server/red-channel-client.h | 13 -
>  3 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/server/red-channel-client-private.h b/server/red-
> channel-client-private.h
> index 593fee5..d01cdbd 100644
> --- a/server/red-channel-client-private.h
> +++ b/server/red-channel-client-private.h
> @@ -50,6 +50,16 @@ typedef struct OutgoingHandler {
>  int size;
>  } OutgoingHandler;
>  
> +typedef struct IncomingHandler {
> +IncomingHandlerInterface *cb;
> +void *opaque;
> +uint8_t header_buf[MAX_HEADER_SIZE];
> +SpiceDataHeaderOpaque header;
> +uint32_t header_pos;
> +uint8_t *msg; // data of the msg following the header. allocated
> by alloc_msg_buf.
> +uint32_t msg_pos;
> +} IncomingHandler;
> +
>  struct RedChannelClientPrivate
>  {
>  RedChannel *channel;
> @@ -95,6 +105,7 @@ struct RedChannelClientPrivate
>  RedChannelClientLatencyMonitor latency_monitor;
>  RedChannelClientConnectivityMonitor connectivity_monitor;
>  
> +IncomingHandler incoming;
>  OutgoingHandler outgoing;
>  };
>  
> diff --git a/server/red-channel-client.c b/server/red-channel-
> client.c
> index 8312d3e..0002951 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -268,8 +268,8 @@ static void
> red_channel_client_constructed(GObject *object)
>  {
>  RedChannelClient *self =  RED_CHANNEL_CLIENT(object);
>  
> -self->incoming.opaque = self;
> -self->incoming.cb = red_channel_get_incoming_handler(self->priv-
> >channel);
> +self->priv->incoming.opaque = self;
> +self->priv->incoming.cb = red_channel_get_incoming_handler(self-
> >priv->channel);
>  
>  self->priv->outgoing.opaque = self;
>  self->priv->outgoing.cb = red_channel_get_outgoing_handler(self-
> >priv->channel);
> @@ -277,15 +277,15 @@ static void
> red_channel_client_constructed(GObject *object)
>  self->priv->outgoing.size = 0;
>  
>  if (red_channel_client_test_remote_common_cap(self,
> SPICE_COMMON_CAP_MINI_HEADER)) {
> -self->incoming.header = mini_header_wrapper;
> +self->priv->incoming.header = mini_header_wrapper;
>  self->priv->send_data.header = mini_header_wrapper;
>  self->priv->is_mini_header = TRUE;
>  } else {
> -self->incoming.header = full_header_wrapper;
> +self->priv->incoming.header = full_header_wrapper;
>  self->priv->send_data.header = full_header_wrapper;
>  self->priv->is_mini_header = FALSE;
>  }
> -self->incoming.header.data = self->incoming.header_buf;
> +self->priv->incoming.header.data = self->priv-
> >incoming.header_buf;
>  }
>  
>  static void red_channel_client_class_init(RedChannelClientClass
> *klass)
> @@ -1178,7 +1178,7 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>  void red_channel_client_receive(RedChannelClient *rcc)
>  {
>  g_object_ref(rcc);
> -red_peer_handle_incoming(rcc->priv->stream, &rcc->incoming);
> +red_peer_handle_incoming(rcc->priv->stream, &rcc->priv-
> >incoming);
>  g_object_unref(rcc);
>  }
>  
> diff --git a/server/red-channel-client.h b/server/red-channel-
> client.h
> index 0d404d1..fada609 100644
> --- a/server/red-channel-client.h
> +++ b/server/red-channel-client.h
> @@ -190,23 +190,10 @@ gboolean
> red_channel_client_set_migration_seamless(RedChannelClient *rcc);
>  void red_channel_client_set_destroying(RedChannelClient *rcc);
>  gboolean red_channel_client_is_destroying(RedChannelClient *rcc);
>  
> -typedef struct IncomingHandler {
> -IncomingHandlerInterface *cb;
> -void *opaque;
> -uint8_t header_buf[MAX_HEADER_SIZE];
> -SpiceDataHeaderOpaque header;
> -uint32_t header_pos;
> -uint8_t *msg; // data of the msg following the header. allocated
> by alloc_msg_buf.
> -uint32_t msg_pos;
> -} IncomingHandler;
> -
>  struct RedChannelClient
>  {
>  GObject parent;
>  
> -/* protected */
> -IncomingHandler incoming;
> -
>  RedChannelClientPrivate *priv;
>  };
>  
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 04/11] sound: Free more on SndChannel finalize

2016-12-22 Thread Jonathon Jongsma
Good, but I think the commit log is a little bit misleading. After
reading the summary, I thought you were fixing a leak. But you're
actually just moving the freeing of these variables to finalize instead
of doing it in detach. I'd prefer something more like:

-
sound: free SndChannel data in finalize()

Move the freeing of SndChannel data members from snd_detach_common() to
the finalize function to encapsulate things a bit more cleanly. It
doesn't really change the behavior or order of destruction since
snd_detach_common() destroys the channel.
-

Acked-by: Jonathon Jongsma 



On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 19 ---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index a645b60..427ae8f 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1436,10 +1436,26 @@ snd_channel_init(SndChannel *self)
>  }
>  
>  static void
> +snd_channel_finalize(GObject *object)
> +{
> +SndChannel *channel = SND_CHANNEL(object);
> +
> +remove_channel(channel);
> +
> +free(channel->volume.volume);
> +channel->volume.volume = NULL;
> +
> +G_OBJECT_CLASS(snd_channel_parent_class)->finalize(object);
> +}
> +
> +static void
>  snd_channel_class_init(SndChannelClass *klass)
>  {
> +GObjectClass *object_class = G_OBJECT_CLASS(klass);
>  RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>  
> +object_class->finalize = snd_channel_finalize;
> +
>  channel_class->config_socket = snd_channel_config_socket;
>  channel_class->alloc_recv_buf =
> snd_channel_client_alloc_recv_buf;
>  channel_class->release_recv_buf =
> snd_channel_client_release_recv_buf;
> @@ -1555,10 +1571,7 @@ static void snd_detach_common(SndChannel
> *channel)
>  }
>  RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
> -remove_channel(channel);
>  reds_unregister_channel(reds, RED_CHANNEL(channel));
> -free(channel->volume.volume);
> -channel->volume.volume = NULL;
>  red_channel_destroy(RED_CHANNEL(channel));
>  }
>  
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 05/11] sound: Use default disconnect for client channels

2016-12-22 Thread Jonathon Jongsma
OK, I don't see any downsides here. 

Acked-by: Jonathon Jongsma 



On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 19 ---
>  1 file changed, 19 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 427ae8f..9263a23 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -819,23 +819,6 @@
> snd_channel_client_release_recv_buf(RedChannelClient *rcc, uint16_t
> type, uint32
>  }
>  }
>  
> -static void snd_disconnect_channel_client(RedChannelClient *rcc)
> -{
> -SndChannel *channel;
> -RedChannel *red_channel = red_channel_client_get_channel(rcc);
> -uint32_t type;
> -
> -channel = SND_CHANNEL(red_channel);
> -spice_assert(channel);
> -g_object_get(red_channel, "channel-type", &type, NULL);
> -
> -spice_debug("channel-type=%d", type);
> -if (channel->connection) {
> -spice_assert(RED_CHANNEL_CLIENT(channel->connection) ==
> rcc);
> -red_channel_client_disconnect(rcc);
> -}
> -}
> -
>  static void snd_set_command(SndChannelClient *client, uint32_t
> command)
>  {
>  if (!client) {
> @@ -1477,7 +1460,6 @@ playback_channel_constructed(GObject *object)
>  G_OBJECT_CLASS(playback_channel_parent_class)-
> >constructed(object);
>  
>  client_cbs.connect = snd_set_playback_peer;
> -client_cbs.disconnect = snd_disconnect_channel_client;
>  client_cbs.migrate = snd_playback_migrate_channel_client;
>  red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> self);
>  
> @@ -1528,7 +1510,6 @@ record_channel_constructed(GObject *object)
>  G_OBJECT_CLASS(record_channel_parent_class)-
> >constructed(object);
>  
>  client_cbs.connect = snd_set_record_peer;
> -client_cbs.disconnect = snd_disconnect_channel_client;
>  client_cbs.migrate = snd_record_migrate_channel_client;
>  red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> self);
>  
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 06/11] sound: Reuse code for snd_set_{playback, record}_peer

2016-12-22 Thread Jonathon Jongsma
Acked-by: Jonathon Jongsma 


On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Almost identical beside the type.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 53 +++-
> ---
>  1 file changed, 16 insertions(+), 37 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 9263a23..74a4cc2 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1086,9 +1086,9 @@ playback_channel_client_constructed(GObject
> *object)
>  snd_send(SND_CHANNEL_CLIENT(playback_client));
>  }
>  
> -static void snd_set_playback_peer(RedChannel *red_channel, RedClient
> *client, RedsStream *stream,
> -  int migration, int
> num_common_caps, uint32_t *common_caps,
> -  int num_caps, uint32_t *caps)
> +static void snd_set_peer(RedChannel *red_channel, RedClient *client,
> RedsStream *stream,
> + int migration, int num_common_caps,
> uint32_t *common_caps,
> + int num_caps, uint32_t *caps, GType type)
>  {
>  SndChannel *channel = SND_CHANNEL(red_channel);
>  GArray *common_caps_array = NULL, *caps_array = NULL;
> @@ -1108,7 +1108,7 @@ static void snd_set_playback_peer(RedChannel
> *red_channel, RedClient *client, Re
>  g_array_append_vals(caps_array, caps, num_caps);
>  }
>  
> -g_initable_new(TYPE_PLAYBACK_CHANNEL_CLIENT,
> +g_initable_new(type,
> NULL, NULL,
> "channel", channel,
> "client", client,
> @@ -1125,6 +1125,15 @@ static void snd_set_playback_peer(RedChannel
> *red_channel, RedClient *client, Re
>  }
>  }
>  
> +static void snd_set_playback_peer(RedChannel *red_channel, RedClient
> *client, RedsStream *stream,
> +  int migration, int
> num_common_caps, uint32_t *common_caps,
> +  int num_caps, uint32_t *caps)
> +{
> +snd_set_peer(red_channel, client, stream, migration,
> + num_common_caps, common_caps, num_caps, caps,
> + TYPE_PLAYBACK_CHANNEL_CLIENT);
> +}
> +
>  static void snd_record_migrate_channel_client(RedChannelClient *rcc)
>  {
>  SndChannel *channel;
> @@ -1342,39 +1351,9 @@ static void snd_set_record_peer(RedChannel
> *red_channel, RedClient *client, Reds
>  int migration, int num_common_caps,
> uint32_t *common_caps,
>  int num_caps, uint32_t *caps)
>  {
> -SndChannel *channel = SND_CHANNEL(red_channel);
> -GArray *common_caps_array = NULL, *caps_array = NULL;
> -
> -if (channel->connection) {
> -red_channel_client_disconnect(RED_CHANNEL_CLIENT(channel-
> >connection));
> -channel->connection = NULL;
> -}
> -
> -if (common_caps) {
> -common_caps_array = g_array_sized_new(FALSE, FALSE, sizeof
> (*common_caps),
> -  num_common_caps);
> -g_array_append_vals(common_caps_array, common_caps,
> num_common_caps);
> -}
> -if (caps) {
> -caps_array = g_array_sized_new(FALSE, FALSE, sizeof (*caps),
> num_caps);
> -g_array_append_vals(caps_array, caps, num_caps);
> -}
> -
> -g_initable_new(TYPE_RECORD_CHANNEL_CLIENT,
> -   NULL, NULL,
> -   "channel", channel,
> -   "client", client,
> -   "stream", stream,
> -   "caps", caps_array,
> -   "common-caps", common_caps_array,
> -   NULL);
> -
> -if (caps_array) {
> -g_array_unref(caps_array);
> -}
> -if (common_caps_array) {
> -g_array_unref(common_caps_array);
> -}
> +snd_set_peer(red_channel, client, stream, migration,
> + num_common_caps, common_caps, num_caps, caps,
> + TYPE_RECORD_CHANNEL_CLIENT);
>  }
>  
>  static void snd_playback_migrate_channel_client(RedChannelClient
> *rcc)
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 07/11] sound: Reuse code for migrating client channels

2016-12-22 Thread Jonathon Jongsma
OK, seems fine.

Acked-by: Jonathon Jongsma 



On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> We support only a single client so don't waste code just
> to check this.
> The worst stuff can happen is that we'll migrate multiple
> connections.
> 
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 35 +--
>  1 file changed, 5 insertions(+), 30 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 74a4cc2..32c712f 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1134,19 +1134,10 @@ static void snd_set_playback_peer(RedChannel
> *red_channel, RedClient *client, Re
>   TYPE_PLAYBACK_CHANNEL_CLIENT);
>  }
>  
> -static void snd_record_migrate_channel_client(RedChannelClient *rcc)
> +static void snd_migrate_channel_client(RedChannelClient *rcc)
>  {
> -SndChannel *channel;
> -RedChannel *red_channel = red_channel_client_get_channel(rcc);
> -
> -channel = SND_CHANNEL(red_channel);
> -spice_assert(channel);
> -
> -if (channel->connection) {
> -spice_assert(RED_CHANNEL_CLIENT(channel->connection) ==
> rcc);
> -snd_set_command(channel->connection, SND_MIGRATE_MASK);
> -snd_send(channel->connection);
> -}
> +snd_set_command(SND_CHANNEL_CLIENT(rcc), SND_MIGRATE_MASK);
> +snd_send(SND_CHANNEL_CLIENT(rcc));
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_record_set_volume(SpiceRecordInstance *sin,
> @@ -1356,22 +1347,6 @@ static void snd_set_record_peer(RedChannel
> *red_channel, RedClient *client, Reds
>   TYPE_RECORD_CHANNEL_CLIENT);
>  }
>  
> -static void snd_playback_migrate_channel_client(RedChannelClient
> *rcc)
> -{
> -SndChannel *channel;
> -RedChannel *red_channel = red_channel_client_get_channel(rcc);
> -
> -channel = SND_CHANNEL(red_channel);
> -spice_assert(channel);
> -spice_debug(NULL);
> -
> -if (channel->connection) {
> -spice_assert(RED_CHANNEL_CLIENT(channel->connection) ==
> rcc);
> -snd_set_command(channel->connection, SND_MIGRATE_MASK);
> -snd_send(channel->connection);
> -}
> -}
> -
>  static void add_channel(SndChannel *channel)
>  {
>  channel->next = snd_channels;
> @@ -1439,7 +1414,7 @@ playback_channel_constructed(GObject *object)
>  G_OBJECT_CLASS(playback_channel_parent_class)-
> >constructed(object);
>  
>  client_cbs.connect = snd_set_playback_peer;
> -client_cbs.migrate = snd_playback_migrate_channel_client;
> +client_cbs.migrate = snd_migrate_channel_client;
>  red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> self);
>  
>  if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1,
> SND_CODEC_ANY_FREQUENCY)) {
> @@ -1489,7 +1464,7 @@ record_channel_constructed(GObject *object)
>  G_OBJECT_CLASS(record_channel_parent_class)-
> >constructed(object);
>  
>  client_cbs.connect = snd_set_record_peer;
> -client_cbs.migrate = snd_record_migrate_channel_client;
> +client_cbs.migrate = snd_migrate_channel_client;
>  red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> self);
>  
>  if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1,
> SND_CODEC_ANY_FREQUENCY)) {
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 08/11] sound: Reuse code to set volume and mute

2016-12-22 Thread Jonathon Jongsma
Acked-by: Jonathon Jongsma 



On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 51 +---
> ---
>  1 file changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 32c712f..fc3a92c 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -827,12 +827,11 @@ static void snd_set_command(SndChannelClient
> *client, uint32_t command)
>  client->command |= command;
>  }
>  
> -SPICE_GNUC_VISIBLE void
> spice_server_playback_set_volume(SpicePlaybackInstance *sin,
> -  uint8_t nchannels,
> -  uint16_t *volume)
> +static void snd_channel_set_volume(SndChannel *channel,
> +   uint8_t nchannels, uint16_t
> *volume)
>  {
> -SpiceVolumeState *st = &sin->st->channel.volume;
> -SndChannelClient *client = sin->st->channel.connection;
> +SpiceVolumeState *st = &channel->volume;
> +SndChannelClient *client = channel->connection;
>  
>  st->volume_nchannels = nchannels;
>  free(st->volume);
> @@ -845,10 +844,17 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_set_volume(SpicePlaybackInstance *
>  snd_send(client);
>  }
>  
> -SPICE_GNUC_VISIBLE void
> spice_server_playback_set_mute(SpicePlaybackInstance *sin, uint8_t
> mute)
> +SPICE_GNUC_VISIBLE void
> spice_server_playback_set_volume(SpicePlaybackInstance *sin,
> +  uint8_t nchannels,
> +  uint16_t *volume)
>  {
> -SpiceVolumeState *st = &sin->st->channel.volume;
> -SndChannelClient *client = sin->st->channel.connection;
> +snd_channel_set_volume(&sin->st->channel, nchannels, volume);
> +}
> +
> +static void snd_channel_set_mute(SndChannel *channel, uint8_t mute)
> +{
> +SpiceVolumeState *st = &channel->volume;
> +SndChannelClient *client = channel->connection;
>  
>  st->mute = mute;
>  
> @@ -859,6 +865,11 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_set_mute(SpicePlaybackInstance *si
>  snd_send(client);
>  }
>  
> +SPICE_GNUC_VISIBLE void
> spice_server_playback_set_mute(SpicePlaybackInstance *sin, uint8_t
> mute)
> +{
> +snd_channel_set_mute(&sin->st->channel, mute);
> +}
> +
>  static void snd_playback_start(SndChannel *channel)
>  {
>  SndChannelClient *client = channel->connection;
> @@ -1144,32 +1155,12 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_set_volume(SpiceRecordInstance *sin,
>  uint8_t nchannels,
>  uint16_t *volume)
>  {
> -SpiceVolumeState *st = &sin->st->channel.volume;
> -SndChannelClient *client = sin->st->channel.connection;
> -
> -st->volume_nchannels = nchannels;
> -free(st->volume);
> -st->volume = spice_memdup(volume, sizeof(uint16_t) * nchannels);
> -
> -if (!client || nchannels == 0)
> -return;
> -
> -snd_set_command(client, SND_VOLUME_MUTE_MASK);
> -snd_send(client);
> +snd_channel_set_volume(&sin->st->channel, nchannels, volume);
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_record_set_mute(SpiceRecordInstance *sin, uint8_t mute)
>  {
> -SpiceVolumeState *st = &sin->st->channel.volume;
> -SndChannelClient *client = sin->st->channel.connection;
> -
> -st->mute = mute;
> -
> -if (!client)
> -return;
> -
> -snd_set_command(client, SND_VOLUME_MUTE_MASK);
> -snd_send(client);
> +snd_channel_set_mute(&sin->st->channel, mute);
>  }
>  
>  static void snd_record_start(SndChannel *channel)
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 10/11] sound: Make clear active and client_active are boolean

2016-12-22 Thread Jonathon Jongsma
Acked-by: Jonathon Jongsma 


On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 33d0e96..351cd8d 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -93,8 +93,8 @@ GType snd_channel_client_get_type(void)
> G_GNUC_CONST;
>  struct SndChannelClient {
>  RedChannelClient parent;
>  
> -int active;
> -int client_active;
> +gboolean active;
> +gboolean client_active;
>  
>  uint32_t command;
>  
> @@ -176,7 +176,7 @@ struct SndChannel {
>  SndChannelClient *connection; /* Only one client is supported */
>  SndChannel *next; /* For the global SndChannel list */
>  
> -int active;
> +gboolean active;
>  SpiceVolumeState volume;
>  uint32_t frequency;
>  };
> @@ -860,7 +860,7 @@ static void snd_playback_start(SndChannel
> *channel)
>  {
>  SndChannelClient *client = channel->connection;
>  
> -channel->active = 1;
> +channel->active = TRUE;
>  if (!client)
>  return;
>  spice_assert(!client->active);
> @@ -883,7 +883,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_stop(SpicePlaybackInstance *sin)
>  {
>  SndChannelClient *client = sin->st->channel.connection;
>  
> -sin->st->channel.active = 0;
> +sin->st->channel.active = FALSE;
>  if (!client)
>  return;
>  PlaybackChannelClient *playback_client =
> PLAYBACK_CHANNEL_CLIENT(client);
> @@ -1153,7 +1153,7 @@ static void snd_record_start(SndChannel
> *channel)
>  {
>  SndChannelClient *client = channel->connection;
>  
> -channel->active = 1;
> +channel->active = TRUE;
>  if (!client) {
>  return;
>  }
> @@ -1179,7 +1179,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_stop(SpiceRecordInstance *sin)
>  {
>  SndChannelClient *client = sin->st->channel.connection;
>  
> -sin->st->channel.active = 0;
> +sin->st->channel.active = FALSE;
>  if (!client)
>  return;
>  spice_assert(client->active);
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v9 11/11] sound: Use memcpy instead of manually copy volume array

2016-12-22 Thread Jonathon Jongsma
On Tue, 2016-12-20 at 17:44 +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  server/sound.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 351cd8d..836a525 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -386,7 +386,6 @@ static int
> snd_playback_send_migrate(PlaybackChannelClient *client)
>  static int snd_send_volume(SndChannelClient *client, uint32_t cap,
> int msg)
>  {
>  SpiceMsgAudioVolume *vol;
> -uint8_t c;
>  RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
>  SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
>  SndChannel *channel =
> SND_CHANNEL(red_channel_client_get_channel(rcc));
> @@ -400,9 +399,8 @@ static int snd_send_volume(SndChannelClient
> *client, uint32_t cap, int msg)
>   st->volume_nchannels * sizeof (uint16_t));
>  red_channel_client_init_send_data(rcc, msg);
>  vol->nchannels = st->volume_nchannels;
> -for (c = 0; c < st->volume_nchannels; ++c) {
> -vol->volume[c] = st->volume[c];
> -}
> +SPICE_VERIFY(sizeof(vol->volume[0]) == sizeof(st->volume[0]));
> +memcpy(vol->volume, st->volume, sizeof(st->volume[0] * st-
> >volume_nchannels));

This doesn't look right to me. Shouldn't the third argument be 

sizeof(st->volume[0]) * st->volume_nchannels

instead of

sizeof(st->volume[0] * st->volume_nchannels)



>  spice_marshall_SpiceMsgAudioVolume(m, vol);
>  
>  red_channel_client_begin_send_message(rcc);

Reviewed-by: Jonathon Jongsma 


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


Re: [Spice-devel] [spice-gtk v1] file-xfer: differentiate error from host/guest and client

2016-12-22 Thread Jonathon Jongsma



On Thu, 2016-12-22 at 14:17 +0100, Victor Toso wrote:
> Hi,
> 
> On Thu, Dec 22, 2016 at 11:35:13AM +0100, Pavel Grunt wrote:
> > Hi,
> > 
> > it works. But do we need a new public symbol ? It is an internal
> > stuff
> > for channel-main.
> > 
> > Pavel
> 
> Yeah, I was also thinking if this is really necessary.
> 
> Application can track SpiceFileTransferTask "finished" signal too, so
> I
> thought this distinction in GError would make sense even though the
> main
> API function still uses SPICE_CLIENT_ERROR_FAILED error code in that
> case.
> 
> The changes in the code are quite minor this way too, but this is not
> really important.
> 
> Pinging Jonathon for hints/ideas/opinions :)

Can you expand a little bit about what actual behavior you're trying to
fix? I'm not quite sure I understand the problem yet.



> 
>   toso
> 
> > 
> > On Wed, 2016-12-21 at 18:55 +0100, Victor Toso wrote:
> > > From: Victor Toso 
> > > 
> > > During file transfer, we can have error and cancellation in
> > > client,
> > > host or guest side.
> > > 
> > > If error happens in the client side, we must send a message to
> > > the
> > > guest, VD_AGENT_FILE_XFER_STATUS, with either _STATUS_CANCELLED
> > > or
> > > _STATUS_ERROR.
> > > 
> > > But the current code is also sending a message to the guest when
> > > error/cancellation does not come from client, leading to
> > > unexpected
> > > messages to be received in host/guest.
> > > 
> > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED is introduced in this
> > > patch
> > > to
> > > mark a SpiceFileTransferTask to be failed due host or guest
> > > problems.
> > > 
> > > The generic error code SPICE_CLIENT_ERROR_FAILED is still used in
> > > spice_main_file_copy_async() API.
> > > 
> > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=99170
> > > Signed-off-by: Victor Toso 
> > > Reported-by: Pavel Grunt 
> > > ---
> > >  src/channel-main.c | 11 +++
> > >  src/spice-client.h |  1 +
> > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/channel-main.c b/src/channel-main.c
> > > index ed5d611..e92d363 100644
> > > --- a/src/channel-main.c
> > > +++ b/src/channel-main.c
> > > @@ -1862,18 +1862,18 @@ static void
> > > main_agent_handle_xfer_status(SpiceMainChannel *channel,
> > >  spice_file_transfer_task_read_async(xfer_task,
> > > file_xfer_read_async_cb, xfer_op);
> > >  return;
> > >  case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> > > -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > >  _("The spice agent cancelled
> > > the file transfer"));
> > >  break;
> > >  case VD_AGENT_FILE_XFER_STATUS_ERROR:
> > > -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > >  _("The spice agent reported
> > > an
> > > error during the file transfer"));
> > >  break;
> > >  case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
> > >  break;
> > >  default:
> > >  g_warn_if_reached();
> > > -error = g_error_new(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > +error = g_error_new(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > >  "unhandled status type: %u", msg-
> > > > result);
> > > 
> > >  break;
> > >  }
> > > @@ -2960,7 +2960,10 @@ static void
> > > file_transfer_operation_task_finished(SpiceFileTransferTask
> > > *xfer_ta
> > >  task_id = spice_file_transfer_task_get_id(xfer_task);
> > >  g_return_if_fail(task_id != 0);
> > >  
> > > -if (error) {
> > > +if (g_error_matches(error, SPICE_CLIENT_ERROR,
> > > +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED)
> > > ) {
> > > +spice_debug("xfer task: %u failed due cancel/error on
> > > server or agent", task_id);
> > > +} else if (error) {
> > >  VDAgentFileXferStatusMessage msg;
> > >  msg.id = task_id;
> > >  if (g_error_matches(error, G_IO_ERROR,
> > > G_IO_ERROR_CANCELLED)) {
> > > diff --git a/src/spice-client.h b/src/spice-client.h
> > > index 32b79ea..8297537 100644
> > > --- a/src/spice-client.h
> > > +++ b/src/spice-client.h
> > > @@ -82,6 +82,7 @@ typedef enum
> > >  SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> > >  SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
> > >  SPICE_CLIENT_ERROR_USB_SERVICE,
> > > +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > >  } SpiceClientError;
> > >  
> > >  #ifndef SPICE_DISABLE_DEPRECATED
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v1] file-xfer: differentiate error from host/guest and client

2016-12-22 Thread Jonathon Jongsma
On Thu, 2016-12-22 at 11:23 -0600, Jonathon Jongsma wrote:
> 
> 
> On Thu, 2016-12-22 at 14:17 +0100, Victor Toso wrote:
> > Hi,
> > 
> > On Thu, Dec 22, 2016 at 11:35:13AM +0100, Pavel Grunt wrote:
> > > Hi,
> > > 
> > > it works. But do we need a new public symbol ? It is an internal
> > > stuff
> > > for channel-main.
> > > 
> > > Pavel
> > 
> > Yeah, I was also thinking if this is really necessary.
> > 
> > Application can track SpiceFileTransferTask "finished" signal too,
> > so
> > I
> > thought this distinction in GError would make sense even though the
> > main
> > API function still uses SPICE_CLIENT_ERROR_FAILED error code in
> > that
> > case.
> > 
> > The changes in the code are quite minor this way too, but this is
> > not
> > really important.
> > 
> > Pinging Jonathon for hints/ideas/opinions :)
> 
> Can you expand a little bit about what actual behavior you're trying
> to
> fix? I'm not quite sure I understand the problem yet.

Sorry, totally overlooked the bugzilla link on your git commit log.
Nevermind that question.


> 
> 
> 
> > 
> >   toso
> > 
> > > 
> > > On Wed, 2016-12-21 at 18:55 +0100, Victor Toso wrote:
> > > > From: Victor Toso 
> > > > 
> > > > During file transfer, we can have error and cancellation in
> > > > client,
> > > > host or guest side.
> > > > 
> > > > If error happens in the client side, we must send a message to
> > > > the
> > > > guest, VD_AGENT_FILE_XFER_STATUS, with either _STATUS_CANCELLED
> > > > or
> > > > _STATUS_ERROR.
> > > > 
> > > > But the current code is also sending a message to the guest
> > > > when
> > > > error/cancellation does not come from client, leading to
> > > > unexpected
> > > > messages to be received in host/guest.
> > > > 
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED is introduced in this
> > > > patch
> > > > to
> > > > mark a SpiceFileTransferTask to be failed due host or guest
> > > > problems.
> > > > 
> > > > The generic error code SPICE_CLIENT_ERROR_FAILED is still used
> > > > in
> > > > spice_main_file_copy_async() API.
> > > > 
> > > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=99170
> > > > Signed-off-by: Victor Toso 
> > > > Reported-by: Pavel Grunt 
> > > > ---
> > > >  src/channel-main.c | 11 +++
> > > >  src/spice-client.h |  1 +
> > > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/src/channel-main.c b/src/channel-main.c
> > > > index ed5d611..e92d363 100644
> > > > --- a/src/channel-main.c
> > > > +++ b/src/channel-main.c
> > > > @@ -1862,18 +1862,18 @@ static void
> > > > main_agent_handle_xfer_status(SpiceMainChannel *channel,
> > > >  spice_file_transfer_task_read_async(xfer_task,
> > > > file_xfer_read_async_cb, xfer_op);
> > > >  return;
> > > >  case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> > > > -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >  _("The spice agent
> > > > cancelled
> > > > the file transfer"));
> > > >  break;
> > > >  case VD_AGENT_FILE_XFER_STATUS_ERROR:
> > > > -error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > +error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >  _("The spice agent
> > > > reported
> > > > an
> > > > error during the file transfer"));
> > > >  break;
> > > >  case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
> > > >  break;
> > > >  default:
> > > >  g_warn_if_reached();
> > > > -error = g_error_new(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > +error = g_error_new(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >  "unhandled status type: %u", msg-
> > > > > result);
> > > > 
> > > >  break;
> > > >  }
> > > > @@ -2960,7 +2960,10 @@ static void
> > > > file_transfer_operation_task_finished(SpiceFileTransferTask
> > > > *xfer_ta
> > > >  task_id = spice_file_transfer_task_get_id(xfer_task);
> > > >  g_return_if_fail(task_id != 0);
> > > >  
> > > > -if (error) {
> > > > +if (g_error_matches(error, SPICE_CLIENT_ERROR,
> > > > +SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILE
> > > > D)
> > > > ) {
> > > > +spice_debug("xfer task: %u failed due cancel/error on
> > > > server or agent", task_id);
> > > > +} else if (error) {
> > > >  VDAgentFileXferStatusMessage msg;
> > > >  msg.id = task_id;
> > > >  if (g_error_matches(error, G_IO_ERROR,
> > > > G_IO_ERROR_CANCELLED)) {
> > > > diff --git a/src/spice-client.h b/src/spice-client.h
> > > > index 32b79ea..8297537 100644
> > > > --- a/src/spice-client.h
> > > > +++ b/src/spice-client.h
> > > > @@ -82,6 +82,7 @@ typedef en