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

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

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

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(+),

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

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

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

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

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

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

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

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:

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

>> 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,

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

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

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

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

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

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

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

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

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

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

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

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

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

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 ---

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

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

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

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

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

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

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

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

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

> > 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

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

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'

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

> 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: > > > >

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

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

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

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

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

> 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

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

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

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

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:

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

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 >

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

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 .

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

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

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

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

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

> > 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. > --- >

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

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

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

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,

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

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 >

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

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. > --- >

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

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

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

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.