[Spice-devel] [PATCH spice-server v2 2/2] red-channel: Move alloc_recv_buf and release_recv_buf to RedChannelClient

Jonathon Jongsma jjongsma at redhat.com
Fri Mar 3 21:18:54 UTC 2017


On Fri, 2017-03-03 at 14:46 +0000, Frediano Ziglio wrote:
> These vfuncs are more appropriate in RedChannelClient.
> The buffer they allocated are related to the client stream
> which is managed directly by RedChannelClient.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> Looking at the patch (mostly spicevmc.c) looks not great as
> you have to have a new class but I think that these method
> are much better into RedChannelClient. Also this fix multiple
> client issues (another prove is on the wrong place).

When I originally read this, I interpreted "multiple client issues" as
"multiple issues with the client". But now I understand that you mean
"issues when using multiple clients" :) Yes, indeed.

I don't think there's a big problem with adding a new derived class.
Yeah, it adds a bit of code, but it's not a big deal, since it's
clearly the right place to handle the buffer allocation.

> 
> About VmcChannelClient name... maybe I should add a prefix
> (Red/Spice) but looking at the code is not clear the right
> one, both are used.

Yeah, the naming in that area is kind of a mess. I'm not sure what the
right name is either. I'd probably be fine with this name for now, but
maybe we should do some renaming of related classes at some point.

> ---
>  server/common-graphics-channel.c  | 21 ++++++++---
>  server/inputs-channel-client.c    | 36 +++++++++++++++++++
>  server/inputs-channel.c           | 34 ------------------
>  server/main-channel-client.c      | 36 +++++++++++++++++++
>  server/main-channel.c             | 36 -------------------
>  server/red-channel-client.c       |  9 ++---
>  server/red-channel-client.h       |  3 ++
>  server/red-channel.c              |  3 +-
>  server/red-channel.h              |  6 ----
>  server/smartcard-channel-client.c | 23 ++++++++----
>  server/smartcard-channel-client.h | 18 ----------
>  server/smartcard.c                |  2 --
>  server/sound.c                    |  8 +++--
>  server/spicevmc.c                 | 73
> +++++++++++++++++++++++++++++++++++++--
>  14 files changed, 188 insertions(+), 120 deletions(-)
> 
> diff --git a/server/common-graphics-channel.c b/server/common-
> graphics-channel.c
> index 6faf1d7..394a68e 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -35,11 +35,13 @@ G_DEFINE_TYPE(CommonGraphicsChannelClient,
> common_graphics_channel_client, RED_T
>  
>  #define GRAPHICS_CHANNEL_PRIVATE(o) \
>      (G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_COMMON_GRAPHICS_CHANNEL,
> CommonGraphicsChannelPrivate))
> +#define GRAPHICS_CHANNEL_CLIENT_PRIVATE(o) \
> +    (G_TYPE_INSTANCE_GET_PRIVATE((o),
> TYPE_COMMON_GRAPHICS_CHANNEL_CLIENT, \
> +    CommonGraphicsChannelClientPrivate))
>  
>  struct CommonGraphicsChannelPrivate
>  {
>      QXLInstance *qxl;
> -    uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
>      int during_target_migrate; /* TRUE when the client that is
> associated with the channel
>                                    is during migration. Turned off
> when the vm is started.
>                                    The flag is used to avoid sending
> messages that are artifacts
> @@ -47,10 +49,14 @@ struct CommonGraphicsChannelPrivate
>                                    of the primary surface) */
>  };
>  
> +struct CommonGraphicsChannelClientPrivate {
> +    uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> +};
> +
> +
>  static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc,
> uint16_t type, uint32_t size)
>  {
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    CommonGraphicsChannel *common =
> COMMON_GRAPHICS_CHANNEL(channel);
> +    CommonGraphicsChannelClient *common =
> COMMON_GRAPHICS_CHANNEL_CLIENT(rcc);
>  
>      /* SPICE_MSGC_MIGRATE_DATA is the only client message whose size
> is dynamic */
>      if (type == SPICE_MSGC_MIGRATE_DATA) {
> @@ -157,8 +163,6 @@
> common_graphics_channel_class_init(CommonGraphicsChannelClass *klass)
>      object_class->set_property =
> common_graphics_channel_set_property;
>  
>      channel_class->config_socket = common_channel_config_socket;
> -    channel_class->alloc_recv_buf = common_alloc_recv_buf;
> -    channel_class->release_recv_buf = common_release_recv_buf;
>  
>      g_object_class_install_property(object_class,
>                                      PROP_QXL,
> @@ -194,9 +198,16 @@ QXLInstance*
> common_graphics_channel_get_qxl(CommonGraphicsChannel *self)
>  static void
>  common_graphics_channel_client_init(CommonGraphicsChannelClient
> *self)
>  {
> +    self->priv = GRAPHICS_CHANNEL_CLIENT_PRIVATE(self);
>  }
>  
>  static void
>  common_graphics_channel_client_class_init(CommonGraphicsChannelClien
> tClass *klass)
>  {
> +    RedChannelClientClass *client_class =
> RED_CHANNEL_CLIENT_CLASS(klass);
> +
> +    g_type_class_add_private(klass,
> sizeof(CommonGraphicsChannelClientPrivate));
> +
> +    client_class->alloc_recv_buf = common_alloc_recv_buf;
> +    client_class->release_recv_buf = common_release_recv_buf;
>  }
> diff --git a/server/inputs-channel-client.c b/server/inputs-channel-
> client.c
> index 72b5c39..c5b8e71 100644
> --- a/server/inputs-channel-client.c
> +++ b/server/inputs-channel-client.c
> @@ -27,15 +27,51 @@ G_DEFINE_TYPE(InputsChannelClient,
> inputs_channel_client, RED_TYPE_CHANNEL_CLIEN
>  #define INPUTS_CHANNEL_CLIENT_PRIVATE(o) \
>      (G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_INPUTS_CHANNEL_CLIENT,
> InputsChannelClientPrivate))
>  
> +// TODO: RECEIVE_BUF_SIZE used to be the same for inputs_channel and
> main_channel
> +// since it was defined once in reds.c which contained both.
> +// Now that they are split we can give a more fitting value for
> inputs - what
> +// should it be?
> +#define REDS_AGENT_WINDOW_SIZE 10
> +#define REDS_NUM_INTERNAL_AGENT_MESSAGES 1
> +
> +// approximate max receive message size
> +#define RECEIVE_BUF_SIZE \
> +    (4096 + (REDS_AGENT_WINDOW_SIZE +
> REDS_NUM_INTERNAL_AGENT_MESSAGES) * SPICE_AGENT_MAX_DATA_SIZE)
> +
>  struct InputsChannelClientPrivate
>  {
>      uint16_t motion_count;
> +    uint8_t recv_buf[RECEIVE_BUF_SIZE];
>  };
>  
> +static uint8_t *
> +inputs_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
> +                                        uint16_t type, uint32_t
> size)
> +{
> +    if (size > RECEIVE_BUF_SIZE) {
> +        spice_printerr("error: too large incoming message");
> +        return NULL;
> +    }
> +
> +    InputsChannelClient *icc = INPUTS_CHANNEL_CLIENT(rcc);
> +    return icc->priv->recv_buf;
> +}
> +
> +static void
> +inputs_channel_client_release_msg_rcv_buf(RedChannelClient *rcc,
> +                                          uint16_t type, uint32_t
> size, uint8_t *msg)
> +{
> +}
> +
>  static void
>  inputs_channel_client_class_init(InputsChannelClientClass *klass)
>  {
> +    RedChannelClientClass *client_class =
> RED_CHANNEL_CLIENT_CLASS(klass);
> +
>      g_type_class_add_private(klass,
> sizeof(InputsChannelClientPrivate));
> +
> +    client_class->alloc_recv_buf =
> inputs_channel_client_alloc_msg_rcv_buf;
> +    client_class->release_recv_buf =
> inputs_channel_client_release_msg_rcv_buf;
>  }
>  
>  static void
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index ec297a2..98413e5 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -43,22 +43,10 @@
>  #include "migration-protocol.h"
>  #include "utils.h"
>  
> -// TODO: RECEIVE_BUF_SIZE used to be the same for inputs_channel and
> main_channel
> -// since it was defined once in reds.c which contained both.
> -// Now that they are split we can give a more fitting value for
> inputs - what
> -// should it be?
> -#define REDS_AGENT_WINDOW_SIZE 10
> -#define REDS_NUM_INTERNAL_AGENT_MESSAGES 1
> -
> -// approximate max receive message size
> -#define RECEIVE_BUF_SIZE \
> -    (4096 + (REDS_AGENT_WINDOW_SIZE +
> REDS_NUM_INTERNAL_AGENT_MESSAGES) * SPICE_AGENT_MAX_DATA_SIZE)
> -
>  struct InputsChannel
>  {
>      RedChannel parent;
>  
> -    uint8_t recv_buf[RECEIVE_BUF_SIZE];
>      VDAgentMouseState mouse_state;
>      int src_during_migrate;
>      SpiceTimer *key_modifiers_timer;
> @@ -153,26 +141,6 @@ const VDAgentMouseState
> *inputs_channel_get_mouse_state(InputsChannel *inputs)
>      return &inputs->mouse_state;
>  }
>  
> -static uint8_t *inputs_channel_alloc_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                 uint16_t type,
> -                                                 uint32_t size)
> -{
> -    InputsChannel *inputs_channel =
> INPUTS_CHANNEL(red_channel_client_get_channel(rcc));
> -
> -    if (size > RECEIVE_BUF_SIZE) {
> -        spice_printerr("error: too large incoming message");
> -        return NULL;
> -    }
> -    return inputs_channel->recv_buf;
> -}
> -
> -static void inputs_channel_release_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                               uint16_t type,
> -                                               uint32_t size,
> -                                               uint8_t *msg)
> -{
> -}
> -
>  #define OUTGOING_OK 0
>  #define OUTGOING_FAILED -1
>  #define OUTGOING_BLOCKED 1
> @@ -628,8 +596,6 @@ inputs_channel_class_init(InputsChannelClass
> *klass)
>      /* channel callbacks */
>      channel_class->on_disconnect = inputs_channel_on_disconnect;
>      channel_class->send_item = inputs_channel_send_item;
> -    channel_class->alloc_recv_buf =
> inputs_channel_alloc_msg_rcv_buf;
> -    channel_class->release_recv_buf =
> inputs_channel_release_msg_rcv_buf;
>      channel_class->handle_migrate_data =
> inputs_channel_handle_migrate_data;
>      channel_class->handle_migrate_flush_mark =
> inputs_channel_handle_migrate_flush_mark;
>  }
> diff --git a/server/main-channel-client.c b/server/main-channel-
> client.c
> index 2b68407..1094dfd 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -46,6 +46,10 @@ G_DEFINE_TYPE(MainChannelClient,
> main_channel_client, RED_TYPE_CHANNEL_CLIENT)
>  #define MAIN_CHANNEL_CLIENT_PRIVATE(o) \
>      (G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_MAIN_CHANNEL_CLIENT,
> MainChannelClientPrivate))
>  
> +// approximate max receive message size for main channel
> +#define MAIN_CHANNEL_RECEIVE_BUF_SIZE \
> +    (4096 + (REDS_AGENT_WINDOW_SIZE +
> REDS_NUM_INTERNAL_AGENT_MESSAGES) * SPICE_AGENT_MAX_DATA_SIZE)
> +
>  struct MainChannelClientPrivate {
>      uint32_t connection_id;
>      uint32_t ping_id;
> @@ -63,6 +67,7 @@ struct MainChannelClientPrivate {
>      int mig_wait_prev_try_seamless;
>      int init_sent;
>      int seamless_mig_dst;
> +    uint8_t recv_buf[MAIN_CHANNEL_RECEIVE_BUF_SIZE];
>  };
>  
>  typedef struct RedPingPipeItem {
> @@ -194,9 +199,37 @@ static void main_channel_client_finalize(GObject
> *object)
>      G_OBJECT_CLASS(main_channel_client_parent_class)-
> >finalize(object);
>  }
>  
> +static uint8_t *
> +main_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
> +                                      uint16_t type, uint32_t size)
> +{
> +    MainChannelClient *mcc = MAIN_CHANNEL_CLIENT(rcc);
> +
> +    if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
> +        RedChannel *channel = red_channel_client_get_channel(rcc);
> +        return
> reds_get_agent_data_buffer(red_channel_get_server(channel), mcc,
> size);
> +    } else if (size > sizeof(mcc->priv->recv_buf)) {
> +        /* message too large, caller will log a message and close
> the connection */
> +        return NULL;
> +    } else {
> +        return mcc->priv->recv_buf;
> +    }
> +}
> +
> +static void
> +main_channel_client_release_msg_rcv_buf(RedChannelClient *rcc,
> +                                        uint16_t type, uint32_t
> size, uint8_t *msg)
> +{
> +    if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
> +        RedChannel *channel = red_channel_client_get_channel(rcc);
> +        reds_release_agent_data_buffer(red_channel_get_server(channe
> l), msg);
> +    }
> +}
> +
>  static void main_channel_client_class_init(MainChannelClientClass
> *klass)
>  {
>      GObjectClass *object_class = G_OBJECT_CLASS(klass);
> +    RedChannelClientClass *client_class =
> RED_CHANNEL_CLIENT_CLASS(klass);
>  
>      g_type_class_add_private(klass,
> sizeof(MainChannelClientPrivate));
>  
> @@ -205,6 +238,9 @@ static void
> main_channel_client_class_init(MainChannelClientClass *klass)
>      object_class->finalize = main_channel_client_finalize;
>      object_class->constructed = main_channel_client_constructed;
>  
> +    client_class->alloc_recv_buf =
> main_channel_client_alloc_msg_rcv_buf;
> +    client_class->release_recv_buf =
> main_channel_client_release_msg_rcv_buf;
> +
>      g_object_class_install_property(object_class,
>                                      PROP_CONNECTION_ID,
>                                      g_param_spec_uint("connection-
> id",
> diff --git a/server/main-channel.c b/server/main-channel.c
> index 307c80f..a43f58c 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -28,15 +28,10 @@
>  #include "main-channel.h"
>  #include "main-channel-client.h"
>  
> -// approximate max receive message size for main channel
> -#define MAIN_CHANNEL_RECEIVE_BUF_SIZE \
> -    (4096 + (REDS_AGENT_WINDOW_SIZE +
> REDS_NUM_INTERNAL_AGENT_MESSAGES) * SPICE_AGENT_MAX_DATA_SIZE)
> -
>  struct MainChannel
>  {
>      RedChannel parent;
>  
> -    uint8_t recv_buf[MAIN_CHANNEL_RECEIVE_BUF_SIZE];
>      // TODO: add refs and release (afrer all clients completed
> migration in one way or the other?)
>      RedsMigSpice mig_target;
>      int num_clients_mig_wait;
> @@ -248,35 +243,6 @@ static int
> main_channel_handle_message(RedChannelClient *rcc, uint16_t type,
>      return TRUE;
>  }
>  
> -static uint8_t *main_channel_alloc_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                               uint16_t type,
> -                                               uint32_t size)
> -{
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    MainChannel *main_chan = MAIN_CHANNEL(channel);
> -    MainChannelClient *mcc = MAIN_CHANNEL_CLIENT(rcc);
> -
> -    if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
> -        return
> reds_get_agent_data_buffer(red_channel_get_server(channel), mcc,
> size);
> -    } else if (size > sizeof(main_chan->recv_buf)) {
> -        /* message too large, caller will log a message and close
> the connection */
> -        return NULL;
> -    } else {
> -        return main_chan->recv_buf;
> -    }
> -}
> -
> -static void main_channel_release_msg_rcv_buf(RedChannelClient *rcc,
> -                                               uint16_t type,
> -                                               uint32_t size,
> -                                               uint8_t *msg)
> -{
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    if (type == SPICE_MSGC_MAIN_AGENT_DATA) {
> -        reds_release_agent_data_buffer(red_channel_get_server(channe
> l), msg);
> -    }
> -}
> -
>  static int main_channel_handle_migrate_flush_mark(RedChannelClient
> *rcc)
>  {
>      RedChannel *channel = red_channel_client_get_channel(rcc);
> @@ -349,8 +315,6 @@ main_channel_class_init(MainChannelClass *klass)
>      /* channel callbacks */
>      channel_class->on_disconnect =
> main_channel_client_on_disconnect;
>      channel_class->send_item = main_channel_client_send_item;
> -    channel_class->alloc_recv_buf = main_channel_alloc_msg_rcv_buf;
> -    channel_class->release_recv_buf =
> main_channel_release_msg_rcv_buf;
>      channel_class->handle_migrate_flush_mark =
> main_channel_handle_migrate_flush_mark;
>      channel_class->handle_migrate_data =
> main_channel_handle_migrate_data;
>  }
> diff --git a/server/red-channel-client.c b/server/red-channel-
> client.c
> index 441d20b..71ad726 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -360,6 +360,9 @@ static void
> red_channel_client_constructed(GObject *object)
>  {
>      RedChannelClient *self =  RED_CHANNEL_CLIENT(object);
>  
> +    RedChannelClientClass *klass =
> RED_CHANNEL_CLIENT_GET_CLASS(self);
> +    spice_assert(klass->alloc_recv_buf && klass->release_recv_buf);
> +
>      self->priv->outgoing.pos = 0;
>      self->priv->outgoing.size = 0;
>  
> @@ -1047,8 +1050,7 @@ void
> red_channel_client_shutdown(RedChannelClient *rcc)
>  static uint8_t *red_channel_client_alloc_msg_buf(RedChannelClient
> *rcc,
>                                                   uint16_t type,
> uint32_t size)
>  {
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
> +    RedChannelClientClass *klass =
> RED_CHANNEL_CLIENT_GET_CLASS(rcc);
>  
>      return klass->alloc_recv_buf(rcc, type, size);
>  }
> @@ -1057,8 +1059,7 @@ static void
> red_channel_client_release_msg_buf(RedChannelClient *rcc,
>                                                 uint16_t type,
> uint32_t size,
>                                                 uint8_t *msg)
>  {
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
> +    RedChannelClientClass *klass =
> RED_CHANNEL_CLIENT_GET_CLASS(rcc);
>  
>      klass->release_recv_buf(rcc, type, size, msg);
>  }
> diff --git a/server/red-channel-client.h b/server/red-channel-
> client.h
> index 1450089..397216d 100644
> --- a/server/red-channel-client.h
> +++ b/server/red-channel-client.h
> @@ -168,6 +168,9 @@ struct RedChannelClient
>  struct RedChannelClientClass
>  {
>      GObjectClass parent_class;
> +
> +    uint8_t *(*alloc_recv_buf)(RedChannelClient *channel, uint16_t
> type, uint32_t size);
> +    void (*release_recv_buf)(RedChannelClient *channel, uint16_t
> type, uint32_t size, uint8_t *msg);
>  };
>  
>  #define SPICE_SERVER_ERROR spice_server_error_quark()
> diff --git a/server/red-channel.c b/server/red-channel.c
> index d78c628..47edc54 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -204,8 +204,7 @@ red_channel_constructed(GObject *object)
>  
>      G_OBJECT_CLASS(red_channel_parent_class)->constructed(object);
>  
> -    spice_assert(klass->on_disconnect &&
> -                 klass->alloc_recv_buf && klass->release_recv_buf);
> +    spice_assert(klass->on_disconnect);
>      spice_assert(klass->handle_migrate_data ||
>                   !(self->priv->migration_flags &
> SPICE_MIGRATE_NEED_DATA_TRANSFER));
>  }
> diff --git a/server/red-channel.h b/server/red-channel.h
> index e076e2a..6866952 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -43,12 +43,8 @@ typedef struct RedChannelClient RedChannelClient;
>  typedef struct RedClient RedClient;
>  typedef struct MainChannelClient MainChannelClient;
>  
> -typedef uint8_t *(*channel_alloc_msg_recv_buf_proc)(RedChannelClient
> *channel,
> -                                                    uint16_t type,
> uint32_t size);
>  typedef int (*channel_handle_message_proc)(RedChannelClient *rcc,
> uint16_t type,
>                                             uint32_t size, void
> *msg);
> -typedef void (*channel_release_msg_recv_buf_proc)(RedChannelClient
> *channel,
> -                                                  uint16_t type,
> uint32_t size, uint8_t *msg);
>  typedef void (*channel_disconnect_proc)(RedChannelClient *rcc);
>  typedef int (*channel_configure_socket_proc)(RedChannelClient *rcc);
>  typedef void (*channel_send_pipe_item_proc)(RedChannelClient *rcc,
> RedPipeItem *item);
> @@ -122,8 +118,6 @@ struct RedChannelClass
>      channel_configure_socket_proc config_socket;
>      channel_disconnect_proc on_disconnect;
>      channel_send_pipe_item_proc send_item;
> -    channel_alloc_msg_recv_buf_proc alloc_recv_buf;
> -    channel_release_msg_recv_buf_proc release_recv_buf;
>      channel_handle_migrate_flush_mark_proc
> handle_migrate_flush_mark;
>      channel_handle_migrate_data_proc handle_migrate_data;
>      channel_handle_migrate_data_get_serial_proc
> handle_migrate_data_get_serial;
> diff --git a/server/smartcard-channel-client.c b/server/smartcard-
> channel-client.c
> index 04c5c04..1e649cb 100644
> --- a/server/smartcard-channel-client.c
> +++ b/server/smartcard-channel-client.c
> @@ -43,6 +43,12 @@ typedef struct RedErrorItem {
>      VSCMsgError  error;
>  } RedErrorItem;
>  
> +static uint8_t *
> +smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
> uint16_t type, uint32_t size);
> +static void
> +smartcard_channel_client_release_msg_rcv_buf(RedChannelClient *rcc,
> uint16_t type,
> +                                             uint32_t size, uint8_t
> *msg);
> +
>  static void smart_card_channel_client_get_property(GObject *object,
>                                                     guint
> property_id,
>                                                     GValue *value,
> @@ -88,6 +94,10 @@ static void
> smart_card_channel_client_class_init(SmartCardChannelClientClass *kl
>  
>      g_type_class_add_private(klass,
> sizeof(SmartCardChannelClientPrivate));
>  
> +    RedChannelClientClass *client_class =
> RED_CHANNEL_CLIENT_CLASS(klass);
> +    client_class->alloc_recv_buf =
> smartcard_channel_client_alloc_msg_rcv_buf;
> +    client_class->release_recv_buf =
> smartcard_channel_client_release_msg_rcv_buf;
> +
>      object_class->get_property =
> smart_card_channel_client_get_property;
>      object_class->set_property =
> smart_card_channel_client_set_property;
>      object_class->dispose = smart_card_channel_client_dispose;
> @@ -119,9 +129,9 @@ SmartCardChannelClient*
> smartcard_channel_client_create(RedChannel *channel,
>      return rcc;
>  }
>  
> -uint8_t *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                    uint16_t type,
> -                                                    uint32_t size)
> +static uint8_t *
> +smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
> +                                           uint16_t type, uint32_t
> size)
>  {
>      SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
>      RedClient *client = red_channel_client_get_client(rcc);
> @@ -152,10 +162,9 @@ uint8_t
> *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
>      }
>  }
>  
> -void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                  uint16_t type,
> -                                                  uint32_t size,
> -                                                  uint8_t *msg)
> +static void
> +smartcard_channel_client_release_msg_rcv_buf(RedChannelClient *rcc,
> +                                             uint16_t type, uint32_t
> size, uint8_t *msg)
>  {
>      SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
>  
> diff --git a/server/smartcard-channel-client.h b/server/smartcard-
> channel-client.h
> index 84181a4..6248a98 100644
> --- a/server/smartcard-channel-client.h
> +++ b/server/smartcard-channel-client.h
> @@ -60,15 +60,6 @@ SmartCardChannelClient*
> smartcard_channel_client_create(RedChannel *channel,
>                                                          int
> monitor_latency,
>                                                          RedChannelCa
> pabilities *caps);
>  
> -uint8_t* smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                    uint16_t type,
> -                                                    uint32_t size);
> -
> -void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                  uint16_t type,
> -                                                  uint32_t size,
> -                                                  uint8_t *msg);
> -
>  int
> smartcard_channel_client_handle_migrate_flush_mark(RedChannelClient
> *rcc);
>  
>  void smartcard_channel_client_on_disconnect(RedChannelClient *rcc);
> @@ -96,15 +87,6 @@ void
> smartcard_channel_client_set_char_device(SmartCardChannelClient *scc,
>  
>  RedCharDeviceSmartcard*
> smartcard_channel_client_get_char_device(SmartCardChannelClient
> *scc);
>  
> -void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                  uint16_t type,
> -                                                  uint32_t size,
> -                                                  uint8_t *msg);
> -
> -uint8_t *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> *rcc,
> -                                                    uint16_t type,
> -                                                    uint32_t size);
> -
>  G_END_DECLS
>  
>  #endif /* SMARTCARD_CHANNEL_CLIENT_H__ */
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 7b8ad01..fcba01b 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -580,8 +580,6 @@
> red_smartcard_channel_class_init(RedSmartcardChannelClass *klass)
>  
>      channel_class->on_disconnect =
> smartcard_channel_client_on_disconnect;
>      channel_class->send_item = smartcard_channel_send_item;
> -    channel_class->alloc_recv_buf =
> smartcard_channel_client_alloc_msg_rcv_buf;
> -    channel_class->release_recv_buf =
> smartcard_channel_client_release_msg_rcv_buf;
>      channel_class->handle_migrate_flush_mark =
> smartcard_channel_client_handle_migrate_flush_mark;
>      channel_class->handle_migrate_data =
> smartcard_channel_client_handle_migrate_data;
>  
> diff --git a/server/sound.c b/server/sound.c
> index 2e614f0..5921b1a 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1334,8 +1334,6 @@ snd_channel_class_init(SndChannelClass *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;
>      channel_class->on_disconnect = snd_channel_on_disconnect;
>  }
>  
> @@ -1485,8 +1483,12 @@ void snd_set_playback_compression(int on)
>  }
>  
>  static void
> -snd_channel_client_class_init(SndChannelClientClass *self)
> +snd_channel_client_class_init(SndChannelClientClass *klass)
>  {
> +    RedChannelClientClass *client_class =
> RED_CHANNEL_CLIENT_CLASS(klass);
> +
> +    client_class->alloc_recv_buf =
> snd_channel_client_alloc_recv_buf;
> +    client_class->release_recv_buf =
> snd_channel_client_release_recv_buf;
>  }
>  
>  static void
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 3951fa2..cbb28bc 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -181,6 +181,42 @@ static void
> red_vmc_channel_port_init(RedVmcChannelPort *self)
>  }
>  G_DEFINE_TYPE(RedVmcChannelPort, red_vmc_channel_port,
> RED_TYPE_VMC_CHANNEL)
>  
> +
> +#define TYPE_VMC_CHANNEL_CLIENT vmc_channel_client_get_type()
> +
> +#define VMC_CHANNEL_CLIENT(obj) \
> +    (G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_VMC_CHANNEL_CLIENT,
> VmcChannelClient))
> +#define VMC_CHANNEL_CLIENT_CLASS(klass) \
> +    (G_TYPE_CHECK_CLASS_CAST((klass), TYPE_VMC_CHANNEL_CLIENT,
> VmcChannelClientClass))
> +#define COMMON_IS_GRAPHICS_CHANNEL_CLIENT(obj) \
> +    (G_TYPE_CHECK_INSTANCE_TYPE((obj), TYPE_VMC_CHANNEL_CLIENT))
> +#define COMMON_IS_GRAPHICS_CHANNEL_CLIENT_CLASS(klass) \
> +    (G_TYPE_CHECK_CLASS_TYPE((klass), TYPE_VMC_CHANNEL_CLIENT))
> +#define VMC_CHANNEL_CLIENT_GET_CLASS(obj) \
> +    (G_TYPE_INSTANCE_GET_CLASS((obj), TYPE_VMC_CHANNEL_CLIENT,
> VmcChannelClientClass))
> +
> +typedef struct VmcChannelClient VmcChannelClient;
> +typedef struct VmcChannelClientClass VmcChannelClientClass;
> +typedef struct VmcChannelClientPrivate VmcChannelClientPrivate;
> +
> +struct VmcChannelClient {
> +    RedChannelClient parent;
> +};
> +
> +struct VmcChannelClientClass {
> +    RedChannelClientClass parent_class;
> +};
> +
> +GType vmc_channel_client_get_type(void) G_GNUC_CONST;
> +
> +G_DEFINE_TYPE(VmcChannelClient, vmc_channel_client,
> RED_TYPE_CHANNEL_CLIENT)
> +
> +static RedChannelClient *
> +vmc_channel_client_create(RedChannel *channel, RedClient *client,
> +                          RedsStream *stream,
> +                          RedChannelCapabilities *caps);
> +
> +
>  static void spicevmc_connect(RedChannel *channel, RedClient *client,
>                               RedsStream *stream, int migration,
>                               RedChannelCapabilities *caps);
> @@ -733,8 +769,6 @@ red_vmc_channel_class_init(RedVmcChannelClass
> *klass)
>  
>      channel_class->on_disconnect =
> spicevmc_red_channel_client_on_disconnect;
>      channel_class->send_item = spicevmc_red_channel_send_item;
> -    channel_class->alloc_recv_buf =
> spicevmc_red_channel_alloc_msg_rcv_buf;
> -    channel_class->release_recv_buf =
> spicevmc_red_channel_release_msg_rcv_buf;
>      channel_class->handle_migrate_flush_mark =
> spicevmc_channel_client_handle_migrate_flush_mark;
>      channel_class->handle_migrate_data =
> spicevmc_channel_client_handle_migrate_data;
>  }
> @@ -786,7 +820,7 @@ static void spicevmc_connect(RedChannel *channel,
> RedClient *client,
>          return;
>      }
>  
> -    rcc = red_channel_client_create(channel, client, stream, FALSE,
> caps);
> +    rcc = vmc_channel_client_create(channel, client, stream, caps);
>      if (!rcc) {
>          return;
>      }
> @@ -952,3 +986,36 @@
> red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
>                          "channel", channel,
>                          NULL);
>  }
> +
> +static void
> +vmc_channel_client_init(VmcChannelClient *self)
> +{
> +}
> +
> +static void
> +vmc_channel_client_class_init(VmcChannelClientClass *klass)
> +{
> +    RedChannelClientClass *client_class =
> RED_CHANNEL_CLIENT_CLASS(klass);
> +
> +    client_class->alloc_recv_buf =
> spicevmc_red_channel_alloc_msg_rcv_buf;
> +    client_class->release_recv_buf =
> spicevmc_red_channel_release_msg_rcv_buf;
> +}
> +
> +static RedChannelClient *
> +vmc_channel_client_create(RedChannel *channel, RedClient *client,
> +                          RedsStream *stream,
> +                          RedChannelCapabilities *caps)
> +{
> +    RedChannelClient *rcc;
> +
> +    rcc = g_initable_new(TYPE_VMC_CHANNEL_CLIENT,
> +                         NULL, NULL,
> +                         "channel", channel,
> +                         "client", client,
> +                         "stream", stream,
> +                         "monitor-latency", false,

Since this is a gboolean type property, we should probably use FALSE
here. I'm not sure if it will cause any problems to use a bool instead,
but it seems safer to use gboolean.

> +                         "caps", caps,
> +                         NULL);
> +
> +    return rcc;
> +}

Aside from this, the patch looks fine.

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>



More information about the Spice-devel mailing list