Re: [Spice-devel] [spice-gtk v2 2/4] spice-channel: spice_channel_read_wire() improvements
Hi,
On Tue, Feb 21, 2017 at 06:18:29PM +0100, Christophe Fergeau wrote:
> On Fri, Feb 03, 2017 at 04:13:38PM +0100, Victor Toso wrote:
> > From: Victor Toso
> >
> > * Removes the reread label by having while(TRUE);
> >
> > The label is being used after g_coroutine_socket_wait() is called,
> > which is context switch while we can't do the reading as it would
> > block. Moving this inside a while() makes the code more straight
> > forward to be read and should not impact the performance.
> >
> > * Move local variables inside the block they will be used.
> >
> > Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
> > Signed-off-by: Victor Toso
> > ---
> > src/spice-channel.c | 38 --
> > 1 file changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > index a17c402..d1714df 100644
> > --- a/src/spice-channel.c
> > +++ b/src/spice-channel.c
> > @@ -1025,32 +1025,34 @@ static int
> > spice_channel_read_wire_nonblocking(SpiceChannel *channel,
> > static int spice_channel_read_wire(SpiceChannel *channel, void *data,
> > size_t len)
> > {
> > SpiceChannelPrivate *c = channel->priv;
> > -GIOCondition cond;
> > -gssize ret;
> >
> > -reread:
> > +while (TRUE) {
> > +gssize ret;
> > +GIOCondition cond;
> >
> > -if (c->has_error) return 0; /* has_error is set by disconnect(),
> > return no error */
> > +if (c->has_error) {
> > +/* has_error is set by disconnect(), return no error */
> > +return 0;
> > +}
> >
> > -ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> > +ret = spice_channel_read_wire_nonblocking(channel, data, len,
> > &cond);
> > +if (ret > 0)
> > +return ret;
> >
> > -if (ret == -1) {
> > -if (cond != 0) {
> > -// TODO: should use
> > g_pollable_input/output_stream_create_source() ?
> > -g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> > -goto reread;
> > -} else {
> > +if (ret == 0) {
> > +CHANNEL_DEBUG(channel, "Closing the connection:
> > spice_channel_read() - ret=0");
> > +c->has_error = TRUE;
> > +return 0;
> > +}
> > +
> > +if (cond == 0) {
> > c->has_error = TRUE;
> > return -errno;
> > }
> > -}
> > -if (ret == 0) {
> > -CHANNEL_DEBUG(channel, "Closing the connection:
> > spice_channel_read() - ret=0");
> > -c->has_error = TRUE;
> > -return 0;
> > -}
> >
> > -return ret;
> > +// TODO: should use g_pollable_input/output_stream_create_source()
> > ?
> > +g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> > +}
>
> So basically what this change is doing is moving all the checks
> leading to early returns first, and then it handles the remaining
> case, which consists in waiting for data availability.
Yep
> As with patch 4/4, I regret that we change for toplevel tests checking
> only for 'ret' value to toplevel tests testing either 'ret' or 'cond'.
> I'd also add a bunch of 'else' while you are at it so that it's
> explicit that everything is mutually exclusive.
Okay, I'll rethink it and try to improve. It is true that checking for
ret before and cond afterwards is worse now then it was before but
if + else if + else should improve it.
> If I were writing these patches, I might even go as far as adding a
> first commit doing a switch to while(TRUE) and replacing the 'goto'
> with 'continue', and doing the reformatting, and then have another
> commit which moves around the various conditions.
>
> Christophe
Sure, I'll do that.
Many thanks for the reviews!
toso
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] smartcard: fix memory leak in vcard_apdu_new
In the error path, 'new_apdu->a_data' is not freed.
This can be triggered by the guest continuely.
Signed-off-by: Li Qiang
---
src/card_7816.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/card_7816.c b/src/card_7816.c
index b598ef9..0082504 100644
--- a/src/card_7816.c
+++ b/src/card_7816.c
@@ -341,12 +341,12 @@ vcard_apdu_new(unsigned char *raw_apdu, int len,
vcard_7816_status_t *status)
new_apdu->a_len = len;
*status = vcard_apdu_set_class(new_apdu);
if (*status != VCARD7816_STATUS_SUCCESS) {
-g_free(new_apdu);
+vcard_apdu_delete(new_apdu);
return NULL;
}
*status = vcard_apdu_set_length(new_apdu);
if (*status != VCARD7816_STATUS_SUCCESS) {
-g_free(new_apdu);
+vcard_apdu_delete(new_apdu);
new_apdu = NULL;
}
return new_apdu;
--
1.8.3.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately
Hi,
On 02/21/2017 06:37 PM, Christophe Fergeau wrote:
On Sun, Feb 19, 2017 at 04:47:17PM +0200, Snir Sheriber wrote:
Remove handling with failures in the SASL authentication
process to separate function
---
src/spice-channel.c | 44 +++-
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/spice-channel.c b/src/spice-channel.c
index af67931..cbf1291 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel *channel,
void *data, size_t length)
return length;
}
+#if HAVE_SASL
/* coroutine context */
-static void spice_channel_failed_authentication(SpiceChannel *channel,
-gboolean invalidPassword)
+static void spice_channel_failed_sasl_authentication(SpiceChannel *channel)
{
SpiceChannelPrivate *c = channel->priv;
+gint err_code; /* Affects the authentication window activated fileds */
if (c->auth_needs_username && c->auth_needs_password)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-
SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
-_("Authentication failed: password and username are
required"));
+err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
else if (c->auth_needs_username)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
-_("Authentication failed: username is required"));
Is there a mechanism that allows only username ?
-else if (c->auth_needs_password)
-g_set_error_literal(&c->error,
-SPICE_CLIENT_ERROR,
-SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
-_("Authentication failed: password is required"));
-else if (invalidPassword)
+err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
+else
+err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
+
+g_set_error_literal(&c->error,
+SPICE_CLIENT_ERROR,
+err_code,
+_("SASL authentication failed"));
I don't understand what you are aiming for with this series. I thought
the goal was to get better error messages on autentication failures, but
this change merges 3 distinct error messages
_("Authentication failed: password and username are required"));
_("Authentication failed: username is required"));
_("Authentication failed: password is required"));
to just
_("SASL authentication failed"));
Christophe
It basically similar issue as in rhbz#1365736 but for the sasl
authentication case..
The auth_needs_username & auth_needs_password are used for signaling the
application if the selected
mechanism requires them, according to that the username\password fields
will be enabled\disabled, imo it's
enough for letting the user know "password and username are required"
The authentication type string is just for letting the user know what
authentication is in use.
This is not significant , but i guess it somehow more clear (or maybe
less confusing) to the user that way..
Snir.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately
On Wed, Feb 22, 2017 at 11:50:21AM +0200, Snir Sheriber wrote:
> Hi,
>
>
> On 02/21/2017 06:37 PM, Christophe Fergeau wrote:
> > On Sun, Feb 19, 2017 at 04:47:17PM +0200, Snir Sheriber wrote:
> > > Remove handling with failures in the SASL authentication
> > > process to separate function
> > > ---
> > > src/spice-channel.c | 44 +++-
> > > 1 file changed, 27 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > > index af67931..cbf1291 100644
> > > --- a/src/spice-channel.c
> > > +++ b/src/spice-channel.c
> > > @@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel
> > > *channel, void *data, size_t length)
> > > return length;
> > > }
> > > +#if HAVE_SASL
> > > /* coroutine context */
> > > -static void spice_channel_failed_authentication(SpiceChannel *channel,
> > > -gboolean invalidPassword)
> > > +static void spice_channel_failed_sasl_authentication(SpiceChannel
> > > *channel)
> > > {
> > > SpiceChannelPrivate *c = channel->priv;
> > > +gint err_code; /* Affects the authentication window activated fileds
> > > */
> > > if (c->auth_needs_username && c->auth_needs_password)
> > > -g_set_error_literal(&c->error,
> > > -SPICE_CLIENT_ERROR,
> > > -
> > > SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
> > > -_("Authentication failed: password and
> > > username are required"));
> > > +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
> > > else if (c->auth_needs_username)
> > > -g_set_error_literal(&c->error,
> > > -SPICE_CLIENT_ERROR,
> > > -SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> > > -_("Authentication failed: username is
> > > required"));
> Is there a mechanism that allows only username ?
I guess in SSO setups, it makes sense to first ask for just a username,
then check for a valid kerberos ticket for that username (or whatever
you use for SSO), and if there is no such ticket, then ask for an
additional authentication token.
> > > -else if (c->auth_needs_password)
> > > -g_set_error_literal(&c->error,
> > > -SPICE_CLIENT_ERROR,
> > > -SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
> > > -_("Authentication failed: password is
> > > required"));
> > > -else if (invalidPassword)
> > > +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
> > > +else
> > > +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
> > > +
> > > +g_set_error_literal(&c->error,
> > > +SPICE_CLIENT_ERROR,
> > > +err_code,
> > > +_("SASL authentication failed"));
> > I don't understand what you are aiming for with this series. I thought
> > the goal was to get better error messages on autentication failures, but
> > this change merges 3 distinct error messages
> > _("Authentication failed: password and username are required"));
> > _("Authentication failed: username is required"));
> > _("Authentication failed: password is required"));
> >
> > to just
> >
> > _("SASL authentication failed"));
> >
> > Christophe
> It basically similar issue as in rhbz#1365736 but for the sasl
> authentication case..
>
> The auth_needs_username & auth_needs_password are used for signaling the
> application if the selected
> mechanism requires them, according to that the username\password fields will
> be enabled\disabled,
Ah, right, so yes, library users can now about the need for username
and/or password through the SPICE_CLIENT_ERROR_AUTH_NEEDS_xxx error code
which is set in the GError returned through a channel-event signal. Then
application *should* disable unneeded fields. (I would not say "will" as
an application could do differently there ;)
> imo it's
> enough for letting the user know "password and username are required"
> The authentication type string is just for letting the user know what
> authentication is in use.
>
> This is not significant , but i guess it somehow more clear (or maybe less
> confusing) to the user that way..
Now I agree, but the commit log really should explain *why* you are
doing the change, and *why* this is not making the error reporting worse
for the user.
In this case, we could even add a few words in the documentation about
the expected behaviour of client applications to the
SPICE_CLIENT_ERROR_AUTH_NEEDS_xxx errors (assuming there is a good place
to document that).
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] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately
> On 21 Feb 2017, at 16:28, Snir Sheriber wrote:
>
> Hi,
>
>
> On 02/20/2017 07:00 PM, Christophe de Dinechin wrote:
>>> On 19 Feb 2017, at 15:47, Snir Sheriber wrote:
>>>
>>> Remove handling with failures in the SASL authentication
>>> process to separate function
>>> ---
>>> src/spice-channel.c | 44 +++-
>>> 1 file changed, 27 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/src/spice-channel.c b/src/spice-channel.c
>>> index af67931..cbf1291 100644
>>> --- a/src/spice-channel.c
>>> +++ b/src/spice-channel.c
>>> @@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel
>>> *channel, void *data, size_t length)
>>> return length;
>>> }
>>>
>>> +#if HAVE_SASL
>>> /* coroutine context */
>>> -static void spice_channel_failed_authentication(SpiceChannel *channel,
>>> -gboolean invalidPassword)
>>> +static void spice_channel_failed_sasl_authentication(SpiceChannel *channel)
>>> {
>>> SpiceChannelPrivate *c = channel->priv;
>>> +gint err_code; /* Affects the authentication window activated fileds */
>>>
>>> if (c->auth_needs_username && c->auth_needs_password)
>>> -g_set_error_literal(&c->error,
>>> -SPICE_CLIENT_ERROR,
>>> -
>>> SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>>> -_("Authentication failed: password and
>>> username are required"));
>>> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
>>> else if (c->auth_needs_username)
>>> -g_set_error_literal(&c->error,
>>> -SPICE_CLIENT_ERROR,
>>> -SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>>> -_("Authentication failed: username is
>>> required"));
>>> -else if (c->auth_needs_password)
>>> -g_set_error_literal(&c->error,
>>> -SPICE_CLIENT_ERROR,
>>> -SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
>>> -_("Authentication failed: password is
>>> required"));
>>> -else if (invalidPassword)
>>> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
>>> +else
>>> +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
>>> +
>>> +g_set_error_literal(&c->error,
>>> +SPICE_CLIENT_ERROR,
>>> +err_code,
>>> +_("SASL authentication failed"));
>> Per the recent discussion (Feb 14 with Christophe F), can’t we map common
>> SASL errors to Spice messages? To me, it’s different if the problem is that
>> I used a wrong password or if the server is down. The message as is seems
>> quite terse.
>>
>> Errors that seem be reportable (although not all of them seem relevant to
>> Spice):
>>
>> SASL_BADAUTH Authentication failure.
>> SASL_NOAUTHZ Authorization failure.
>> SASL_EXPIRED The passphrase expired and must be reset.
>> SASL_DISABLEDAccount disabled.
>> SASL_NOUSER User not found.
>> SASL_BADVERS Version mismatch with plug-in.
>> SASL_NOVERIFYThe user exists, but there is no verifier for the user.
>> SASL_WEAKPASSThe passphrase is too weak for security policy.
>> SASL_NOUSERPASS User supplied passwords are not permitted.
>>
>>
>> Some that may need to be “translated” in Spicese if they ever get back to us:
>>
>> SASL_TOOWEAK The mechanism is too weak for this user.
>> SASL_ENCRYPT Encryption is needed to use this mechanism.
>> SASL_TRANS One time use of a plaintext password will enable
>> requested mechanism for user.
>>
>> Others should probably collected into a “default” in a switch statement,
>> something like “Unexpected SASL error code ”.
>>
> As link result/error? I guess it would be the best , but first it requires to
> inform the client in some other way that it can stop waiting for the sasl
> server start\step result (currently it just freeing the link)
> btw according to sasl docs the full error string should be sent
Yes, but not translated. We can always add the SASL errors to the po files.
Christophe
>>> +
>>> +c->event = SPICE_CHANNEL_ERROR_AUTH;
>>> +
>>> +c->has_error = TRUE; /* force disconnect */
>>> +}
>>> +#endif
>>> +
>>> +/* coroutine context */
>>> +static void spice_channel_failed_authentication(SpiceChannel *channel,
>>> +gboolean invalidPassword)
>>> +{
>>> +SpiceChannelPrivate *c = channel->priv;
>>> +
>>> +if (invalidPassword)
>>> g_set_error_literal(&c->error,
>>> SPICE_CLIENT_ERROR,
>>> SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
>>> @@ -1808,7 +1818,7 @@ error:
>>> if (saslconn)
>>> sasl_dispose(&saslconn);
>>>
>>> -spice_channel_failed_authentication(channel, FALSE);
>>> +spice_channel_failed_sasl_authentication(channel);
>>>
Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately
Hey, On Wed, Feb 22, 2017 at 11:50:21AM +0200, Snir Sheriber wrote: > The authentication type string is just for letting the user know what > authentication is in use. Sorry, forgot to reply to this bit, I don't think saying "SASL" in this string is useful, this is going to be very cryptic to the users (I'm not even sure I knew what SASL was before working on SPICE), and imo is just relevant to sysadmins and spice-gtk developers, so making sure debug logs indicate which kind of auth mechanism is used should be enough. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server] Define a new interface for Qemu to pass EGL display
The new interface pass directly the EGL display to make
possible to extract texture data if necessary.
Also allows to use the same device using hardware
encoders.
Signed-off-by: Frediano Ziglio
---
server/red-qxl.c | 10 ++
server/spice-qxl.h | 3 +++
server/spice-server.syms | 5 +
3 files changed, 18 insertions(+)
diff --git a/server/red-qxl.c b/server/red-qxl.c
index b6b3770..367a428 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -60,6 +60,8 @@ struct QXLState {
RedsState *reds;
RedWorker *worker;
+void *egl_display;
+
pthread_mutex_t scanout_mutex;
SpiceMsgDisplayGlScanoutUnix scanout;
struct AsyncCommand *gl_draw_async;
@@ -865,6 +867,14 @@ void red_qxl_put_gl_scanout(QXLInstance *qxl,
SpiceMsgDisplayGlScanoutUnix *scan
}
SPICE_GNUC_VISIBLE
+void spice_qxl_gl_setup(QXLInstance *qxl,
+void *egl_display)
+{
+QXLState *qxl_state = qxl->st;
+qxl_state->egl_display = egl_display;
+}
+
+SPICE_GNUC_VISIBLE
void spice_qxl_gl_scanout(QXLInstance *qxl,
int fd,
uint32_t width, uint32_t height,
diff --git a/server/spice-qxl.h b/server/spice-qxl.h
index b8910bf..07da0dc 100644
--- a/server/spice-qxl.h
+++ b/server/spice-qxl.h
@@ -114,6 +114,9 @@ void spice_qxl_gl_draw_async(QXLInstance *instance,
uint32_t x, uint32_t y,
uint32_t w, uint32_t h,
uint64_t cookie);
+/* since spice 0.13.4 */
+void spice_qxl_gl_setup(QXLInstance *instance,
+void *egl_display);
typedef struct QXLDrawArea {
uint8_t *buf;
diff --git a/server/spice-server.syms b/server/spice-server.syms
index edf04a4..8aeb40f 100644
--- a/server/spice-server.syms
+++ b/server/spice-server.syms
@@ -173,3 +173,8 @@ SPICE_SERVER_0.13.2 {
global:
spice_server_set_video_codecs;
} SPICE_SERVER_0.13.1;
+
+SPICE_SERVER_0.13.4 {
+global:
+spice_qxl_gl_setup;
+} SPICE_SERVER_0.13.2;
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately
On Wed, Feb 22, 2017 at 11:47:10AM +0100, Christophe Fergeau wrote:
> On Wed, Feb 22, 2017 at 11:50:21AM +0200, Snir Sheriber wrote:
> > Hi,
> >
> >
> > On 02/21/2017 06:37 PM, Christophe Fergeau wrote:
> > > On Sun, Feb 19, 2017 at 04:47:17PM +0200, Snir Sheriber wrote:
> > > > Remove handling with failures in the SASL authentication
> > > > process to separate function
> > > > ---
> > > > src/spice-channel.c | 44 +++-
> > > > 1 file changed, 27 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > > > index af67931..cbf1291 100644
> > > > --- a/src/spice-channel.c
> > > > +++ b/src/spice-channel.c
> > > > @@ -1113,28 +1113,38 @@ static int spice_channel_read(SpiceChannel
> > > > *channel, void *data, size_t length)
> > > > return length;
> > > > }
> > > > +#if HAVE_SASL
> > > > /* coroutine context */
> > > > -static void spice_channel_failed_authentication(SpiceChannel *channel,
> > > > -gboolean
> > > > invalidPassword)
> > > > +static void spice_channel_failed_sasl_authentication(SpiceChannel
> > > > *channel)
> > > > {
> > > > SpiceChannelPrivate *c = channel->priv;
> > > > +gint err_code; /* Affects the authentication window activated
> > > > fileds */
> > > > if (c->auth_needs_username && c->auth_needs_password)
> > > > -g_set_error_literal(&c->error,
> > > > -SPICE_CLIENT_ERROR,
> > > > -
> > > > SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
> > > > -_("Authentication failed: password and
> > > > username are required"));
> > > > +err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
> > > > else if (c->auth_needs_username)
> > > > -g_set_error_literal(&c->error,
> > > > -SPICE_CLIENT_ERROR,
> > > > -SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> > > > -_("Authentication failed: username is
> > > > required"));
> > Is there a mechanism that allows only username ?
>
> I guess in SSO setups, it makes sense to first ask for just a username,
> then check for a valid kerberos ticket for that username (or whatever
> you use for SSO), and if there is no such ticket, then ask for an
> additional authentication token.
If you want to correctly use SASL then you should not make any
assumptions about which credentials you'll be asked for. Even if a
mechanism wants the username *and* password, it is permitted to
ask for them in separate steps of the handshake. So you might need
to popup a dialog to ask for username, and then later ask for
password in a new dialog popup. It is upto the mechanism plugin
to decide which to ask for at which point, so the app can not
predict that.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server] Define a new interface for Qemu to pass EGL display
Hi
- Original Message -
> The new interface pass directly the EGL display to make
> possible to extract texture data if necessary.
> Also allows to use the same device using hardware
> encoders.
I would like to see how this is going to be used before amending it, but a few
questions below:
>
> Signed-off-by: Frediano Ziglio
> ---
> server/red-qxl.c | 10 ++
> server/spice-qxl.h | 3 +++
> server/spice-server.syms | 5 +
> 3 files changed, 18 insertions(+)
>
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index b6b3770..367a428 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -60,6 +60,8 @@ struct QXLState {
> RedsState *reds;
> RedWorker *worker;
>
> +void *egl_display;
> +
Why not use EGLDisplay type?
> pthread_mutex_t scanout_mutex;
> SpiceMsgDisplayGlScanoutUnix scanout;
> struct AsyncCommand *gl_draw_async;
> @@ -865,6 +867,14 @@ void red_qxl_put_gl_scanout(QXLInstance *qxl,
> SpiceMsgDisplayGlScanoutUnix *scan
> }
>
> SPICE_GNUC_VISIBLE
> +void spice_qxl_gl_setup(QXLInstance *qxl,
> +void *egl_display)
> +{
> +QXLState *qxl_state = qxl->st;
> +qxl_state->egl_display = egl_display;
> +}
I would rather name the function spice_qxl_set_egl_display()
> +
> +SPICE_GNUC_VISIBLE
> void spice_qxl_gl_scanout(QXLInstance *qxl,
>int fd,
>uint32_t width, uint32_t height,
> diff --git a/server/spice-qxl.h b/server/spice-qxl.h
> index b8910bf..07da0dc 100644
> --- a/server/spice-qxl.h
> +++ b/server/spice-qxl.h
> @@ -114,6 +114,9 @@ void spice_qxl_gl_draw_async(QXLInstance *instance,
> uint32_t x, uint32_t y,
> uint32_t w, uint32_t h,
> uint64_t cookie);
> +/* since spice 0.13.4 */
> +void spice_qxl_gl_setup(QXLInstance *instance,
> +void *egl_display);
>
> typedef struct QXLDrawArea {
> uint8_t *buf;
> diff --git a/server/spice-server.syms b/server/spice-server.syms
> index edf04a4..8aeb40f 100644
> --- a/server/spice-server.syms
> +++ b/server/spice-server.syms
> @@ -173,3 +173,8 @@ SPICE_SERVER_0.13.2 {
> global:
> spice_server_set_video_codecs;
> } SPICE_SERVER_0.13.1;
> +
> +SPICE_SERVER_0.13.4 {
> +global:
> +spice_qxl_gl_setup;
> +} SPICE_SERVER_0.13.2;
> --
> 2.9.3
>
> ___
> 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 spice-server] Define a new interface for Qemu to pass EGL display
>
> Hi
>
> - Original Message -
> > The new interface pass directly the EGL display to make
> > possible to extract texture data if necessary.
> > Also allows to use the same device using hardware
> > encoders.
>
> I would like to see how this is going to be used before amending it, but a
> few questions below:
>
https://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=virgl-spice&id=a0b6ba1130e79ca59484462a2a4b7a64d301b474
http://gstreamer-devel.966125.n4.nabble.com/Sharing-EGL-context-with-glimagesink-td4673980.html
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > server/red-qxl.c | 10 ++
> > server/spice-qxl.h | 3 +++
> > server/spice-server.syms | 5 +
> > 3 files changed, 18 insertions(+)
> >
> > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > index b6b3770..367a428 100644
> > --- a/server/red-qxl.c
> > +++ b/server/red-qxl.c
> > @@ -60,6 +60,8 @@ struct QXLState {
> > RedsState *reds;
> > RedWorker *worker;
> >
> > +void *egl_display;
> > +
>
> Why not use EGLDisplay type?
>
It's the same type so don't require any header include.
> > pthread_mutex_t scanout_mutex;
> > SpiceMsgDisplayGlScanoutUnix scanout;
> > struct AsyncCommand *gl_draw_async;
> > @@ -865,6 +867,14 @@ void red_qxl_put_gl_scanout(QXLInstance *qxl,
> > SpiceMsgDisplayGlScanoutUnix *scan
> > }
> >
> > SPICE_GNUC_VISIBLE
> > +void spice_qxl_gl_setup(QXLInstance *qxl,
> > +void *egl_display)
> > +{
> > +QXLState *qxl_state = qxl->st;
> > +qxl_state->egl_display = egl_display;
> > +}
>
> I would rather name the function spice_qxl_set_egl_display()
>
> > +
> > +SPICE_GNUC_VISIBLE
> > void spice_qxl_gl_scanout(QXLInstance *qxl,
> >int fd,
> >uint32_t width, uint32_t height,
> > diff --git a/server/spice-qxl.h b/server/spice-qxl.h
> > index b8910bf..07da0dc 100644
> > --- a/server/spice-qxl.h
> > +++ b/server/spice-qxl.h
> > @@ -114,6 +114,9 @@ void spice_qxl_gl_draw_async(QXLInstance *instance,
> > uint32_t x, uint32_t y,
> > uint32_t w, uint32_t h,
> > uint64_t cookie);
> > +/* since spice 0.13.4 */
> > +void spice_qxl_gl_setup(QXLInstance *instance,
> > +void *egl_display);
> >
> > typedef struct QXLDrawArea {
> > uint8_t *buf;
> > diff --git a/server/spice-server.syms b/server/spice-server.syms
> > index edf04a4..8aeb40f 100644
> > --- a/server/spice-server.syms
> > +++ b/server/spice-server.syms
> > @@ -173,3 +173,8 @@ SPICE_SERVER_0.13.2 {
> > global:
> > spice_server_set_video_codecs;
> > } SPICE_SERVER_0.13.1;
> > +
> > +SPICE_SERVER_0.13.4 {
> > +global:
> > +spice_qxl_gl_setup;
> > +} SPICE_SERVER_0.13.2;
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 5/6] qxl-wddm-dod: Timer-based VSync interrupt indication
> On Thu, Feb 16, 2017 at 5:54 PM, Frediano Ziglio < fzig...@redhat.com >
> wrote:
> > >
>
> > > In case the driver supports VSync control feature, it
>
> > > maintains timer for VSync interrupt indication.
>
> > > In further commits this timer will be started upon
>
> > > class driver request. The interrupt notification and
>
> > > related DPC do not access device registers.
>
> > >
>
> > > Signed-off-by: Yuri Benditovich < yuri.benditov...@daynix.com >
>
> > > ---
>
> > > qxldod/QxlDod.cpp | 82
>
> > > +++
>
> > > qxldod/QxlDod.h | 14 ++
>
> > > 2 files changed, 96 insertions(+)
>
> > >
>
> > > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
>
> > > index cd22446..9175300 100755
>
> > > --- a/qxldod/QxlDod.cpp
>
> > > +++ b/qxldod/QxlDod.cpp
>
> > > @@ -82,6 +82,12 @@ QxlDod::QxlDod(_In_ DEVICE_OBJECT*
> > > pPhysicalDeviceObject)
>
> > > : m_pPhysicalDevice(pP
>
> > > RtlZeroMemory(m_CurrentModes, sizeof(m_CurrentModes));
>
> > > RtlZeroMemory(&m_PointerShape, sizeof(m_PointerShape));
>
> > > m_pHWDevice = NULL;
>
> > > +
>
> > > + KeInitializeDpc(&m_VsyncTimerDpc, VsyncTimerProcGate, this);
>
> > > + KeInitializeTimer(&m_VsyncTimer);
>
> > > + m_VsyncFiredCounter = 0;
>
> > > + m_bVsyncEnabled = FALSE;
>
> > > +
>
> > > DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
>
> > > }
>
> > >
>
> > > @@ -198,6 +204,7 @@ NTSTATUS QxlDod::StopDevice(VOID)
>
> > > {
>
> > > PAGED_CODE();
>
> > > m_Flags.DriverStarted = FALSE;
>
> > > + EnableVsync(FALSE);
>
> > > return STATUS_SUCCESS;
>
> > > }
>
> > >
>
> > > @@ -518,6 +525,7 @@ NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST
>
> > > DXGKARG_QUERYADAPTERINFO* pQueryAda
>
> > > pDriverCaps->PointerCaps.Color = 1;
>
> > >
>
> > > pDriverCaps->SupportNonVGA = m_pHWDevice->IsBIOSCompatible();
>
> > > + pDriverCaps->SchedulingCaps.VSyncPowerSaveAware =
>
> > > g_bSupportVSync;
>
> > >
>
> > I think we could set this to TRUE. If OS supports VSync will deactivate
>
> > if not should do nothing or try to deactivate.
>
> > Could be if set to FALSE that OS still expect VSync coming or don't
>
> > enable some energy saving policy.
>
> Setting it to TRUE when VSync is not supported may create some side-effect in
> Windows,
> as this combination is not expected. Setting it to FALSE when VSync is
> supported also does
> not make too much sense as then the driver will send indications also when
> the system
> is completely inactive. So, only meaningful choice for us is to keep it equal
> to VSync control.
> > > DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s 1\n", __FUNCTION__));
>
> > > return STATUS_SUCCESS;
>
> > > @@ -4813,6 +4821,14 @@ BOOLEAN QxlDevice::InterruptRoutine(_In_
>
> > > PDXGKRNL_INTERFACE pDxgkInterface, _In_
>
> > > }
>
> > >
>
> > > QXL_NON_PAGED
>
> > > +VOID QxlDevice::VSyncInterruptPostProcess(_In_ PDXGKRNL_INTERFACE
>
> > > pDxgkInterface)
>
> > > +{
>
> > > + if (!pDxgkInterface->DxgkCbQueueDpc(pDxgkInterface->DeviceHandle)) {
>
> > > + DbgPrint(TRACE_LEVEL_WARNING, ("---> %s can't enqueue DPC, pending
>
> > > interrupts %X\n", __FUNCTION__, m_Pending));
>
> > > + }
>
> > > +}
>
> > > +
>
> > > +QXL_NON_PAGED
>
> > > VOID QxlDevice::DpcRoutine(PVOID)
>
> > > {
>
> > > LONG intStatus = InterlockedExchange(&m_Pending, 0);
>
> > > @@ -4914,3 +4930,69 @@ NTSTATUS
>
> > > HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf
>
> > > }
>
> > > return Status;
>
> > > }
>
> > > +
>
> > > +// Vga device does not generate interrupts
>
> > > +QXL_NON_PAGED VOID VgaDevice::VSyncInterruptPostProcess(_In_
>
> > > PDXGKRNL_INTERFACE pxface)
>
> > > +{
>
> > > + pxface->DxgkCbQueueDpc(pxface->DeviceHandle);
>
> > > +}
>
> > > +
>
> > > +QXL_NON_PAGED VOID QxlDod::IndicateVSyncInterrupt()
>
> > > +{
>
> > Maybe is more flexible if this function returns BOOLEAN.
>
> > > + DXGKARGCB_NOTIFY_INTERRUPT_DATA data = {};
>
> > > + data.InterruptType = DXGK_INTERRUPT_DISPLAYONLY_VSYNC;
>
> > > + m_DxgkInterface.DxgkCbNotifyInterrupt(m_DxgkInterface.DeviceHandle,
>
> > > &data);
>
> > > + m_pHWDevice->VSyncInterruptPostProcess(&m_DxgkInterface);
>
> > > +}
>
> > > +
>
> > > +QXL_NON_PAGED BOOLEAN QxlDod::VsyncTimerSynchRoutine(PVOID context)
>
> > > +{
>
> > > + QxlDod* pQxl = reinterpret_cast(context);
>
> > > + pQxl->IndicateVSyncInterrupt();
>
> > > + return FALSE;
>
> > Here would be
>
> > return pQxl->IndicateVSyncInterrupt();
>
> > so we could handle the return value from the member function (if needed
>
> > in the future).
>
> This procedure currently returns boolean just because Windows
> synch routine prototype defined such a way without any reason.
> There is no meaningful information that we can pass back on this boolean.
> When synch procedure needs to return something little bigger than boolean,
> local context passed to the routine does the job.
> > > +}
>
> > > +
Re: [Spice-devel] [PATCH v3 6/6] qxl-wddm-dod: Preparation for VSync activation
>
> Prepare all the procedures needed for VSync feature but
> do not turn it on yet. Manual uncomment still required to
> enable it.
> Advanced users may enable VSync feature and report functional
> problem with it. The driver is expected to pass all the formal
> tests under HLK 1607 with device rev.3 and rev.4 but with
> device rev.4 on setups with high load or long round-trip delay
> video system may detect timeout during rendering operation
> and disable the driver with error 43.
>
> Signed-off-by: Yuri Benditovich
> ---
> qxldod/driver.cpp | 52
> qxldod/driver.h | 15 +++
> 2 files changed, 67 insertions(+)
>
> diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
> index dc84aa8..5bd57b4 100755
> --- a/qxldod/driver.cpp
> +++ b/qxldod/driver.cpp
> @@ -32,6 +32,19 @@ DriverEntry(
>
> DbgPrint(TRACE_LEVEL_FATAL, ("---> KMDOD build on on %s %s\n", __DATE__,
> __TIME__));
>
> +RTL_OSVERSIONINFOW versionInfo;
> +versionInfo.dwOSVersionInfoSize = sizeof(versionInfo);
> +
> +
> +RtlGetVersion(&versionInfo);
> +if (versionInfo.dwBuildNumber >= 14393 || versionInfo.dwBuildNumber <=
> 9600)
Can you put a comment in the code on why we need to exclude 9600 - 14393 range?
A bug?
> +{
> +//g_bSupportVSync = TRUE;
I would add also a comment before this line why this is commented
out.
> +}
> +DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for %d.%d.%d\n",
> +g_bSupportVSync ? "en" : "dis",
> +versionInfo.dwMajorVersion, versionInfo.dwMinorVersion,
> versionInfo.dwBuildNumber));
> +
> // Initialize DDI function pointers and dxgkrnl
> KMDDOD_INITIALIZATION_DATA InitialData = {0};
>
> @@ -67,6 +80,11 @@ DriverEntry(
> InitialData.DxgkDdiStopDeviceAndReleasePostDisplayOwnership =
> DodStopDeviceAndReleasePostDisplayOwnership;
> InitialData.DxgkDdiSystemDisplayEnable =
> DodSystemDisplayEnable;
> InitialData.DxgkDdiSystemDisplayWrite = DodSystemDisplayWrite;
> +if (g_bSupportVSync)
> +{
> +InitialData.DxgkDdiControlInterrupt = DodControlInterrupt;
> +InitialData.DxgkDdiGetScanLine = DodGetScanLine;
> +}
>
> NTSTATUS Status = DxgkInitializeDisplayOnlyDriver(pDriverObject,
> pRegistryPath, &InitialData);
> if (!NT_SUCCESS(Status))
> @@ -559,6 +577,40 @@ DodQueryVidPnHWCapability(
> return pQxl->QueryVidPnHWCapability(pVidPnHWCaps);
> }
>
> +NTSTATUS
> +APIENTRY
> +DodControlInterrupt(
> +IN_CONST_HANDLE hAdapter,
> +IN_CONST_DXGK_INTERRUPT_TYPEInterruptType,
> +IN_BOOLEAN EnableInterrupt
> +)
> +{
> +PAGED_CODE();
> +QxlDod* pQxl = reinterpret_cast(hAdapter);
> +if (InterruptType == DXGK_INTERRUPT_DISPLAYONLY_VSYNC)
> +{
> +pQxl->EnableVsync(EnableInterrupt);
> +return STATUS_SUCCESS;
> +}
> +return STATUS_NOT_IMPLEMENTED;
> +}
> +
> +NTSTATUS
> +APIENTRY
> +DodGetScanLine(
> +IN_CONST_HANDLE hAdapter,
> +INOUT_PDXGKARG_GETSCANLINE pGetScanLine
> +)
> +{
> +PAGED_CODE();
> +// Currently we do not see any practical use case when this procedure is
> called
> +// IDirectDraw has an interface for querying scan line
> +// Leave it not implemented like remote desktop does
> +// until we recognize use case for more intelligent implementation
> +DbgPrint(TRACE_LEVEL_ERROR, ("<---> %s\n", __FUNCTION__));
> +return STATUS_NOT_IMPLEMENTED;
> +}
> +
> //END: Paged Code
> #pragma code_seg(pop)
>
> diff --git a/qxldod/driver.h b/qxldod/driver.h
> index 97b9415..2dcbda4 100755
> --- a/qxldod/driver.h
> +++ b/qxldod/driver.h
> @@ -217,6 +217,21 @@ DodSystemDisplayWrite(
> _In_ UINT PositionX,
> _In_ UINT PositionY);
>
> +NTSTATUS
> +APIENTRY
> +DodControlInterrupt(
> +IN_CONST_HANDLE hAdapter,
> +IN_CONST_DXGK_INTERRUPT_TYPEInterruptType,
> +IN_BOOLEAN EnableInterrupt
> +);
> +
> +NTSTATUS
> +APIENTRY
> +DodGetScanLine(
> +IN_CONST_HANDLE hAdapter,
> +INOUT_PDXGKARG_GETSCANLINE pGetScanLine
> +);
> +
> #if DBG
>
> extern int nDebugLevel;
Otherwise
Acked-by: Frediano Ziglio
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1] gtk-session: reply agent's clipboard request on failure
From: Victor Toso
We need to reply back to the agent all clipboard requests even in case
of failure otherwise it will have a pending request. The following
error message can be seen in afterwards, when client sends down some
clipboard data:
> clipboard: selection requests pending on clipboard ownership
> change, clearing
An easy way to reproduce this is:
1-) In client, copy image from lo-draw (selection or ctrl+c)
2-) In guest, paste it to GEdit (mouse3 our ctrl+v)
3-) Move to the client
4-) Move back to the guest
5-) see error on vdagent logs
The reason for failure is that client's clipboard contains different
data type (image) then what was requested from guest's editor (text)
While at it, include extra debug message as it might be hard to
identify why clipboard did not work.
Resolves: rhbz#1409854
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 0426d8f..315bc26 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -940,28 +940,33 @@ static void clipboard_received_text_cb(GtkClipboard
*clipboard,
if (self == NULL)
return;
+selection = get_selection_from_clipboard(self->priv, clipboard);
+g_return_if_fail(selection != -1);
+
if (text == NULL) {
SPICE_DEBUG("Failed to retrieve clipboard text");
-return;
+goto notify_agent;
}
g_return_if_fail(SPICE_IS_GTK_SESSION(self));
-selection = get_selection_from_clipboard(self->priv, clipboard);
-g_return_if_fail(selection != -1);
-
len = strlen(text);
if (!check_clipboard_size_limits(self, len)) {
-return;
+SPICE_DEBUG("Failed sized limits of clipboard text (%d bytes)", len);
+len = 0;
+goto notify_agent;
}
/* gtk+ internal utf8 newline is always LF, even on windows */
conv = fixup_clipboard_text(self, text, &len);
if (!check_clipboard_size_limits(self, len)) {
-g_free(conv);
-return;
+SPICE_DEBUG("Failed sized limits of clipboard text (%d bytes)", len);
+g_clear_pointer(&conv, g_free);
+len = 0;
+goto notify_agent;
}
+notify_agent:
spice_main_clipboard_selection_notify(self->priv->main, selection,
VD_AGENT_CLIPBOARD_UTF8_TEXT,
(guchar *)(conv ?: text), len);
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 0/7] clipboard_get_targets() on spice-gtk-session.c
From: Victor Toso Hi, Minor changes that I was doing while playing with clipboard. The first one seems important the others might be somewhat an improvement or not. Let me know ;) Cheers, Victor Toso (7): gtk-session: check if retrieving clipboard data failed gtk-session: initialize array without memset gtk-session: use clear variable for array's size gtk-session: do an early check of clipboard grab gtk-session: move atom's debug gtk-session: better debug GdkAtoms that are parsed gtk-session: move variables to internal scope src/spice-gtk-session.c | 55 - 1 file changed, 31 insertions(+), 24 deletions(-) -- 2.9.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 3/7] gtk-session: use clear variable for array's size
From: Victor Toso
There is no need to use an index variable to keep track of the number
of VD_AGENT_CLIPBOARD types we are storing.
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index bc47f6b..cda2c3a 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -615,6 +615,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
char *name;
int a, m, t;
int selection;
+gint num_types;
if (s->main == NULL)
return;
@@ -631,6 +632,8 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
}
}
+/* Set all Atoms that matches our current protocol implementation */
+num_types = 0;
for (a = 0; a < n_atoms; a++) {
name = gdk_atom_name(atoms[a]);
for (m = 0; m < SPICE_N_ELEMENTS(atom2agent); m++) {
@@ -646,6 +649,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
if (types[t] == 0) {
/* add type to empty slot */
types[t] = atom2agent[m].vdagent;
+num_types++;
break;
}
}
@@ -653,16 +657,17 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
}
g_free(name);
}
-for (t = 0; t < SPICE_N_ELEMENTS(atom2agent); t++) {
-if (types[t] == 0) {
-break;
-}
+
+if (num_types == 0) {
+SPICE_DEBUG("No GdkAtoms will be sent from %d", n_atoms);
+return;
}
-if (!s->clip_grabbed[selection] && t > 0) {
+
+if (!s->clip_grabbed[selection]) {
s->clip_grabbed[selection] = TRUE;
if (spice_main_agent_test_capability(s->main,
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
-spice_main_clipboard_selection_grab(s->main, selection, types, t);
+spice_main_clipboard_selection_grab(s->main, selection, types,
num_types);
/* Sending a grab causes the agent to do an implicit release */
s->nclip_targets[selection] = 0;
}
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 1/7] gtk-session: check if retrieving clipboard data failed
From: Victor Toso
In case of failure, the GdkAtom *atoms will be set to NULL.
Including this check to an early return.
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 0426d8f..3c78e6a 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -604,7 +604,8 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
{
SpiceGtkSession *self = free_weak_ref(user_data);
-if (self == NULL)
+
+if (self == NULL || atoms == NULL)
return;
g_return_if_fail(SPICE_IS_GTK_SESSION(self));
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 2/7] gtk-session: initialize array without memset
From: Victor Toso
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 3c78e6a..bc47f6b 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -611,7 +611,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
g_return_if_fail(SPICE_IS_GTK_SESSION(self));
SpiceGtkSessionPrivate *s = self->priv;
-guint32 types[SPICE_N_ELEMENTS(atom2agent)];
+guint32 types[SPICE_N_ELEMENTS(atom2agent)] = { 0 };
char *name;
int a, m, t;
int selection;
@@ -631,7 +631,6 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
}
}
-memset(types, 0, sizeof(types));
for (a = 0; a < n_atoms; a++) {
name = gdk_atom_name(atoms[a]);
for (m = 0; m < SPICE_N_ELEMENTS(atom2agent); m++) {
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 4/7] gtk-session: do an early check of clipboard grab
From: Victor Toso
As we will not be doing anything in case clipboard is already grabbed
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index cda2c3a..651e438 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -623,6 +623,11 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
selection = get_selection_from_clipboard(s, clipboard);
g_return_if_fail(selection != -1);
+if (s->clip_grabbed[selection]) {
+SPICE_DEBUG("Clipboard is already grabbed, ignoring %d atoms",
n_atoms);
+return;
+}
+
SPICE_DEBUG("%s:", __FUNCTION__);
if (spice_util_get_debug()) {
for (a = 0; a < n_atoms; a++) {
@@ -663,14 +668,13 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
return;
}
-if (!s->clip_grabbed[selection]) {
-s->clip_grabbed[selection] = TRUE;
+s->clip_grabbed[selection] = TRUE;
-if (spice_main_agent_test_capability(s->main,
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
-spice_main_clipboard_selection_grab(s->main, selection, types,
num_types);
-/* Sending a grab causes the agent to do an implicit release */
-s->nclip_targets[selection] = 0;
-}
+if (spice_main_agent_test_capability(s->main,
VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
+spice_main_clipboard_selection_grab(s->main, selection, types,
num_types);
+
+/* Sending a grab causes the agent to do an implicit release */
+s->nclip_targets[selection] = 0;
}
static void clipboard_owner_change(GtkClipboard*clipboard,
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 6/7] gtk-session: better debug GdkAtoms that are parsed
From: Victor Toso
By including what is discarded or not.
To improve coherence, using prefix 'saving' and 'discarding' to both
debug messages:
> saving:'image/bmp'
> discarding:'application/x-openoffice-bitmap;windows_formatname="Bitmap"'
> saving:'image/png'
> discarding:'application/x-openoffice-wmf;windows_formatname="Image WMF"'
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 8682229..cd546d3 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -645,7 +645,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
}
if (types[t] == 0) {
/* add type to empty slot */
-SPICE_DEBUG("'%s'", name);
+SPICE_DEBUG("saving:'%s'", name);
types[t] = atom2agent[m].vdagent;
num_types++;
break;
@@ -653,6 +653,10 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
}
break;
}
+
+if (m == SPICE_N_ELEMENTS(atom2agent))
+SPICE_DEBUG("discarding:'%s'", name);
+
g_free(name);
}
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 5/7] gtk-session: move atom's debug
From: Victor Toso
We already iterate in all n_atoms and get its name with
gdk_atom_name(), let's move the debug there too;
As spice_util_get_debug() uses g_once(), this should not impact
performance or might improve it with one memory allocation less.
While at it, move the generic SPICE_DEBUG of the function to the top.
It should help us identify when the function was triggered but not
used due the early returns.
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 651e438..8682229 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -604,6 +604,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
{
SpiceGtkSession *self = free_weak_ref(user_data);
+SPICE_DEBUG("%s:", __FUNCTION__);
if (self == NULL || atoms == NULL)
return;
@@ -628,15 +629,6 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
return;
}
-SPICE_DEBUG("%s:", __FUNCTION__);
-if (spice_util_get_debug()) {
-for (a = 0; a < n_atoms; a++) {
-name = gdk_atom_name(atoms[a]);
-SPICE_DEBUG(" \"%s\"", name);
-g_free(name);
-}
-}
-
/* Set all Atoms that matches our current protocol implementation */
num_types = 0;
for (a = 0; a < n_atoms; a++) {
@@ -653,6 +645,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
}
if (types[t] == 0) {
/* add type to empty slot */
+SPICE_DEBUG("'%s'", name);
types[t] = atom2agent[m].vdagent;
num_types++;
break;
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 7/7] gtk-session: move variables to internal scope
From: Victor Toso
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index cd546d3..595b327 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -613,8 +613,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
SpiceGtkSessionPrivate *s = self->priv;
guint32 types[SPICE_N_ELEMENTS(atom2agent)] = { 0 };
-char *name;
-int a, m, t;
+int a;
int selection;
gint num_types;
@@ -632,7 +631,9 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
/* Set all Atoms that matches our current protocol implementation */
num_types = 0;
for (a = 0; a < n_atoms; a++) {
-name = gdk_atom_name(atoms[a]);
+gint m, t;
+gchar *name = gdk_atom_name(atoms[a]);
+
for (m = 0; m < SPICE_N_ELEMENTS(atom2agent); m++) {
if (strcasecmp(name, atom2agent[m].xatom) != 0) {
continue;
--
2.9.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 6/6] qxl-wddm-dod: Preparation for VSync activation
Prepare all the procedures needed for VSync feature but
do not turn it on yet. Manual uncomment still required to
enable it.
Advanced users may enable VSync feature and report functional
problem with it. The driver is expected to pass all the formal
tests under HLK 1607 with device rev.3 and rev.4 but with
device rev.4 on setups with high load or long round-trip delay
video system may detect timeout during rendering operation
and disable the driver with error 43.
Signed-off-by: Yuri Benditovich
---
qxldod/driver.cpp | 64 +++
qxldod/driver.h | 15 +
2 files changed, 79 insertions(+)
diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
index dc84aa8..1946147 100755
--- a/qxldod/driver.cpp
+++ b/qxldod/driver.cpp
@@ -32,6 +32,31 @@ DriverEntry(
DbgPrint(TRACE_LEVEL_FATAL, ("---> KMDOD build on on %s %s\n", __DATE__,
__TIME__));
+RTL_OSVERSIONINFOW versionInfo;
+versionInfo.dwOSVersionInfoSize = sizeof(versionInfo);
+
+RtlGetVersion(&versionInfo);
+
+// VSync control is NOT planned to be enabled on Win10 builds
+// before RS1. Enabling it on DOD driver causes OS to stop the driver
+// with error 43 when the system turns off display due to idle setting.
+// On Windows 8.1 (9200 and 9600) till now no problem observed
+// (OS does not send VSync enable command)
+// On Windows 10RS1 (14393) enabling VSync control activates
+// watchdog policy and creates high sensitivity to long (> 2 sec)
+// processing in PresentDisplayOnly callback (stop with error 43)
+
+if (versionInfo.dwBuildNumber >= 14393 || versionInfo.dwBuildNumber <=
9600)
+{
+// we will uncomment the line below after we address all the problems
+// related to enabled VSync control in Win10RS1
+
+//g_bSupportVSync = TRUE;
+}
+DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for %d.%d.%d\n",
+g_bSupportVSync ? "en" : "dis",
+versionInfo.dwMajorVersion, versionInfo.dwMinorVersion,
versionInfo.dwBuildNumber));
+
// Initialize DDI function pointers and dxgkrnl
KMDDOD_INITIALIZATION_DATA InitialData = {0};
@@ -67,6 +92,11 @@ DriverEntry(
InitialData.DxgkDdiStopDeviceAndReleasePostDisplayOwnership =
DodStopDeviceAndReleasePostDisplayOwnership;
InitialData.DxgkDdiSystemDisplayEnable = DodSystemDisplayEnable;
InitialData.DxgkDdiSystemDisplayWrite = DodSystemDisplayWrite;
+if (g_bSupportVSync)
+{
+InitialData.DxgkDdiControlInterrupt = DodControlInterrupt;
+InitialData.DxgkDdiGetScanLine = DodGetScanLine;
+}
NTSTATUS Status = DxgkInitializeDisplayOnlyDriver(pDriverObject,
pRegistryPath, &InitialData);
if (!NT_SUCCESS(Status))
@@ -559,6 +589,40 @@ DodQueryVidPnHWCapability(
return pQxl->QueryVidPnHWCapability(pVidPnHWCaps);
}
+NTSTATUS
+APIENTRY
+DodControlInterrupt(
+IN_CONST_HANDLE hAdapter,
+IN_CONST_DXGK_INTERRUPT_TYPEInterruptType,
+IN_BOOLEAN EnableInterrupt
+)
+{
+PAGED_CODE();
+QxlDod* pQxl = reinterpret_cast(hAdapter);
+if (InterruptType == DXGK_INTERRUPT_DISPLAYONLY_VSYNC)
+{
+pQxl->EnableVsync(EnableInterrupt);
+return STATUS_SUCCESS;
+}
+return STATUS_NOT_IMPLEMENTED;
+}
+
+NTSTATUS
+APIENTRY
+DodGetScanLine(
+IN_CONST_HANDLE hAdapter,
+INOUT_PDXGKARG_GETSCANLINE pGetScanLine
+)
+{
+PAGED_CODE();
+// Currently we do not see any practical use case when this procedure is
called
+// IDirectDraw has an interface for querying scan line
+// Leave it not implemented like remote desktop does
+// until we recognize use case for more intelligent implementation
+DbgPrint(TRACE_LEVEL_ERROR, ("<---> %s\n", __FUNCTION__));
+return STATUS_NOT_IMPLEMENTED;
+}
+
//END: Paged Code
#pragma code_seg(pop)
diff --git a/qxldod/driver.h b/qxldod/driver.h
index 97b9415..e39e386 100755
--- a/qxldod/driver.h
+++ b/qxldod/driver.h
@@ -217,6 +217,21 @@ DodSystemDisplayWrite(
_In_ UINT PositionX,
_In_ UINT PositionY);
+NTSTATUS
+APIENTRY
+DodControlInterrupt(
+IN_CONST_HANDLE hAdapter,
+IN_CONST_DXGK_INTERRUPT_TYPEInterruptType,
+IN_BOOLEAN EnableInterrupt
+);
+
+NTSTATUS
+APIENTRY
+DodGetScanLine(
+IN_CONST_HANDLE hAdapter,
+INOUT_PDXGKARG_GETSCANLINE pGetScanLine
+);
+
#if DBG
extern int nDebugLevel;
--
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] [PATCH v2 spice-gtk 1/2] authentication: Handle failed SASL authentication separately
On 02/22/2017 12:58 PM, Christophe Fergeau wrote: Hey, On Wed, Feb 22, 2017 at 11:50:21AM +0200, Snir Sheriber wrote: The authentication type string is just for letting the user know what authentication is in use. Sorry, forgot to reply to this bit, I don't think saying "SASL" in this string is useful, this is going to be very cryptic to the users (I'm not even sure I knew what SASL was before working on SPICE), and imo is just relevant to sysadmins and spice-gtk developers, so making sure debug logs indicate which kind of auth mechanism is used should be enough. Yes, you are right, i guess it's enough (and the log also shows the selected mechanism by sasl lib) Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 6/6] qxl-wddm-dod: Preparation for VSync activation
>
> Prepare all the procedures needed for VSync feature but
> do not turn it on yet. Manual uncomment still required to
> enable it.
> Advanced users may enable VSync feature and report functional
> problem with it. The driver is expected to pass all the formal
> tests under HLK 1607 with device rev.3 and rev.4 but with
> device rev.4 on setups with high load or long round-trip delay
> video system may detect timeout during rendering operation
> and disable the driver with error 43.
>
> Signed-off-by: Yuri Benditovich
Merged the entire set
Thanks,
Frediano
> ---
> qxldod/driver.cpp | 64
> +++
> qxldod/driver.h | 15 +
> 2 files changed, 79 insertions(+)
>
> diff --git a/qxldod/driver.cpp b/qxldod/driver.cpp
> index dc84aa8..1946147 100755
> --- a/qxldod/driver.cpp
> +++ b/qxldod/driver.cpp
> @@ -32,6 +32,31 @@ DriverEntry(
>
> DbgPrint(TRACE_LEVEL_FATAL, ("---> KMDOD build on on %s %s\n", __DATE__,
> __TIME__));
>
> +RTL_OSVERSIONINFOW versionInfo;
> +versionInfo.dwOSVersionInfoSize = sizeof(versionInfo);
> +
> +RtlGetVersion(&versionInfo);
> +
> +// VSync control is NOT planned to be enabled on Win10 builds
> +// before RS1. Enabling it on DOD driver causes OS to stop the driver
> +// with error 43 when the system turns off display due to idle setting.
> +// On Windows 8.1 (9200 and 9600) till now no problem observed
> +// (OS does not send VSync enable command)
> +// On Windows 10RS1 (14393) enabling VSync control activates
> +// watchdog policy and creates high sensitivity to long (> 2 sec)
> +// processing in PresentDisplayOnly callback (stop with error 43)
> +
> +if (versionInfo.dwBuildNumber >= 14393 || versionInfo.dwBuildNumber <=
> 9600)
> +{
> +// we will uncomment the line below after we address all the
> problems
> +// related to enabled VSync control in Win10RS1
> +
> +//g_bSupportVSync = TRUE;
> +}
> +DbgPrint(TRACE_LEVEL_WARNING, ("VSync support %sabled for %d.%d.%d\n",
> +g_bSupportVSync ? "en" : "dis",
> +versionInfo.dwMajorVersion, versionInfo.dwMinorVersion,
> versionInfo.dwBuildNumber));
> +
> // Initialize DDI function pointers and dxgkrnl
> KMDDOD_INITIALIZATION_DATA InitialData = {0};
>
> @@ -67,6 +92,11 @@ DriverEntry(
> InitialData.DxgkDdiStopDeviceAndReleasePostDisplayOwnership =
> DodStopDeviceAndReleasePostDisplayOwnership;
> InitialData.DxgkDdiSystemDisplayEnable =
> DodSystemDisplayEnable;
> InitialData.DxgkDdiSystemDisplayWrite = DodSystemDisplayWrite;
> +if (g_bSupportVSync)
> +{
> +InitialData.DxgkDdiControlInterrupt = DodControlInterrupt;
> +InitialData.DxgkDdiGetScanLine = DodGetScanLine;
> +}
>
> NTSTATUS Status = DxgkInitializeDisplayOnlyDriver(pDriverObject,
> pRegistryPath, &InitialData);
> if (!NT_SUCCESS(Status))
> @@ -559,6 +589,40 @@ DodQueryVidPnHWCapability(
> return pQxl->QueryVidPnHWCapability(pVidPnHWCaps);
> }
>
> +NTSTATUS
> +APIENTRY
> +DodControlInterrupt(
> +IN_CONST_HANDLE hAdapter,
> +IN_CONST_DXGK_INTERRUPT_TYPEInterruptType,
> +IN_BOOLEAN EnableInterrupt
> +)
> +{
> +PAGED_CODE();
> +QxlDod* pQxl = reinterpret_cast(hAdapter);
> +if (InterruptType == DXGK_INTERRUPT_DISPLAYONLY_VSYNC)
> +{
> +pQxl->EnableVsync(EnableInterrupt);
> +return STATUS_SUCCESS;
> +}
> +return STATUS_NOT_IMPLEMENTED;
> +}
> +
> +NTSTATUS
> +APIENTRY
> +DodGetScanLine(
> +IN_CONST_HANDLE hAdapter,
> +INOUT_PDXGKARG_GETSCANLINE pGetScanLine
> +)
> +{
> +PAGED_CODE();
> +// Currently we do not see any practical use case when this procedure is
> called
> +// IDirectDraw has an interface for querying scan line
> +// Leave it not implemented like remote desktop does
> +// until we recognize use case for more intelligent implementation
> +DbgPrint(TRACE_LEVEL_ERROR, ("<---> %s\n", __FUNCTION__));
> +return STATUS_NOT_IMPLEMENTED;
> +}
> +
> //END: Paged Code
> #pragma code_seg(pop)
>
> diff --git a/qxldod/driver.h b/qxldod/driver.h
> index 97b9415..e39e386 100755
> --- a/qxldod/driver.h
> +++ b/qxldod/driver.h
> @@ -217,6 +217,21 @@ DodSystemDisplayWrite(
> _In_ UINT PositionX,
> _In_ UINT PositionY);
>
> +NTSTATUS
> +APIENTRY
> +DodControlInterrupt(
> +IN_CONST_HANDLE hAdapter,
> +IN_CONST_DXGK_INTERRUPT_TYPEInterruptType,
> +IN_BOOLEAN EnableInterrupt
> +);
> +
> +NTSTATUS
> +APIENTRY
> +DodGetScanLine(
> +IN_CONST_HANDLE hAdapter,
> +INOUT_PDXGKARG_GETSCANLINE pGetScanLine
> +);
> +
> #if DBG
>
> extern int nDebugLevel;
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.
Re: [Spice-devel] [PATCH] Don't close all but one display during reboot.
Hi, I'll go against the fix as I don't see any reason for the "assumption". Let's split the question into 2 things, max monitor number and which monitor is enabled. The max monitor number seems correct now, controlled by qemu together with qxl. And it's used to control the client menu for monitors. The monitor enable should be "remembered" only by the guest OS.while can be controlled by both client and qxl. They won't affect each other, and the init-boot/reboot process should be similar: 1. (Re)Boot with primary VGA -- qemu send maxim one monitor with it enabled -- client have 1 window (or close extra ones) 2. Qxl loaded (early in guest boot) -- qxl send maxim n monitor with one enabled -- client can have 4 window, but only one enabled 3. Guest enable some monitors -- qxl send maxim n monitor with x enabled -- client have corresponding windows enabled (and placed correctly, e.g. onto different monitors) Everything looks fine above, but why do we have the bug? The symptom looks as if the guest "forget" to enable the monitor or someone change it. Let's dig it out. Regards,Daimon On Tuesday, February 21, 2017 1:00 AM, Christophe de Dinechin wrote: Is it possible to make the max number of monitors something persistent (e.g. set / get that using libvirt), so that the behavior would be consistent between a first boot and a reboot? Christophe On 20 Feb 2017, at 15:49, Jonathon Jongsma wrote: On Thu, 2017-02-02 at 10:30 -0600, Jonathon Jongsma wrote: On Thu, 2017-02-02 at 05:29 -0500, Frediano Ziglio wrote: When a guest is rebooted, the QXL driver gets unloaded at some point in the reboot process. When the driver is unloaded, the spice server sets a single flag to FALSE: RedWorker::driver_cap_monitors_config. This flag indicates whether the driver is capable of sending its own monitors config messages to the client. The only place this flag is used is when a new primary surface is created. If this flag is true, the server assumes that the driver will send its own monitors config very soon after the surface is created. If it's false, the server directly sends its own temporary monitors config message to the client since the driver cannot. This temporary monitors config message is based on the size of the just-created primary surface and always has a maximum monitor count of 1. This flag is initialized to false at startup so that in early boot (e.g. vga text mode), the server will send out these 'temporary' monitor config messages to the client whenever the primary surface is destroyed and re- created. This causes the client to resize its window to match the size of the guest. When the QXL driver is eventually loaded and starts taking over the monitors config responsibilities, we set this flag to true and the server stops sending monitors config messages out on its own. If we reboot and set this flag to false, it will result in the server sending a monitors config message to the client indicating that the guest now supports a maximum of 1 monitor. If the guest happens to have more than one display window open, it will destroy those extra windows because they exceed the maximum allowed number of monitors. This means that if you reboot a guest with 4 monitors, after reboot it will only have 1 monitor. After reading this paragraph and the bug I don't see the issue. I mean, if on my laptop I reboot when I reboot I get a single monitor till the OS and other stuff kick in. After a reboot the firmware use one monitor or if capable do the mirroring but always the same way. I think the issue is that on first boot the guest activate the additional monitors and the client do the same while on second boot (reboot) not so to me looks like more an issue of the client instead of the server. Well, it could be considered a client issue to some extent, but not an easy one to fix. I would try to get a network capture to look at DisplayChannel messages if they are more or less the same for first boot and reboot. If they are the same I would look at the client state to check that while booting it's the same. The initial boot and the reboot are the same, and that's basically why the problem exists. At initial boot, it brings up a single display. And on reboot, it also brings up a single display. The issue is that we want the reboot to behave differently. Initial boot: - - In early boot, the server sends monitors config message when primary surface is created. monitors=1, max-monitors=1 - client only shows a single window, disables menu items for enabling additional displays because the guest doesn't support more than 1 - When QXL driver is loaded and takes over, it takes over sending monitors config messages. monitors=1, max-monitors=4 - client still shows a single window, but now the menu items for enabling additional menus are enabled - client enables a second monitor. Client sends monitors-config request to server - QXL driver
Re: [Spice-devel] Windows drivers for virgl?
On Tue, 2017-02-21 at 12:41 +0200, Yan Vugenfirer wrote: > Hi Benrooz, > > I am adding Vadim to the thread. He is working on virtio-gpu for > Windows. > We do have some working prototype of virtio-gpu dod driver. I'm thinking about adding OpenGL Installable Client Driver as the next step. Then we are going to move forward adding Direct3D support. Both these steps mentioned above will require help from the spice team. All the best, Vadim. > Best regards, > Yan. > > > > > On 21 Feb 2017, at 12:39, Behrooz Shabani > > wrote: > > > > Hi All, > > > > I discussed the state of windows drivers on IRC with the developers > > awhile back but as not everybody is on the channel, I wanted to ask > > here as well: > > > > Are you aware of any implementation efforts regarding Windows > > drivers of virgl? Maybe someone can provide some insights on how to > > approach it? We are willing to help the process. > > > > -- > > regards, > > behrooz > > ___ > > 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
[Spice-devel] How to config video and image compression?
I learned that video and image compression can be chosen server initiation from https://www.spice-space.org/spice-user-manual.html. How can I change the configuration for the compression ? I want to play 720p video smoothly in the guest(some loss or distortion does not matter). What is the best configuration for me? I got that I can change option of graphics in libvirt like: Which option should I change ? Can I also customize more parameter about the vga(such as fps, bitrate ...) ? Regards, John ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
