[Spice-devel] [PATCH 2/4] char-device: add 'self' param to vfuncs

[Jonathon Jongsma]

Add a 'self' parameter to all of the char device virtual functions so
that we don't have to play games with the 'opaque' pointer.
---
 server/char-device.c  | 31 +--
 server/char-device.h  | 20 ++--
 server/reds.c | 24 +---
 server/smartcard-channel-client.c |  2 +-
 server/smartcard.c| 32 
 server/spicevmc.c | 39 ---
 6 files changed, 69 insertions(+), 79 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index 318bf3c..3b70066 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
 int during_read_from_device;
 int during_write_to_device;
 
-void *opaque;
 SpiceServer *reds;
 };
 
@@ -88,7 +87,6 @@ enum {
 PROP_SPICE_SERVER,
 PROP_CLIENT_TOKENS_INTERVAL,
 PROP_SELF_TOKENS,
-PROP_OPAQUE
 };
 
 static void red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer 
*write_buf);
@@ -99,7 +97,7 @@ red_char_device_read_one_msg_from_device(RedCharDevice *dev)
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   return klass->read_one_msg_from_device(dev->priv->sin, dev->priv->opaque);
+   return klass->read_one_msg_from_device(dev, dev->priv->sin);
 }
 
 static void
@@ -109,7 +107,7 @@ red_char_device_send_msg_to_client(RedCharDevice *dev,
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   klass->send_msg_to_client(msg, client, dev->priv->opaque);
+   klass->send_msg_to_client(dev, msg, client);
 }
 
 static void
@@ -119,7 +117,7 @@ red_char_device_send_tokens_to_client(RedCharDevice *dev,
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   klass->send_tokens_to_client(client, tokens, dev->priv->opaque);
+   klass->send_tokens_to_client(dev, client, tokens);
 }
 
 static void
@@ -128,7 +126,7 @@ red_char_device_on_free_self_token(RedCharDevice *dev)
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
if (klass->on_free_self_token != NULL) {
-   klass->on_free_self_token(dev->priv->opaque);
+   klass->on_free_self_token(dev);
}
 }
 
@@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice *dev, RedClient 
*client)
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   klass->remove_client(client, dev->priv->opaque);
+   klass->remove_client(dev, client);
 }
 
 static void red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
@@ -686,11 +684,6 @@ void red_char_device_reset_dev_instance(RedCharDevice *dev,
 g_object_notify(G_OBJECT(dev), "sin");
 }
 
-void *red_char_device_opaque_get(RedCharDevice *dev)
-{
-return dev->priv->opaque;
-}
-
 void red_char_device_destroy(RedCharDevice *char_dev)
 {
 g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
@@ -1041,9 +1034,6 @@ red_char_device_get_property(GObject*object,
 case PROP_SELF_TOKENS:
 g_value_set_uint64(value, self->priv->num_self_tokens);
 break;
-case PROP_OPAQUE:
-g_value_set_pointer(value, self->priv->opaque);
-break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
 }
@@ -1071,9 +1061,6 @@ red_char_device_set_property(GObject  *object,
 case PROP_SELF_TOKENS:
 self->priv->num_self_tokens = g_value_get_uint64(value);
 break;
-case PROP_OPAQUE:
-self->priv->opaque = g_value_get_pointer(value);
-break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
 }
@@ -1156,14 +1143,6 @@ red_char_device_class_init(RedCharDeviceClass *klass)
 0, G_MAXUINT64, 0,
 G_PARAM_STATIC_STRINGS 
|
 G_PARAM_READWRITE));
-g_object_class_install_property(object_class,
-PROP_OPAQUE,
-g_param_spec_pointer("opaque",
- "opaque",
- "User data to pass to 
callbacks",
-  G_PARAM_STATIC_STRINGS |
-  G_PARAM_READWRITE));
-
 }
 
 static void
diff --git a/server/char-device.h b/server/char-device.h
index 44e9504..3b87023 100644
--- a/server/char-device.h
+++ b/server/char-device.h
@@ -56,25 +56,27 @@ struct RedCharDeviceClass
 
 /* reads from the device till reaching a msg that should be sent to the 
client,
  * or till the reading fails */
-RedPipeItem* (*read_one_msg_from_device)(SpiceCharDeviceInstance *sin,
- 

[Spice-devel] [PATCH 4/4] spicevmc: use 'channel' instead of 'state'

[Jonathon Jongsma]

After renaming the object to RedVmcChannel, the local variables still
used the old 'state' terminology. Changing these variables to 'channel'
makes things a bit more consistent.
---
 server/spicevmc.c | 146 +++---
 1 file changed, 73 insertions(+), 73 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 49de9cc..664896c 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -328,12 +328,12 @@ static void 
spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
  *  - a new pipe item with the compressed data in it upon success
  */
 #ifdef USE_LZ4
-static RedVmcPipeItem* try_compress_lz4(RedVmcChannel *state, int n, 
RedVmcPipeItem *msg_item)
+static RedVmcPipeItem* try_compress_lz4(RedVmcChannel *channel, int n, 
RedVmcPipeItem *msg_item)
 {
 RedVmcPipeItem *msg_item_compressed;
 int compressed_data_count;
 
-if (reds_stream_get_family(red_channel_client_get_stream(state->rcc)) == 
AF_UNIX) {
+if (reds_stream_get_family(red_channel_client_get_stream(channel->rcc)) == 
AF_UNIX) {
 /* AF_LOCAL - data will not be compressed */
 return NULL;
 }
@@ -341,7 +341,7 @@ static RedVmcPipeItem* try_compress_lz4(RedVmcChannel 
*state, int n, RedVmcPipeI
 /* n <= threshold - data will not be compressed */
 return NULL;
 }
-if (!red_channel_test_remote_cap(RED_CHANNEL(state), 
SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4)) {
+if (!red_channel_test_remote_cap(RED_CHANNEL(channel), 
SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4)) {
 /* Client doesn't have compression cap - data will not be compressed */
 return NULL;
 }
@@ -370,25 +370,25 @@ static RedPipeItem 
*spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
SpiceCharDeviceInstance 
*sin)
 {
 RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
-RedVmcChannel *state = RED_VMC_CHANNEL(vmc->channel);
+RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
 SpiceCharDeviceInterface *sif;
 RedVmcPipeItem *msg_item;
 int n;
 
 sif = spice_char_device_get_interface(sin);
 
-if (!state->rcc) {
+if (!channel->rcc) {
 return NULL;
 }
 
-if (!state->pipe_item) {
+if (!channel->pipe_item) {
 msg_item = spice_new0(RedVmcPipeItem, 1);
 msg_item->type = SPICE_DATA_COMPRESSION_TYPE_NONE;
 red_pipe_item_init(_item->base, RED_PIPE_ITEM_TYPE_SPICEVMC_DATA);
 } else {
-spice_assert(state->pipe_item->buf_used == 0);
-msg_item = state->pipe_item;
-state->pipe_item = NULL;
+spice_assert(channel->pipe_item->buf_used == 0);
+msg_item = channel->pipe_item;
+channel->pipe_item = NULL;
 }
 
 n = sif->read(sin, msg_item->buf,
@@ -398,7 +398,7 @@ static RedPipeItem 
*spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
 #ifdef USE_LZ4
 RedVmcPipeItem *msg_item_compressed;
 
-msg_item_compressed = try_compress_lz4(state, n, msg_item);
+msg_item_compressed = try_compress_lz4(channel, n, msg_item);
 if (msg_item_compressed != NULL) {
 return _item_compressed->base;
 }
@@ -407,7 +407,7 @@ static RedPipeItem 
*spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
 msg_item->buf_used = n;
 return _item->base;
 } else {
-state->pipe_item = msg_item;
+channel->pipe_item = msg_item;
 return NULL;
 }
 }
@@ -417,24 +417,24 @@ static void 
spicevmc_chardev_send_msg_to_client(RedCharDevice *self,
 RedClient *client)
 {
 RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
-RedVmcChannel *state = RED_VMC_CHANNEL(vmc->channel);
+RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
 
-spice_assert(red_channel_client_get_client(state->rcc) == client);
+spice_assert(red_channel_client_get_client(channel->rcc) == client);
 red_pipe_item_ref(msg);
-red_channel_client_pipe_add_push(state->rcc, msg);
+red_channel_client_pipe_add_push(channel->rcc, msg);
 }
 
 static void spicevmc_port_send_init(RedChannelClient *rcc)
 {
-RedVmcChannel *state = 
RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
+RedVmcChannel *channel = 
RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 SpiceCharDeviceInstance *sin;
 RedPortInitPipeItem *item = spice_malloc(sizeof(RedPortInitPipeItem));
 
-g_object_get(state->chardev, "sin", , NULL);
+g_object_get(channel->chardev, "sin", , NULL);
 
 red_pipe_item_init(>base, RED_PIPE_ITEM_TYPE_PORT_INIT);
 item->name = strdup(sin->portname);
-item->opened = state->port_opened;
+item->opened = channel->port_opened;
 red_channel_client_pipe_add_push(rcc, >base);
 }
 
@@ -458,13 +458,13 @@ static void spicevmc_char_dev_remove_client(RedCharDevice 
*self,
 RedClient 

[Spice-devel] [PATCH 1/4] spicevmc: Channel is owned by device

[Jonathon Jongsma]

Previously, spicevmc_device_connect() created a channel, which then
internally created a device. Then we returned the internal device from
the channel to the caller. The channel essentially owned the device, but
it makes more sense for the device to own the channel, because a channel
can't really exist without the associated device.

So this refactory inverts things: spicevmc_device_connect() now creates
a char device and returns that directly. Internally, that device creates
an associated channel.
---
 server/spicevmc.c | 122 +-
 1 file changed, 83 insertions(+), 39 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index d095378..522814e 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -78,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass 
RedCharDeviceSpiceVmcClass;
 
 struct RedCharDeviceSpiceVmc {
 RedCharDevice parent;
+uint8_t channel_type;
 RedVmcChannel *channel;
 };
 
@@ -89,7 +90,7 @@ struct RedCharDeviceSpiceVmcClass
 static GType red_char_device_spicevmc_get_type(void) G_GNUC_CONST;
 static RedCharDevice *red_char_device_spicevmc_new(SpiceCharDeviceInstance 
*sin,
RedsState *reds,
-   void *opaque);
+   uint8_t channel_type);
 
 G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc, 
RED_TYPE_CHAR_DEVICE)
 
@@ -109,8 +110,7 @@ struct RedVmcChannel
 RedChannel parent;
 
 RedChannelClient *rcc;
-RedCharDevice *chardev;
-SpiceCharDeviceInstance *chardev_sin;
+RedCharDevice *chardev; /* weak */
 RedVmcPipeItem *pipe_item;
 RedCharDeviceWriteBuffer *recv_from_client_buf;
 uint8_t port_opened;
@@ -180,7 +180,7 @@ G_DEFINE_TYPE(RedVmcChannelPort, red_vmc_channel_port, 
RED_TYPE_VMC_CHANNEL)
 
 enum {
 PROP0,
-PROP_DEVICE_INSTANCE
+PROP_CHAR_DEVICE
 };
 
 static void
@@ -193,8 +193,8 @@ red_vmc_channel_get_property(GObject *object,
 
 switch (property_id)
 {
-case PROP_DEVICE_INSTANCE:
-g_value_set_pointer(value, self->chardev_sin);
+case PROP_CHAR_DEVICE:
+g_value_set_object(value, self->chardev);
 break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
@@ -211,8 +211,9 @@ red_vmc_channel_set_property(GObject *object,
 
 switch (property_id)
 {
-case PROP_DEVICE_INSTANCE:
-self->chardev_sin = g_value_get_pointer(value);
+case PROP_CHAR_DEVICE:
+/* don't take a ref */
+self->chardev = g_value_get_object(value);
 break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
@@ -241,8 +242,6 @@ red_vmc_channel_constructed(GObject *object)
 
 red_channel_init_outgoing_messages_window(RED_CHANNEL(self));
 
-self->chardev = red_char_device_spicevmc_new(self->chardev_sin, reds, 
self);
-
 reds_register_channel(reds, RED_CHANNEL(self));
 }
 
@@ -265,7 +264,7 @@ red_vmc_channel_finalize(GObject *object)
 }
 
 static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t 
channel_type,
-  SpiceCharDeviceInstance *sin)
+  RedCharDeviceSpiceVmc *chardev)
 {
 GType gtype = G_TYPE_NONE;
 static uint8_t id[256] = { 0, };
@@ -282,7 +281,9 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, 
uint8_t channel_type,
 break;
 default:
 g_error("Unsupported channel_type for red_vmc_channel_new(): %u", 
channel_type);
+return NULL;
 }
+
 return g_object_new(gtype,
 "spice-server", reds,
 "core-interface", reds_get_core_interface(reds),
@@ -291,7 +292,7 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, 
uint8_t channel_type,
 "handle-acks", FALSE,
 "migration-flags",
 (SPICE_MIGRATE_NEED_FLUSH | 
SPICE_MIGRATE_NEED_DATA_TRANSFER),
-"device-instance", sin,
+"char-device", chardev,
 NULL);
 }
 
@@ -427,9 +428,11 @@ static RedVmcChannel 
*spicevmc_red_channel_client_get_state(RedChannelClient *rc
 static void spicevmc_port_send_init(RedChannelClient *rcc)
 {
 RedVmcChannel *state = spicevmc_red_channel_client_get_state(rcc);
-SpiceCharDeviceInstance *sin = state->chardev_sin;
+SpiceCharDeviceInstance *sin;
 RedPortInitPipeItem *item = spice_malloc(sizeof(RedPortInitPipeItem));
 
+g_object_get(state->chardev, "sin", , NULL);
+
 red_pipe_item_init(>base, RED_PIPE_ITEM_TYPE_PORT_INIT);
 item->name = strdup(sin->portname);
 item->opened = state->port_opened;
@@ -487,6 +490,7 @@ static int 

[Spice-devel] [PATCH 0/4] More spicevmc cleanups

[Jonathon Jongsma]

This patch series should apply on top of the series recently sent by Frediano.
It's an attempt to clarify ownership between the channel and device a bit, and
adds a few additional minor cleanups.

Jonathon Jongsma (4):
  spicevmc: Channel is owned by device
  char-device: add 'self' param to vfuncs
  Remove spicevmc_red_channel_client_get_state()
  spicevmc: use 'channel' instead of 'state'

 server/char-device.c  |  31 +---
 server/char-device.h  |  20 +--
 server/reds.c |  24 ++--
 server/smartcard-channel-client.c |   2 +-
 server/smartcard.c|  32 ++---
 server/spicevmc.c | 291 ++
 6 files changed, 214 insertions(+), 186 deletions(-)

-- 
2.7.4

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

[Spice-devel] [PATCH 3/4] Remove spicevmc_red_channel_client_get_state()

[Jonathon Jongsma]

This helper function does nothing but cast the return from
red_channel_client_get_channel(), and it has a confusing name
(_get_state(), but returns a channel)
---
 server/spicevmc.c | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 1a97b68..49de9cc 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -424,15 +424,9 @@ static void 
spicevmc_chardev_send_msg_to_client(RedCharDevice *self,
 red_channel_client_pipe_add_push(state->rcc, msg);
 }
 
-static RedVmcChannel *spicevmc_red_channel_client_get_state(RedChannelClient 
*rcc)
-{
-RedChannel *channel = red_channel_client_get_channel(rcc);
-return RED_VMC_CHANNEL(channel);
-}
-
 static void spicevmc_port_send_init(RedChannelClient *rcc)
 {
-RedVmcChannel *state = spicevmc_red_channel_client_get_state(rcc);
+RedVmcChannel *state = 
RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 SpiceCharDeviceInstance *sin;
 RedPortInitPipeItem *item = spice_malloc(sizeof(RedPortInitPipeItem));
 
@@ -505,7 +499,7 @@ static void 
spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
 return;
 }
 
-state = spicevmc_red_channel_client_get_state(rcc);
+state = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 
 /* partial message which wasn't pushed to device */
 red_char_device_write_buffer_release(state->chardev, 
>recv_from_client_buf);
@@ -543,7 +537,7 @@ static int 
spicevmc_channel_client_handle_migrate_data(RedChannelClient *rcc,
 SpiceMigrateDataSpiceVmc *mig_data;
 RedVmcChannel *state;
 
-state = spicevmc_red_channel_client_get_state(rcc);
+state = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 
 header = (SpiceMigrateDataHeader *)message;
 mig_data = (SpiceMigrateDataSpiceVmc *)(header + 1);
@@ -609,7 +603,7 @@ static int 
spicevmc_red_channel_client_handle_message_parsed(RedChannelClient *r
 SpiceCharDeviceInstance *sin;
 SpiceCharDeviceInterface *sif;
 
-state = spicevmc_red_channel_client_get_state(rcc);
+state = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 g_object_get(state->chardev, "sin", , NULL);
 sif = spice_char_device_get_interface(sin);
 
@@ -645,7 +639,7 @@ static uint8_t 
*spicevmc_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
 RedVmcChannel *state;
 RedClient *client = red_channel_client_get_client(rcc);
 
-state = spicevmc_red_channel_client_get_state(rcc);
+state = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 
 switch (type) {
 case SPICE_MSGC_SPICEVMC_DATA:
@@ -673,7 +667,7 @@ static void 
spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
 {
 RedVmcChannel *state;
 
-state = spicevmc_red_channel_client_get_state(rcc);
+state = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 
 switch (type) {
 case SPICE_MSGC_SPICEVMC_DATA:
@@ -712,7 +706,7 @@ static void 
spicevmc_red_channel_send_migrate_data(RedChannelClient *rcc,
 {
 RedVmcChannel *state;
 
-state = spicevmc_red_channel_client_get_state(rcc);
+state = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, item);
 spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_SPICEVMC_MAGIC);
 spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_SPICEVMC_VERSION);
-- 
2.7.4

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

Re: [Spice-devel] [PATCH spice-server v3 0/4] Changes a fix how RedCharDeviceSpiceVmc is freed

[Jonathon Jongsma]

ACK series

On Thu, 2016-11-03 at 16:33 +, Frediano Ziglio wrote:
> Move destruction to finalize/dispose methods instead of freeing
> from an external function.
> This make spicevmc_device_disconnect much more similar to
> smartcard_device_disconnect.
> 
> Frediano Ziglio (3):
>   spicevmc: Free pipe_item in finalize
>   spicevmc: More RedVmcChannel::recv_from_client_buf cleanup to
> finalize
>   spicevmc: Clear RedVmcChannel on red_char_device_spicevmc_dispose
> 
> Jonathon Jongsma (1):
>   spicevmc: store channel in char device
> 
>  server/spicevmc.c | 68 +++
> 
>  1 file changed, 48 insertions(+), 20 deletions(-)
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] Delay the exit call for the exit on disconnect function.

[Jeremy White]

>> On 11/03/2016 04:58 AM, Pavel Grunt wrote:
>>> moving it down can cause server to not exit in case client==NULL
>>
>> I spent some time and could not persuade myself of a case where this
>> function would be called with client == NULL.  Am I missing
>> something?
> 
> Same here, you are right, but things are changing a lot recently...
> And the check is there
> 
> There are more possibilities - one of these is to make sure it will
> not return earlier, like:
> ..
> if (!reds->config->exit_on_disconnect &&
> (!client || client->disconnecting))

Hmm.  I'm concerned that obfuscates the code a bit and muddles the
question of whether or not client can be null (for example, it implies
that client cannot be null when exit_on_disconnect is set, but it
otherwise can be).

The !client check was introduced by this commit:

commit 1078dc04edc406950e5f6d91bae456411eaa4a47
Author: Alon Levy 
Date:   Tue Aug 23 14:13:16 2011 +0300

server/reds: reds_client_disconnect: remove wrong check for
reds_main_channel_connected

The "channel->disconnecting" parameter already protects against
recursion.

Removed fixed TODOs.

I don't see any evidence that the '!client' check was really required;
it seems like something Alon may have injected reflexively.  It was not
checked prior to that patch.

So I'm inclined to switch it to just:

  if (client->disconnecting)

I think that communicates our understanding to future developers more
clearly, and it parallels similar checks in other places in the code.

Thoughts?

> ..
> 
> and move the if (reds->config->exit_on_disconnect) { exit()}
> like you did (or more to the end of the function)
> 
> 
>>
>>>
>>> OT: if we consider multiple client connections
>>> (SPICE_DEBUG_ALLOW_MC=1) than it is still leaking
>>
>> Yah.  Arguably, the exit on disconnect option should be mutually
>> exclusive of the ability to allow multiple clients.  That's not
>> enforced, currently, although this behavior does enforce it at first
>> client disconnect :-/.
> 
> maybe it can exit when all the clients disconnects - if the concern is
>  to clean properly all the clients? 
> 
> (but in the end everything is cleaned on exit, no ?)

Well, my specific concern is getting the event notification so that I
can run specialized code on disconnect; it's not so much about internal
Spice cleanup.

(For full edification, I've got code in x11spice that flips the desktop
background red when someone connects, and then relies on this disconnect
notice to put the desktop back to normal).

Cheers,

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

Re: [Spice-devel] [PATCH v2 2/3] char-device: add 'self' param to vfuncs

[Jonathon Jongsma]

On Thu, 2016-11-03 at 11:02 -0400, Frediano Ziglio wrote:
> > 
> > 
> > Add a 'self' parameter to all of the char device virtual functions
> > so
> > that we don't have to play games with the 'opaque' pointer.
> > ---
> > Changes in v2:
> >  - remove 'opaque' property from red_char_device_spicevmc_new()
> >  - use g_param_spec_object() instead of g_param_spec_pointer() for
> > 'channel'
> >    property
> > 
> >  server/char-device.c  | 31 ++---
> >  server/char-device.h  | 20 
> >  server/reds.c | 24 +-
> >  server/smartcard-channel-client.c |  2 +-
> >  server/smartcard.c| 32 ++---
> >  server/spicevmc.c | 97
> >  ---
> >  6 files changed, 125 insertions(+), 81 deletions(-)
> > 
> > diff --git a/server/char-device.c b/server/char-device.c
> > index 318bf3c..3b70066 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
> >  int during_read_from_device;
> >  int during_write_to_device;
> >  
> > -void *opaque;
> >  SpiceServer *reds;
> >  };
> >  
> > @@ -88,7 +87,6 @@ enum {
> >  PROP_SPICE_SERVER,
> >  PROP_CLIENT_TOKENS_INTERVAL,
> >  PROP_SELF_TOKENS,
> > -PROP_OPAQUE
> >  };
> >  
> >  static void
> > red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
> >  *write_buf);
> > @@ -99,7 +97,7 @@
> > red_char_device_read_one_msg_from_device(RedCharDevice
> > *dev)
> >  {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >  
> > -   return klass->read_one_msg_from_device(dev->priv->sin,
> > dev->priv->opaque);
> > +   return klass->read_one_msg_from_device(dev, dev->priv->sin);
> >  }
> >  
> >  static void
> > @@ -109,7 +107,7 @@
> > red_char_device_send_msg_to_client(RedCharDevice *dev,
> >  {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >  
> > -   klass->send_msg_to_client(msg, client, dev->priv->opaque);
> > +   klass->send_msg_to_client(dev, msg, client);
> >  }
> >  
> >  static void
> > @@ -119,7 +117,7 @@
> > red_char_device_send_tokens_to_client(RedCharDevice *dev,
> >  {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >  
> > -   klass->send_tokens_to_client(client, tokens, dev->priv-
> > >opaque);
> > +   klass->send_tokens_to_client(dev, client, tokens);
> >  }
> >  
> >  static void
> > @@ -128,7 +126,7 @@
> > red_char_device_on_free_self_token(RedCharDevice *dev)
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >  
> > if (klass->on_free_self_token != NULL) {
> > -   klass->on_free_self_token(dev->priv->opaque);
> > +   klass->on_free_self_token(dev);
> > }
> >  }
> >  
> > @@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice
> > *dev,
> > RedClient *client)
> >  {
> > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> >  
> > -   klass->remove_client(client, dev->priv->opaque);
> > +   klass->remove_client(dev, client);
> >  }
> >  
> >  static void
> > red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> > @@ -686,11 +684,6 @@ void
> > red_char_device_reset_dev_instance(RedCharDevice
> > *dev,
> >  g_object_notify(G_OBJECT(dev), "sin");
> >  }
> >  
> > -void *red_char_device_opaque_get(RedCharDevice *dev)
> > -{
> > -return dev->priv->opaque;
> > -}
> > -
> >  void red_char_device_destroy(RedCharDevice *char_dev)
> >  {
> >  g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
> > @@ -1041,9 +1034,6 @@
> > red_char_device_get_property(GObject*object,
> >  case PROP_SELF_TOKENS:
> >  g_value_set_uint64(value, self->priv-
> > >num_self_tokens);
> >  break;
> > -case PROP_OPAQUE:
> > -g_value_set_pointer(value, self->priv->opaque);
> > -break;
> >  default:
> >  G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> > pspec);
> >  }
> > @@ -1071,9 +1061,6 @@
> > red_char_device_set_property(GObject  *object,
> >  case PROP_SELF_TOKENS:
> >  self->priv->num_self_tokens =
> > g_value_get_uint64(value);
> >  break;
> > -case PROP_OPAQUE:
> > -self->priv->opaque = g_value_get_pointer(value);
> > -break;
> >  default:
> >  G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> > pspec);
> >  }
> > @@ -1156,14 +1143,6 @@
> > red_char_device_class_init(RedCharDeviceClass *klass)
> >  0,
> > G_MAXUINT64, 0,
> >  G_PARAM_ST
> > ATIC_STRINGS
> >  |
> >  G_PARAM_RE
> > ADWRITE));
> > -g_object_class_install_property(object_class,
> > -PROP_OPAQUE,
> > -  

Re: [Spice-devel] [drm/qxl v2 7/7] qxl: Allow resolution which are not multiple of 8

[Gerd Hoffmann]

On Do, 2016-11-03 at 12:41 +0100, Christophe Fergeau wrote:
> On Thu, Nov 03, 2016 at 09:53:48AM +0100, Gerd Hoffmann wrote:
> > On Mi, 2016-11-02 at 18:00 +0100, Christophe Fergeau wrote:
> > > The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
> > > the resolutions we are going to present to user-space are going to be
> > > rounded down to a multiple of 8. In the QXL arbitrary resolution case,
> > > this is not useful.
> > > This commit forces the actual width/height that was requested by the
> > > client in the drm_display_mode structure rather than keeping the
> > > rounded version.
> > 
> > Hmm, not sure this is a good idea.  There are probably reasons why
> > drm_cvt_mode is rounding down ...
> 
> Yeah, I'm sure there are reasons, but I don't know what they are.
> drm_cvt_mode seems to be calculating various frequencies and timings
> related to refreshing real world monitors, and this seems to be defined
> by some VESA standard. Maybe the rounding down is because the real-world
> monitors or VESA require it.

No worries here, we are in the virtual world, it for sure wouldn't break
monitors ;)

> Or maybe other parts of the
> kernel/userspace rely on this rounding down.

This is where I suspect we could run in trouble.  Odd resolutions simply
don't happen on physical hardware, all usual resolutions are a multiple
of 8, most of them are even a multiple of 16.

Various image/video formats use 16x16 blocks.
The qemu vnc server operates on 16x16 blocks too (and we had bugs in the
past with odd resolutions).

Also scanlines and cachelines align nicely if you don't use odd
resolutions.

> I unfortunately don't know
> :(

I don't have definitive answers too, just a gut feeling that this might
cause trouble.

Maybe we should add a module option for this?  So there is an easy
(as-in: doesn't require a kernel rebuild) way out in case it causes
trouble in certain setups?

cheers,
  Gerd

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

Re: [Spice-devel] [spice-server PATCH 8/8] red-record-qxl: child_output_setup: check fcntl return value

[Uri Lublin]

On 10/17/2016 01:08 PM, Frediano Ziglio wrote:


Also replaced "continue" in while block with an empty
block (added curly braces).



I think you split this in 7/8.


Right, I'll remove it.




Found by coverity.

Signed-off-by: Uri Lublin 
---
 server/red-record-qxl.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
index adc487b..21fc35f 100644
--- a/server/red-record-qxl.c
+++ b/server/red-record-qxl.c
@@ -845,13 +845,18 @@ void red_record_qxl_command(RedRecord *record,
RedMemSlotInfo *slots,
 static void child_output_setup(gpointer user_data)
 {
 int fd = GPOINTER_TO_INT(user_data);
+int r;

 while (dup2(fd, STDOUT_FILENO) < 0 && errno == EINTR) {
 }
 close(fd);

 // make sure file is not closed calling exec()
-fcntl(STDOUT_FILENO, F_SETFD, 0);
+while ((r = fcntl(STDOUT_FILENO, F_SETFD, 0)) < 0 && errno == EINTR) {
+}
+if (r < 0) {
+spice_error("fcntl F_SETFD failed (%d)\n", errno);
+}
 }

 RedRecord *red_record_new(const char *filename)


I tried to understand better this.
First: fcntl(F_SETFD) cannot return EINTR so there's no reason to check
 (unless you check for kernel bugs).
This would change the code to

if (fcntl(STDOUT_FILENO, F_SETFD, 0) < 0) {
spice_error("fcntl F_SETFD failed (%d)\n", errno);
}

Note however that probably this won't do what you are expecting.
This will put the message in the log and then spice-server will continue
and write the recording into a closed pipe so all fprintf internally will
call write which will return EPIPE.

Looking at the called function the file descriptor is the file descriptor
of "f" which is not created with O_CLOEXEC flag so the easier way to remove
this warning is just remove the fcntl line (which is doing nothing).


I think you are right, but also because of the dup2 that is a few lines
above the fcntl.



About O_CLOEXEC would be worth perhaps setting this flag after setting
up file/pipe (before the call to fwrite in red_record_new), something
like

if (fcntl(fileno(f), F_SETFD, O_CLOEXEC) < 0) {
spice_error("fcntl F_SETFD failed (%d)\n", errno);
}

it's a bit racy but not racy would require using G_SPAWN_CLOEXEC_PIPES
in g_spawn_async_with_pipes which is supported since GLib 2.40 or manually
build the pipe with pipe2 and O_CLOEXEC and passing it to 
g_spawn_async_with_pipes
(and opening the record file with "w+e" instead of just "w+").
But I don't think that risking to leak this file descriptor is a big deal...



right again.

I also noticed that if there are 2 (or more) QXL
devices, the same file is used for all, which is problematic

Thanks,
Uri.

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

[Spice-devel] [PATCH qxl-wddm-dod v3] Configurable version information in binary and INF

[yuri . benditovich]

From: Yuri Benditovich 

Changes after initial submission:
* Changed _VERSION_Vx_ to VERSION_Vx
* Default version set to 10.0.0.13000
* removed redundant ifdefs in RC file
* Fixed typos in RC file

Yuri Benditovich (1):
  Configurable version information in binary and INF

 qxldod/Version.props  | 33 +
 qxldod/qxldod.rc  | 23 +--
 qxldod/qxldod.vcxproj |  1 +
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 qxldod/Version.props

-- 
2.7.0.windows.1

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

Re: [Spice-devel] [spice-server PATCH 4/8] main-channel: getpeername/getsockname return early if no sockfd

[Uri Lublin]

On 10/17/2016 01:38 PM, Frediano Ziglio wrote:


I'm not sure that needed as it seems getpeername/getsockname
accept int fd and return -1 for fd=-1

Signed-off-by: Uri Lublin 
---
 server/main-channel.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/server/main-channel.c b/server/main-channel.c
index bf84694..9ff4dcd 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -281,12 +281,24 @@ MainChannelClient *main_channel_link(MainChannel
*channel, RedClient *client,

 int main_channel_getsockname(MainChannel *main_chan, struct sockaddr *sa,
 socklen_t *salen)
 {
-return main_chan ?
getsockname(red_channel_get_first_socket(_chan->base), sa, salen) : -1;
+int socketfd;
+
+if (!main_chan || ((socketfd =
red_channel_get_first_socket(_chan->base)) < 0)) {
+return -1;
+}
+
+return getsockname(socketfd, sa, salen);
 }

 int main_channel_getpeername(MainChannel *main_chan, struct sockaddr *sa,
 socklen_t *salen)
 {
-return main_chan ?
getpeername(red_channel_get_first_socket(_chan->base), sa, salen) : -1;
+int socketfd;
+
+if (!main_chan || ((socketfd =
red_channel_get_first_socket(_chan->base)) < 0)) {
+return -1;
+}
+
+return getpeername(socketfd, sa, salen);
 }

 // TODO: ? shouldn't it disonnect all clients? or shutdown all
 main_channels?


Mumble... I don't know why but it does not look that good.

These functions assume that there are only one client.
They are used only by spice_server_get_sock_info and spice_server_get_peer_info
which are no more used by recent Qemu so mainly they are dead code.
Perhaps spice_server_get_sock_info and spice_server_get_peer_info should
check for the first client and call getpeername/getsockname by themself?
Or just adding a main_channel_get_socket instead of 2 obsolete main_channel_*
functions?


Yes, you are correct.
So, should we deprecate those spice_server_get_*_info calls ?

Yes, you are correct.
Currently multi-client is still considered experimental, but we
do have a lot of code to make it almost work.
So, should we deprecate those spice_servet_get_*_info calls ?



OT: red_channel_get_first_socket is used only for obsolete or wrong code.


Due to the assumption there is only one client.


OT: main_channel_close is causing a dandling file descriptor.


Yes, this function can iterate over all clients and close their
socketfd.

Thanks,
Uri.

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

[Spice-devel] [PATCH spice-server v3 3/4] spicevmc: More RedVmcChannel::recv_from_client_buf cleanup to finalize

[Frediano Ziglio]

No reason why this should be done only on spicevmc_device_disconnect.
red_char_device_write_buffer_release can be called with first pointer
NULL.

Signed-off-by: Frediano Ziglio 
---
 server/spicevmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 1eadbbb..b643787 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -256,6 +256,7 @@ red_vmc_channel_finalize(GObject *object)
 {
 RedVmcChannel *self = RED_VMC_CHANNEL(object);
 
+red_char_device_write_buffer_release(self->chardev, 
>recv_from_client_buf);
 if (self->pipe_item) {
 red_pipe_item_unref(>pipe_item->base);
 }
@@ -884,7 +885,6 @@ void spicevmc_device_disconnect(RedsState *reds, 
SpiceCharDeviceInstance *sin)
 channel = vmc->channel;
 vmc->channel = NULL;
 
-red_char_device_write_buffer_release(channel->chardev, 
>recv_from_client_buf);
 /* FIXME */
 red_char_device_destroy(RED_CHAR_DEVICE(vmc));
 channel->chardev = NULL;
-- 
2.7.4

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

[Spice-devel] [PATCH spice-server v3 2/4] spicevmc: Free pipe_item in finalize

[Frediano Ziglio]

Assure field is freed at the end and not used or allocate again.

Signed-off-by: Frediano Ziglio 
---
 server/spicevmc.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 8e23248..1eadbbb 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -251,6 +251,18 @@ red_vmc_channel_init(RedVmcChannel *self)
 {
 }
 
+static void
+red_vmc_channel_finalize(GObject *object)
+{
+RedVmcChannel *self = RED_VMC_CHANNEL(object);
+
+if (self->pipe_item) {
+red_pipe_item_unref(>pipe_item->base);
+}
+
+G_OBJECT_CLASS(red_vmc_channel_parent_class)->finalize(object);
+}
+
 static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t 
channel_type,
   SpiceCharDeviceInstance *sin)
 {
@@ -756,6 +768,7 @@ red_vmc_channel_class_init(RedVmcChannelClass *klass)
 object_class->get_property = red_vmc_channel_get_property;
 object_class->set_property = red_vmc_channel_set_property;
 object_class->constructed = red_vmc_channel_constructed;
+object_class->finalize = red_vmc_channel_finalize;
 
 channel_class->handle_parsed = 
spicevmc_red_channel_client_handle_message_parsed;
 
@@ -878,7 +891,6 @@ void spicevmc_device_disconnect(RedsState *reds, 
SpiceCharDeviceInstance *sin)
 sin->st = NULL;
 
 reds_unregister_channel(reds, RED_CHANNEL(channel));
-free(channel->pipe_item);
 red_channel_destroy(RED_CHANNEL(channel));
 }
 
-- 
2.7.4

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

[Spice-devel] [PATCH spice-server v3 1/4] spicevmc: store channel in char device

[Frediano Ziglio]

From: Jonathon Jongsma 

Store the channel in the char device object, and unref it in dispose.
Previously, we were just creating a channel and potentially allowing it
to leak.  There may be better long-term solutions, but this works for
now.
---
 server/spicevmc.c | 46 ++
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 7067bc7..8e23248 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -47,6 +47,9 @@
 #define BUF_SIZE (64 * 1024 + 32)
 #define COMPRESS_THRESHOLD 1000
 
+typedef struct RedVmcChannel RedVmcChannel;
+typedef struct RedVmcChannelClass RedVmcChannelClass;
+
 typedef struct RedVmcPipeItem {
 RedPipeItem base;
 
@@ -75,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass 
RedCharDeviceSpiceVmcClass;
 
 struct RedCharDeviceSpiceVmc {
 RedCharDevice parent;
+RedVmcChannel *channel;
 };
 
 struct RedCharDeviceSpiceVmcClass
@@ -89,9 +93,6 @@ static RedCharDevice 
*red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
 
 G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc, 
RED_TYPE_CHAR_DEVICE)
 
-#define RED_CHAR_DEVICE_SPICEVMC_PRIVATE(o) \
-(G_TYPE_INSTANCE_GET_PRIVATE ((o), RED_TYPE_CHAR_DEVICE_SPICEVMC, 
RedCharDeviceSpiceVmcPrivate))
-
 #define RED_TYPE_VMC_CHANNEL red_vmc_channel_get_type()
 
 #define RED_VMC_CHANNEL(obj) \
@@ -103,9 +104,6 @@ G_DEFINE_TYPE(RedCharDeviceSpiceVmc, 
red_char_device_spicevmc, RED_TYPE_CHAR_DEV
 #define RED_VMC_CHANNEL_GET_CLASS(obj) \
 (G_TYPE_INSTANCE_GET_CLASS((obj), RED_TYPE_VMC_CHANNEL, 
RedVmcChannelClass))
 
-typedef struct RedVmcChannel RedVmcChannel;
-typedef struct RedVmcChannelClass RedVmcChannelClass;
-
 struct RedVmcChannel
 {
 RedChannel parent;
@@ -855,28 +853,33 @@ RedCharDevice *spicevmc_device_connect(RedsState *reds,
SpiceCharDeviceInstance *sin,
uint8_t channel_type)
 {
+RedCharDeviceSpiceVmc *dev_state;
 RedVmcChannel *state = red_vmc_channel_new(reds, channel_type, sin);
 
-return state->chardev;
+dev_state = RED_CHAR_DEVICE_SPICEVMC(state->chardev);
+dev_state->channel = state;
+
+return RED_CHAR_DEVICE(dev_state);
 }
 
 /* Must be called from RedClient handling thread. */
 void spicevmc_device_disconnect(RedsState *reds, SpiceCharDeviceInstance *sin)
 {
-RedVmcChannel *state;
+RedVmcChannel *channel;
+RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(sin->st);
 
-/* FIXME */
-state = (RedVmcChannel 
*)red_char_device_opaque_get((RedCharDevice*)sin->st);
+channel = vmc->channel;
+vmc->channel = NULL;
 
-red_char_device_write_buffer_release(state->chardev, 
>recv_from_client_buf);
+red_char_device_write_buffer_release(channel->chardev, 
>recv_from_client_buf);
 /* FIXME */
-red_char_device_destroy((RedCharDevice*)sin->st);
-state->chardev = NULL;
+red_char_device_destroy(RED_CHAR_DEVICE(vmc));
+channel->chardev = NULL;
 sin->st = NULL;
 
-reds_unregister_channel(reds, RED_CHANNEL(state));
-free(state->pipe_item);
-red_channel_destroy(RED_CHANNEL(state));
+reds_unregister_channel(reds, RED_CHANNEL(channel));
+free(channel->pipe_item);
+red_channel_destroy(RED_CHANNEL(channel));
 }
 
 SPICE_GNUC_VISIBLE void spice_server_port_event(SpiceCharDeviceInstance *sin, 
uint8_t event)
@@ -904,10 +907,21 @@ SPICE_GNUC_VISIBLE void 
spice_server_port_event(SpiceCharDeviceInstance *sin, ui
 }
 
 static void
+red_char_device_spicevmc_dispose(GObject *object)
+{
+RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
+
+g_clear_object(>channel);
+}
+
+static void
 red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass *klass)
 {
+GObjectClass *object_class = G_OBJECT_CLASS(klass);
 RedCharDeviceClass *char_dev_class = RED_CHAR_DEVICE_CLASS(klass);
 
+object_class->dispose = red_char_device_spicevmc_dispose;
+
 char_dev_class->read_one_msg_from_device = 
spicevmc_chardev_read_msg_from_dev;
 char_dev_class->send_msg_to_client = spicevmc_chardev_send_msg_to_client;
 char_dev_class->send_tokens_to_client = 
spicevmc_char_dev_send_tokens_to_client;
-- 
2.7.4

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

[Spice-devel] [PATCH spice-server v3 0/4] Changes a fix how RedCharDeviceSpiceVmc is freed

[Frediano Ziglio]

Move destruction to finalize/dispose methods instead of freeing
from an external function.
This make spicevmc_device_disconnect much more similar to
smartcard_device_disconnect.

Frediano Ziglio (3):
  spicevmc: Free pipe_item in finalize
  spicevmc: More RedVmcChannel::recv_from_client_buf cleanup to finalize
  spicevmc: Clear RedVmcChannel on red_char_device_spicevmc_dispose

Jonathon Jongsma (1):
  spicevmc: store channel in char device

 server/spicevmc.c | 68 +++
 1 file changed, 48 insertions(+), 20 deletions(-)

-- 
2.7.4

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

Re: [Spice-devel] [PATCH] Delay the exit call for the exit on disconnect function.

[Pavel Grunt]

Hi Jeremy,

On Thu, 2016-11-03 at 10:33 -0500, Jeremy White wrote:
> Hi Pavel,
> 
> Thanks for the review.
> 
> On 11/03/2016 04:58 AM, Pavel Grunt wrote:
> > moving it down can cause server to not exit in case client==NULL
> 
> I spent some time and could not persuade myself of a case where this
> function would be called with client == NULL.  Am I missing
> something?

Same here, you are right, but things are changing a lot recently...
And the check is there

There are more possibilities - one of these is to make sure it will
not return earlier, like:
..
if (!reds->config->exit_on_disconnect &&
(!client || client->disconnecting))
..

and move the if (reds->config->exit_on_disconnect) { exit()}
like you did (or more to the end of the function)


> 
> > 
> > OT: if we consider multiple client connections
> > (SPICE_DEBUG_ALLOW_MC=1) than it is still leaking
> 
> Yah.  Arguably, the exit on disconnect option should be mutually
> exclusive of the ability to allow multiple clients.  That's not
> enforced, currently, although this behavior does enforce it at first
> client disconnect :-/.

maybe it can exit when all the clients disconnects - if the concern is
 to clean properly all the clients? 

(but in the end everything is cleaned on exit, no ?)

Thanks,
Pavel

> 
> Cheers,
> 
> Jeremy
> 
> > 
> > Pavel
> > 
> > On Wed, 2016-11-02 at 10:42 -0500, Jeremy White wrote:
> > > This will allow client cleanup to happen.
> > > 
> > > Signed-off-by: Jeremy White 
> > > ---
> > >  server/reds.c | 12 ++--
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/server/reds.c b/server/reds.c
> > > index c235421..e380dd1 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -559,12 +559,6 @@ void reds_client_disconnect(RedsState
> > > *reds,
> > > RedClient *client)
> > >  {
> > >  RedsMigTargetClient *mig_client;
> > >  
> > > -if (reds->config->exit_on_disconnect)
> > > -{
> > > -spice_info("Exiting server because of client
> > > disconnect.\n");
> > > -exit(0);
> > > -}
> > > -
> > >  if (!client || client->disconnecting) {
> > >  spice_debug("client %p already during disconnection",
> > > client);
> > >  return;
> > > @@ -599,6 +593,12 @@ void reds_client_disconnect(RedsState
> > > *reds,
> > > RedClient *client)
> > >  reds->num_clients--;
> > >  red_client_destroy(client);
> > >  
> > > +if (reds->config->exit_on_disconnect)
> > > +{
> > > +spice_info("Exiting server because of client
> > > disconnect.\n");
> > > +exit(0);
> > > +}
> > > +
> > > // TODO: we need to handle agent properly for all
> > > clients
> > > (e.g., cut and paste, how? Maybe throw away messages
> > > // if we are in the middle of one from another client)
> > >  if (reds->num_clients == 0) {
> 
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] Delay the exit call for the exit on disconnect function.

[Jeremy White]

Hi Pavel,

Thanks for the review.

On 11/03/2016 04:58 AM, Pavel Grunt wrote:
> moving it down can cause server to not exit in case client==NULL

I spent some time and could not persuade myself of a case where this
function would be called with client == NULL.  Am I missing something?

> 
> OT: if we consider multiple client connections
> (SPICE_DEBUG_ALLOW_MC=1) than it is still leaking

Yah.  Arguably, the exit on disconnect option should be mutually
exclusive of the ability to allow multiple clients.  That's not
enforced, currently, although this behavior does enforce it at first
client disconnect :-/.

Cheers,

Jeremy

> 
> Pavel
> 
> On Wed, 2016-11-02 at 10:42 -0500, Jeremy White wrote:
>> This will allow client cleanup to happen.
>>
>> Signed-off-by: Jeremy White 
>> ---
>>  server/reds.c | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/server/reds.c b/server/reds.c
>> index c235421..e380dd1 100644
>> --- a/server/reds.c
>> +++ b/server/reds.c
>> @@ -559,12 +559,6 @@ void reds_client_disconnect(RedsState *reds,
>> RedClient *client)
>>  {
>>  RedsMigTargetClient *mig_client;
>>  
>> -if (reds->config->exit_on_disconnect)
>> -{
>> -spice_info("Exiting server because of client
>> disconnect.\n");
>> -exit(0);
>> -}
>> -
>>  if (!client || client->disconnecting) {
>>  spice_debug("client %p already during disconnection",
>> client);
>>  return;
>> @@ -599,6 +593,12 @@ void reds_client_disconnect(RedsState *reds,
>> RedClient *client)
>>  reds->num_clients--;
>>  red_client_destroy(client);
>>  
>> +if (reds->config->exit_on_disconnect)
>> +{
>> +spice_info("Exiting server because of client
>> disconnect.\n");
>> +exit(0);
>> +}
>> +
>> // TODO: we need to handle agent properly for all clients
>> (e.g., cut and paste, how? Maybe throw away messages
>> // if we are in the middle of one from another client)
>>  if (reds->num_clients == 0) {

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

Re: [Spice-devel] [PATCH v2 2/3] char-device: add 'self' param to vfuncs

[Frediano Ziglio]

> 
> Add a 'self' parameter to all of the char device virtual functions so
> that we don't have to play games with the 'opaque' pointer.
> ---
> Changes in v2:
>  - remove 'opaque' property from red_char_device_spicevmc_new()
>  - use g_param_spec_object() instead of g_param_spec_pointer() for 'channel'
>property
> 
>  server/char-device.c  | 31 ++---
>  server/char-device.h  | 20 
>  server/reds.c | 24 +-
>  server/smartcard-channel-client.c |  2 +-
>  server/smartcard.c| 32 ++---
>  server/spicevmc.c | 97
>  ---
>  6 files changed, 125 insertions(+), 81 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 318bf3c..3b70066 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
>  int during_read_from_device;
>  int during_write_to_device;
>  
> -void *opaque;
>  SpiceServer *reds;
>  };
>  
> @@ -88,7 +87,6 @@ enum {
>  PROP_SPICE_SERVER,
>  PROP_CLIENT_TOKENS_INTERVAL,
>  PROP_SELF_TOKENS,
> -PROP_OPAQUE
>  };
>  
>  static void red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
>  *write_buf);
> @@ -99,7 +97,7 @@ red_char_device_read_one_msg_from_device(RedCharDevice
> *dev)
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   return klass->read_one_msg_from_device(dev->priv->sin,
> dev->priv->opaque);
> +   return klass->read_one_msg_from_device(dev, dev->priv->sin);
>  }
>  
>  static void
> @@ -109,7 +107,7 @@ red_char_device_send_msg_to_client(RedCharDevice *dev,
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   klass->send_msg_to_client(msg, client, dev->priv->opaque);
> +   klass->send_msg_to_client(dev, msg, client);
>  }
>  
>  static void
> @@ -119,7 +117,7 @@ red_char_device_send_tokens_to_client(RedCharDevice *dev,
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   klass->send_tokens_to_client(client, tokens, dev->priv->opaque);
> +   klass->send_tokens_to_client(dev, client, tokens);
>  }
>  
>  static void
> @@ -128,7 +126,7 @@ red_char_device_on_free_self_token(RedCharDevice *dev)
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> if (klass->on_free_self_token != NULL) {
> -   klass->on_free_self_token(dev->priv->opaque);
> +   klass->on_free_self_token(dev);
> }
>  }
>  
> @@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice *dev,
> RedClient *client)
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   klass->remove_client(client, dev->priv->opaque);
> +   klass->remove_client(dev, client);
>  }
>  
>  static void red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> @@ -686,11 +684,6 @@ void red_char_device_reset_dev_instance(RedCharDevice
> *dev,
>  g_object_notify(G_OBJECT(dev), "sin");
>  }
>  
> -void *red_char_device_opaque_get(RedCharDevice *dev)
> -{
> -return dev->priv->opaque;
> -}
> -
>  void red_char_device_destroy(RedCharDevice *char_dev)
>  {
>  g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
> @@ -1041,9 +1034,6 @@ red_char_device_get_property(GObject*object,
>  case PROP_SELF_TOKENS:
>  g_value_set_uint64(value, self->priv->num_self_tokens);
>  break;
> -case PROP_OPAQUE:
> -g_value_set_pointer(value, self->priv->opaque);
> -break;
>  default:
>  G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
>  }
> @@ -1071,9 +1061,6 @@ red_char_device_set_property(GObject  *object,
>  case PROP_SELF_TOKENS:
>  self->priv->num_self_tokens = g_value_get_uint64(value);
>  break;
> -case PROP_OPAQUE:
> -self->priv->opaque = g_value_get_pointer(value);
> -break;
>  default:
>  G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
>  }
> @@ -1156,14 +1143,6 @@ red_char_device_class_init(RedCharDeviceClass *klass)
>  0, G_MAXUINT64, 0,
>  
> G_PARAM_STATIC_STRINGS
>  |
>  G_PARAM_READWRITE));
> -g_object_class_install_property(object_class,
> -PROP_OPAQUE,
> -g_param_spec_pointer("opaque",
> - "opaque",
> - "User data to pass
> to callbacks",
> -  G_PARAM_STATIC_STRINGS
> |
> -  G_PARAM_READWRITE));
> -
>  }
>  
>  static 

[Spice-devel] [PATCH v2 2/3] char-device: add 'self' param to vfuncs

[Jonathon Jongsma]

Add a 'self' parameter to all of the char device virtual functions so
that we don't have to play games with the 'opaque' pointer.
---
Changes in v2:
 - remove 'opaque' property from red_char_device_spicevmc_new()
 - use g_param_spec_object() instead of g_param_spec_pointer() for 'channel'
   property

 server/char-device.c  | 31 ++---
 server/char-device.h  | 20 
 server/reds.c | 24 +-
 server/smartcard-channel-client.c |  2 +-
 server/smartcard.c| 32 ++---
 server/spicevmc.c | 97 ---
 6 files changed, 125 insertions(+), 81 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index 318bf3c..3b70066 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
 int during_read_from_device;
 int during_write_to_device;
 
-void *opaque;
 SpiceServer *reds;
 };
 
@@ -88,7 +87,6 @@ enum {
 PROP_SPICE_SERVER,
 PROP_CLIENT_TOKENS_INTERVAL,
 PROP_SELF_TOKENS,
-PROP_OPAQUE
 };
 
 static void red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer 
*write_buf);
@@ -99,7 +97,7 @@ red_char_device_read_one_msg_from_device(RedCharDevice *dev)
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   return klass->read_one_msg_from_device(dev->priv->sin, dev->priv->opaque);
+   return klass->read_one_msg_from_device(dev, dev->priv->sin);
 }
 
 static void
@@ -109,7 +107,7 @@ red_char_device_send_msg_to_client(RedCharDevice *dev,
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   klass->send_msg_to_client(msg, client, dev->priv->opaque);
+   klass->send_msg_to_client(dev, msg, client);
 }
 
 static void
@@ -119,7 +117,7 @@ red_char_device_send_tokens_to_client(RedCharDevice *dev,
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   klass->send_tokens_to_client(client, tokens, dev->priv->opaque);
+   klass->send_tokens_to_client(dev, client, tokens);
 }
 
 static void
@@ -128,7 +126,7 @@ red_char_device_on_free_self_token(RedCharDevice *dev)
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
if (klass->on_free_self_token != NULL) {
-   klass->on_free_self_token(dev->priv->opaque);
+   klass->on_free_self_token(dev);
}
 }
 
@@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice *dev, RedClient 
*client)
 {
RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
 
-   klass->remove_client(client, dev->priv->opaque);
+   klass->remove_client(dev, client);
 }
 
 static void red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
@@ -686,11 +684,6 @@ void red_char_device_reset_dev_instance(RedCharDevice *dev,
 g_object_notify(G_OBJECT(dev), "sin");
 }
 
-void *red_char_device_opaque_get(RedCharDevice *dev)
-{
-return dev->priv->opaque;
-}
-
 void red_char_device_destroy(RedCharDevice *char_dev)
 {
 g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
@@ -1041,9 +1034,6 @@ red_char_device_get_property(GObject*object,
 case PROP_SELF_TOKENS:
 g_value_set_uint64(value, self->priv->num_self_tokens);
 break;
-case PROP_OPAQUE:
-g_value_set_pointer(value, self->priv->opaque);
-break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
 }
@@ -1071,9 +1061,6 @@ red_char_device_set_property(GObject  *object,
 case PROP_SELF_TOKENS:
 self->priv->num_self_tokens = g_value_get_uint64(value);
 break;
-case PROP_OPAQUE:
-self->priv->opaque = g_value_get_pointer(value);
-break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
 }
@@ -1156,14 +1143,6 @@ red_char_device_class_init(RedCharDeviceClass *klass)
 0, G_MAXUINT64, 0,
 G_PARAM_STATIC_STRINGS 
|
 G_PARAM_READWRITE));
-g_object_class_install_property(object_class,
-PROP_OPAQUE,
-g_param_spec_pointer("opaque",
- "opaque",
- "User data to pass to 
callbacks",
-  G_PARAM_STATIC_STRINGS |
-  G_PARAM_READWRITE));
-
 }
 
 static void
diff --git a/server/char-device.h b/server/char-device.h
index 44e9504..3b87023 100644
--- a/server/char-device.h
+++ b/server/char-device.h
@@ -56,25 +56,27 @@ struct RedCharDeviceClass
 
 /* reads from the device till reaching a msg that should be sent to the 
client,
  * or till the reading fails */
-RedPipeItem* 

Re: [Spice-devel] [PATCH spice-server v2] Manage lifetime of RedVmcChannel

[Frediano Ziglio]

> On Wed, 2016-11-02 at 13:05 -0400, Frediano Ziglio wrote:
> > > 
> > > 
> > > On Wed, 2016-11-02 at 08:31 -0400, Frediano Ziglio wrote:
> > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > On Wed, Nov 02, 2016 at 04:34:47AM -0400, Frediano Ziglio
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > From: Jonathon Jongsma 
> > > > > > > 
> > > > > > > Store the channel in the char device object, and unref it
> > > > > > > in
> > > > > > > dispose.
> > > > > > > Previously, we were just creating a channel and allowing it
> > > > > > > to
> > > > > > > leak.
> > > > > > > There may be better long-term solutions, but this works for
> > > > > > > now.
> > > > > > > ---
> > > > > > >  server/spicevmc.c | 27 ---
> > > > > > >  1 file changed, 20 insertions(+), 7 deletions(-)
> > > > > > > 
> > > > > > > Changes:
> > > > > > > - squashed fixup.
> > > > > > > 
> > > > > > > diff --git a/server/spicevmc.c b/server/spicevmc.c
> > > > > > > index 7067bc7..a77e868 100644
> > > > > > > --- a/server/spicevmc.c
> > > > > > > +++ b/server/spicevmc.c
> > > > > > > @@ -47,6 +47,9 @@
> > > > > > >  #define BUF_SIZE (64 * 1024 + 32)
> > > > > > >  #define COMPRESS_THRESHOLD 1000
> > > > > > >  
> > > > > > > +typedef struct RedVmcChannel RedVmcChannel;
> > > > > > > +typedef struct RedVmcChannelClass RedVmcChannelClass;
> > > > > > > +
> > > > > > >  typedef struct RedVmcPipeItem {
> > > > > > >  RedPipeItem base;
> > > > > > >  
> > > > > > > @@ -75,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass
> > > > > > > RedCharDeviceSpiceVmcClass;
> > > > > > >  
> > > > > > >  struct RedCharDeviceSpiceVmc {
> > > > > > >  RedCharDevice parent;
> > > > > > > +RedVmcChannel *channel;
> > > > > > >  };
> > > > > > >  
> > > > > > >  struct RedCharDeviceSpiceVmcClass
> > > > > > > @@ -89,9 +93,6 @@ static RedCharDevice
> > > > > > > *red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
> > > > > > >  
> > > > > > >  G_DEFINE_TYPE(RedCharDeviceSpiceVmc,
> > > > > > > red_char_device_spicevmc,
> > > > > > >  RED_TYPE_CHAR_DEVICE)
> > > > > > >  
> > > > > > > -#define RED_CHAR_DEVICE_SPICEVMC_PRIVATE(o) \
> > > > > > > -(G_TYPE_INSTANCE_GET_PRIVATE ((o),
> > > > > > > RED_TYPE_CHAR_DEVICE_SPICEVMC,
> > > > > > > RedCharDeviceSpiceVmcPrivate))
> > > > > > > -
> > > > > > >  #define RED_TYPE_VMC_CHANNEL red_vmc_channel_get_type()
> > > > > > >  
> > > > > > >  #define RED_VMC_CHANNEL(obj) \
> > > > > > > @@ -103,9 +104,6 @@ G_DEFINE_TYPE(RedCharDeviceSpiceVmc,
> > > > > > > red_char_device_spicevmc, RED_TYPE_CHAR_DEV
> > > > > > >  #define RED_VMC_CHANNEL_GET_CLASS(obj) \
> > > > > > >  (G_TYPE_INSTANCE_GET_CLASS((obj),
> > > > > > > RED_TYPE_VMC_CHANNEL,
> > > > > > >  RedVmcChannelClass))
> > > > > > >  
> > > > > > > -typedef struct RedVmcChannel RedVmcChannel;
> > > > > > > -typedef struct RedVmcChannelClass RedVmcChannelClass;
> > > > > > > -
> > > > > > >  struct RedVmcChannel
> > > > > > >  {
> > > > > > >  RedChannel parent;
> > > > > > > @@ -855,9 +853,13 @@ RedCharDevice
> > > > > > > *spicevmc_device_connect(RedsState
> > > > > > > *reds,
> > > > > > > SpiceCharDeviceInst
> > > > > > > ance
> > > > > > > *sin,
> > > > > > > uint8_t
> > > > > > > channel_type)
> > > > > > >  {
> > > > > > > +RedCharDeviceSpiceVmc *dev_state;
> > > > > > >  RedVmcChannel *state = red_vmc_channel_new(reds,
> > > > > > > channel_type, sin);
> > > > > > >  
> > > > > > > -return state->chardev;
> > > > > > > +dev_state = RED_CHAR_DEVICE_SPICEVMC(state->chardev);
> > > > > > > +dev_state->channel = state;
> > > > > > > +
> > > > > > > +return RED_CHAR_DEVICE(dev_state);
> > > > > > >  }
> > > > > > >  
> > > > > > >  /* Must be called from RedClient handling thread. */
> > > > > > > @@ -904,10 +906,21 @@ SPICE_GNUC_VISIBLE void
> > > > > > > spice_server_port_event(SpiceCharDeviceInstance *sin, ui
> > > > > > >  }
> > > > > > >  
> > > > > > >  static void
> > > > > > > +red_char_device_spicevmc_dispose(GObject *object)
> > > > > > > +{
> > > > > > > +RedCharDeviceSpiceVmc *self =
> > > > > > > RED_CHAR_DEVICE_SPICEVMC(object);
> > > > > > > +
> > > > > > > +g_clear_object(>channel);
> > > > > > 
> > > > > > This call produce a double free.
> > > > > > The channel is already freed in spicevmc_device_disconnect.
> > > > > 
> > > > > But g_clear_object() verify if channel is null before the unref
> > > > > and
> > > > > set
> > > > > it to NULL afterwards.
> > > > > 
> > > > 
> > > > Doing a NULL check on a dangling pointer is not much helpful.
> > > 
> > > Hmm, you're right. If we're going to remove the 'opaque' property
> > > (in a
> > > different patch), I think we do need to keep a pointer to the
> > > channel
> > > inside of the char device. But perhaps it should be a weak
> > > 

Re: [Spice-devel] [PATCH qxl-wddm-dod v2 1/1] Configurable version information in binary and INF

[Dmitry Fleytman]

Acked-by: Dmitry Fleytman 

> On 2 Nov 2016, at 02:26 AM, yuri.benditov...@daynix.com wrote:
> 
> From: Yuri Benditovich 
> 
> Version information in INF file is configured by
> environment variables.
> The same version information placed in driver binary.
> To set required version from external build engine,
> define following environment variables:
> VERSION_V1, VERSION_V2, VERSION_V3, VERSION_V4
> 
> Signed-off-by: Yuri Benditovich 
> ---
> qxldod/Version.props  | 33 +
> qxldod/qxldod.rc  | 23 +--
> qxldod/qxldod.vcxproj |  1 +
> 3 files changed, 55 insertions(+), 2 deletions(-)
> create mode 100644 qxldod/Version.props
> 
> diff --git a/qxldod/Version.props b/qxldod/Version.props
> new file mode 100644
> index 000..bdf02a9
> --- /dev/null
> +++ b/qxldod/Version.props
> @@ -0,0 +1,33 @@
> +
> +
> +http://schemas.microsoft.com/developer/msbuild/2003; 
> TreatAsLocalProperty="Platform">
> +  
> +
> +100
> +
> +6
> +
> +0
> +
> +13
> +
> $(VERSION_V1).$(VERSION_V2).$(VERSION_V3).$(VERSION_V4)
> +  
> +
> +  
> +  
> +
> +  
> VERSION_V3=$(VERSION_V3);VERSION_V4=$(VERSION_V4);VERSION_V1=$(VERSION_V1);VERSION_V2=$(VERSION_V2);%(PreprocessorDefinitions)
> +
> +
> +  
> VERSION_V3=$(VERSION_V3);VERSION_V4=$(VERSION_V4);VERSION_V1=$(VERSION_V1);VERSION_V2=$(VERSION_V2);%(PreprocessorDefinitions)
> +
> +
> +  $(STAMPINF_VERSION)
> +
> +  
> +
> diff --git a/qxldod/qxldod.rc b/qxldod/qxldod.rc
> index 7d7c326..de45b37 100755
> --- a/qxldod/qxldod.rc
> +++ b/qxldod/qxldod.rc
> @@ -2,11 +2,30 @@
> 
> #include 
> 
> +#undef VER_PRODUCTBUILD
> +#undef VER_PRODUCTBUILD_QFE
> +#undef VER_PRODUCTNAME_STR
> +#undef VER_PRODUCTMAJORVERSION
> +#undef VER_PRODUCTMINORVERSION
> +#undef VER_COMPANYNAME_STR
> +#undef VER_LEGALTRADEMARKS_STR
> +#undef VER_LEGALCOPYRIGHT_STR
> +
> +#define VER_COMPANYNAME_STR "Red Hat Inc."
> +#define VER_LEGALTRADEMARKS_STR ""
> +#define VER_LEGALCOPYRIGHT_STR  "Copyright (C) 2008-2016 Red Hat Inc."
> +
> +#define VER_PRODUCTMAJORVERSION VERSION_V1
> +#define VER_PRODUCTMINORVERSION VERSION_V2
> +#define VER_PRODUCTBUILDVERSION_V3
> +#define VER_PRODUCTBUILD_QFEVERSION_V4
> +
> #define VER_FILETYPEVFT_DRV
> #define VER_FILESUBTYPE VFT2_DRV_DISPLAY
> -#define VER_FILEDESCRIPTION_STR "QQL WDDM DOD"
> -#define VER_INTERNALNAME_STR"qxlod.sys"
> +#define VER_FILEDESCRIPTION_STR "QXL WDDM DOD"
> +#define VER_INTERNALNAME_STR"qxldod.sys"
> #define VER_ORIGINALFILENAME_STR"qxldod.sys"
> +#define VER_PRODUCTNAME_STR VER_FILEDESCRIPTION_STR
> 
> #define VER_LANGNEUTRAL
> #include "common.ver"
> diff --git a/qxldod/qxldod.vcxproj b/qxldod/qxldod.vcxproj
> index 2c10158..1766a61 100755
> --- a/qxldod/qxldod.vcxproj
> +++ b/qxldod/qxldod.vcxproj
> @@ -44,6 +44,7 @@
> WDM
>   
>   
> +  
>Condition="'$(Configuration)|$(Platform)'=='Win10Debug|Win32'" 
> Label="Configuration">
> 
> 
> -- 
> 2.7.0.windows.1
> 

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

[Spice-devel] [PATCH spice] spice-options-test: Convert to gtest to catch criticals

[Pavel Grunt]

Fail on glib warnings instead of ignoring them

Signed-off-by: Pavel Grunt 
---
 server/tests/spice-options-test.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/server/tests/spice-options-test.c 
b/server/tests/spice-options-test.c
index e762461..72ec215 100644
--- a/server/tests/spice-options-test.c
+++ b/server/tests/spice-options-test.c
@@ -26,7 +26,7 @@
 #define g_assert_nonnull g_assert
 #endif
 
-int main(int argc, char *argv[])
+static void agent_options(void)
 {
 SpiceCoreInterface *core ;
 SpiceServer *server = spice_server_new();
@@ -49,6 +49,13 @@ int main(int argc, char *argv[])
 spice_server_set_agent_file_xfer(server, 0);
 
 spice_server_destroy(server);
+}
+
+int main(int argc, char *argv[])
+{
+g_test_init(, , NULL);
+
+g_test_add_func("/server/agent options", agent_options);
 
-return 0;
+return g_test_run();
 }
-- 
2.10.1

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

Re: [Spice-devel] [PATCH 1/3] spicevmc: store channel in char device

[Frediano Ziglio]

> Store the channel in the char device object, and unref it in dispose.
> Previously, we were just creating a channel and potentially allowing it
> to leak.  There may be better long-term solutions, but this works for
> now.
> ---
> Changes from last version:
>  - set channel to NULL in spicevmc_device_disconnect() to avoid the
>  double-free
>mentioned by Frediano
> 

I was considering 1/3 and 3/3 patches. Would make sense to
have just the spicevmc_device_disconnect (and similar smartcard) to just
unref the device however g_clear_object on chardev is not enough:
- device destruction should call reds_unregister_channel to prevent
  possible future RedChannelClient to connect to the channel;
- destruction should call red_channel_destroy and not just g_object_unref
  (called by g_clear_object) to close current clients (no reasons to have
  client to handle closed devices).
Note that the above cannot/shouldn't be moved to the channel destructor
as there will be external (like from RedChannelClient) reference to
the channel that will prevent these stuff to be executed.

Possibly there could be a red_char_device_clear_channel(RedChannel **p_channel)
utility that check NULL pointer and do all these stuff to be called
by Smartcard/SpiceVmc instead of g_clear_pointer.

>  server/spicevmc.c | 46 ++
>  1 file changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 7067bc7..8e23248 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -47,6 +47,9 @@
>  #define BUF_SIZE (64 * 1024 + 32)
>  #define COMPRESS_THRESHOLD 1000
>  
> +typedef struct RedVmcChannel RedVmcChannel;
> +typedef struct RedVmcChannelClass RedVmcChannelClass;
> +
>  typedef struct RedVmcPipeItem {
>  RedPipeItem base;
>  
> @@ -75,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass
> RedCharDeviceSpiceVmcClass;
>  
>  struct RedCharDeviceSpiceVmc {
>  RedCharDevice parent;
> +RedVmcChannel *channel;
>  };
>  
>  struct RedCharDeviceSpiceVmcClass
> @@ -89,9 +93,6 @@ static RedCharDevice
> *red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
>  
>  G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc,
>  RED_TYPE_CHAR_DEVICE)
>  
> -#define RED_CHAR_DEVICE_SPICEVMC_PRIVATE(o) \
> -(G_TYPE_INSTANCE_GET_PRIVATE ((o), RED_TYPE_CHAR_DEVICE_SPICEVMC,
> RedCharDeviceSpiceVmcPrivate))
> -
>  #define RED_TYPE_VMC_CHANNEL red_vmc_channel_get_type()
>  
>  #define RED_VMC_CHANNEL(obj) \
> @@ -103,9 +104,6 @@ G_DEFINE_TYPE(RedCharDeviceSpiceVmc,
> red_char_device_spicevmc, RED_TYPE_CHAR_DEV
>  #define RED_VMC_CHANNEL_GET_CLASS(obj) \
>  (G_TYPE_INSTANCE_GET_CLASS((obj), RED_TYPE_VMC_CHANNEL,
>  RedVmcChannelClass))
>  
> -typedef struct RedVmcChannel RedVmcChannel;
> -typedef struct RedVmcChannelClass RedVmcChannelClass;
> -
>  struct RedVmcChannel
>  {
>  RedChannel parent;
> @@ -855,28 +853,33 @@ RedCharDevice *spicevmc_device_connect(RedsState *reds,
> SpiceCharDeviceInstance *sin,
> uint8_t channel_type)
>  {
> +RedCharDeviceSpiceVmc *dev_state;
>  RedVmcChannel *state = red_vmc_channel_new(reds, channel_type, sin);
>  
> -return state->chardev;
> +dev_state = RED_CHAR_DEVICE_SPICEVMC(state->chardev);
> +dev_state->channel = state;
> +
> +return RED_CHAR_DEVICE(dev_state);
>  }
>  
>  /* Must be called from RedClient handling thread. */
>  void spicevmc_device_disconnect(RedsState *reds, SpiceCharDeviceInstance
>  *sin)
>  {
> -RedVmcChannel *state;
> +RedVmcChannel *channel;
> +RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(sin->st);
>  
> -/* FIXME */
> -state = (RedVmcChannel
> *)red_char_device_opaque_get((RedCharDevice*)sin->st);
> +channel = vmc->channel;
> +vmc->channel = NULL;
>  
> -red_char_device_write_buffer_release(state->chardev,
> >recv_from_client_buf);
> +red_char_device_write_buffer_release(channel->chardev,
> >recv_from_client_buf);
>  /* FIXME */
> -red_char_device_destroy((RedCharDevice*)sin->st);
> -state->chardev = NULL;
> +red_char_device_destroy(RED_CHAR_DEVICE(vmc));
> +channel->chardev = NULL;
>  sin->st = NULL;
>  
> -reds_unregister_channel(reds, RED_CHANNEL(state));
> -free(state->pipe_item);
> -red_channel_destroy(RED_CHANNEL(state));
> +reds_unregister_channel(reds, RED_CHANNEL(channel));
> +free(channel->pipe_item);
> +red_channel_destroy(RED_CHANNEL(channel));
>  }
>  
>  SPICE_GNUC_VISIBLE void spice_server_port_event(SpiceCharDeviceInstance
>  *sin, uint8_t event)
> @@ -904,10 +907,21 @@ SPICE_GNUC_VISIBLE void
> spice_server_port_event(SpiceCharDeviceInstance *sin, ui
>  }
>  
>  static void
> +red_char_device_spicevmc_dispose(GObject *object)
> +{
> +RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
> +
> +g_clear_object(>channel);
> +}
> +
> +static void
> 

Re: [Spice-devel] [PATCH spice-gtk] Adjust include header to new location of macros

[Victor Toso]

Hi,

On Thu, Nov 03, 2016 at 01:10:20PM +0100, Pavel Grunt wrote:
> On Thu, 2016-11-03 at 13:05 +0100, Victor Toso wrote:
> > Hi,
> > 
> > On Tue, Nov 01, 2016 at 05:08:01PM +0100, Pavel Grunt wrote:
> > > minor & major macros were moved to sysmacros.h
> > > 
> > > usbutil.c: In function ‘spice_usbutil_get_sysfs_attribute’:
> > > usbutil.c:110:14: warning: ‘__major_from_sys_types’ is deprecated:
> > >   In the GNU C Library, `major' is defined by .
> > >   For historical compatibility, it is currently defined by
> > >    as well, but we plan to remove this soon.
> > >   To use `major', include  directly.
> > >   If you did not intend to use a system-defined macro `major',
> > >   you should #undef it after including .
> > >   [-Wdeprecated-declarations]
> > >   major(stat_buf.st_rdev), minor(stat_buf.st_rdev),
> > > attribute);
> > > ---
> > >  src/usbutil.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/usbutil.c b/src/usbutil.c
> > > index 7bfbe44..c4dd17c 100644
> > > --- a/src/usbutil.c
> > > +++ b/src/usbutil.c
> > > @@ -30,7 +30,10 @@
> > >  #ifdef __linux__
> > >  #include 
> > >  #include 
> > > +#include 
> > > +#ifndef major /* major and minor macros were moved to
> > > sys/sysmacros from sys/types.h */
> > >  #include 
> > > +#endif
> > 
> > So, sys/types.h was included only for major macro?
> 
> yes, for major and minor macros
> 
> > The patch removes the include if major is defined.
> > 
> > Well, it builds without warnings with your patch so I think it is
> > fine
> > but I would consider using #undef after  as the warning
> > suggests.
> there would be no "major" macro after that

 | To use `major', include  directly.
 | If you did not intend to use a system-defined macro `major',
 | you should #undef it after including .

I mean, undef after sys/types.h and then include it from the right
header (sys/sysmacros.h) (so you don't have two macros with same
name...)

Either way is fine to me.

> 
> > 
> > Should be fine either way.
> > 
> > Acked-by: Victor Toso 
> > 
> > >  #include 
> > >  #endif
> > >  #include "usbutil.h"
> > > -- 
> > > 2.10.1
> > > 
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel


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

Re: [Spice-devel] [PATCH spice-gtk] Adjust include header to new location of macros

[Pavel Grunt]

On Thu, 2016-11-03 at 13:05 +0100, Victor Toso wrote:
> Hi,
> 
> On Tue, Nov 01, 2016 at 05:08:01PM +0100, Pavel Grunt wrote:
> > minor & major macros were moved to sysmacros.h
> > 
> > usbutil.c: In function ‘spice_usbutil_get_sysfs_attribute’:
> > usbutil.c:110:14: warning: ‘__major_from_sys_types’ is deprecated:
> >   In the GNU C Library, `major' is defined by .
> >   For historical compatibility, it is currently defined by
> >    as well, but we plan to remove this soon.
> >   To use `major', include  directly.
> >   If you did not intend to use a system-defined macro `major',
> >   you should #undef it after including .
> >   [-Wdeprecated-declarations]
> >   major(stat_buf.st_rdev), minor(stat_buf.st_rdev),
> > attribute);
> > ---
> >  src/usbutil.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/usbutil.c b/src/usbutil.c
> > index 7bfbe44..c4dd17c 100644
> > --- a/src/usbutil.c
> > +++ b/src/usbutil.c
> > @@ -30,7 +30,10 @@
> >  #ifdef __linux__
> >  #include 
> >  #include 
> > +#include 
> > +#ifndef major /* major and minor macros were moved to
> > sys/sysmacros from sys/types.h */
> >  #include 
> > +#endif
> 
> So, sys/types.h was included only for major macro?

yes, for major and minor macros

> The patch removes the include if major is defined.
> 
> Well, it builds without warnings with your patch so I think it is
> fine
> but I would consider using #undef after  as the warning
> suggests.
there would be no "major" macro after that

> 
> Should be fine either way.
> 
> Acked-by: Victor Toso 
> 
> >  #include 
> >  #endif
> >  #include "usbutil.h"
> > -- 
> > 2.10.1
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 2/3] char-device: add 'self' param to vfuncs

[Pavel Grunt]

On Thu, 2016-11-03 at 06:43 -0400, Frediano Ziglio wrote:
> > 
> > Hi, it looks good to me, just an extra line addition in
> > red_vmc_channel_class_init
> > 
> > Ack
> > 
> > Pavel
> > 
> 
> Guy, how do you test? I just launched a VM and got 3, not 1
> critical messages...
> 
I got one


> Frediano
> 
> > On Wed, 2016-11-02 at 16:25 -0500, Jonathon Jongsma wrote:
> > > Add a 'self' parameter to all of the char device virtual
> > > functions
> > > so
> > > that we don't have to play games with the 'opaque' pointer.
> > > ---
> > >  server/char-device.c  | 31 ++---
> > >  server/char-device.h  | 20 
> > >  server/reds.c | 23 ++
> > >  server/smartcard-channel-client.c |  2 +-
> > >  server/smartcard.c| 32 ++---
> > >  server/spicevmc.c | 96
> > > ---
> > >  6 files changed, 124 insertions(+), 80 deletions(-)
> > > 
> > > diff --git a/server/char-device.c b/server/char-device.c
> > > index 318bf3c..3b70066 100644
> > > --- a/server/char-device.c
> > > +++ b/server/char-device.c
> > > @@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
> > >  int during_read_from_device;
> > >  int during_write_to_device;
> > >  
> > > -void *opaque;
> > >  SpiceServer *reds;
> > >  };
> > >  
> > > @@ -88,7 +87,6 @@ enum {
> > >  PROP_SPICE_SERVER,
> > >  PROP_CLIENT_TOKENS_INTERVAL,
> > >  PROP_SELF_TOKENS,
> > > -PROP_OPAQUE
> > >  };
> > >  
> > >  static void
> > > red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
> > > *write_buf);
> > > @@ -99,7 +97,7 @@
> > > red_char_device_read_one_msg_from_device(RedCharDevice *dev)
> > >  {
> > > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> > >  
> > > -   return klass->read_one_msg_from_device(dev->priv->sin, dev-
> > > > priv->opaque);
> > > 
> > > +   return klass->read_one_msg_from_device(dev, dev->priv->sin);
> > >  }
> > >  
> > >  static void
> > > @@ -109,7 +107,7 @@
> > > red_char_device_send_msg_to_client(RedCharDevice
> > > *dev,
> > >  {
> > > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> > >  
> > > -   klass->send_msg_to_client(msg, client, dev->priv->opaque);
> > > +   klass->send_msg_to_client(dev, msg, client);
> > >  }
> > >  
> > >  static void
> > > @@ -119,7 +117,7 @@
> > > red_char_device_send_tokens_to_client(RedCharDevice *dev,
> > >  {
> > > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> > >  
> > > -   klass->send_tokens_to_client(client, tokens, dev->priv-
> > > >opaque);
> > > +   klass->send_tokens_to_client(dev, client, tokens);
> > >  }
> > >  
> > >  static void
> > > @@ -128,7 +126,7 @@
> > > red_char_device_on_free_self_token(RedCharDevice
> > > *dev)
> > > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> > >  
> > > if (klass->on_free_self_token != NULL) {
> > > -   klass->on_free_self_token(dev->priv->opaque);
> > > +   klass->on_free_self_token(dev);
> > > }
> > >  }
> > >  
> > > @@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice
> > > *dev, RedClient *client)
> > >  {
> > > RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> > >  
> > > -   klass->remove_client(client, dev->priv->opaque);
> > > +   klass->remove_client(dev, client);
> > >  }
> > >  
> > >  static void
> > > red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> > > @@ -686,11 +684,6 @@ void
> > > red_char_device_reset_dev_instance(RedCharDevice *dev,
> > >  g_object_notify(G_OBJECT(dev), "sin");
> > >  }
> > >  
> > > -void *red_char_device_opaque_get(RedCharDevice *dev)
> > > -{
> > > -return dev->priv->opaque;
> > > -}
> > > -
> > >  void red_char_device_destroy(RedCharDevice *char_dev)
> > >  {
> > >  g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
> > > @@ -1041,9 +1034,6 @@
> > > red_char_device_get_property(GObject*object,
> > >  case PROP_SELF_TOKENS:
> > >  g_value_set_uint64(value, self->priv-
> > > >num_self_tokens);
> > >  break;
> > > -case PROP_OPAQUE:
> > > -g_value_set_pointer(value, self->priv->opaque);
> > > -break;
> > >  default:
> > >  G_OBJECT_WARN_INVALID_PROPERTY_ID(object,
> > > property_id,
> > > pspec);
> > >  }
> > > @@ -1071,9 +1061,6 @@
> > > red_char_device_set_property(GObject  *object,
> > >  case PROP_SELF_TOKENS:
> > >  self->priv->num_self_tokens =
> > > g_value_get_uint64(value);
> > >  break;
> > > -case PROP_OPAQUE:
> > > -self->priv->opaque = g_value_get_pointer(value);
> > > -break;
> > >  default:
> > >  G_OBJECT_WARN_INVALID_PROPERTY_ID(object,
> > > property_id,
> > > pspec);
> > >  }
> > > @@ -1156,14 +1143,6 @@
> > > red_char_device_class_init(RedCharDeviceClass
> > > *klass)
> > >   

Re: [Spice-devel] [PATCH spice-gtk] Adjust include header to new location of macros

[Victor Toso]

Hi,

On Tue, Nov 01, 2016 at 05:08:01PM +0100, Pavel Grunt wrote:
> minor & major macros were moved to sysmacros.h
>
> usbutil.c: In function ‘spice_usbutil_get_sysfs_attribute’:
> usbutil.c:110:14: warning: ‘__major_from_sys_types’ is deprecated:
>   In the GNU C Library, `major' is defined by .
>   For historical compatibility, it is currently defined by
>as well, but we plan to remove this soon.
>   To use `major', include  directly.
>   If you did not intend to use a system-defined macro `major',
>   you should #undef it after including .
>   [-Wdeprecated-declarations]
>   major(stat_buf.st_rdev), minor(stat_buf.st_rdev), attribute);
> ---
>  src/usbutil.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/usbutil.c b/src/usbutil.c
> index 7bfbe44..c4dd17c 100644
> --- a/src/usbutil.c
> +++ b/src/usbutil.c
> @@ -30,7 +30,10 @@
>  #ifdef __linux__
>  #include 
>  #include 
> +#include 
> +#ifndef major /* major and minor macros were moved to sys/sysmacros from 
> sys/types.h */
>  #include 
> +#endif

So, sys/types.h was included only for major macro?
The patch removes the include if major is defined.

Well, it builds without warnings with your patch so I think it is fine
but I would consider using #undef after  as the warning
suggests.

Should be fine either way.

Acked-by: Victor Toso 

>  #include 
>  #endif
>  #include "usbutil.h"
> -- 
> 2.10.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


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

Re: [Spice-devel] [drm/qxl v2 7/7] qxl: Allow resolution which are not multiple of 8

[Christophe Fergeau]

On Thu, Nov 03, 2016 at 09:53:48AM +0100, Gerd Hoffmann wrote:
> On Mi, 2016-11-02 at 18:00 +0100, Christophe Fergeau wrote:
> > The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
> > the resolutions we are going to present to user-space are going to be
> > rounded down to a multiple of 8. In the QXL arbitrary resolution case,
> > this is not useful.
> > This commit forces the actual width/height that was requested by the
> > client in the drm_display_mode structure rather than keeping the
> > rounded version.
> 
> Hmm, not sure this is a good idea.  There are probably reasons why
> drm_cvt_mode is rounding down ...

Yeah, I'm sure there are reasons, but I don't know what they are.
drm_cvt_mode seems to be calculating various frequencies and timings
related to refreshing real world monitors, and this seems to be defined
by some VESA standard. Maybe the rounding down is because the real-world
monitors or VESA require it. Or maybe other parts of the
kernel/userspace rely on this rounding down. I unfortunately don't know
:( Any guidance there whether that's ok, or whether I should approach
this differently would be very useful.

Christophe


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

Re: [Spice-devel] [spice-server PATCH 7/8] red-record-qxl: replace continue with empty block

[Uri Lublin]

On 10/17/2016 12:52 PM, Frediano Ziglio wrote:


Spice coding style suggests to use curly braces
for while blocks.

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

diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
index 8af2c9c..adc487b 100644
--- a/server/red-record-qxl.c
+++ b/server/red-record-qxl.c
@@ -846,8 +846,8 @@ static void child_output_setup(gpointer user_data)
 {
 int fd = GPOINTER_TO_INT(user_data);

-while (dup2(fd, STDOUT_FILENO) < 0 && errno == EINTR)
-continue;
+while (dup2(fd, STDOUT_FILENO) < 0 && errno == EINTR) {
+}
 close(fd);

 // make sure file is not closed calling exec()


There is another occurrence in the same file.


Yes, I'll modify both.


It's weird to see a close bracket just before a statement, perhaps a combination
of the two like

while (dup2(fd, STDOUT_FILENO) < 0 && errno == EINTR) {
continue;
}

is more "unexpected" ?


OK, I do not find it weird but I can leave the "continue;" in the block.

Thanks,
Uri

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

Re: [Spice-devel] [PATCH 3/3] Smartcard: store channel in device

[Frediano Ziglio]

> 
> Commit 542bc478 removed the global smartcard channel, but never stored the
> newly-created channel anywhere. To avoid leaking the channel, store the
> channel
> as a private member of the device it is associated with. It will be unreffed
> when its associated device is destroyed.
> ---
>  server/smartcard.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 5b18abe..bc1ada4 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -115,6 +115,7 @@ struct RedCharDeviceSmartcardPrivate {
>  
>  SmartCardChannelClient*scc; // client providing the remote card
>  int  reader_added; // has reader_add been sent to the
>  device
> +RedSmartcardChannel *channel;
>  };
>  
>  typedef struct RedMsgItem {
> @@ -136,7 +137,6 @@ static int smartcard_char_device_add_to_readers(RedsState
> *reds, SpiceCharDevice
>  static RedMsgItem *smartcard_char_device_on_message_from_device(
>  RedCharDeviceSmartcard *dev, VSCMsgHeader *header);
>  static RedCharDeviceSmartcard *smartcard_device_new(RedsState *reds,
>  SpiceCharDeviceInstance *sin);
> -static void smartcard_init(RedsState *reds);
>  
>  static void smartcard_read_buf_prepare(RedCharDeviceSmartcard *dev,
>  VSCMsgHeader *vheader)
>  {
> @@ -256,7 +256,7 @@ static int smartcard_char_device_add_to_readers(RedsState
> *reds, SpiceCharDevice
>  }
>  dev->priv->reader_id = g_smartcard_readers.num;
>  g_smartcard_readers.sin[g_smartcard_readers.num++] = char_device;
> -smartcard_init(reds);
> +dev->priv->channel = red_smartcard_channel_new(reds);
>  return 0;
>  }
>  
> @@ -602,13 +602,15 @@
> red_smartcard_channel_class_init(RedSmartcardChannelClass *klass)
>  
>  }
>  
> -static void smartcard_init(RedsState *reds)
> +static void
> +red_char_device_smartcard_dispose(GObject *object)
>  {
> -spice_assert(!reds_find_channel(reds, SPICE_CHANNEL_SMARTCARD, 0));

Here you removed the check for singleton. Are we going to support more 
smartcards?

> +RedCharDeviceSmartcard *self = RED_CHAR_DEVICE_SMARTCARD(object);
>  
> -red_smartcard_channel_new(reds);
> -}
> +g_clear_object(>priv->channel);
>  
> +G_OBJECT_CLASS(red_char_device_smartcard_parent_class)->dispose(object);
> +}
>  
>  static void
>  red_char_device_smartcard_finalize(GObject *object)
> @@ -628,6 +630,7 @@
> red_char_device_smartcard_class_init(RedCharDeviceSmartcardClass *klass)
>  
>  g_type_class_add_private(klass, sizeof (RedCharDeviceSmartcardPrivate));
>  
> +object_class->dispose = red_char_device_smartcard_dispose;
>  object_class->finalize = red_char_device_smartcard_finalize;
>  
>  char_dev_class->read_one_msg_from_device =
>  smartcard_read_msg_from_device;

Looks like this patch is only partial.
I think there is a missing part similar to spicevmc_device_disconnect.
Who is going to unregister the channel?
Note that is possible that channel will continue to stay alive even when
the device is deleted as long as clients are attached to the channel
(this is prevented by red_channel_destroy call for SpiceVmc).
I agree on the strong reference from device to channel; application asked
for a SpiceCharDeviceInstance and will release such device.

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

Re: [Spice-devel] [PATCH] Delay the exit call for the exit on disconnect function.

[Pavel Grunt]

moving it down can cause server to not exit in case client==NULL

OT: if we consider multiple client connections
(SPICE_DEBUG_ALLOW_MC=1) than it is still leaking

Pavel

On Wed, 2016-11-02 at 10:42 -0500, Jeremy White wrote:
> This will allow client cleanup to happen.
> 
> Signed-off-by: Jeremy White 
> ---
>  server/reds.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index c235421..e380dd1 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -559,12 +559,6 @@ void reds_client_disconnect(RedsState *reds,
> RedClient *client)
>  {
>  RedsMigTargetClient *mig_client;
>  
> -if (reds->config->exit_on_disconnect)
> -{
> -spice_info("Exiting server because of client
> disconnect.\n");
> -exit(0);
> -}
> -
>  if (!client || client->disconnecting) {
>  spice_debug("client %p already during disconnection",
> client);
>  return;
> @@ -599,6 +593,12 @@ void reds_client_disconnect(RedsState *reds,
> RedClient *client)
>  reds->num_clients--;
>  red_client_destroy(client);
>  
> +if (reds->config->exit_on_disconnect)
> +{
> +spice_info("Exiting server because of client
> disconnect.\n");
> +exit(0);
> +}
> +
> // TODO: we need to handle agent properly for all clients
> (e.g., cut and paste, how? Maybe throw away messages
> // if we are in the middle of one from another client)
>  if (reds->num_clients == 0) {
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Erratic mouse behavior (client mode)

[Pavel Duda]

Hi Victor,
works fine on fc25. I will also try to use the latest spice build on fc24 (or just migrate to fc25 as soon as possible :)).
 
Thanks & Have a Nice Day
 
Pavel Duda| tel: +420 27213 1108 | email: pavel.d...@cz.ibm.com |IBM Ceska republika, spol. s r. o.Registered address: V Parku 2294/4, Prague 4 - Chodov, Zip Code: 148 00Company ID: 14890992Entered in the Commercial Register maintained by the Municipal Court in Prague (Part C, Entry 692) 
 
 
- Original message -From: Victor Toso To: Pavel Duda/Czech Republic/IBM@IBMCZCc: spice-devel@lists.freedesktop.orgSubject: Re: [Spice-devel] Erratic mouse behavior (client mode)Date: Tue, Nov 1, 2016 12:09 PM 
Hi,On Mon, Oct 31, 2016 at 03:11:05PM +, Pavel Duda wrote:> Hello,> I'm experiencing weird bug on Fedora 24 when using mouse in client mode> together with IBM Sametime.Is it possible to test with Fedora 25? It should have at least thispatch [0] which is mostly related to server-side mode but it could helphere.[0] https://cgit.freedesktop.org/spice/spice-gtk/commit/?id=951c21d01712cd309239a94fb883e0fda1bf6a42> The cursor position and mouse clicks are fine as long as I do not open> some chat window in the IBM Sametime (see attached video).>> I have tried wo success> - to use multiple clients like spicy, remote-viewer, virt-viewer> - various guest OS tested: RHEL6, Ubuntu 16.04, Windows 7> - use guest with and without vdagent> - two different Sametime versions> - switching the window manager (Gnome 3, Gnome 2, Cinnamon)>> Switching the mouse mode to server works. The unmodified guests are also> working on another machines with the same versions of Sametime> so this really seems like an issue in FC24 and only when ST chat window is> open (ST contact list doesn't have any impact).>> The --spice-debug or --debug option for clients do not give much clues> however I'm not an expert so I might miss something there...> Is there some way how to find out what is causing this? Any advice how> should I proceed and what to check?>> Packages:> spice-glib-0.32-2.fc24.x86_64> spice-gtk-0.32-2.fc24.x86_64> spice-gtk3-0.32-2.fc24.x86_64> spice-gtk-tools-0.32-2.fc24.x86_64> spice-protocol-0.12.11-1.fc24.noarch> spice-server-0.12.8-1.fc24.x86_64> spice-xpi-2.8.90-10.fc24.x86_64> virt-viewer-4.0-1.fc24.x86_64> qemu-kvm-2.6.2-2.fc24.x86_64>> Thanks> & Have a Nice Day>>> S pozdravem / Kind regards>> Pavel Duda>> IBM Czech Republic>>> Phone:> +420-27213-1108>  IBM CR s.r.o.>> E-Mail:> pavel_d...@cz.ibm.com>  V Parku 2294/4>>>  14800 Praha>>>  Czech Republic>>>  >>> IBM Ceska republika, spol. s r. o.> Registered address: Prague 4, Chodov, V Parku 2294/4, Zip Code: 148 00,> Company ID: 14890992> Entered in the Commercial Register maintained by the Municipal Court in> Prague (Part C, Entry 692)>>> ___> Spice-devel mailing list> Spice-devel@lists.freedesktop.org> https://lists.freedesktop.org/mailman/listinfo/spice-devel 
 
 

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

Re: [Spice-devel] [PATCH 3/3] Smartcard: store channel in device

[Pavel Grunt]

Ack

On Wed, 2016-11-02 at 16:25 -0500, Jonathon Jongsma wrote:
> Commit 542bc478 removed the global smartcard channel, but never
> stored the
> newly-created channel anywhere. To avoid leaking the channel, store
> the channel
> as a private member of the device it is associated with. It will be
> unreffed
> when its associated device is destroyed.
> ---
>  server/smartcard.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 5b18abe..bc1ada4 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -115,6 +115,7 @@ struct RedCharDeviceSmartcardPrivate {
>  
>  SmartCardChannelClient*scc; // client providing the remote
> card
>  int  reader_added; // has reader_add been sent
> to the device
> +RedSmartcardChannel *channel;
>  };
>  
>  typedef struct RedMsgItem {
> @@ -136,7 +137,6 @@ static int
> smartcard_char_device_add_to_readers(RedsState *reds,
> SpiceCharDevice
>  static RedMsgItem *smartcard_char_device_on_message_from_device(
>  RedCharDeviceSmartcard *dev, VSCMsgHeader *header);
>  static RedCharDeviceSmartcard *smartcard_device_new(RedsState
> *reds, SpiceCharDeviceInstance *sin);
> -static void smartcard_init(RedsState *reds);
>  
>  static void smartcard_read_buf_prepare(RedCharDeviceSmartcard *dev,
> VSCMsgHeader *vheader)
>  {
> @@ -256,7 +256,7 @@ static int
> smartcard_char_device_add_to_readers(RedsState *reds,
> SpiceCharDevice
>  }
>  dev->priv->reader_id = g_smartcard_readers.num;
>  g_smartcard_readers.sin[g_smartcard_readers.num++] =
> char_device;
> -smartcard_init(reds);
> +dev->priv->channel = red_smartcard_channel_new(reds);
>  return 0;
>  }
>  
> @@ -602,13 +602,15 @@
> red_smartcard_channel_class_init(RedSmartcardChannelClass *klass)
>  
>  }
>  
> -static void smartcard_init(RedsState *reds)
> +static void
> +red_char_device_smartcard_dispose(GObject *object)
>  {
> -spice_assert(!reds_find_channel(reds, SPICE_CHANNEL_SMARTCARD,
> 0));
> +RedCharDeviceSmartcard *self =
> RED_CHAR_DEVICE_SMARTCARD(object);
>  
> -red_smartcard_channel_new(reds);
> -}
> +g_clear_object(>priv->channel);
>  
> +G_OBJECT_CLASS(red_char_device_smartcard_parent_class)-
> >dispose(object);
> +}
>  
>  static void
>  red_char_device_smartcard_finalize(GObject *object)
> @@ -628,6 +630,7 @@
> red_char_device_smartcard_class_init(RedCharDeviceSmartcardClass
> *klass)
>  
>  g_type_class_add_private(klass, sizeof
> (RedCharDeviceSmartcardPrivate));
>  
> +object_class->dispose = red_char_device_smartcard_dispose;
>  object_class->finalize = red_char_device_smartcard_finalize;
>  
>  char_dev_class->read_one_msg_from_device =
> smartcard_read_msg_from_device;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH 2/3] char-device: add 'self' param to vfuncs

[Pavel Grunt]

Hi, it looks good to me, just an extra line addition in
red_vmc_channel_class_init

Ack

Pavel

On Wed, 2016-11-02 at 16:25 -0500, Jonathon Jongsma wrote:
> Add a 'self' parameter to all of the char device virtual functions
> so
> that we don't have to play games with the 'opaque' pointer.
> ---
>  server/char-device.c  | 31 ++---
>  server/char-device.h  | 20 
>  server/reds.c | 23 ++
>  server/smartcard-channel-client.c |  2 +-
>  server/smartcard.c| 32 ++---
>  server/spicevmc.c | 96
> ---
>  6 files changed, 124 insertions(+), 80 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 318bf3c..3b70066 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -66,7 +66,6 @@ struct RedCharDevicePrivate {
>  int during_read_from_device;
>  int during_write_to_device;
>  
> -void *opaque;
>  SpiceServer *reds;
>  };
>  
> @@ -88,7 +87,6 @@ enum {
>  PROP_SPICE_SERVER,
>  PROP_CLIENT_TOKENS_INTERVAL,
>  PROP_SELF_TOKENS,
> -PROP_OPAQUE
>  };
>  
>  static void
> red_char_device_write_buffer_unref(RedCharDeviceWriteBuffer
> *write_buf);
> @@ -99,7 +97,7 @@
> red_char_device_read_one_msg_from_device(RedCharDevice *dev)
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   return klass->read_one_msg_from_device(dev->priv->sin, dev-
> >priv->opaque);
> +   return klass->read_one_msg_from_device(dev, dev->priv->sin);
>  }
>  
>  static void
> @@ -109,7 +107,7 @@ red_char_device_send_msg_to_client(RedCharDevice
> *dev,
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   klass->send_msg_to_client(msg, client, dev->priv->opaque);
> +   klass->send_msg_to_client(dev, msg, client);
>  }
>  
>  static void
> @@ -119,7 +117,7 @@
> red_char_device_send_tokens_to_client(RedCharDevice *dev,
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   klass->send_tokens_to_client(client, tokens, dev->priv->opaque);
> +   klass->send_tokens_to_client(dev, client, tokens);
>  }
>  
>  static void
> @@ -128,7 +126,7 @@ red_char_device_on_free_self_token(RedCharDevice
> *dev)
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> if (klass->on_free_self_token != NULL) {
> -   klass->on_free_self_token(dev->priv->opaque);
> +   klass->on_free_self_token(dev);
> }
>  }
>  
> @@ -137,7 +135,7 @@ red_char_device_remove_client(RedCharDevice
> *dev, RedClient *client)
>  {
> RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
>  
> -   klass->remove_client(client, dev->priv->opaque);
> +   klass->remove_client(dev, client);
>  }
>  
>  static void
> red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> @@ -686,11 +684,6 @@ void
> red_char_device_reset_dev_instance(RedCharDevice *dev,
>  g_object_notify(G_OBJECT(dev), "sin");
>  }
>  
> -void *red_char_device_opaque_get(RedCharDevice *dev)
> -{
> -return dev->priv->opaque;
> -}
> -
>  void red_char_device_destroy(RedCharDevice *char_dev)
>  {
>  g_return_if_fail(RED_IS_CHAR_DEVICE(char_dev));
> @@ -1041,9 +1034,6 @@
> red_char_device_get_property(GObject*object,
>  case PROP_SELF_TOKENS:
>  g_value_set_uint64(value, self->priv->num_self_tokens);
>  break;
> -case PROP_OPAQUE:
> -g_value_set_pointer(value, self->priv->opaque);
> -break;
>  default:
>  G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
>  }
> @@ -1071,9 +1061,6 @@
> red_char_device_set_property(GObject  *object,
>  case PROP_SELF_TOKENS:
>  self->priv->num_self_tokens =
> g_value_get_uint64(value);
>  break;
> -case PROP_OPAQUE:
> -self->priv->opaque = g_value_get_pointer(value);
> -break;
>  default:
>  G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
>  }
> @@ -1156,14 +1143,6 @@ red_char_device_class_init(RedCharDeviceClass
> *klass)
>  0,
> G_MAXUINT64, 0,
>  G_PARAM_STA
> TIC_STRINGS |
>  G_PARAM_REA
> DWRITE));
> -g_object_class_install_property(object_class,
> -PROP_OPAQUE,
> -g_param_spec_pointer("opaque",
> - "opaque",
> - "User data
> to pass to callbacks",
> -  G_PARAM_STATI
> C_STRINGS |
> -  G_PARAM_READW
> RITE));
> -
>  }
>  
>  static void
> diff --git a/server/char-device.h 

Re: [Spice-devel] [drm/qxl v2 7/7] qxl: Allow resolution which are not multiple of 8

[Gerd Hoffmann]

On Mi, 2016-11-02 at 18:00 +0100, Christophe Fergeau wrote:
> The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
> the resolutions we are going to present to user-space are going to be
> rounded down to a multiple of 8. In the QXL arbitrary resolution case,
> this is not useful.
> This commit forces the actual width/height that was requested by the
> client in the drm_display_mode structure rather than keeping the
> rounded version.

Hmm, not sure this is a good idea.  There are probably reasons why
drm_cvt_mode is rounding down ...

cheers,
  Gerd

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

Re: [Spice-devel] [PATCH 1/3] spicevmc: store channel in char device

[Pavel Grunt]

On Wed, 2016-11-02 at 16:25 -0500, Jonathon Jongsma wrote:
> Store the channel in the char device object, and unref it in
> dispose.
> Previously, we were just creating a channel and potentially allowing
> it
> to leak.  There may be better long-term solutions, but this works
> for
> now.
Acked-by: Pavel Grunt 
> ---
> Changes from last version:
>  - set channel to NULL in spicevmc_device_disconnect() to avoid the
> double-free
>    mentioned by Frediano
> 
>  server/spicevmc.c | 46 ++
> 
>  1 file changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 7067bc7..8e23248 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -47,6 +47,9 @@
>  #define BUF_SIZE (64 * 1024 + 32)
>  #define COMPRESS_THRESHOLD 1000
>  
> +typedef struct RedVmcChannel RedVmcChannel;
> +typedef struct RedVmcChannelClass RedVmcChannelClass;
> +
>  typedef struct RedVmcPipeItem {
>  RedPipeItem base;
>  
> @@ -75,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass
> RedCharDeviceSpiceVmcClass;
>  
>  struct RedCharDeviceSpiceVmc {
>  RedCharDevice parent;
> +RedVmcChannel *channel;
>  };
>  
>  struct RedCharDeviceSpiceVmcClass
> @@ -89,9 +93,6 @@ static RedCharDevice
> *red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
>  
>  G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc,
> RED_TYPE_CHAR_DEVICE)
>  
> -#define RED_CHAR_DEVICE_SPICEVMC_PRIVATE(o) \
> -(G_TYPE_INSTANCE_GET_PRIVATE ((o),
> RED_TYPE_CHAR_DEVICE_SPICEVMC, RedCharDeviceSpiceVmcPrivate))
> -
>  #define RED_TYPE_VMC_CHANNEL red_vmc_channel_get_type()
>  
>  #define RED_VMC_CHANNEL(obj) \
> @@ -103,9 +104,6 @@ G_DEFINE_TYPE(RedCharDeviceSpiceVmc,
> red_char_device_spicevmc, RED_TYPE_CHAR_DEV
>  #define RED_VMC_CHANNEL_GET_CLASS(obj) \
>  (G_TYPE_INSTANCE_GET_CLASS((obj), RED_TYPE_VMC_CHANNEL,
> RedVmcChannelClass))
>  
> -typedef struct RedVmcChannel RedVmcChannel;
> -typedef struct RedVmcChannelClass RedVmcChannelClass;
> -
>  struct RedVmcChannel
>  {
>  RedChannel parent;
> @@ -855,28 +853,33 @@ RedCharDevice
> *spicevmc_device_connect(RedsState *reds,
> SpiceCharDeviceInstance
> *sin,
> uint8_t channel_type)
>  {
> +RedCharDeviceSpiceVmc *dev_state;
>  RedVmcChannel *state = red_vmc_channel_new(reds, channel_type,
> sin);
>  
> -return state->chardev;
> +dev_state = RED_CHAR_DEVICE_SPICEVMC(state->chardev);
> +dev_state->channel = state;
> +
> +return RED_CHAR_DEVICE(dev_state);
>  }
>  
>  /* Must be called from RedClient handling thread. */
>  void spicevmc_device_disconnect(RedsState *reds,
> SpiceCharDeviceInstance *sin)
>  {
> -RedVmcChannel *state;
> +RedVmcChannel *channel;
> +RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(sin->st);
>  
> -/* FIXME */
> -state = (RedVmcChannel
> *)red_char_device_opaque_get((RedCharDevice*)sin->st);
> +channel = vmc->channel;
> +vmc->channel = NULL;
>  
> -red_char_device_write_buffer_release(state->chardev, 
> >recv_from_client_buf);
> +red_char_device_write_buffer_release(channel->chardev,
> >recv_from_client_buf);
>  /* FIXME */
> -red_char_device_destroy((RedCharDevice*)sin->st);
> -state->chardev = NULL;
> +red_char_device_destroy(RED_CHAR_DEVICE(vmc));
> +channel->chardev = NULL;
>  sin->st = NULL;
>  
> -reds_unregister_channel(reds, RED_CHANNEL(state));
> -free(state->pipe_item);
> -red_channel_destroy(RED_CHANNEL(state));
> +reds_unregister_channel(reds, RED_CHANNEL(channel));
> +free(channel->pipe_item);
> +red_channel_destroy(RED_CHANNEL(channel));
>  }
>  
>  SPICE_GNUC_VISIBLE void
> spice_server_port_event(SpiceCharDeviceInstance *sin, uint8_t event)
> @@ -904,10 +907,21 @@ SPICE_GNUC_VISIBLE void
> spice_server_port_event(SpiceCharDeviceInstance *sin, ui
>  }
>  
>  static void
> +red_char_device_spicevmc_dispose(GObject *object)
> +{
> +RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
> +
> +g_clear_object(>channel);
> +}
> +
> +static void
>  red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass
> *klass)
>  {
> +GObjectClass *object_class = G_OBJECT_CLASS(klass);
>  RedCharDeviceClass *char_dev_class =
> RED_CHAR_DEVICE_CLASS(klass);
>  
> +object_class->dispose = red_char_device_spicevmc_dispose;
> +
>  char_dev_class->read_one_msg_from_device =
> spicevmc_chardev_read_msg_from_dev;
>  char_dev_class->send_msg_to_client =
> spicevmc_chardev_send_msg_to_client;
>  char_dev_class->send_tokens_to_client =
> spicevmc_char_dev_send_tokens_to_client;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel