On Fri, 2017-08-25 at 10:54 +0100, Frediano Ziglio wrote:
> QXL device requires a QXL interface which imply all set
> of requirements like memory sharing.
> Currently RedCursorChannel uses QXL to release resources
> allocated from such devices.
> However this is quite a limitation as potentially we
ok
Acked-by: Jonathon Jongsma
On Fri, 2017-08-25 at 10:54 +0100, Frediano Ziglio wrote:
> Setting the capability is not enough, each stream must be enabled
> so do so if client support them.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/stream-channel.c | 19 +++
> 1 file
Acked-by: Jonathon Jongsma
On Fri, 2017-08-25 at 10:54 +0100, Frediano Ziglio wrote:
> Do not allow the guest to fill host memory.
> Also having a huge queue mainly cause to have a higher video
> latency.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/stream-channel.c | 41
> ++
I guess this depends on patch 2 ("notify client... dynamically"), so my
concerns there will potentially affect this patch. on additional
thought below.
On Fri, 2017-08-25 at 10:54 +0100, Frediano Ziglio wrote:
> This allows a better id allocation as devices are created after
> fixed ones.
> Also
On Fri, 2017-08-25 at 10:54 +0100, Frediano Ziglio wrote:
> When guest close the device the host device has to be reset too.
> This make easier to restart the guest device which can happen in case
> of reboot, agent issues or if we want to update the agent.
>
> Signed-off-by: Frediano Ziglio
> --
Acked-by: Jonathon Jongsma
On Fri, 2017-08-25 at 10:54 +0100, Frediano Ziglio wrote:
> Currently, red_char_device_reset() stops the device, clears all
> pending
> messages, and clears its device instance. After this function is
> called,
> the char device will not work again until it is assigned
On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> Handle stream data from device sending to the channel.
> The StreamChannel will forward the data to the clients using standard
> DisplayChannel messages, and will create and destroy streams as
> necessary.
>
> Signed-off-by: Frediano Zigl
Acked-by: Jonathon Jongsma
On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> So can be used by the device to communicate with the clients.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/stream-device.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/serv
I don't think the word "really" is necessary in the summary ("really
base").
The changes (passing ID to stream_channel_new(), registering channel in
constructor, etc) are an improvement
Acked-by: Jonathon Jongsma
On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> Currently only compile
On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> Parse the data sent from the guest to the streaming device.
For completeness perhaps add "At the moment, the data is simply
discarded after it is parsed."
>
> Signed-off-by: Frediano Ziglio
> ---
> server/stream-device.c | 115
> +
How Spice handle threading
--
Lot of code run in a single thread.
Qemu usually calls Spice from the same thread except on call backs to
code already run in different threads.
Channel run mostly on a single thread except on creation and
destroying which MUST be done in the
On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> This allows to add channels after the client is connected.
"allows to add" -> "allows the server to add"
>
> Signed-off-by: Frediano Ziglio
> ---
> server/main-channel-client.c | 50
>
> se
>
> On Fri, 2017-08-25 at 05:39 -0400, Frediano Ziglio wrote:
> > >
> > > I had a similar comment in a different patch during the last
> > > version of
> > > the series, but I personally would prefer a signal to handle this
> > > situation. For example, StreamChannel could have a "supported-
> >
On Thu, Aug 24, 2017 at 03:37:20PM +0100, Frediano Ziglio wrote:
> The channels list was not protected by a lock in red_client_destroy.
> This could cause for instance a RedChannelClient object to be
> created while scanning the list so potentially modifying the
> list while scanning with all race
>
> shortlog: "Avoid to" is not really grammatically correct. It should be
> something like "Avoid using" or "Don't use"
>
> On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> > This patch allocate VMC IDs finding the first ID not used
>
> "allocates"
> "by finding"
>
> > instead of us
On Fri, Aug 25, 2017 at 11:30:29AM -0400, Frediano Ziglio wrote:
> >
> > On Fri, Aug 25, 2017 at 10:33:28AM -0400, Frediano Ziglio wrote:
> > > >
> > > > On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> > > > > ORC lirabry is used internally by GStreamer to generate code
> > > >
On Thu, Aug 24, 2017 at 03:37:18PM +0100, Frediano Ziglio wrote:
> A RedClient can be freed from the main thread following
> a main channel disconnection.
I'd mention reds_client_disconnect or some concrete function name so
that someone wanting to follow that code has something to start from.
> T
>
> On Fri, Aug 25, 2017 at 10:33:28AM -0400, Frediano Ziglio wrote:
> > >
> > > On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> > > > ORC lirabry is used internally by GStreamer to generate code
> > > > dynamically.
> > >
> > > "The ORC library"
> > >
> > > > If ORC cannot a
shortlog: "Avoid to" is not really grammatically correct. It should be
something like "Avoid using" or "Don't use"
On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> This patch allocate VMC IDs finding the first ID not used
"allocates"
"by finding"
> instead of using a global variable a
On Fri, Aug 25, 2017 at 11:04:35AM -0400, Frediano Ziglio wrote:
> >
> > On Thu, Aug 24, 2017 at 03:37:16PM +0100, Frediano Ziglio wrote:
> > > You could easily trigger this issue using multiple monitors and
> > > a modified client with this patch:
> > >
> > > --- a/src/channel-main.c
> > > +++ b
>
> On Thu, Aug 24, 2017 at 03:37:16PM +0100, Frediano Ziglio wrote:
> > You could easily trigger this issue using multiple monitors and
> > a modified client with this patch:
> >
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -1699,6 +1699,7 @@ static gboolean _channel_new(chan
On Fri, Aug 25, 2017 at 10:33:28AM -0400, Frediano Ziglio wrote:
> >
> > On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> > > ORC lirabry is used internally by GStreamer to generate code
> > > dynamically.
> >
> > "The ORC library"
> >
> > > If ORC cannot allocate executable me
On Thu, Aug 24, 2017 at 03:37:16PM +0100, Frediano Ziglio wrote:
> You could easily trigger this issue using multiple monitors and
> a modified client with this patch:
>
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -1699,6 +1699,7 @@ static gboolean _channel_new(channel_new_t *c)
>
>
> On Fri, Aug 25, 2017 at 11:24:41AM +0100, Frediano Ziglio wrote:
> > The data pointer for client callbacks was not used.
> > The code was saving this information using callback registration
> > and g_object_set_data. Remove the g_object_set_data using
> > the data registered.
>
> Ah, hmm, I a
>
> >
> > On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> > > ORC lirabry is used internally by GStreamer to generate code
> > > dynamically.
> >
> > "The ORC library"
> >
> > > If ORC cannot allocate executable memory the failure cause
> > > an abort(3) to be called.
> >
>
>
> On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> > ORC lirabry is used internally by GStreamer to generate code
> > dynamically.
>
> "The ORC library"
>
> > If ORC cannot allocate executable memory the failure cause
> > an abort(3) to be called.
>
> "memory, the failure ca
Instead of using a separate parameter and field store
in the same structure.
This allows to pass the pointer while passing callbacks
and also make easier to understand that "data" field
is related to callbacks.
Also allows to retrieve client callback saved pointer.
The data pointer for client callb
On Fri, Aug 18, 2017 at 12:32:12PM +0100, Frediano Ziglio wrote:
> ORC lirabry is used internally by GStreamer to generate code
> dynamically.
"The ORC library"
> If ORC cannot allocate executable memory the failure cause
> an abort(3) to be called.
"memory, the failure causes"
> This happens o
On Fri, 2017-08-25 at 05:39 -0400, Frediano Ziglio wrote:
> >
> > I had a similar comment in a different patch during the last
> > version of
> > the series, but I personally would prefer a signal to handle this
> > situation. For example, StreamChannel could have a "supported-
> > codecs"
> > pro
On Thu, Aug 24, 2017 at 03:37:19PM +0100, Frediano Ziglio wrote:
> Allows to access it safely from different threads.
I guess it would be worth mentioning the code paths where it's needed in
the commit log.
Christophe
>
> Signed-off-by: Frediano Ziglio
> ---
> server/red-client.c | 8 +++-
Acked-by: Christophe Fergeau
On Thu, Aug 24, 2017 at 03:37:17PM +0100, Frediano Ziglio wrote:
> If a DisplayChannelClient cannot be instantiated capabilities
> are not released correctly.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/red-worker.c | 3 +--
> 1 file changed, 1 insertion(+),
Trace the number of loops done processing display commands
and the number of loops in which the queue was full.
Signed-off-by: Frediano Ziglio
---
This patch came from quite ancien time when we were debugging
the loop to understand some issues due to GLib integration.
Was marked as "statistics fr
Acked-by: Christophe Fergeau
On Fri, Aug 25, 2017 at 11:24:40AM +0100, Frediano Ziglio wrote:
> Client callbacks in sound channels do not use registered
> data so don't pass a valid pointer making clear from
> source that the parameter is not used.
>
> Signed-off-by: Frediano Ziglio
> ---
> s
On Fri, Aug 25, 2017 at 11:24:41AM +0100, Frediano Ziglio wrote:
> The data pointer for client callbacks was not used.
> The code was saving this information using callback registration
> and g_object_set_data. Remove the g_object_set_data using
> the data registered.
Ah, hmm, I actually had diffe
On Fri, Aug 25, 2017 at 11:24:39AM +0100, Frediano Ziglio wrote:
> A RedChannelClient is always attached to a valid RedChannel.
Yup, RedChannelClient::channel is construct-only, and
RedChannelClient::constructed already assumes that it has a non-NULL
channel.
Acked-by: Christophe Fergeau
>
> S
On Fri, Aug 25, 2017 at 07:37:05AM -0400, Frediano Ziglio wrote:
> >
> > I would propagate the types, something like this:
> >
> > https://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=teuf2&id=6930f008fd0dcbffc9e0c7a47e49dcdd1013b1e1
> >
> > Beside that,
> >
> > Acked-by: Frediano Zigli
Acked-by: Christophe Fergeau
On Fri, Aug 25, 2017 at 01:44:00PM +0100, Frediano Ziglio wrote:
> If you are testing for NULL data this means that variable could be
> NULL so avoid to access before the check to make sure the check is hit.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/cursor-
Sure,
Acked-by: Christophe Fergeau
On Fri, Aug 25, 2017 at 01:36:51PM +0100, Frediano Ziglio wrote:
> display variable already contains the DCC_TO_DC(dcc) value so
> reuse it.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/dcc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
If you are testing for NULL data this means that variable could be
NULL so avoid to access before the check to make sure the check is hit.
Signed-off-by: Frediano Ziglio
---
server/cursor-channel.c | 3 ++-
server/dcc-send.c | 11 ---
server/stream.c | 3 ++-
3 files chan
display variable already contains the DCC_TO_DC(dcc) value so
reuse it.
Signed-off-by: Frediano Ziglio
---
server/dcc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/dcc.c b/server/dcc.c
index 4490507b..2778bb88 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -297,7
>
> On Thu, Aug 24, 2017 at 05:11:34AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, 2017-08-22 at 08:54 +0100, Frediano Ziglio wrote:
> > > > The leak detector we use currently is not enough to detect
> > > > some kind of leak in DisplayChannel so manully test.
> > > > These tests are enable
On Thu, Aug 24, 2017 at 05:11:34AM -0400, Frediano Ziglio wrote:
> >
> > On Tue, 2017-08-22 at 08:54 +0100, Frediano Ziglio wrote:
> > > The leak detector we use currently is not enough to detect
> > > some kind of leak in DisplayChannel so manully test.
> > > These tests are enabled only when --e
Acked-by: Christophe Fergeau
On Fri, Aug 25, 2017 at 09:46:24AM +0100, Frediano Ziglio wrote:
> reds_get_n_clients is a single line and is used only by
> spice_server_get_num_clients.
> The 2 functions have very similar names so inlining
> reds_get_n_clients does not make code less readable.
>
>
> I would propagate the types, something like this:
>
> https://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=teuf2&id=6930f008fd0dcbffc9e0c7a47e49dcdd1013b1e1
>
> Beside that,
>
> Acked-by: Frediano Ziglio
>
Updated version of proposed fixup at
https://cgit.freedesktop.org/~fziglio
Client callbacks in sound channels do not use registered
data so don't pass a valid pointer making clear from
source that the parameter is not used.
Signed-off-by: Frediano Ziglio
---
server/sound.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/sound.c b/server/s
The data pointer for client callbacks was not used.
The code was saving this information using callback registration
and g_object_set_data. Remove the g_object_set_data using
the data registered.
Signed-off-by: Frediano Ziglio
---
server/red-channel.c | 5 +
server/red-channel.h | 2 ++
se
A RedChannelClient is always attached to a valid RedChannel.
Signed-off-by: Frediano Ziglio
---
server/red-qxl.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/server/red-qxl.c b/server/red-qxl.c
index 53f3338b..ba869e54 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@
Do not allow the guest to fill host memory.
Also having a huge queue mainly cause to have a higher video
latency.
Signed-off-by: Frediano Ziglio
---
server/stream-channel.c | 41 -
server/stream-channel.h | 10 ++
server/stream-device.c | 35 +
This is an hacky patch that tryes to reduce lag
---
server/stream-device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/stream-device.c b/server/stream-device.c
index 5ab41396..88e5abba 100644
--- a/server/stream-device.c
+++ b/server/stream-device.c
@@ -182,7 +182,7
Setting the capability is not enough, each stream must be enabled
so do so if client support them.
Signed-off-by: Frediano Ziglio
---
server/stream-channel.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/server/stream-channel.c b/server/stream-channel.c
index 34c8a351.
When guest close the device the host device has to be reset too.
This make easier to restart the guest device which can happen in case
of reboot, agent issues or if we want to update the agent.
Signed-off-by: Frediano Ziglio
---
server/stream-channel.c | 30 ++
server
When a new client is connected we must restart the stream so new
clients can receive correct data without having to wait for the
next full screen (which on idle screen could take ages).
On disconnection we should tell the guest to stop streaming
not wasting resources to stream not needed data.
Sig
This allows a better id allocation as devices are created after
fixed ones.
Also will allow to support more easily multiple monitor.
Signed-off-by: Frediano Ziglio
---
server/stream-device.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/ser
QXL device requires a QXL interface which imply all set
of requirements like memory sharing.
Currently RedCursorChannel uses QXL to release resources
allocated from such devices.
However this is quite a limitation as potentially we
can implement cursors in a virtual device or in a different
one.
Th
The channel needs to communicate when it receive a new
stream request from the guest.
Signed-off-by: Frediano Ziglio
---
server/stream-channel.c | 12
server/stream-channel.h | 6 ++
2 files changed, 18 insertions(+)
diff --git a/server/stream-channel.c b/server/stream-channel
Send proper messages to client to see the display.
Currently a blank display will be showed to the clients
connecting.
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/stream-channel.c | 52 +++--
1 file changed, 50 insertions(+),
TODO: reuse message code buffering
code to handle msg_buf can be reused by different messages
we should limit msg_buf base on type of messages
Signed-off-by: Frediano Ziglio
---
server/stream-device.c | 142 -
1 file changed, 141 insert
Currently only compile, not used and not much sense
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/Makefile.am | 2 +
server/stream-channel.c | 171
server/stream-channel.h | 53 +++
3 files changed, 226
So can be used by the device to communicate with the clients.
Signed-off-by: Frediano Ziglio
---
server/stream-device.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/server/stream-device.c b/server/stream-device.c
index e343f1f5..0551e5b5 100644
--- a/server/stream-device.c
This patch allocate VMC IDs finding the first ID not used
instead of using a global variable and incrementing the value
for each channel created.
This solve some potential issues:
- remove the global state potentially making possible
to use multiple SpiceServer on the same process;
- avoid to pot
Signed-off-by: Frediano Ziglio
---
server/stream-device.c | 35 ++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/server/stream-device.c b/server/stream-device.c
index 88e5abba..3d33ca13 100644
--- a/server/stream-device.c
+++ b/server/stream-device
Start showing something when we have a surface and stream
instead of showing a blank screen which is now not useful.
Was useful for debugging purposes to understand that the
new channel was sending messages correctly to client and
client could handle them.
Signed-off-by: Frediano Ziglio
Acked-by:
This allows to add channels after the client is connected.
Signed-off-by: Frediano Ziglio
---
server/main-channel-client.c | 50
server/main-channel-client.h | 3 +++
server/main-channel.c| 6 ++
server/main-channel.h| 3 +++
se
Currently, red_char_device_reset() stops the device, clears all pending
messages, and clears its device instance. After this function is called,
the char device will not work again until it is assigned a new device
instance and restarted. This is fine for the vdagent char device, which
is currently
Remove the fixed size stream and support any display size.
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/stream-channel.c | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/server/stream-channel.c b/server/stream-channel.c
Handle messages from clients.
Send some messages to clients.
Signed-off-by: Frediano Ziglio
Acked-by: Jonathon Jongsma
---
server/stream-channel.c | 41 -
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/server/stream-channel.c b/server/strea
Parse the data sent from the guest to the streaming device.
Signed-off-by: Frediano Ziglio
---
server/stream-device.c | 115 ++---
1 file changed, 110 insertions(+), 5 deletions(-)
diff --git a/server/stream-device.c b/server/stream-device.c
index f3a
Handle stream data from device sending to the channel.
The StreamChannel will forward the data to the clients using standard
DisplayChannel messages, and will create and destroy streams as
necessary.
Signed-off-by: Frediano Ziglio
---
server/stream-channel.c | 111 +++
Add a stub device in guest.
The aim of this device is to make it possible for the guest to send a
stream through a DisplayChannel (in the sense of protocol channel).
This stub allows the guest to send some data and you can see some debug
lines of data arrived on host logs.
Signed-off-by: Frediano
This patcheset add a device to allow guest machines with assigned
hardware devices to send encoded video and other informations that
will be handled as a normal display by Spice.
New GPU devices are capable to capture and encode video in an
efficient way. As the GPU in this case is assigned to the
>
> I had a similar comment in a different patch during the last version of
> the series, but I personally would prefer a signal to handle this
> situation. For example, StreamChannel could have a "supported-codecs"
> property, Then when a new client connected, or a client disconnected,
> it would
>
> On Wed, 2017-08-23 at 10:14 +0100, Frediano Ziglio wrote:
> > Remove the fixed size stream and support any display size.
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > server/stream-channel.c | 31 +--
> > 1 file changed, 25 insertions(+), 6 deletions(-)
> >
>
> On Wed, 2017-08-23 at 10:14 +0100, Frediano Ziglio wrote:
> > Handle stream data from device sending to the channel.
> > Channel will forward to clients as proper DisplayChannel
> > messaging creating and destroying the channel stream as
> > needed.
>
> This last sentence is a bit unclear. Pe
>
> On Wed, 2017-08-23 at 10:14 +0100, Frediano Ziglio wrote:
> > So can be used by the device to communicate with the clients.
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > server/stream-device.c | 20
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/server/
reds_get_n_clients is a single line and is used only by
spice_server_get_num_clients.
The 2 functions have very similar names so inlining
reds_get_n_clients does not make code less readable.
Signed-off-by: Frediano Ziglio
---
server/reds.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions
75 matches
Mail list logo
