Re: [Spice-devel] [PATCH linux vdagent] Add systemd socket activation
vdagentd_messages,
> +VDAGENTD_NO_MESSAGES, debug);
> +own_socket = FALSE;
> +} else
> +/* systemd socket activation not enabled, create our own */
> +#endif /* WITH_SYSTEMD_SOCKET_ACTIVATION */
> +{
> +server = udscs_create_server(vdagentd_socket, agent_connect,
> + agent_read_complete, agent_disconnect,
> + vdagentd_messages, VDAGENTD_NO_MESSAGES,
> + debug);
> +}
> +
> if (!server) {
> if (errno == EADDRINUSE) {
> syslog(LOG_CRIT, "Fatal the server socket %s exists already.
> Delete it?",
> @@ -1147,11 +1172,15 @@ int main(int argc, char *argv[])
> }
> return 1;
> }
> -if (chmod(vdagentd_socket, 0666)) {
> -syslog(LOG_CRIT, "Fatal could not change permissions on %s: %m",
> - vdagentd_socket);
> -udscs_destroy_server(server);
> -return 1;
> +
> +/* no need to set permissions on a socket that was provided by systemd */
> +if (own_socket) {
> +if (chmod(vdagentd_socket, 0666)) {
> +syslog(LOG_CRIT, "Fatal could not change permissions on %s: %m",
> + vdagentd_socket);
> +udscs_destroy_server(server);
> +return 1;
> +}
> }
>
> if (do_daemonize)
> @@ -1181,8 +1210,12 @@ int main(int argc, char *argv[])
> vdagent_virtio_port_destroy(&virtio_port);
> session_info_destroy(session_info);
> udscs_destroy_server(server);
> -if (unlink(vdagentd_socket) != 0)
> -syslog(LOG_ERR, "unlink %s: %s", vdagentd_socket, strerror(errno));
> +
> +/* leave the socket around if it was provided by systemd */
> +if (own_socket) {
> +if (unlink(vdagentd_socket) != 0)
> +syslog(LOG_ERR, "unlink %s: %s", vdagentd_socket,
> strerror(errno));
> +}
> syslog(LOG_INFO, "vdagentd quitting, returning status %d", retval);
>
> if (do_daemonize)
> --
> 2.9.4
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Remote Viewer for Windows XP with UsbDk support
On Sun, Mar 5, 2017 at 1:40 PM, Sameeh Jubran wrote: > > > On Sun, Mar 5, 2017 at 12:37 PM, Victor Toso wrote: >> >> Hi, >> >> On Sun, Mar 05, 2017 at 11:46:01AM +0200, Sameeh Jubran wrote: >> > Hi all, >> > >> > I am trying to run Remote Viewer inside Windows XP while using UsbDk for >> > redirection, However when installing the latest version of Virt-Viewer >> > on >> > XP and try to run Remote Viewer some Dlls seem to be missing. >> > >> > Anyone knows where can I find / compile a version that supports UsbDk >> > and >> > runs on Windows XP? >> >> Can you provide which dll's are missing? > > dwmapi.dll >> >> Just to be sure, you are using the virt-viewer 5.0 from >> https://virt-manager.org/download/ ? > > Yes, I tried v5.0 v4.0 and v3.0 all of them require this DLL. > v2.0 works on XP but I am not sure it supports UsbDk, can you confirm this? The only version with UsbDk support is the latest one (5.0). Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Remote Viewer for Windows XP with UsbDk support
On Sun, Mar 5, 2017 at 8:46 PM, Fabiano Fidêncio wrote: > On Sun, Mar 5, 2017 at 3:41 PM, Frediano Ziglio wrote: >> >> On Sun, Mar 5, 2017 at 12:37 PM, Victor Toso wrote: >>> >>> Hi, >>> >>> On Sun, Mar 05, 2017 at 11:46:01AM +0200, Sameeh Jubran wrote: >>> > Hi all, >>> > >>> > I am trying to run Remote Viewer inside Windows XP while using UsbDk for >>> > redirection, However when installing the latest version of Virt-Viewer >>> > on >>> > XP and try to run Remote Viewer some Dlls seem to be missing. >>> > >>> > Anyone knows where can I find / compile a version that supports UsbDk >>> > and >>> > runs on Windows XP? >>> >>> Can you provide which dll's are missing? >> >> dwmapi.dll >> >> >> $ grep -ri dwmapi >> Binary file Program Files/VirtViewer v5.0-256/bin/libgdk-3-0.dll matches >> >> $ i686-w64-mingw32-objdump -x Program\ Files/VirtViewer\ >> v5.0-256/bin/libgdk-3-0.dll >> ... >> DLL Name: dwmapi.dll >> vma: Hint/Ord Member-Name Bound-To >> 9725c 3 DwmEnableBlurBehindWindow >> 97278 15 DwmIsCompositionEnabled >> ... >> >> Don't ask me why they are needed. >> >> dwmapi.dll is the "Desktop Window Manager" and from >> https://msdn.microsoft.com/en-us/library/windows/desktop/aa969540(v=vs.85).aspx >> was introduced in Vista. >> >> To stub them DwmIsCompositionEnabled and DwmEnableBlurBehindWindow should >> return S_FALSE. >> >> They are used in gtk+, sources gdk/win32/gdkwindow-win32.c and >> ./gdk/win32/gdkscreen-win32.c. >>> >>> Just to be sure, y >>> >>> ou are using the virt-viewer 5.0 from >>> https://virt-manager.org/download/ ? >> >> Yes, I tried v5.0 v4.0 and v3.0 all of them require this DLL. >> v2.0 works on XP but I am not sure it supports UsbDk, can you confirm this? >>> >>> >>> Cheers, >>> toso >> >> Do we support Windows XP as client? > > No. We do not support windows XP as client. > Gtk3 dropped Windows XP support a while ago and we dropped Gtk2 > support a while ago. Being a bit more precise here ... on 4.0 we dropped support to Gtk2. And here you can see the discussion about Windows XP: https://www.redhat.com/archives/virt-tools-list/2015-November/msg00176.html This thread continues on December 2015. Just follow it up and you'll see when and why the decision was taken. Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Remote Viewer for Windows XP with UsbDk support
On Sun, Mar 5, 2017 at 3:41 PM, Frediano Ziglio wrote: > > On Sun, Mar 5, 2017 at 12:37 PM, Victor Toso wrote: >> >> Hi, >> >> On Sun, Mar 05, 2017 at 11:46:01AM +0200, Sameeh Jubran wrote: >> > Hi all, >> > >> > I am trying to run Remote Viewer inside Windows XP while using UsbDk for >> > redirection, However when installing the latest version of Virt-Viewer >> > on >> > XP and try to run Remote Viewer some Dlls seem to be missing. >> > >> > Anyone knows where can I find / compile a version that supports UsbDk >> > and >> > runs on Windows XP? >> >> Can you provide which dll's are missing? > > dwmapi.dll > > > $ grep -ri dwmapi > Binary file Program Files/VirtViewer v5.0-256/bin/libgdk-3-0.dll matches > > $ i686-w64-mingw32-objdump -x Program\ Files/VirtViewer\ > v5.0-256/bin/libgdk-3-0.dll > ... > DLL Name: dwmapi.dll > vma: Hint/Ord Member-Name Bound-To > 9725c 3 DwmEnableBlurBehindWindow > 97278 15 DwmIsCompositionEnabled > ... > > Don't ask me why they are needed. > > dwmapi.dll is the "Desktop Window Manager" and from > https://msdn.microsoft.com/en-us/library/windows/desktop/aa969540(v=vs.85).aspx > was introduced in Vista. > > To stub them DwmIsCompositionEnabled and DwmEnableBlurBehindWindow should > return S_FALSE. > > They are used in gtk+, sources gdk/win32/gdkwindow-win32.c and > ./gdk/win32/gdkscreen-win32.c. >> >> Just to be sure, y >> >> ou are using the virt-viewer 5.0 from >> https://virt-manager.org/download/ ? > > Yes, I tried v5.0 v4.0 and v3.0 all of them require this DLL. > v2.0 works on XP but I am not sure it supports UsbDk, can you confirm this? >> >> >> Cheers, >> toso > > Do we support Windows XP as client? No. We do not support windows XP as client. Gtk3 dropped Windows XP support a while ago and we dropped Gtk2 support a while ago. Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] gitlab
On Fri, Sep 30, 2016 at 1:48 PM, Victor Toso wrote: > Hi again! > > On Wed, Jun 22, 2016 at 04:48:39PM +0200, Victor Toso wrote: >> Hi all, >> >> I created a spice group in gitlab [0] mirroring the repository from >> freedesktop which should be updated every hour. >> >> [0] https://gitlab.com/groups/spice > > https://gitlab.com/spice works too. > >> >> But I would like to discuss the transition to use gitlab for source >> code at some time soon. One of the main reasons is the management for >> users and repos being much easier with no need to file a bug and wait >> some fdo admin to have time to do it. >> >> As pointed by Daniel [1] while discussing the migration of libosinfo >> to gitlab, we would still be under open source infrastructure with >> good self-service API for managing the project plus other benefits. >> >> [1] https://www.redhat.com/archives/libosinfo/2016-March/msg0.html >> >> The idea is keeping freedesktop and github [2] as a mirror of gitlab >> repos. I'm not sure about moving from bugzilla to the issues thing but >> I guess it might make sense in the future. >> >> [2] https://github.com/SPICE >> >> The pull requests can be disable int gitlab (while we can't on github) >> but I wonder if should do that? >> >> As we are at it, I would suggest to rename: >> >> * linux/vd_agent to spice-vdagent (currently vdagent on gitlab) >> * win32/vd_agent to spice-vdagent-win (currently vdagent-win on gitlab) > > * Marc-André did not agree with renaming ^ > >> >> It would be great to hear from current contributors about this! >> >> Cheers, > > So, I put this on hold for some time but it would be great to discuss > this once more. > > Last week I started to play with continuous integration in gitlab [0] > which can be very useful to track if given commit is breaking > builds, tests, etc. We also plan to setup a copr [1] repo to provide > fedora builds to latest upstream. Pavel has worked in a script [2] that > does that (if commit passes in previous stages like building, tests). > > [0] https://about.gitlab.com/gitlab-ci/ > [1] https://copr.fedorainfracloud.org/ > [2] https://gitlab.com/xerus/ci/tree/master I guess is worth to check the CentOS CI. It can be easily integrated with github/gitlab. virt-viewer already makes use of that, libvirt as well as far as I remember. > > Does anyone disagree with moving to gitlab and making freedesktop a > mirror? I'm not part of the project anymore, but +1 from my side. > > Cheers, > toso > > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel > Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [RFC] Adding support to ssh-agent-forward
On Mon, Sep 5, 2016 at 1:36 PM, Victor Toso wrote:
> Hi,
>
> On Sat, Sep 03, 2016 at 01:49:06PM +0200, Fabiano Fidêncio wrote:
>> Howdy!
>>
>> Here is a small patch series adding support to ssh-agent-forward.
>> This patch series have been tested using a usable (but not 100%
>> working) spice-ssh-agent[0].
>>
>> The spice-related part seems to be working fine. There are still a
>> few improvements to be done on spice-ssh-agent[0[, but that shouldn't
>> be a blocker and patches are more than welcome.
>>
>> There is also some work done on virt-viewer[1] and there will be some
>> work on virt-manager (adding support to add the new ssh-channel to a
>> VM).
>>
>> I really would appreciate with we can have the spice{,-proto,-common}
>> merged before the next release.
>>
>> For spice-gtk it can be merged after 0.33 though due to the issues I
>> am still having with the spice-ssh-agent[0].
>
> What issues?
That's a good question.
Let me resurrect my test VM and I'll summarize the issues here in
order to avoid you (or anyone else who picks this up for review) to
waste time on this.
Gimme a day or two and I'll update the spice-ssh-agent branch (just in
order to be sure that it will work) and I'll get back with a detailed
description of the points that were failing for me.
Deal? :-)
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/6] [spice-proto] Add support to ssh channel
On Sat, Sep 3, 2016 at 2:29 PM, Fabiano Fidêncio wrote:
> On Sat, Sep 3, 2016 at 2:02 PM, Victor Toso wrote:
>> Hi,
>>
>> On Sat, Sep 03, 2016 at 01:49:07PM +0200, Fabiano Fidêncio wrote:
>>> ---
>>
>> Could you please include some documentation about the protocol in the
>> commit log? Check webdav ~ 6e5ea8d802ac8 or port ~ f188fb7a89086
>
> Okay, I'll send a v2 at some point (already switched context to Sagar's work).
> I hope it doesn't block you from reviewing the patches. In any case,
> let me try to help you to help me :-)
>
> Here you can find the protocol description:
> https://tools.ietf.org/html/draft-ietf-secsh-agent-02
Let me try to be a bit more specific here:
I'm basically sending back and forth the ssh-agent packet that has the
following format:
uint32 length
byte type
data[length -1] data payload
(as described on
https://tools.ietf.org/html/draft-ietf-secsh-agent-02#section-1.1)
Any of the internals should _not_ matter, as we don't mess with the
packet (we just deliver it to the client's ssh agent).
>>
>> toso
>>
>>> spice.proto | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/spice.proto b/spice.proto
>>> index 0bfc515..eb99742 100644
>>> --- a/spice.proto
>>> +++ b/spice.proto
>>> @@ -1423,6 +1423,9 @@ channel PortChannel : SpicevmcChannel {
>>> channel WebDAVChannel : PortChannel {
>>> };
>>>
>>> +channel SshChannel : PortChannel {
>>> +};
>>> +
>>> protocol Spice {
>>> MainChannel main = 1;
>>> DisplayChannel display;
>>> @@ -1435,4 +1438,5 @@ protocol Spice {
>>> UsbredirChannel usbredir;
>>> PortChannel port;
>>> WebDAVChannel webdav;
>>> +SshChannel ssh;
>>> };
>>> --
>>> 2.7.4
>>>
>>> ___
>>> Spice-devel mailing list
>>> Spice-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> Best Regards,
> --
> Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/6] [spice-proto] Add support to ssh channel
On Sat, Sep 3, 2016 at 2:02 PM, Victor Toso wrote:
> Hi,
>
> On Sat, Sep 03, 2016 at 01:49:07PM +0200, Fabiano Fidêncio wrote:
>> ---
>
> Could you please include some documentation about the protocol in the
> commit log? Check webdav ~ 6e5ea8d802ac8 or port ~ f188fb7a89086
Okay, I'll send a v2 at some point (already switched context to Sagar's work).
I hope it doesn't block you from reviewing the patches. In any case,
let me try to help you to help me :-)
Here you can find the protocol description:
https://tools.ietf.org/html/draft-ietf-secsh-agent-02
>
> toso
>
>> spice.proto | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/spice.proto b/spice.proto
>> index 0bfc515..eb99742 100644
>> --- a/spice.proto
>> +++ b/spice.proto
>> @@ -1423,6 +1423,9 @@ channel PortChannel : SpicevmcChannel {
>> channel WebDAVChannel : PortChannel {
>> };
>>
>> +channel SshChannel : PortChannel {
>> +};
>> +
>> protocol Spice {
>> MainChannel main = 1;
>> DisplayChannel display;
>> @@ -1435,4 +1438,5 @@ protocol Spice {
>> UsbredirChannel usbredir;
>> PortChannel port;
>> WebDAVChannel webdav;
>> +SshChannel ssh;
>> };
>> --
>> 2.7.4
>>
>> ___
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 4/6] [spice] Associate org.spice-space.ssh.0 port to a ssh channel
For example, with qemu, a ssh channel can be created this way:
-chardev spiceport,name=org.spice-space.ssh.0,...
And redirected to a virtio port:
-device virserialport,...,name=org.spice-space.ssh.0
Signed-off-by: Fabiano Fidêncio
---
server/reds.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/server/reds.c b/server/reds.c
index 74f7727..6815062 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3235,6 +3235,8 @@ static int
spice_server_char_device_add_interface(SpiceServer *reds,
else if (strcmp(char_device->subtype, SUBTYPE_PORT) == 0) {
if (strcmp(char_device->portname, "org.spice-space.webdav.0") == 0) {
dev_state = spicevmc_device_connect(reds, char_device,
SPICE_CHANNEL_WEBDAV);
+} else if (strcmp(char_device->portname, "org.spice-space.ssh.0") ==
0) {
+dev_state = spicevmc_device_connect(reds, char_device,
SPICE_CHANNEL_SSH);
} else {
dev_state = spicevmc_device_connect(reds, char_device,
SPICE_CHANNEL_PORT);
}
@@ -3933,6 +3935,7 @@ SPICE_GNUC_VISIBLE int
spice_server_set_channel_security(SpiceServer *s, const c
#endif
[ SPICE_CHANNEL_USBREDIR ] = "usbredir",
[ SPICE_CHANNEL_WEBDAV ] = "webdav",
+[ SPICE_CHANNEL_SSH ] = "ssh",
};
int i;
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 3/6] [spice] spice-common submodule update
The update is needed in order to introduce support to the brand new SSH_SPICE_CHANNEL. Signed-off-by: Fabiano Fidêncio --- spice-common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spice-common b/spice-common index 38047fb..a6e090d 16 --- a/spice-common +++ b/spice-common @@ -1 +1 @@ -Subproject commit 38047fb46f7e1211bbc20d9a81c8fae19b8f8bf4 +Subproject commit a6e090d4a8ece58ca9b751d6539394b1c71fe177 -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 5/6] [spice-gtk] spice-common submodule update
The update is needed in order to introduce support to the brand new SSH_SPICE_CHANNEL. Signed-off-by: Fabiano Fidêncio --- spice-common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spice-common b/spice-common index 62f3024..a6e090d 16 --- a/spice-common +++ b/spice-common @@ -1 +1 @@ -Subproject commit 62f3024f4220766761269618bf3df143ff5c9956 +Subproject commit a6e090d4a8ece58ca9b751d6539394b1c71fe177 -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 2/6] [spice-common] Add support to ssh channel
---
spice/enums.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/spice/enums.h b/spice/enums.h
index 1d0c2db..1d61daf 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -422,6 +422,7 @@ enum {
SPICE_CHANNEL_USBREDIR,
SPICE_CHANNEL_PORT,
SPICE_CHANNEL_WEBDAV,
+SPICE_CHANNEL_SSH,
SPICE_END_CHANNEL
};
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [RFC] Adding support to ssh-agent-forward
Howdy!
Here is a small patch series adding support to ssh-agent-forward.
This patch series have been tested using a usable (but not 100%
working) spice-ssh-agent[0].
The spice-related part seems to be working fine. There are still a
few improvements to be done on spice-ssh-agent[0[, but that shouldn't
be a blocker and patches are more than welcome.
There is also some work done on virt-viewer[1] and there will be some
work on virt-manager (adding support to add the new ssh-channel to a
VM).
I really would appreciate with we can have the spice{,-proto,-common}
merged before the next release.
For spice-gtk it can be merged after 0.33 though due to the issues I
am still having with the spice-ssh-agent[0].
If you're interested in contributing to the spice-ssh-agent[0], please.
feel free to send me direct emails with your patches or open a pull
request on gitlab.
[0]: https://gitlab.com/fidencio/spice-ssh-agent
[1]: https://gitlab.com/fidencio/virt-viewer/tree/wip/ssh-agent-forward
spice-proto:
Fabiano Fidêncio (1):
Add support to ssh channel
spice/enums.h | 1 +
1 file changed, 1 insertion(+)
spice-common:
Fabiano Fidêncio (1):
Add support to ssh channel
spice.proto | 4
1 file changed, 4 insertions(+)
spice:
Fabiano Fidêncio (2):
spice-common submodule update
Associate org.spice-space.ssh.0 port to a ssh channel
server/reds.c | 3 +++
spice-common | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
spice-gtk:
Fabiano Fidêncio (2):
spice-common submodule update
Add channel-ssh
configure.ac | 26 ++
doc/reference/spice-gtk-sections.txt | 17 ++
doc/reference/spice-gtk.types| 2 +
spice-common | 2 +-
src/Makefile.am | 3 +
src/channel-ssh.c| 454
+++
src/channel-ssh.h| 68 ++
src/map-file | 1 +
src/spice-channel.c | 10 +
src/spice-client.h | 1 +
src/spice-glib-sym-file | 1 +
11 files changed, 584 insertions(+), 1 deletion(-)
create mode 100644 src/channel-ssh.c
create mode 100644 src/channel-ssh.h
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 1/6] [spice-proto] Add support to ssh channel
---
spice.proto | 4
1 file changed, 4 insertions(+)
diff --git a/spice.proto b/spice.proto
index 0bfc515..eb99742 100644
--- a/spice.proto
+++ b/spice.proto
@@ -1423,6 +1423,9 @@ channel PortChannel : SpicevmcChannel {
channel WebDAVChannel : PortChannel {
};
+channel SshChannel : PortChannel {
+};
+
protocol Spice {
MainChannel main = 1;
DisplayChannel display;
@@ -1435,4 +1438,5 @@ protocol Spice {
UsbredirChannel usbredir;
PortChannel port;
WebDAVChannel webdav;
+SshChannel ssh;
};
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 6/6] [spice-gtk] Add channel-ssh
The brand new channel-ssh has been created in order to do the
communication between the guest and the client's ssh agent. With the new
agent and a little bit of configuration on the guest we are able to
forward the ssh-agent through a spice channel.
The configuration needed on the guest side (part of OpenSSH 7.3 release)
is quite simple, just add in the guest's ssh config file:
Host: hostname
IdentityAgent: /var/run/spice-ssh-agentd/spice-ssh-agent-sock
It will forward the agent when connecting to that specify hostname.
Signed-off-by: Fabiano Fidêncio
---
configure.ac | 26 ++
doc/reference/spice-gtk-sections.txt | 17 ++
doc/reference/spice-gtk.types| 2 +
src/Makefile.am | 3 +
src/channel-ssh.c| 454 +++
src/channel-ssh.h| 68 ++
src/map-file | 1 +
src/spice-channel.c | 10 +
src/spice-client.h | 1 +
src/spice-glib-sym-file | 1 +
10 files changed, 583 insertions(+)
create mode 100644 src/channel-ssh.c
create mode 100644 src/channel-ssh.h
diff --git a/configure.ac b/configure.ac
index f3e7f8d..5beb5c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -188,6 +188,31 @@ PKG_CHECK_MODULES(CAIRO, cairo >= 1.2.0)
PKG_CHECK_MODULES(GTHREAD, gthread-2.0)
+AC_ARG_ENABLE([ssh-agent-forward],
+ AS_HELP_STRING([--enable-ssh-agent-forward=@<:@auto/yes/no@:>@],
+ [Enable ssh-agent-forward support @<:@default=auto@:>@]),
+ [],
+ [enable_ssh_agent_forward="auto"])
+
+
+if test "x$enable_ssh_agent_forward" = "xno"; then
+ have_ssh_agent_forward="no"
+else
+ PKG_CHECK_MODULES(GIO,
+[glib-2.0 >= 2.43.90 gio-2.0 > 2.43.90],
+[have_ssh_agent_forward=yes],
+[have_ssh_agent_forward=no])
+
+ if test "x$have_ssh_agent_forward" = "xno" && test
"x$enable_ssh_agent_forward" = "xyes"; then
+AC_MSG_ERROR([ssh-agent-forward support explicitly requested, but some
required packages are not available])
+ fi
+fi
+
+AS_IF([test "x$have_ssh_agent_forward" = "xyes"],
+ AC_DEFINE([USE_SSH_AGENT_FORWARD], [1], [Define if supporting
ssh-agent-forward]))
+
+AM_CONDITIONAL([WITH_SSH_AGENT_FORWARD], [test "x$have_ssh_agent_forward" =
"xyes"])
+
AC_ARG_ENABLE([webdav],
AS_HELP_STRING([--enable-webdav=@<:@auto/yes/no@:>@],
[Enable webdav support @<:@default=auto@:>@]),
@@ -618,6 +643,7 @@ AC_MSG_NOTICE([
DBus: ${have_dbus}
WebDAV support: ${have_phodav}
LZ4 support: ${have_lz4}
+ssh-agent forward support: ${have_ssh_agent_forward}
Now type 'make' to build $PACKAGE
diff --git a/doc/reference/spice-gtk-sections.txt
b/doc/reference/spice-gtk-sections.txt
index 386e737..98be852 100644
--- a/doc/reference/spice-gtk-sections.txt
+++ b/doc/reference/spice-gtk-sections.txt
@@ -527,3 +527,20 @@ SpiceFileTransferTask
SpiceFileTransferTaskClass
SpiceFileTransferTaskPrivate
+
+
+channel-ssh
+SpiceSshChannel
+SpiceSshChannel
+SpiceSshChannelClass
+
+SPICE_IS_SSH_CHANNEL
+SPICE_IS_SSH_CHANNEL_CLASS
+SPICE_TYPE_SSH_CHANNEL
+SPICE_SSH_CHANNEL
+SPICE_SSH_CHANNEL_CLASS
+SPICE_SSH_CHANNEL_GET_CLASS
+spice_ssh_channel_get_type
+
+SpiceSshChannelPrivate
+
diff --git a/doc/reference/spice-gtk.types b/doc/reference/spice-gtk.types
index e14ae1b..ea07d5b 100644
--- a/doc/reference/spice-gtk.types
+++ b/doc/reference/spice-gtk.types
@@ -12,6 +12,7 @@
#include "channel-playback.h"
#include "channel-record.h"
#include "channel-smartcard.h"
+#include "channel-ssh.h"
#include "channel-usbredir.h"
#include "channel-webdav.h"
#include "spice-gtk-session.h"
@@ -40,6 +41,7 @@ spice_session_verify_get_type
spice_smartcard_channel_get_type
spice_smartcard_manager_get_type
spice_session_verify_get_type
+spice_ssh_channel_get_type
spice_usbredir_channel_get_type
spice_usb_device_get_type
spice_usb_device_manager_get_type
diff --git a/src/Makefile.am b/src/Makefile.am
index 7542580..87fdfe9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -254,6 +254,7 @@ libspice_client_glib_2_0_la_SOURCES =
\
channel-port.c \
channel-record.c\
channel-smartcard.c \
+ channel-ssh.c \
channel-usbredir.c \
channel-usbredir-priv.h \
smartcard-manager.c \
@@ -306,6 +307,7 @@ libspice_client_glibinclude_H
Re: [Spice-devel] [PATCH spice-gtk] widget: Fix rendering issues with CSD on Windows
On Fri, Aug 12, 2016 at 11:38 PM, Pavel Grunt wrote:
> Replace GDK_WINDOW_HWND by gdk_win32_window_get_impl_hwnd() which gets
> the HWND directly, without any side effects.
>
> Related:
> https://bugzilla.redhat.com/show_bug.cgi?id=1352216
> ---
> src/spice-widget.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 7c1c756..ae56d1b 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -1417,7 +1417,7 @@ static gboolean
> check_for_grab_key_released(SpiceDisplay *display, int type, int
> static void update_display(SpiceDisplay *display)
> {
> #ifdef G_OS_WIN32
> -win32_window = display ?
> GDK_WINDOW_HWND(gtk_widget_get_window(GTK_WIDGET(display))) : NULL;
> +win32_window = display ?
> gdk_win32_window_get_impl_hwnd(gtk_widget_get_window(GTK_WIDGET(display))) :
> NULL;
> #endif
> }
>
> --
> 2.9.2
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
ACK!
(PS: And this patch must be backported to the mingw-spice-gtk on Fedora).
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] vmcstream: set the right result for the task
On Wed, Jul 27, 2016 at 12:42 PM, Fabiano Fidêncio wrote:
> On Wed, Jul 27, 2016 at 12:39 PM, Christophe Fergeau
> wrote:
>> On Wed, Jul 27, 2016 at 11:17:38AM +0200, Fabiano Fidêncio wrote:
>>> This bogus code was introduced when switching to GTask API. Seems that
>>> while writing those patches I just overlooked this part and set the wrong
>>> result for the task. As part of the problems introduced (and now fixed)
>>> you can notice that no output stream was being sent to the guest.
>>>
>>> Signed-off-by: Fabiano Fidêncio
>>> ---
>>> src/vmcstream.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>>> index c536f71..ffb1ba4 100644
>>> --- a/src/vmcstream.c
>>> +++ b/src/vmcstream.c
>>> @@ -411,7 +411,7 @@ write_cb(GObject *source_object,
>>> {
>>> GTask *task = user_data;
>>>
>>> -g_task_return_pointer(task, g_object_ref(task), g_object_unref);
>>> +g_task_return_pointer(task, g_object_ref(res), g_object_unref);
>>>
>>> g_object_unref(task);
>>> }
>>
>> You'll also need
>>
>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>> index c536f71..32d8b5e 100644
>> --- a/src/vmcstream.c
>> +++ b/src/vmcstream.c
>> @@ -399,9 +399,13 @@ spice_vmc_output_stream_write_finish(GOutputStream
>> *stream,
>> {
>> SpiceVmcOutputStream *self = SPICE_VMC_OUTPUT_STREAM(stream);
>> GAsyncResult *res = g_task_propagate_pointer(G_TASK(simple), error);
>> +gssize bytes_written;
>>
>> SPICE_DEBUG("spicevmc write finish");
>> -return spice_vmc_write_finish(self->channel, res, error);
>> +bytes_written = spice_vmc_write_finish(self->channel, res, error);
>> +g_object_unref(res);
>> +
>> +return bytes_written;
>> }
>>
>> static void
>>
>>
>> (but this is arguably a slightly different bug which was already existing
>> before your change).
>
> Feel free to squash my patch into your change or push both separately.
Hmm. With your patch I can see the following critical
(remote-viewer:18180): GSpice-CRITICAL **:
spice_vmc_input_stream_read_all_async: assertion 'self->task == NULL'
failed
So, please, let's go for my patch and then we can get back to your
changes later on.
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk v2] vmcstream: set the right result for the task
On Wed, Jul 27, 2016 at 12:39 PM, Christophe Fergeau
wrote:
> On Wed, Jul 27, 2016 at 11:17:38AM +0200, Fabiano Fidêncio wrote:
>> This bogus code was introduced when switching to GTask API. Seems that
>> while writing those patches I just overlooked this part and set the wrong
>> result for the task. As part of the problems introduced (and now fixed)
>> you can notice that no output stream was being sent to the guest.
>>
>> Signed-off-by: Fabiano Fidêncio
>> ---
>> src/vmcstream.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>> index c536f71..ffb1ba4 100644
>> --- a/src/vmcstream.c
>> +++ b/src/vmcstream.c
>> @@ -411,7 +411,7 @@ write_cb(GObject *source_object,
>> {
>> GTask *task = user_data;
>>
>> -g_task_return_pointer(task, g_object_ref(task), g_object_unref);
>> +g_task_return_pointer(task, g_object_ref(res), g_object_unref);
>>
>> g_object_unref(task);
>> }
>
> You'll also need
>
> diff --git a/src/vmcstream.c b/src/vmcstream.c
> index c536f71..32d8b5e 100644
> --- a/src/vmcstream.c
> +++ b/src/vmcstream.c
> @@ -399,9 +399,13 @@ spice_vmc_output_stream_write_finish(GOutputStream
> *stream,
> {
> SpiceVmcOutputStream *self = SPICE_VMC_OUTPUT_STREAM(stream);
> GAsyncResult *res = g_task_propagate_pointer(G_TASK(simple), error);
> +gssize bytes_written;
>
> SPICE_DEBUG("spicevmc write finish");
> -return spice_vmc_write_finish(self->channel, res, error);
> +bytes_written = spice_vmc_write_finish(self->channel, res, error);
> +g_object_unref(res);
> +
> +return bytes_written;
> }
>
> static void
>
>
> (but this is arguably a slightly different bug which was already existing
> before your change).
Feel free to squash my patch into your change or push both separately.
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk v2] vmcstream: set the right result for the task
This bogus code was introduced when switching to GTask API. Seems that
while writing those patches I just overlooked this part and set the wrong
result for the task. As part of the problems introduced (and now fixed)
you can notice that no output stream was being sent to the guest.
Signed-off-by: Fabiano Fidêncio
---
src/vmcstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vmcstream.c b/src/vmcstream.c
index c536f71..ffb1ba4 100644
--- a/src/vmcstream.c
+++ b/src/vmcstream.c
@@ -411,7 +411,7 @@ write_cb(GObject *source_object,
{
GTask *task = user_data;
-g_task_return_pointer(task, g_object_ref(task), g_object_unref);
+g_task_return_pointer(task, g_object_ref(res), g_object_unref);
g_object_unref(task);
}
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk] vmcstream: set the right result for the task
This bogus code was introduced when switching to GTask API. Seems that
while writing those patches I just overlooked this part and set the wrong
result for the task. As part of the problems introduced (and now fixed)
you can notice that no output stream was being sent to the guest.
Signed-off-by: Fabiano Fidêncio
---
src/vmcstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vmcstream.c b/src/vmcstream.c
index c536f71..7c7af44 100644
--- a/src/vmcstream.c
+++ b/src/vmcstream.c
@@ -411,7 +411,7 @@ write_cb(GObject *source_object,
{
GTask *task = user_data;
-g_task_return_pointer(task, g_object_ref(task), g_object_unref);
+g_task_return_pointer(task, res, NULL);
g_object_unref(task);
}
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common] codegen: Do not generate extra null check
On Mon, Jul 25, 2016 at 6:40 PM, Pavel Grunt wrote:
> Spotted by coverity
Although I remember seeing free (NULL); crashing in some really old
BDSs, it's a no-op in pretty much any modern system.
> ---
> python_modules/demarshal.py | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
> index 2252f37..de2ccd0 100644
> --- a/python_modules/demarshal.py
> +++ b/python_modules/demarshal.py
> @@ -1098,8 +1098,7 @@ def write_msg_parser(writer, message):
> writer.newline()
> if writer.has_error_check:
> writer.label("error")
> -with writer.block("if (data != NULL)"):
> -writer.statement("free(data)")
> +writer.statement("free(data)")
> writer.statement("return NULL")
> writer.end_block()
>
> --
> 2.9.2
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Acked-by: Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Help :Build error of "configure: error: Package requirements (libxml-2.0 >= 2.6.0) were not met:"
On Thu, Jun 23, 2016 at 7:14 AM, 小泉 wrote:
> Hi all
>
> I build virt-viewer for Windows on my fedora based on following instruction.
>
> Building Instructions/Client
> http://www.spice-space.org/page/Building_Instructions/Client
>
> I see "configure: error: Package requirements (libxml-2.0 >= 2.6.0) were
> not met:" error message when I run "mingw32-configure --prefix=$INST_ROOT".
> But libxml-2.0 already has been installed on my fedora.
> I investigated about this problem for several days. However I could not find
> out good solution.
> Could you please tell me what I should do?
>
> Here is error message:
> [**@localhost virt-viewer]$ mingw32-configure --prefix=$INST_ROOT
> checking for a BSD-compatible install... /usr/bin/install -c
> checking whether build environment is sane... yes
> ---
> checking for LIBXML2... no
> configure: error: Package requirements (libxml-2.0 >= 2.6.0) were not met:
>
> No package 'libxml-2.0' found
>
> Consider adjusting the PKG_CONFIG_PATH environment variable if you
> installed software in a non-standard prefix.
>
> ---
>
> But libxml-2.0 is in my fedora.
> [**@localhost virt-viewer]$ rpm -qa | grep libxml
> python-libxml2-2.9.3-3.fc24.x86_64
> libxml2-static-2.9.3-3.fc24.x86_64
> libxml2-debuginfo-2.9.3-3.fc24.x86_64
> libxml2-devel-2.9.3-3.fc24.x86_64
> libxml2-2.9.3-3.fc24.x86_64
>
> koizumi
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
You need mingw{32,64}-libxml2 installed.
In the top dir of your virt-viewer checkout, do: dnf builddep
mingw-virt-viewer.spec
It should be enough for getting all the deps you need.
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click
On Mon, Jun 20, 2016 at 5:34 PM, Marc-André Lureau wrote: > Hi > > On Mon, Jun 20, 2016 at 5:33 PM, Marc-André Lureau > wrote: >> On Mon, Jun 20, 2016 at 5:28 PM, Fabiano Fidêncio >> wrote: >>>> gtk_widget_set_can_focus(widget, true); >>>> +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true); >>> >>> I'd just do: s/true/TRUE before pushing, but no need to resend the >>> patch because of this is just my preference :-) >> >> The line above uses lowercase 'true', I guess I'll change it too then ;) >> > > Oh well, 'true' seems to win anyway: > elmarco@anjohibe:~/src/spice/spice-gtk (master *%)$ git grep -c TRUE > src/spice-widget.c > src/spice-widget.c:17 > elmarco@anjohibe:~/src/spice/spice-gtk (master *%)$ git grep -c true > src/spice-widget.c > src/spice-widget.c:38 > > > -- > Marc-André Lureau So, keep it as it is. Feel free to push the patch, Marc-André. -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk] widget: fix keyboard ungrab after click
On Mon, Jun 20, 2016 at 3:22 PM, wrote:
> From: Marc-André Lureau
>
> Since the switch to a container widget (gtkstack then gtkeventbox), the
> grab may be lost when clicking on the display. Since events are treated
> at the top level container, set widget "above-child" to trap all of them
> to solve this.
>
> Fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=96595
>
> Signed-off-by: Marc-André Lureau
> Reported-by: Frediano Ziglio
> ---
> src/spice-widget.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 4ca74bf..4cfbcfa 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -632,6 +632,8 @@ static void spice_display_init(SpiceDisplay *display)
>GDK_KEY_PRESS_MASK |
>GDK_SCROLL_MASK);
> gtk_widget_set_can_focus(widget, true);
> +gtk_event_box_set_above_child(GTK_EVENT_BOX(widget), true);
I'd just do: s/true/TRUE before pushing, but no need to resend the
patch because of this is just my preference :-)
> +
> d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
> d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
> d->mouse_cursor = get_blank_cursor();
> --
> 2.7.4
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Acked-by: Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2] sasl: fix SASL GSSAPI by allowing NULL username
From: Alexander Bokovoy
SASL GSSAPI module will try to negotiate authentication based on the
credentials in the default credentials cache. It does not matter if
SPICE knows username or not as SASL negotiation will pass through the
discovered name from the GSSAPI module.
Signed-off-by: Alexander Bokovoy
Acked-by: Fabiano Fidêncio
---
Sending the patch to the ML for the record.
I already ACKed the patch and anyone objects I'll push it Tomorrow.
---
src/spice-channel.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/spice-channel.c b/src/spice-channel.c
index c6e548d..0eb0e61 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1387,11 +1387,10 @@ spice_channel_gather_sasl_credentials(SpiceChannel
*channel,
switch (interact[ninteract].id) {
case SASL_CB_AUTHNAME:
case SASL_CB_USER:
-if (spice_session_get_username(c->session) == NULL)
-return FALSE;
-
-interact[ninteract].result =
spice_session_get_username(c->session);
-interact[ninteract].len = strlen(interact[ninteract].result);
+if (spice_session_get_username(c->session) != NULL) {
+interact[ninteract].result =
spice_session_get_username(c->session);
+interact[ninteract].len = strlen(interact[ninteract].result);
+}
break;
case SASL_CB_PASS:
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI
On Mon, Jun 6, 2016 at 4:07 PM, Alexander Bokovoy wrote: > On Mon, 06 Jun 2016, Daniel P. Berrange wrote: >> >> On Mon, Jun 06, 2016 at 04:34:09PM +0300, Alexander Bokovoy wrote: >>> >>> On Mon, 06 Jun 2016, Daniel P. Berrange wrote: >>> > On Mon, Jun 06, 2016 at 09:01:10AM -0400, Marc-André Lureau wrote: >>> > > Hi >>> > > >>> > > - Original Message - >>> > > > I'm sending Alexander Bokovoy's patch as it is, also here is some >>> > > > notes from >>> > > > him: >>> > > > >>> > > > "I'd really like to find a way to do it with pure SASL properties >>> > > > so that the >>> > > > code would work for both SPNEGO and Kerberos. SPNEGO NTLMSSP would >>> > > > make it >>> > > > working for environments where you don't have Kerberos but what we >>> > > > have >>> > > > right now should be fine for pure Kerberos environments like >>> > > > FreeIPA or >>> > > > Active Directory." >>> > > > >>> > > > And also his blog post: >>> > > > >>> > > > https://vda.li/en/posts/2016/05/30/Single-sign-on-to-virtual-machines/ >>> > > > >>> > > > On one hand I think would be good to have this issue partially >>> > > > fixed (as per >>> > > > Alexander's comment) for 0.32, on the other hand I don't like >>> > > > calling these >>> > > > kerberos functions directly. Also, we probably would have to add a >>> > > > kerberos >>> > > > check/option on configure, right? I can do that without any >>> > > > problems, but I >>> > > > firstly would like to hear the opinions from other people in the >>> > > > project. >>> > > >>> > > Yes, it will have to be optional (especially because compiling krb5 >>> > > on mingw is *hard* - last time I checked) >>> > >>> > Even compiling cryus-sasl is hard - indeed last I looked fedora didn't >>> > have any mingw packages for it. >>> > >>> > > >>> > > > I'm willing to re-work this patch after the release and try to find >>> > > > an ideal >>> > > > solution (if possible) and also spend some more time digging into >>> > > > the >>> > > > differences on handling this between gtk-vnc and spice-gtk. >>> > > >>> > > From his blog, I gathered that it worked with gtk-vnc but not with >>> > > spice-gtk. Why do we need krb specific code when gtk-vnc doesn't need >>> > > it? >>> > >>> > It looks like the code is trying to set a default username based on the >>> > current kerberos credential the user has. gtk-vnc doesn't bother trying >>> > todo this - the user just always has to supply the username explicitly >>> > IMHO it would be fine for spice-gtk todo the same and avoid the krb >>> > dep/ >>> I tried that. Let me get a bit deeper into details, though. >>> >>> Cyrus SASL GSSAPI would work if you provide NULL username but the code >>> in spice-gtk rejects such usernames: >>> >>> https://cgit.freedesktop.org/spice/spice-gtk/tree/src/spice-channel.c#n1390 >> >> >> Hmm, that code looks really rather wrong - it is clearly making a bogus >> assumption that a NULL username will result in auth failure - it should >> definitely be left upto the SASL library to decide that on the server >> side. > > On the client side, you mean. > >>> I tried to allow NULL username here but the problem is that we need >>> eventually to set actual username so that SPICE communication can >>> continue. And if SASL GSSAPI module did find default credentials, we >>> need to pick up the username from them. This is possible theoretically >>> but all my attempts to do so caused SPICE server side to drop actual >>> SPICE connection. >> >> >> I'm not sure what failure you just remove that check, but I think we >> need to investigate that further, as I don't think that check for >> NULL is right. > > It is wrong, for sure. > > Hm.. I retried again with a simple patch (attached) and it worked this > time. Nice, I really like the patch. You have an ACK from me and if we don't have any objections in the next days I'll push your patch _before_ the 0.32 release. > > -- > / Alexander Bokovoy Thanks for patch and best regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI
On Mon, Jun 6, 2016 at 3:01 PM, Marc-André Lureau wrote: > Hi > > - Original Message - >> I'm sending Alexander Bokovoy's patch as it is, also here is some notes from >> him: >> >> "I'd really like to find a way to do it with pure SASL properties so that the >> code would work for both SPNEGO and Kerberos. SPNEGO NTLMSSP would make it >> working for environments where you don't have Kerberos but what we have >> right now should be fine for pure Kerberos environments like FreeIPA or >> Active Directory." >> >> And also his blog post: >> https://vda.li/en/posts/2016/05/30/Single-sign-on-to-virtual-machines/ >> >> On one hand I think would be good to have this issue partially fixed (as per >> Alexander's comment) for 0.32, on the other hand I don't like calling these >> kerberos functions directly. Also, we probably would have to add a kerberos >> check/option on configure, right? I can do that without any problems, but I >> firstly would like to hear the opinions from other people in the project. > > Yes, it will have to be optional (especially because compiling krb5 on mingw > is *hard* - last time I checked) Currently we build mingw-spice-gtk with --without-sasl (both fedora and downstream). So, it won't be that problematic. > >> I'm willing to re-work this patch after the release and try to find an ideal >> solution (if possible) and also spend some more time digging into the >> differences on handling this between gtk-vnc and spice-gtk. > > From his blog, I gathered that it worked with gtk-vnc but not with spice-gtk. > Why do we need krb specific code when gtk-vnc doesn't need it? Yeah, that's part of my plan to setup the environment and dig into it as soon as I have time for it. > >> >> Please, as I'm not whether Alexander is subscribed to our mailing list or >> not, >> let's keep him CC'ed for any further interaction. >> > > Thanks again Alexander Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] Support SASL GSSAPI
On Mon, Jun 6, 2016 at 2:51 PM, Fabiano Fidêncio wrote: > I'm sending Alexander Bokovoy's patch as it is, also here is some notes from > him: > > "I'd really like to find a way to do it with pure SASL properties so that the > code would work for both SPNEGO and Kerberos. SPNEGO NTLMSSP would make it > working for environments where you don't have Kerberos but what we have > right now should be fine for pure Kerberos environments like FreeIPA or > Active Directory." > > And also his blog post: > https://vda.li/en/posts/2016/05/30/Single-sign-on-to-virtual-machines/ > > On one hand I think would be good to have this issue partially fixed (as per > Alexander's comment) for 0.32, on the other hand I don't like calling these > kerberos functions directly. Also, we probably would have to add a kerberos > check/option on configure, right? I can do that without any problems, but I > firstly would like to hear the opinions from other people in the project. Alexander just pointed out (on #freeIPA channel) that we don't need the kerberos checks as these come to us via Cyrus-SASL already. > > I'm willing to re-work this patch after the release and try to find an ideal > solution (if possible) and also spend some more time digging into the > differences on handling this between gtk-vnc and spice-gtk. > > Please, as I'm not whether Alexander is subscribed to our mailing list or not, > let's keep him CC'ed for any further interaction. > > Alexander Bokovoy (1): > spice-channel: support SASL GSSAPI > > src/spice-channel.c | 61 > +++++ > 1 file changed, 57 insertions(+), 4 deletions(-) > > -- > 2.7.4 > Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk] spice-channel: support SASL GSSAPI
From: Alexander Bokovoy
Support SASL GSSAPI Kerberos by discovering default credential from
default Kerberos credentials cache.
If default Kerberos credential is missing, fallback to standard method
with password and username.
Signed-of-by: Alexander Bokovoy
---
src/spice-channel.c | 61 +
1 file changed, 57 insertions(+), 4 deletions(-)
diff --git a/src/spice-channel.c b/src/spice-channel.c
index c6e548d..af49d8a 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1358,17 +1358,32 @@ static gchar *addr_to_string(GSocketAddress *addr)
static gboolean
spice_channel_gather_sasl_credentials(SpiceChannel *channel,
- sasl_interact_t *interact)
+ sasl_interact_t *interact,
+ sasl_conn_t *sasl_conn)
{
SpiceChannelPrivate *c;
-int ninteract;
+int ninteract, rc;
gboolean ret = TRUE;
+const char *mechname = NULL;
+gboolean is_gssapi = FALSE;
+krb5_context krbctx = NULL;
+krb5_ccache ccache = NULL;
+krb5_principal defprinc = NULL;
+krb5_error_code krberr = 0;
+char *username = NULL;
g_return_val_if_fail(channel != NULL, FALSE);
g_return_val_if_fail(channel->priv != NULL, FALSE);
c = channel->priv;
+if (sasl_conn != NULL) {
+rc = sasl_getprop(sasl_conn, SASL_MECHNAME, (const void**) &mechname);
+if (rc == SASL_OK && g_strcmp0(mechname, "GSSAPI") == 0) {
+is_gssapi = TRUE;
+}
+}
+
/* FIXME: we could keep connection open and ask connection details if
missing */
for (ninteract = 0 ; interact[ninteract].id != 0 ; ninteract++) {
@@ -1387,6 +1402,44 @@ spice_channel_gather_sasl_credentials(SpiceChannel
*channel,
switch (interact[ninteract].id) {
case SASL_CB_AUTHNAME:
case SASL_CB_USER:
+/* For SASL GSSAPI derive user name from the default Kerberos
credential */
+if (is_gssapi && spice_session_get_username(c->session) == NULL) {
+krberr = krb5_init_context(&krbctx);
+if (krberr) {
+g_critical("Cannot initialize Kerberos context while using
SASL GSSAPI");
+return FALSE;
+}
+krberr = krb5_cc_default(krbctx, &ccache);
+if (krberr) {
+krb5_free_context(krbctx);
+g_critical("Default Kerberos credential cache not found.
Need username and password to initialize");
+return FALSE;
+}
+krberr = krb5_cc_get_principal(krbctx, ccache, &defprinc);
+if (krberr) {
+krb5_cc_close(krbctx, ccache);
+krb5_free_context(krbctx);
+g_critical("Kerberos user principal not found. Need
username and password to initialize");
+return FALSE;
+}
+
+krberr = krb5_unparse_name_flags(krbctx, defprinc,
KRB5_PRINCIPAL_UNPARSE_SHORT, &username);
+if (krberr) {
+krb5_free_principal(krbctx, defprinc);
+krb5_cc_close(krbctx, ccache);
+krb5_free_context(krbctx);
+g_critical("Kerberos user principal cannot be parsed. Need
username and password to initialize");
+return FALSE;
+}
+
+g_object_set(c->session, "username", username, NULL);
+
+krb5_free_unparsed_name(krbctx, username);
+krb5_free_principal(krbctx, defprinc);
+krb5_cc_close(krbctx, ccache);
+krb5_free_context(krbctx);
+}
+
if (spice_session_get_username(c->session) == NULL)
return FALSE;
@@ -1595,7 +1648,7 @@ restart:
/* Need to gather some credentials from the client */
if (err == SASL_INTERACT) {
-if (!spice_channel_gather_sasl_credentials(channel, interact)) {
+if (!spice_channel_gather_sasl_credentials(channel, interact,
saslconn)) {
CHANNEL_DEBUG(channel, "Failed to collect auth credentials");
goto error;
}
@@ -1679,7 +1732,7 @@ restart:
/* Need to gather some credentials from the client */
if (err == SASL_INTERACT) {
if (!spice_channel_gather_sasl_credentials(channel,
- interact)) {
+ interact, saslconn)) {
CHANNEL_DEBUG(channel, "%s", "Failed to collect auth
credentials");
goto error;
}
--
2.7.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk] Support SASL GSSAPI
I'm sending Alexander Bokovoy's patch as it is, also here is some notes from him: "I'd really like to find a way to do it with pure SASL properties so that the code would work for both SPNEGO and Kerberos. SPNEGO NTLMSSP would make it working for environments where you don't have Kerberos but what we have right now should be fine for pure Kerberos environments like FreeIPA or Active Directory." And also his blog post: https://vda.li/en/posts/2016/05/30/Single-sign-on-to-virtual-machines/ On one hand I think would be good to have this issue partially fixed (as per Alexander's comment) for 0.32, on the other hand I don't like calling these kerberos functions directly. Also, we probably would have to add a kerberos check/option on configure, right? I can do that without any problems, but I firstly would like to hear the opinions from other people in the project. I'm willing to re-work this patch after the release and try to find an ideal solution (if possible) and also spend some more time digging into the differences on handling this between gtk-vnc and spice-gtk. Please, as I'm not whether Alexander is subscribed to our mailing list or not, let's keep him CC'ed for any further interaction. Alexander Bokovoy (1): spice-channel: support SASL GSSAPI src/spice-channel.c | 61 + 1 file changed, 57 insertions(+), 4 deletions(-) -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2] avoid integer underflow under 32 bit architectures
On Fri, Jun 3, 2016 at 2:03 PM, Frediano Ziglio wrote: > The segment_size computation on 32 bit can lead to big numbers which > can lead to negative offset. As we test we don't overrun the buffer > avoid to underrun it as we don't have a check for this. The last sentence in the commit message is a bit confusing. "let s avoid to underrun the buffer, as it's not checked", maybe? > > Signed-off-by: Frediano Ziglio > --- > server/red-parse-qxl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c > index 7678c7e..721c861 100644 > --- a/server/red-parse-qxl.c > +++ b/server/red-parse-qxl.c > @@ -276,6 +276,9 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int > group_id, > count = start->count; > segment_size = sizeof(SpicePathSeg) + (uint64_t) count * > sizeof(SpicePointFix); > mem_size += sizeof(SpicePathSeg *) + SPICE_ALIGN(segment_size, 4); > +/* avoid going backward with 32 bit architectures */ > +spice_assert((uint64_t) count * sizeof(QXLPointFix) > + <= (char*) end - (char*) &start->points[0]); > start = (QXLPathSeg*)(&start->points[count]); > } > > -- > 2.7.4 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel Acked-by: Fabiano Fidêncio -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/2] fix integer overflows in red_get_path
On Fri, Jun 3, 2016 at 2:03 PM, Frediano Ziglio wrote:
> Use 64 bit arithmetic to avoid overflows.
> The multiplication between count and a constant can overflow.
>
> Signed-off-by: Frediano Ziglio
> ---
> server/red-parse-qxl.c | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index 0fdf912..7678c7e 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -246,7 +246,8 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int
> group_id,
> bool free_data;
> QXLPath *qxl;
> SpicePath *red;
> -size_t size, mem_size, mem_size2, dsize, segment_size;
> +size_t size;
> +uint64_t mem_size, mem_size2, segment_size;
> int n_segments;
> int i;
> uint32_t count;
> @@ -273,7 +274,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int
> group_id,
> while (start+1 < end) {
> n_segments++;
> count = start->count;
> -segment_size = sizeof(SpicePathSeg) + count * sizeof(SpicePointFix);
> +segment_size = sizeof(SpicePathSeg) + (uint64_t) count *
> sizeof(SpicePointFix);
> mem_size += sizeof(SpicePathSeg *) + SPICE_ALIGN(segment_size, 4);
> start = (QXLPathSeg*)(&start->points[count]);
> }
> @@ -292,14 +293,8 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots,
> int group_id,
>
> /* Protect against overflow in size calculations before
> writing to memory */
> -spice_assert(mem_size2 + sizeof(SpicePathSeg) > mem_size2);
> -mem_size2 += sizeof(SpicePathSeg);
> -spice_assert(count < UINT32_MAX / sizeof(SpicePointFix));
> -dsize = count * sizeof(SpicePointFix);
> -spice_assert(mem_size2 + dsize > mem_size2);
> -mem_size2 += dsize;
> -
> /* Verify that we didn't overflow due to guest changing data */
> +mem_size2 += sizeof(SpicePathSeg) + (uint64_t) count *
> sizeof(SpicePointFix);
> spice_assert(mem_size2 <= mem_size);
>
> seg->flags = start->flags;
> --
> 2.7.4
>
> _______
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Acked-by: Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk 1/2] file-trasfer-task: Protect header include
On Wed, May 25, 2016 at 7:30 AM, Pavel Grunt wrote: > As stated in the commit d2f33178c40ac51b1c8b1bf796a328631d9869c7 Glib > applications should only include "spice-client.h". > --- > src/spice-file-transfer-task.h | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/src/spice-file-transfer-task.h b/src/spice-file-transfer-task.h > index 18dc509..fba132e 100644 > --- a/src/spice-file-transfer-task.h > +++ b/src/spice-file-transfer-task.h > @@ -18,6 +18,12 @@ > #ifndef __SPICE_FILE_TRANSFER_TASK_H__ > #define __SPICE_FILE_TRANSFER_TASK_H__ > > +#if !defined(__SPICE_CLIENT_H_INSIDE__) && !defined(SPICE_COMPILATION) > +#warning "Only can be included directly" > +#endif > + > +#include "spice-client.h" > + > #include > > G_BEGIN_DECLS > -- > 2.8.3 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel ACK both patches! ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] glz: simplification of do_match and PIXEL_ID
On Wed, May 18, 2016 at 8:20 PM, Uri Lublin wrote:
> Following commit e8017dd366afabbd099a13d7d455a37fd50be124 here
> are some more ifdef simplification.
>
> For PIXEL_ID and PIXEL_DIST macros,
> the ifndef LZ_PLT defines are special cases of the LZ_PLT
> (#else) defines, when pix_per_byte is 1
>
> Now do_match can accept pix_per_byte param for all different
> definitions (implementations).
>
> Signed-off-by: Uri Lublin
> ---
>
> If people prefer I can split it to two patches, one for
> the macros and one for do_match
>
> ---
> server/glz-encode.tmpl.c | 14 +++---
> 1 file changed, 3 insertions(+), 11 deletions(-)
Patch looks good, but II didn't try to build the patch.
I'm assuming you did, you have my ACK.
Acked-by: Fabiano Fidêncio
>
> diff --git a/server/glz-encode.tmpl.c b/server/glz-encode.tmpl.c
> index 6d2b000..033a810 100644
> --- a/server/glz-encode.tmpl.c
> +++ b/server/glz-encode.tmpl.c
> @@ -128,18 +128,12 @@
>
> #endif
>
> -#ifndef LZ_PLT
> -#define PIXEL_ID(pix_ptr, seg_ptr) \
> -((pix_ptr) - ((PIXEL *)(seg_ptr)->lines) + (seg_ptr)->pixels_so_far)
> -#define PIXEL_DIST(src_pix_ptr, src_seg_ptr, ref_pix_ptr, ref_seg_ptr,
> pix_per_byte) \
> -(PIXEL_ID(src_pix_ptr,src_seg_ptr) - PIXEL_ID(ref_pix_ptr, ref_seg_ptr))
> -#else
> #define PIXEL_ID(pix_ptr, seg_ptr, pix_per_byte) \
> (((pix_ptr) - ((PIXEL *)(seg_ptr)->lines)) * pix_per_byte +
> (seg_ptr)->pixels_so_far)
> +
> #define PIXEL_DIST(src_pix_ptr, src_seg_ptr, ref_pix_ptr, ref_seg_ptr,
> pix_per_byte) \
> ((PIXEL_ID(src_pix_ptr,src_seg_ptr, pix_per_byte) - \
> PIXEL_ID(ref_pix_ptr, ref_seg_ptr, pix_per_byte)) / pix_per_byte)
> -#endif
>
> /* returns the length of the match. 0 if no match.
>if image_distance = 0, pixel_distance is the distance between the matching
> pixels.
> @@ -149,9 +143,7 @@ static inline size_t FNAME(do_match)(SharedDictionary
> *dict,
> const PIXEL *ref_limit,
> WindowImageSegment *ip_seg, const
> PIXEL *ip,
> const PIXEL *ip_limit,
> -#ifdef LZ_PLT
> int pix_per_byte,
> -#endif
> size_t *o_image_dist, size_t
> *o_pix_distance)
> {
> int encode_size;
> @@ -262,6 +254,8 @@ static void FNAME(compress_seg)(Encoder *encoder,
> uint32_t seg_idx, PIXEL *from,
> int copy = copied;
> #ifdef LZ_PLT
> int pix_per_byte = PLT_PIXELS_PER_BYTE[encoder->cur_image.type];
> +#else
> +int pix_per_byte = 1;
> #endif
>
> #ifdef DEBUG_ENCODE
> @@ -342,9 +336,7 @@ static void FNAME(compress_seg)(Encoder *encoder,
> uint32_t seg_idx, PIXEL *from,
> ref_limit = (PIXEL *)ref_seg->lines_end;
>
> len = FNAME(do_match)(encoder->dict, ref_seg, ref,
> ref_limit, seg, ip, ip_bound,
> -#ifdef LZ_PLT
>pix_per_byte,
> -#endif
> &image_dist, &pix_dist);
>
> #ifdef CHAINED_HASH
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v3] file-xfer: fix segfault on agent disconnection
On Mon, May 16, 2016 at 8:50 AM, Victor Toso wrote:
> We are checking self->priv->error but accessing the argument GError *
> which is NULL and leads to a segfault.
>
> Program received signal SIGSEGV, Segmentation fault.
> spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
> error=0x0) at channel-main.c:2963
> 2963VDAgentFileXferStatusMessage msg = {
> (gdb) bt
> #0 spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
> error=0x0) at channel-main.c:2963
> #1 in file_xfer_data_flushed_cb (source_object=0x7cc1d0, res=0x953390,
> user_data=user_data@entry=0x7fffd0006f00) at channel-main.c:1857
> #2 in g_task_return_now (task=0x953390) at gtask.c:1108
> #3 in g_task_return (task=0x953390, type=) at gtask.c:1166
> #4 in flush_foreach_remove (key=, value=,
> user_data=) at channel-main.c:928
> #5 in g_hash_table_foreach_remove_or_steal (hash_table=0x70cea0,
> func=func@entry=0x75616f10 ,
> user_data=user_data@entry=0x0, notify=notify@entry=1) at ghash.c:1492
> #6 in g_hash_table_foreach_remove (hash_table=,
> func=func@entry=0x75616f10 ,
> user_data=user_data@entry=0x0) at ghash.c:1538
> #7 in file_xfer_flushed (success=0, channel=0x7cc1d0) at channel-main.c:936
> #8 spice_main_channel_reset_agent (channel=0x7cc1d0) at channel-main.c:466
> #9 set_agent_connected (channel=0x7cc1d0, connected=connected@entry=0) at
> channel-main.c:1572
> #10 in spice_main_channel_reset (channel=0x7cc1d0, migrating=0) at
> channel-main.c:485
> #11 in spice_channel_coroutine (data=0x7cc1d0) at spice-channel.c:2564
> #12 in coroutine_trampoline (cc=0x7cb860) at coroutine_ucontext.c:63
> #13 in continuation_trampoline (i0=, i1=) at
> continuation.c:55
> #14 in ?? () from /lib64/libc.so.6
> #15 in ?? ()
> #16 in ?? ()
> Backtrace stopped: Cannot access memory at address
> ---
> src/channel-main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 2905d7b..dec5d73 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -2964,7 +2964,7 @@ static void
> spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> if (self->priv->error) {
> VDAgentFileXferStatusMessage msg = {
> .id = self->priv->id,
> -.result = error->code == G_IO_ERROR_CANCELLED ?
> +.result = self->priv->error->code == G_IO_ERROR_CANCELLED ?
> VD_AGENT_FILE_XFER_STATUS_CANCELLED :
> VD_AGENT_FILE_XFER_STATUS_ERROR,
> };
> agent_msg_queue_many(self->priv->channel, VD_AGENT_FILE_XFER_STATUS,
> @@ -2986,7 +2986,7 @@ static void
> spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> self);
> self->priv->pending = TRUE;
> signal:
> -g_signal_emit(self, task_signals[SIGNAL_FINISHED], 0, error);
> +g_signal_emit(self, task_signals[SIGNAL_FINISHED], 0, self->priv->error);
> }
>
>
> --
> 2.5.5
>
Acked-by: Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] gtask-helper: include functions to check coroutine context
Toso,
On Sat, May 14, 2016 at 10:05 PM, Victor Toso wrote:
> Hi,
>
> On Sat, May 14, 2016 at 06:34:41PM +0200, Fabiano Fidêncio wrote:
>> On Sat, May 14, 2016 at 5:03 PM, Victor Toso wrote:
>> > If we are in coroutine context, fallback to return in idle; Otherwise,
>> > let GTask decide if callback should be called or not.
>> > ---
>> > src/gtask-helper.h | 45 +
>> > 1 file changed, 45 insertions(+)
>> >
>> > diff --git a/src/gtask-helper.h b/src/gtask-helper.h
>> > index 81c041f..8771021 100644
>> > --- a/src/gtask-helper.h
>> > +++ b/src/gtask-helper.h
>> > @@ -29,6 +29,51 @@ void g_task_helper_return_error_in_idle(GTask *task,
>> > GError *error);
>> > void g_task_helper_return_new_error_in_idle(GTask *task, GQuark domain,
>> > gint code, const char *format, ...);
>> > void g_task_helper_return_pointer_in_idle(GTask *task, gpointer result,
>> > GDestroyNotify result_destroy);
>> >
>> > +#define g_task_helper_return_boolean(...) \
>> > +do { \
>> > +if (coroutine_self_is_main()) { \
>> > +g_task_return_boolean(__VA_ARGS__); \
>> > +} else { \
>> > +g_task_helper_return_boolean_in_idle(__VA_ARGS__); \
>> > +} \
>> > +} while (0)
>> > +
>> > +#define g_task_helper_return_int(...) \
>> > +do { \
>> > +if (coroutine_self_is_main()) { \
>> > +g_task_return_int(__VA_ARGS__); \
>> > +} else { \
>> > +g_task_helper_return_int_in_idle(__VA_ARGS__); \
>> > +} \
>> > +} while (0)
>> > +
>> > +#define g_task_helper_return_error(...) \
>> > +do { \
>> > +if (coroutine_self_is_main()) { \
>> > +g_task_return_error(__VA_ARGS__); \
>> > +} else { \
>> > +g_task_helper_return_error_in_idle(__VA_ARGS__); \
>> > +} \
>> > +} while (0)
>> > +
>> > +#define g_task_helper_return_new_error(...) \
>> > +do { \
>> > +if (coroutine_self_is_main()) { \
>> > +g_task_return_new_error(__VA_ARGS__); \
>> > +} else { \
>> > +g_task_helper_return_new_error_in_idle(__VA_ARGS__); \
>> > +} \
>> > +} while (0)
>> > +
>> > +#define g_task_helper_return_pointer(...) \
>> > +do { \
>> > +if (coroutine_self_is_main()) { \
>> > +g_task_return_pointer(__VA_ARGS__); \
>> > +} else { \
>> > +g_task_helper_return_pointer_in_idle(__VA_ARGS__); \
>> > +} \
>> > +} while (0)
>> > +
>> > G_END_DECLS
>> >
>> > #endif /* __G_TASK_HELPER_H__ */
>> > --
>> > 2.5.5
>> >
>> > ___
>> > Spice-devel mailing list
>> > Spice-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>> NACK!
>> This is not what we agreed on IRC yesterday :-\
>> Anyways, with
>> https://lists.freedesktop.org/archives/spice-devel/2016-May/029040.html
>> this patch doesn't seem to be needed anymore.
>
> Yes, this is not needed. No, it isn't what we agreed on IRC, just the
> idea that is. I realized while testing that this would not needed but I
> thought it would be good to share the patch anyway. I should have
> mentioned this in the email.
>
> Sorry for taking your time and thanks for reviewing.
There is nothing to be sorry about!
I'd like to have the g_task_helper_* in anyways, but without the
smartness (I mean, just returning in idle).
I do believe it's cleaner/better than just adding an idle. But it's
not up to me.
> toso
>
>> ___
>> 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
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2] file-xfer: fix segfault on agent disconnection
Toso,
On Sat, May 14, 2016 at 10:09 PM, Victor Toso wrote:
> Hi,
>
> On Sat, May 14, 2016 at 05:56:45PM +0200, Fabiano Fidêncio wrote:
>> On May 14, 2016 17:29, "Victor Toso" wrote:
>> >
>> > We are checking self->priv->error but accessing the argument GError *
>> > which is NULL and leads to a segfault.
>> >
>> > Program received signal SIGSEGV, Segmentation fault.
>> > spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
>> error=0x0) at channel-main.c:2963
>> > 2963VDAgentFileXferStatusMessage msg = {
>> > (gdb) bt
>> > #0 spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
>> error=0x0) at channel-main.c:2963
>> > #1 in file_xfer_data_flushed_cb (source_object=0x7cc1d0, res=0x953390,
>> user_data=user_data@entry=0x7fffd0006f00) at channel-main.c:1857
>> > #2 in g_task_return_now (task=0x953390) at gtask.c:1108
>> > #3 in g_task_return (task=0x953390, type=) at
>> gtask.c:1166
>> > #4 in flush_foreach_remove (key=, value=,
>> user_data=) at channel-main.c:928
>> > #5 in g_hash_table_foreach_remove_or_steal (hash_table=0x70cea0,
>> func=func@entry=0x75616f10 ,
>> user_data=user_data@entry=0x0, notify=notify@entry=1) at ghash.c:1492
>> > #6 in g_hash_table_foreach_remove (hash_table=,
>> func=func@entry=0x75616f10 ,
>> user_data=user_data@entry=0x0) at ghash.c:1538
>> > #7 in file_xfer_flushed (success=0, channel=0x7cc1d0) at
>> channel-main.c:936
>> > #8 spice_main_channel_reset_agent (channel=0x7cc1d0) at
>> channel-main.c:466
>> > #9 set_agent_connected (channel=0x7cc1d0, connected=connected@entry=0)
>> at channel-main.c:1572
>> > #10 in spice_main_channel_reset (channel=0x7cc1d0, migrating=0) at
>> channel-main.c:485
>> > #11 in spice_channel_coroutine (data=0x7cc1d0) at spice-channel.c:2564
>> > #12 in coroutine_trampoline (cc=0x7cb860) at coroutine_ucontext.c:63
>> > #13 in continuation_trampoline (i0=, i1=)
>> at continuation.c:55
>> > #14 in ?? () from /lib64/libc.so.6
>> > #15 in ?? ()
>> > #16 in ?? ()
>> > Backtrace stopped: Cannot access memory at address
>> > ---
>> > src/channel-main.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/src/channel-main.c b/src/channel-main.c
>> > index 2905d7b..692cfe7 100644
>> > --- a/src/channel-main.c
>> > +++ b/src/channel-main.c
>> > @@ -2964,7 +2964,7 @@ static void
>> spice_file_transfer_task_completed(SpiceFileTransferTask *self,
>> > if (self->priv->error) {
>> > VDAgentFileXferStatusMessage msg = {
>> > .id = self->priv->id,
>> > -.result = error->code == G_IO_ERROR_CANCELLED ?
>> > +.result = self->priv->error->code == G_IO_ERROR_CANCELLED ?
>> > VD_AGENT_FILE_XFER_STATUS_CANCELLED :
>> VD_AGENT_FILE_XFER_STATUS_ERROR,
>> > };
>> > agent_msg_queue_many(self->priv->channel,
>> VD_AGENT_FILE_XFER_STATUS,
>> > --
>> > 2.5.5
>> >
>>
>> Victor, as we talked on IRC your patch solves the problem, fust to comments:
>>
>> First one, shouldn't we emmit self->priv->error instead of error in the end
>> of this function?
>>
>> Second one, from a quick look in the code, I have the impression that we
>> can get rid of the private Error variable and just use the error that we
>> are propagating between the callbacks. A deeper read in the code would be
>> needed though in order to affirm that.
>>
>> Best Regards,
>
> I would rather that we fix the segfault in one patch and improve the
> logic in a different patch/series. I plan to look into this code
> carefully during this next week but I hope your suggestions are not a
> blocker.
The suggestion about emitting the signal, if needed, is a blocker.
The other one may be done while you re-work this code path.
>
> toso
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] gtask-helper: include functions to check coroutine context
On Sat, May 14, 2016 at 5:03 PM, Victor Toso wrote:
> If we are in coroutine context, fallback to return in idle; Otherwise,
> let GTask decide if callback should be called or not.
> ---
> src/gtask-helper.h | 45 +
> 1 file changed, 45 insertions(+)
>
> diff --git a/src/gtask-helper.h b/src/gtask-helper.h
> index 81c041f..8771021 100644
> --- a/src/gtask-helper.h
> +++ b/src/gtask-helper.h
> @@ -29,6 +29,51 @@ void g_task_helper_return_error_in_idle(GTask *task,
> GError *error);
> void g_task_helper_return_new_error_in_idle(GTask *task, GQuark domain, gint
> code, const char *format, ...);
> void g_task_helper_return_pointer_in_idle(GTask *task, gpointer result,
> GDestroyNotify result_destroy);
>
> +#define g_task_helper_return_boolean(...) \
> +do { \
> +if (coroutine_self_is_main()) { \
> +g_task_return_boolean(__VA_ARGS__); \
> +} else { \
> +g_task_helper_return_boolean_in_idle(__VA_ARGS__); \
> +} \
> +} while (0)
> +
> +#define g_task_helper_return_int(...) \
> +do { \
> +if (coroutine_self_is_main()) { \
> +g_task_return_int(__VA_ARGS__); \
> +} else { \
> +g_task_helper_return_int_in_idle(__VA_ARGS__); \
> +} \
> +} while (0)
> +
> +#define g_task_helper_return_error(...) \
> +do { \
> +if (coroutine_self_is_main()) { \
> +g_task_return_error(__VA_ARGS__); \
> +} else { \
> +g_task_helper_return_error_in_idle(__VA_ARGS__); \
> +} \
> +} while (0)
> +
> +#define g_task_helper_return_new_error(...) \
> +do { \
> +if (coroutine_self_is_main()) { \
> +g_task_return_new_error(__VA_ARGS__); \
> +} else { \
> +g_task_helper_return_new_error_in_idle(__VA_ARGS__); \
> +} \
> +} while (0)
> +
> +#define g_task_helper_return_pointer(...) \
> +do { \
> +if (coroutine_self_is_main()) { \
> +g_task_return_pointer(__VA_ARGS__); \
> +} else { \
> +g_task_helper_return_pointer_in_idle(__VA_ARGS__); \
> +} \
> +} while (0)
> +
> G_END_DECLS
>
> #endif /* __G_TASK_HELPER_H__ */
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
NACK!
This is not what we agreed on IRC yesterday :-\
Anyways, with
https://lists.freedesktop.org/archives/spice-devel/2016-May/029040.html
this patch doesn't seem to be needed anymore.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2] file-xfer: fix segfault on agent disconnection
On May 14, 2016 17:29, "Victor Toso" wrote:
>
> We are checking self->priv->error but accessing the argument GError *
> which is NULL and leads to a segfault.
>
> Program received signal SIGSEGV, Segmentation fault.
> spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
error=0x0) at channel-main.c:2963
> 2963VDAgentFileXferStatusMessage msg = {
> (gdb) bt
> #0 spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
error=0x0) at channel-main.c:2963
> #1 in file_xfer_data_flushed_cb (source_object=0x7cc1d0, res=0x953390,
user_data=user_data@entry=0x7fffd0006f00) at channel-main.c:1857
> #2 in g_task_return_now (task=0x953390) at gtask.c:1108
> #3 in g_task_return (task=0x953390, type=) at
gtask.c:1166
> #4 in flush_foreach_remove (key=, value=,
user_data=) at channel-main.c:928
> #5 in g_hash_table_foreach_remove_or_steal (hash_table=0x70cea0,
func=func@entry=0x75616f10 ,
user_data=user_data@entry=0x0, notify=notify@entry=1) at ghash.c:1492
> #6 in g_hash_table_foreach_remove (hash_table=,
func=func@entry=0x75616f10 ,
user_data=user_data@entry=0x0) at ghash.c:1538
> #7 in file_xfer_flushed (success=0, channel=0x7cc1d0) at
channel-main.c:936
> #8 spice_main_channel_reset_agent (channel=0x7cc1d0) at
channel-main.c:466
> #9 set_agent_connected (channel=0x7cc1d0, connected=connected@entry=0)
at channel-main.c:1572
> #10 in spice_main_channel_reset (channel=0x7cc1d0, migrating=0) at
channel-main.c:485
> #11 in spice_channel_coroutine (data=0x7cc1d0) at spice-channel.c:2564
> #12 in coroutine_trampoline (cc=0x7cb860) at coroutine_ucontext.c:63
> #13 in continuation_trampoline (i0=, i1=)
at continuation.c:55
> #14 in ?? () from /lib64/libc.so.6
> #15 in ?? ()
> #16 in ?? ()
> Backtrace stopped: Cannot access memory at address
> ---
> src/channel-main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 2905d7b..692cfe7 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -2964,7 +2964,7 @@ static void
spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> if (self->priv->error) {
> VDAgentFileXferStatusMessage msg = {
> .id = self->priv->id,
> -.result = error->code == G_IO_ERROR_CANCELLED ?
> +.result = self->priv->error->code == G_IO_ERROR_CANCELLED ?
> VD_AGENT_FILE_XFER_STATUS_CANCELLED :
VD_AGENT_FILE_XFER_STATUS_ERROR,
> };
> agent_msg_queue_many(self->priv->channel,
VD_AGENT_FILE_XFER_STATUS,
> --
> 2.5.5
>
Victor, as we talked on IRC your patch solves the problem, fust to comments:
First one, shouldn't we emmit self->priv->error instead of error in the end
of this function?
Second one, from a quick look in the code, I have the impression that we
can get rid of the private Error variable and just use the error that we
are propagating between the callbacks. A deeper read in the code would be
needed though in order to affirm that.
Best Regards,
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v1 2/2] file-xfer: fix file path leak
On Thu, May 12, 2016 at 10:57 PM, Victor Toso wrote:
> On Thu, May 12, 2016 at 10:52:09PM +0200, Fabiano Fidêncio wrote:
>> On Thu, May 12, 2016 at 10:42 PM, Victor Toso wrote:
>> > ---
>> > src/channel-main.c | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/channel-main.c b/src/channel-main.c
>> > index fb0630e..2e2fe86 100644
>> > --- a/src/channel-main.c
>> > +++ b/src/channel-main.c
>> > @@ -2956,8 +2956,10 @@ static void
>> > spice_file_transfer_task_completed(SpiceFileTransferTask *self,
>> > if (self->priv->error)
>> > g_clear_error(&error);
>> > if (error) {
>> > +gchar *path = g_file_get_path(self->priv->file);
>> > SPICE_DEBUG("File %s xfer failed: %s",
>> > -g_file_get_path(self->priv->file), error->message);
>> > +path, error->message);
>> > +g_free(path);
>> > self->priv->error = error;
>> > }
>> >
>> > --
>> > 2.5.5
>> >
>> > ___
>> > Spice-devel mailing list
>> > Spice-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>> Acked-by: Fabiano Fidêncio
>>
>> Just out of curiosity, did you get it just by reading the code or on
>> valgrind usage?
>
> valgrind
Okay, feel free to mention it (or not) in the commit log before
pushing the patch.
>
>> --
>> Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v1 2/2] file-xfer: fix file path leak
On Thu, May 12, 2016 at 10:42 PM, Victor Toso wrote:
> ---
> src/channel-main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index fb0630e..2e2fe86 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -2956,8 +2956,10 @@ static void
> spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> if (self->priv->error)
> g_clear_error(&error);
> if (error) {
> +gchar *path = g_file_get_path(self->priv->file);
> SPICE_DEBUG("File %s xfer failed: %s",
> -g_file_get_path(self->priv->file), error->message);
> +path, error->message);
> +g_free(path);
> self->priv->error = error;
> }
>
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Acked-by: Fabiano Fidêncio
Just out of curiosity, did you get it just by reading the code or on
valgrind usage?
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v1 1/2] file-xfer: fix segfault on agent disconnection
On Thu, May 12, 2016 at 10:42 PM, Victor Toso wrote:
> As one can see by the backtrace, the reason for the segfault is that
> g_task_return_now() is called under coroutine context and in
> spice_file_transfer_task_completed() we access memory that the
> coroutine context has no access.
>
> With GTask integration its callback must respect the coroutine context
> or return-in-idle so the callback can be called in the main context.
>
> In this specific case the memory access can be fixed also due the
> error that is present which is agent being disconnected during a
> file-xfer. In that situation, we should not try to send a message to
> the agent.
>
> Program received signal SIGSEGV, Segmentation fault.
> spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
> error=0x0) at channel-main.c:2963
> 2963VDAgentFileXferStatusMessage msg = {
> (gdb) bt
> #0 spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
> error=0x0) at channel-main.c:2963
> #1 in file_xfer_data_flushed_cb (source_object=0x7cc1d0, res=0x953390,
> user_data=user_data@entry=0x7fffd0006f00) at channel-main.c:1857
> #2 in g_task_return_now (task=0x953390) at gtask.c:1108
> #3 in g_task_return (task=0x953390, type=) at gtask.c:1166
> #4 in flush_foreach_remove (key=, value=,
> user_data=) at channel-main.c:928
> #5 in g_hash_table_foreach_remove_or_steal (hash_table=0x70cea0,
> func=func@entry=0x75616f10 ,
> user_data=user_data@entry=0x0, notify=notify@entry=1) at ghash.c:1492
> #6 in g_hash_table_foreach_remove (hash_table=,
> func=func@entry=0x75616f10 ,
> user_data=user_data@entry=0x0) at ghash.c:1538
> #7 in file_xfer_flushed (success=0, channel=0x7cc1d0) at channel-main.c:936
> #8 _reset_agent (channel=0x7cc1d0) at channel-main.c:466
> #9 d (channel=0x7cc1d0, connected=connected@entry=0) at channel-main.c:1572
> #10 in spice_main_channel_reset (channel=0x7cc1d0, migrating=0) at
> channel-main.c:485
> #11 in spice_channel_coroutine (data=0x7cc1d0) at spice-channel.c:2564
> #12 in coroutine_trampoline (cc=0x7cb860) at coroutine_ucontext.c:63
> #13 in continuation_trampoline (i0=, i1=) at
> continuation.c:55
> #14 in ?? () from /lib64/libc.so.6
> #15 in ?? ()
> #16 in ?? ()
> Backtrace stopped: Cannot access memory at address
> ---
> src/channel-main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index f7789dd..fb0630e 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -2950,6 +2950,8 @@ void spice_main_set_display_enabled(SpiceMainChannel
> *channel, int id, gboolean
> static void spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> GError *error)
> {
> +SpiceMainChannelPrivate *main_priv = self->priv->channel->priv;
Ouch, quite ugly :-\.
IMO would be better to introduce a
spice_main_channel_get_agent_connected() function or something like
that.
Do you think it would make sense?.
> +
> /* In case of multiple errors we only report the first error */
> if (self->priv->error)
> g_clear_error(&error);
> @@ -2959,7 +2961,7 @@ static void
> spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> self->priv->error = error;
> }
>
> -if (self->priv->error) {
> +if (self->priv->error && main_priv->agent_connected) {
> VDAgentFileXferStatusMessage msg = {
> .id = self->priv->id,
> .result = error->code == G_IO_ERROR_CANCELLED ?
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Reviewed-by: Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-windows v1] vdagent-win: check for locked session
On Fri, May 6, 2016 at 5:35 PM, Victor Toso wrote:
> Hi,
>
> On Fri, May 06, 2016 at 05:21:40PM +0200, Pavel Grunt wrote:
>> Hey, it looks good. But according to docs, it will not work on WinXP
>>
>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa383828%28v=vs.85%29.a
>> spx
>>
Okay. OTOH you can find some documents mentioning the very same API
when dealing specifically with Windows XP. Here is one example:
https://support.microsoft.com/en-us/kb/310153 .. but you can find a
few more if you google for it.
>> Pavel
>
> Interesting, I did not see that. Thanks!
>
> I would still keep the patch as it is fairly simple and then make a
> patch that could fix WinXP later on.
Totally agreed. Actually, talking about _upstream_ _only_, makes no
sense to try to find a solution for a dead OS.
What I'd like to suggest is:
- Test with Windows XP
- Let us know about the result of your tests
- In case there is an issue with Windows XP, just mention it in the commit log.
> The fix on WinXP could be done in
> the vdservice, by parsing WTS_SESSION_LOCK/UNLOCK from
> SERVICE_CONTROL_SESSIONCHANGE and then trying to avoid the file-xfer to
> start in the vdagent.
> What I'm missing is the communication between vdservice and vdagent
> which is nothing like we have in the linux vdagent.
>
> Thanks for taking a look at this,
> toso
>
>>
>> On Fri, 2016-05-06 at 13:50 +0200, Victor Toso wrote:
>> > From WTSRegisterSessionNotification() we are aware of session changes
>> > (WM_WTSSESSION_CHANGE) such as Lock/Unlock events.
>> >
>> > We can use that to disable features such as drag-and-drop.
>> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323628
>> > ---
>> > vdagent/vdagent.cpp | 20 +++-
>> > 1 file changed, 19 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
>> > index c976665..a01bc70 100644
>> > --- a/vdagent/vdagent.cpp
>> > +++ b/vdagent/vdagent.cpp
>> > @@ -146,6 +146,7 @@ private:
>> > uint32_t _in_msg_pos;
>> > bool _pending_input;
>> > bool _running;
>> > +bool _session_is_locked;
>> > bool _desktop_switch;
>> > DesktopLayout* _desktop_layout;
>> > bool _updating_display_config;
>> > @@ -205,6 +206,7 @@ VDAgent::VDAgent()
>> > , _in_msg_pos (0)
>> > , _pending_input (false)
>> > , _running (false)
>> > +, _session_is_locked (false)
>> > , _desktop_switch (false)
>> > , _desktop_layout (NULL)
>> > , _display_setting (VD_AGENT_REGISTRY_KEY)
>> > @@ -1282,7 +1284,19 @@ void VDAgent::dispatch_message(VDAgentMessage* msg,
>> > uint32_t port)
>> > case VD_AGENT_ANNOUNCE_CAPABILITIES:
>> > res =
>> > handle_announce_capabilities((VDAgentAnnounceCapabilities*)msg-
>> > >data, msg->size);
>> > break;
>> > -case VD_AGENT_FILE_XFER_START:
>> > +case VD_AGENT_FILE_XFER_START: {
>> > +VDAgentFileXferStatusMessage status;
>> > +if (_session_is_locked) {
>> > +VDAgentFileXferStartMessage *s = (VDAgentFileXferStartMessage
>> > *)msg->data;
>> > +status.id = s->id;
>> > +status.result = VD_AGENT_FILE_XFER_STATUS_ERROR;
>> > +vd_printf("Fail to start file-xfer %u due: Locked session",
>> > status.id);
>> > +write_message(VD_AGENT_FILE_XFER_STATUS, sizeof(status),
>> > &status);
>> > +} else if (_file_xfer.dispatch(msg, &status)) {
>> > +write_message(VD_AGENT_FILE_XFER_STATUS, sizeof(status),
>> > &status);
>> > +}
>> > +break;
>> > +}
>> > case VD_AGENT_FILE_XFER_STATUS:
>> > case VD_AGENT_FILE_XFER_DATA: {
>> > VDAgentFileXferStatusMessage status;
>> > @@ -1489,6 +1503,10 @@ LRESULT CALLBACK VDAgent::wnd_proc(HWND hwnd, UINT
>> > message, WPARAM wparam, LPARA
>> > case WM_WTSSESSION_CHANGE:
>> > if (wparam == WTS_SESSION_LOGON) {
>> > a->set_control_event(CONTROL_LOGON);
>> > +} else if (wparam == WTS_SESSION_LOCK) {
>> > +a->_session_is_locked = true;
>> > +} else if (wparam == WTS_SESSION_UNLOCK) {
>> > +a->_session_is_locked = false;
>> > }
>> > break;
>> > default:
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Best Regards,
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] spice-options: Stop skipping method
On Wed, May 4, 2016 at 6:41 PM, Visarion-Mingopol Alexandru-Viorel wrote: > Sure :) Pushed! > > On Wed, May 4, 2016 at 7:33 PM, Fabiano Fidêncio > wrote: >> >> Visarion, >> >> On Wed, May 4, 2016 at 5:19 PM, Alexandru Visarion >> wrote: >> > From: Visarion-Mingopol Alexandru >> > >> > There is no reason to not include the spice_get_option_group >> > method in the generated bindings, as it can be useful. >> > >> > It was removed in the commit 2db9b8dd037e22d2b04e8e2aeecfd685524b7fef, >> > for the reason of causing warnings, but no warnings show up now. >> > --- >> > src/spice-option.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/src/spice-option.c b/src/spice-option.c >> > index 46ae530..7a17150 100644 >> > --- a/src/spice-option.c >> > +++ b/src/spice-option.c >> > @@ -177,7 +177,7 @@ static gboolean parse_preferred_compression(const >> > gchar *option_name, const gcha >> > } >> > >> > /** >> > - * spice_get_option_group: (skip) >> > + * spice_get_option_group: >> > * >> > * Gets commandline options. >> > * >> > -- >> > 2.5.5 >> > >> > ___ >> > Spice-devel mailing list >> > Spice-devel@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/spice-devel >> >> If you agree, I would like to change the short log to: >> "doc,gir: Don't skip spice_get_option_group()" >> >> Let me know if you agree that I can do the change before pushing (and >> you don't need to submit a v3). >> >> Acked-by: Fabiano Fidêncio >> >> Best Regards, >> -- >> Fabiano Fidêncio > > > > > -- > Visarion-Mingopol Alexandru-Viorel > Telefon : 0729614060 > Best Bucuresti -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] spice-options: Stop skipping method
Visarion, On Wed, May 4, 2016 at 5:19 PM, Alexandru Visarion wrote: > From: Visarion-Mingopol Alexandru > > There is no reason to not include the spice_get_option_group > method in the generated bindings, as it can be useful. > > It was removed in the commit 2db9b8dd037e22d2b04e8e2aeecfd685524b7fef, > for the reason of causing warnings, but no warnings show up now. > --- > src/spice-option.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/spice-option.c b/src/spice-option.c > index 46ae530..7a17150 100644 > --- a/src/spice-option.c > +++ b/src/spice-option.c > @@ -177,7 +177,7 @@ static gboolean parse_preferred_compression(const gchar > *option_name, const gcha > } > > /** > - * spice_get_option_group: (skip) > + * spice_get_option_group: > * > * Gets commandline options. > * > -- > 2.5.5 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel If you agree, I would like to change the short log to: "doc,gir: Don't skip spice_get_option_group()" Let me know if you agree that I can do the change before pushing (and you don't need to submit a v3). Acked-by: Fabiano Fidêncio Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] spice-options: Stop skipping method
On Wed, May 4, 2016 at 8:45 AM, Alexandru Visarion wrote: > From: Visarion-Mingopol Alexandru > > There is no reason to not include the spice_get_option_group > method in the generated bindings, as it can be useful. Seems the skip added on commit 2db9b8dd037e22d2b04e8e2aeecfd685524b7fef when fixing a lot of gtkdoc/giscan warnings. Adding it back doesn't bring any new warning (as far as I can see). Would be good to have it mentioned in the commit message. > --- > src/spice-option.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/spice-option.c b/src/spice-option.c > index 46ae530..7a17150 100644 > --- a/src/spice-option.c > +++ b/src/spice-option.c > @@ -177,7 +177,7 @@ static gboolean parse_preferred_compression(const gchar > *option_name, const gcha > } > > /** > - * spice_get_option_group: (skip) > + * spice_get_option_group: > * > * Gets commandline options. > * > -- > 2.5.5 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel Reviewed-by: Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2 v4] add a program to test redirection on Windows
TE);
>> > +CloseHandle(pi.hProcess);
>> > +
>> > +log_f = fopen("log.txt", "a");
>> > +assert(log_f);
>> > +setbuf(log_f, NULL);
>> > +fprintf(log_f, "< sub command\n");
>> > +
>> > +SetStdHandle(STD_OUTPUT_HANDLE, old_out);
>> > +SetStdHandle(STD_ERROR_HANDLE, old_err);
>> > +}
>> > +
>> > +// simple dirty function to read first line in a file
>> > +static char *
>> > +read_file(const char *fn)
>> > +{
>> > +FILE *f = fopen(fn, "r");
>> > +assert(f);
>> > +
>> > +// dirty but fast
>> > +static char buf[1024];
>> > +
>> > +memset(buf, 0, sizeof(buf));
>> > +if (!fgets(buf, sizeof(buf), f))
>> > +memset(buf, 0, sizeof(buf));
>> > +
>> > +fclose(f);
>> > +return buf;
>> > +}
>> > +
>> > +static char *
>> > +read_console(void)
>> > +{
>> > +CONSOLE_SCREEN_BUFFER_INFO csbiInfo;
>> > +assert(GetConsoleScreenBufferInfo(h_console, &csbiInfo));
>> > +
>> > +fprintf(log_f, "size %d %d\n", csbiInfo.dwSize.X, csbiInfo.dwSize.Y);
>> > +fprintf(log_f, "win %d %d %d %d\n", csbiInfo.srWindow.Left,
>> > csbiInfo.srWindow.Top, csbiInfo.srWindow.Right, csbiInfo.srWindow.Bottom);
>> > +
>> > +COORD size;
>> > +size.Y = csbiInfo.srWindow.Bottom + 1;
>> > +size.X = csbiInfo.srWindow.Right + 1;
>> > +
>> > +COORD coord = { 0, 0 };
>> > +
>> > +CHAR_INFO *buf = calloc(size.Y * size.X, sizeof(CHAR_INFO));
>> > +assert(buf);
>> > +
>> > +SMALL_RECT rect;
>> > +rect.Top = 0;
>> > +rect.Left = 0;
>> > +rect.Bottom = size.Y - 1;
>> > +rect.Right = size.X - 1;
>> > +
>> > +// read console content
>> > +assert(ReadConsoleOutput(h_console, buf, size, coord, &rect));
>> > +
>> > +// convert to just text
>> > +unsigned n;
>> > +char *char_buf = (char *) buf;
>> > +for (n = 0; n < size.X * size.Y; ++n)
>> > +char_buf[n] = buf[n].Char.AsciiChar;
>> > +char_buf[n] = 0;
>> > +
>> > +// remove additional spaces
>> > +char *p;
>> > +while ((p=strstr(char_buf, " ")) != NULL)
>> > +memmove(p, p+1, strlen(p));
>> > +
>> > +return char_buf;
>> > +}
>> > +
>> > +static void
>> > +check(BOOL redir_out, BOOL redir_err)
>> > +{
>> > +++num_test;
>> > +fprintf(log_f, "check method %d console %d redir_out %d redir_err
>> > %d\n",
>> > +(int) redir_method, has_console, redir_out, redir_err);
>> > +
>> > +char stdout_line[64], stderr_line[64];
>> > +
>> > +sprintf(stdout_line, "stdout %d line", num_test);
>> > +sprintf(stderr_line, "stderr %d line", num_test);
>> > +
>> > +DeleteFile("stdout.txt");
>> > +DeleteFile("stderr.txt");
>> > +
>> > +HANDLE out = CreateFile("stdout.txt", GENERIC_WRITE, 0, NULL,
>> > CREATE_ALWAYS, 0, NULL);
>> > +assert(out != INVALID_HANDLE_VALUE);
>> > +HANDLE err = CreateFile("stderr.txt", GENERIC_WRITE, 0, NULL,
>> > CREATE_ALWAYS, 0, NULL);
>> > +assert(err != INVALID_HANDLE_VALUE);
>> > +
>> > +exec(redir_out ? out : INVALID_HANDLE_VALUE,
>> > + redir_err ? err : INVALID_HANDLE_VALUE);
>> > +
>> > +CloseHandle(out);
>> > +CloseHandle(err);
>> > +
>> > +// check file output
>> > +if (redir_out) {
>> > +char *data = read_file("stdout.txt");
>> > +assert(strstr(data, stdout_line) != NULL);
>> > +}
>> > +if (redir_err) {
>> > +char *data = read_file("stderr.txt");
>> > +assert(strstr(data, stderr_line) != NULL);
>> > +}
>> > +
>> > +DeleteFile("stdout.txt");
>> > +DeleteFile("stderr.txt");
>> > +
>> > +// check console output
>> > +if (!has_console)
>> > +return;
>> > +
>> > +char *data = read_console();
>> > +fprintf(log_f, "\nconsole: %s\nstdout expected: %s\nstderr expected:
>> > %s\n", data, stdout_line, stderr_line);
>> > +if (!redir_out)
>> > +assert(strstr(data, stdout_line) != NULL);
>> > +
>> > +if (!redir_err)
>> > +assert(strstr(data, stderr_line) != NULL);
>> > +free(data);
>> > +}
>> > +
>> > +static void
>> > +all_checks(void)
>> > +{
>> > +check(TRUE, FALSE);
>> > +check(FALSE, TRUE);
>> > +check(TRUE, TRUE);
>> > +
>> > +assert(AllocConsole());
>> > +has_console = TRUE;
>> > +h_console = GetStdHandle(STD_OUTPUT_HANDLE);
>> > +fprintf(log_f, "got console handles %p %p\n", h_console,
>> > GetStdHandle(STD_ERROR_HANDLE));
>> > +
>> > +check(FALSE, FALSE);
>> > +check(TRUE, FALSE);
>> > +check(FALSE, TRUE);
>> > +check(TRUE, TRUE);
>> > +
>> > +assert(FreeConsole());
>> > +has_console = FALSE;
>> > +h_console = INVALID_HANDLE_VALUE;
>> > +}
>> > +
>> > +int WINAPI WinMain(HINSTANCE hInstance G_GNUC_UNUSED, HINSTANCE
>> > hPrevInstance G_GNUC_UNUSED,
>> > + LPSTR lpCmdLine, int nShowCmd G_GNUC_UNUSED)
>> > +{
>> > +static const char cmd_seps[] = " \t\r\n\v";
>> > +
>> > +char *argv[10], *p;
>> > +int argc = 0;
>> > +
>> > +// parse arguments
>> > +for (p = strtok(lpCmdLine, cmd_seps); p && argc < 10; p = strtok(NULL,
>> > cmd_seps))
>> > +argv[argc++] = p;
>> > +argv[argc] = NULL;
>> > +
>> > +log_f = fopen("log.txt", argc >= 1 ? "a" : "w");
>> > +assert(log_f);
>> > +setbuf(log_f, NULL);
>> > +
>> > +fprintf(log_f, "argc %d argv[0] %s \n", argc, argv[0]);
>> > +
>> > +if (argc >= 1) {
>> > +return called_test(argc, argv);
>> > +}
>> > +
>> > +// main program, call ourself with different arguments and settings
>> > +redir_method = METHOD_CREATEPROCESS;
>> > +all_checks();
>> > +
>> > +redir_method = METHOD_STDHANDLE;
>> > +all_checks();
>> > +
>> > +fprintf(log_f, "everything ok\n");
>> > +
>> > +fclose(log_f);
>> > +return 0;
>> > +}
>> > --
>> > 2.5.5
>> >
>> > ___
>> > Spice-devel mailing list
>> > Spice-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>>
>>
>> --
>> Fabiano Fidêncio
>>
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/2 v3] Attempt to manage redirection in a way similar to Unix
On Mon, May 2, 2016 at 11:43 PM, Fabiano Fidêncio wrote:
> On Fri, Apr 29, 2016 at 4:09 PM, Frediano Ziglio wrote:
>> This patch allows remote-viewer to redirect output/error streams to
>> files.
>> Also if launched from a console program (for instance from the command
>> prompt) you are able to see output from the console where you launch
>> the program.
>> This allow to launch the program with a syntax like
>> > remote-viewer.exe --debug > log.txt 2>&1
>> or simply
>> > remote-viewer.exe --debug
>>
>> Signed-off-by: Frediano Ziglio
>> ---
>> src/virt-viewer-util.c | 36 +---
>> 1 file changed, 29 insertions(+), 7 deletions(-)
>>
>> Changes from previous version:
>> - fixed different redirection combinations;
>> - tested on Windows XP, 7 and 10.
>>
>> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
>> index 8cf52ec..ebe039c 100644
>> --- a/src/virt-viewer-util.c
>> +++ b/src/virt-viewer-util.c
>> @@ -253,6 +253,17 @@ static void log_handler(const gchar *log_domain,
>> g_log_default_handler(log_domain, log_level, message, unused_data);
>> }
>>
>> +#ifdef G_OS_WIN32
>> +static BOOL is_handle_valid(HANDLE h)
>> +{
>> +if (h == INVALID_HANDLE_VALUE || h == NULL)
>> +return FALSE;
>> +
>> +DWORD flags;
>> +return GetHandleInformation(h, &flags);
>> +}
>> +#endif
>> +
>> void virt_viewer_util_init(const char *appname)
>> {
>> #ifdef G_OS_WIN32
>> @@ -265,13 +276,24 @@ void virt_viewer_util_init(const char *appname)
>> */
>> CreateMutexA(0, 0, "VirtViewerMutex");
>>
>> -if (AttachConsole(ATTACH_PARENT_PROCESS) != 0) {
>> -freopen("CONIN$", "r", stdin);
>> -freopen("CONOUT$", "w", stdout);
>> -freopen("CONOUT$", "w", stderr);
>> -dup2(fileno(stdin), STDIN_FILENO);
>> -dup2(fileno(stdout), STDOUT_FILENO);
>> -dup2(fileno(stderr), STDERR_FILENO);
>> +/* Get redirection from parent */
>> +BOOL out_valid = is_handle_valid(GetStdHandle(STD_OUTPUT_HANDLE));
>> +BOOL err_valid = is_handle_valid(GetStdHandle(STD_ERROR_HANDLE));
>> +
>> +/*
>> + * If not all output are redirected try to redirect to parent console.
>> + * If parent has no console (for instance as launched from GUI) just
>> + * rely on default (no output).
>> + */
>> +if ((!out_valid || !err_valid) && AttachConsole(ATTACH_PARENT_PROCESS))
>> {
>> +if (!out_valid) {
>> +freopen("CONOUT$", "w", stdout);
>> + dup2(fileno(stdout), STDOUT_FILENO);
>> +}
>> +if (!err_valid) {
>> + freopen("CONOUT$", "w", stderr);
>> +dup2(fileno(stderr), STDERR_FILENO);
>> +}
>> }
>>
>> /* Disable input method handling so that the Zenkaku_Hankaku can be
>> passed
>> --
>> 2.5.5
>>
>> ___
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
> Acked-by: Fabiano Fidêncio
Slightly modified the commit short-log and pushed.
Thanks for the patch!
> --
> Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2 v4] add a program to test redirection on Windows
ck method %d console %d redir_out %d redir_err %d\n",
> +(int) redir_method, has_console, redir_out, redir_err);
> +
> +char stdout_line[64], stderr_line[64];
> +
> +sprintf(stdout_line, "stdout %d line", num_test);
> +sprintf(stderr_line, "stderr %d line", num_test);
> +
> +DeleteFile("stdout.txt");
> +DeleteFile("stderr.txt");
> +
> +HANDLE out = CreateFile("stdout.txt", GENERIC_WRITE, 0, NULL,
> CREATE_ALWAYS, 0, NULL);
> +assert(out != INVALID_HANDLE_VALUE);
> +HANDLE err = CreateFile("stderr.txt", GENERIC_WRITE, 0, NULL,
> CREATE_ALWAYS, 0, NULL);
> +assert(err != INVALID_HANDLE_VALUE);
> +
> +exec(redir_out ? out : INVALID_HANDLE_VALUE,
> + redir_err ? err : INVALID_HANDLE_VALUE);
> +
> +CloseHandle(out);
> +CloseHandle(err);
> +
> +// check file output
> +if (redir_out) {
> +char *data = read_file("stdout.txt");
> +assert(strstr(data, stdout_line) != NULL);
> +}
> +if (redir_err) {
> +char *data = read_file("stderr.txt");
> +assert(strstr(data, stderr_line) != NULL);
> +}
> +
> +DeleteFile("stdout.txt");
> +DeleteFile("stderr.txt");
> +
> +// check console output
> +if (!has_console)
> +return;
> +
> +char *data = read_console();
> +fprintf(log_f, "\nconsole: %s\nstdout expected: %s\nstderr expected:
> %s\n", data, stdout_line, stderr_line);
> +if (!redir_out)
> +assert(strstr(data, stdout_line) != NULL);
> +
> +if (!redir_err)
> +assert(strstr(data, stderr_line) != NULL);
> +free(data);
> +}
> +
> +static void
> +all_checks(void)
> +{
> +check(TRUE, FALSE);
> +check(FALSE, TRUE);
> +check(TRUE, TRUE);
> +
> +assert(AllocConsole());
> +has_console = TRUE;
> +h_console = GetStdHandle(STD_OUTPUT_HANDLE);
> +fprintf(log_f, "got console handles %p %p\n", h_console,
> GetStdHandle(STD_ERROR_HANDLE));
> +
> +check(FALSE, FALSE);
> +check(TRUE, FALSE);
> +check(FALSE, TRUE);
> +check(TRUE, TRUE);
> +
> +assert(FreeConsole());
> +has_console = FALSE;
> +h_console = INVALID_HANDLE_VALUE;
> +}
> +
> +int WINAPI WinMain(HINSTANCE hInstance G_GNUC_UNUSED, HINSTANCE
> hPrevInstance G_GNUC_UNUSED,
> + LPSTR lpCmdLine, int nShowCmd G_GNUC_UNUSED)
> +{
> +static const char cmd_seps[] = " \t\r\n\v";
> +
> +char *argv[10], *p;
> +int argc = 0;
> +
> +// parse arguments
> +for (p = strtok(lpCmdLine, cmd_seps); p && argc < 10; p = strtok(NULL,
> cmd_seps))
> +argv[argc++] = p;
> +argv[argc] = NULL;
> +
> +log_f = fopen("log.txt", argc >= 1 ? "a" : "w");
> +assert(log_f);
> +setbuf(log_f, NULL);
> +
> +fprintf(log_f, "argc %d argv[0] %s \n", argc, argv[0]);
> +
> +if (argc >= 1) {
> +return called_test(argc, argv);
> +}
> +
> +// main program, call ourself with different arguments and settings
> +redir_method = METHOD_CREATEPROCESS;
> +all_checks();
> +
> +redir_method = METHOD_STDHANDLE;
> +all_checks();
> +
> +fprintf(log_f, "everything ok\n");
> +
> +fclose(log_f);
> +return 0;
> +}
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] spice_get_option_group is skipped and has messy output
Visarion, On Tue, May 3, 2016 at 12:05 AM, Visarion-Mingopol Alexandru-Viorel wrote: > > Firstly, the spice_get_option_group is skipped when creating the Vala > binding and I need it for inserting the spice-gtk help menu in gnome-boxes. > > Secondly, the spice group options have a weird appearance on the standard > terminal wifth width 80. > Because their entries are so long[1], all the descriptions are automatically > put too much to the right[2], instead of the normal, more pleasant look[3] > > I am willing to fix it myself, if a bug will be opened. Please, feel free to provide a series fixing those issues and we will review and, hopefully, push them if everything is okay. If you need a bug opened, please, feel free to open in our bugzilla: https://bugs.freedesktop.org/ ... (but that's not mandatory). > > -- > Visarion-Mingopol Alexandru-Viorel > Telefon : 0729614060 > Best Bucuresti > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel > Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/2 v3] Attempt to manage redirection in a way similar to Unix
On Fri, Apr 29, 2016 at 4:09 PM, Frediano Ziglio wrote:
> This patch allows remote-viewer to redirect output/error streams to
> files.
> Also if launched from a console program (for instance from the command
> prompt) you are able to see output from the console where you launch
> the program.
> This allow to launch the program with a syntax like
> > remote-viewer.exe --debug > log.txt 2>&1
> or simply
> > remote-viewer.exe --debug
>
> Signed-off-by: Frediano Ziglio
> ---
> src/virt-viewer-util.c | 36 +---
> 1 file changed, 29 insertions(+), 7 deletions(-)
>
> Changes from previous version:
> - fixed different redirection combinations;
> - tested on Windows XP, 7 and 10.
>
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index 8cf52ec..ebe039c 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -253,6 +253,17 @@ static void log_handler(const gchar *log_domain,
> g_log_default_handler(log_domain, log_level, message, unused_data);
> }
>
> +#ifdef G_OS_WIN32
> +static BOOL is_handle_valid(HANDLE h)
> +{
> +if (h == INVALID_HANDLE_VALUE || h == NULL)
> +return FALSE;
> +
> +DWORD flags;
> +return GetHandleInformation(h, &flags);
> +}
> +#endif
> +
> void virt_viewer_util_init(const char *appname)
> {
> #ifdef G_OS_WIN32
> @@ -265,13 +276,24 @@ void virt_viewer_util_init(const char *appname)
> */
> CreateMutexA(0, 0, "VirtViewerMutex");
>
> -if (AttachConsole(ATTACH_PARENT_PROCESS) != 0) {
> -freopen("CONIN$", "r", stdin);
> -freopen("CONOUT$", "w", stdout);
> -freopen("CONOUT$", "w", stderr);
> -dup2(fileno(stdin), STDIN_FILENO);
> -dup2(fileno(stdout), STDOUT_FILENO);
> -dup2(fileno(stderr), STDERR_FILENO);
> +/* Get redirection from parent */
> +BOOL out_valid = is_handle_valid(GetStdHandle(STD_OUTPUT_HANDLE));
> +BOOL err_valid = is_handle_valid(GetStdHandle(STD_ERROR_HANDLE));
> +
> +/*
> + * If not all output are redirected try to redirect to parent console.
> + * If parent has no console (for instance as launched from GUI) just
> + * rely on default (no output).
> + */
> +if ((!out_valid || !err_valid) && AttachConsole(ATTACH_PARENT_PROCESS)) {
> +if (!out_valid) {
> +freopen("CONOUT$", "w", stdout);
> +dup2(fileno(stdout), STDOUT_FILENO);
> +}
> +if (!err_valid) {
> +freopen("CONOUT$", "w", stderr);
> +dup2(fileno(stderr), STDERR_FILENO);
> +}
> }
>
> /* Disable input method handling so that the Zenkaku_Hankaku can be
> passed
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Acked-by: Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/2 v3] add a program to test redirection on Windows
;, num_test);
> +sprintf(stderr_line, "stderr %d line", num_test);
> +
> +DeleteFile("stdout.txt");
> +DeleteFile("stderr.txt");
> +
> +HANDLE out = CreateFile("stdout.txt", GENERIC_WRITE, 0, NULL,
> CREATE_ALWAYS, 0, NULL);
> +assert(out != INVALID_HANDLE_VALUE);
> +HANDLE err = CreateFile("stderr.txt", GENERIC_WRITE, 0, NULL,
> CREATE_ALWAYS, 0, NULL);
> +assert(err != INVALID_HANDLE_VALUE);
> +
> +exec(redir_out ? out : INVALID_HANDLE_VALUE,
> + redir_err ? err : INVALID_HANDLE_VALUE);
> +
> +CloseHandle(out);
> +CloseHandle(err);
> +
> +// check file output
> +if (redir_out) {
> +char *data = read_file("stdout.txt");
> +assert(strstr(data, stdout_line) != NULL);
> +}
> +if (redir_err) {
> +char *data = read_file("stderr.txt");
> +assert(strstr(data, stderr_line) != NULL);
> +}
> +
> +DeleteFile("stdout.txt");
> +DeleteFile("stderr.txt");
> +
> +// check console output
> +if (!has_console)
> +return;
> +
> +char *data = read_console();
> +fprintf(log_f, "\nconsole: %s\nstdout expected: %s\nstderr expected:
> %s\n", data, stdout_line, stderr_line);
> +if (!redir_out)
> +assert(strstr(data, stdout_line) != NULL);
> +
> +if (!redir_err)
> +assert(strstr(data, stderr_line) != NULL);
> +free(data);
> +}
> +
> +static void
> +all_checks(void)
> +{
> +check(TRUE, FALSE);
> +check(FALSE, TRUE);
> +check(TRUE, TRUE);
> +
> +assert(AllocConsole());
> +has_console = TRUE;
> +h_console = GetStdHandle(STD_OUTPUT_HANDLE);
> +fprintf(log_f, "got console handles %p %p\n", h_console,
> GetStdHandle(STD_ERROR_HANDLE));
> +
> +check(FALSE, FALSE);
> +check(TRUE, FALSE);
> +check(FALSE, TRUE);
> +check(TRUE, TRUE);
> +
> +assert(FreeConsole());
> +has_console = FALSE;
> +h_console = INVALID_HANDLE_VALUE;
> +}
> +
> +int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR
> lpCmdLine, int nShowCmd)
> +{
> +char *argv[10], *p;
> +int argc = 0;
> +
> +// parse arguments
> +for (p = strtok(lpCmdLine, CMD_SEPS); p && argc < 10; p = strtok(NULL,
> CMD_SEPS))
> +argv[argc++] = p;
> +argv[argc] = NULL;
> +
> +log_f = fopen("log.txt", argc >= 1 ? "a" : "w");
> +assert(log_f);
> +setbuf(log_f, NULL);
> +
> +fprintf(log_f, "argc %d argv[0] %s \n", argc, argv[0]);
> +
> +if (argc >= 1) {
> +return called_test(argc, argv);
> +}
> +
> +// main program, call ourself with different arguments and settings
> +redir_method = METHOD_CREATEPROCESS;
> +all_checks();
> +
> +redir_method = METHOD_STDHANDLE;
> +all_checks();
> +
> +fprintf(log_f, "everything ok\n");
> +
> +fclose(log_f);
> +return 0;
> +}
> --
> 2.5.5
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Attempt to manage redirection in a way similar to Unix
On Tue, Apr 26, 2016 at 12:29 PM, Frediano Ziglio wrote:
> ping
>
The idea is good, but didn't work according to my tests with a Win7 machine :-\
> - Original Message -
>>
>> This patch allows remote-viewer to redirect output/error streams to
>> files.
>> Also if launched from a console program (for instance from the command
>> prompt) you are able to see output from the console where you launch
>> the program.
>> This allow to launch the program with a syntax like
>> > remote-viewer.exe --debug > log.txt 2>&1
>> or simply
>> > remote-viewer.exe --debug
>>
>> Signed-off-by: Frediano Ziglio
>> ---
>> src/virt-viewer-util.c | 30 +++---
>> 1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
>> index 8cf52ec..fb8604b 100644
>> --- a/src/virt-viewer-util.c
>> +++ b/src/virt-viewer-util.c
>> @@ -265,13 +265,29 @@ void virt_viewer_util_init(const char *appname)
>> */
>> CreateMutexA(0, 0, "VirtViewerMutex");
>>
>> -if (AttachConsole(ATTACH_PARENT_PROCESS) != 0) {
>> -freopen("CONIN$", "r", stdin);
>> -freopen("CONOUT$", "w", stdout);
>> -freopen("CONOUT$", "w", stderr);
>> -dup2(fileno(stdin), STDIN_FILENO);
>> -dup2(fileno(stdout), STDOUT_FILENO);
>> -dup2(fileno(stderr), STDERR_FILENO);
>> +/* Get redirection from parent */
>> +STARTUPINFO si;
>> +memset(&si, 0, sizeof(si));
>> +si.cb = sizeof(si);
>> +si.dwFlags = STARTF_USESTDHANDLES;
>> +GetStartupInfo(&si);
>> +gboolean out_valid = si.hStdOutput != INVALID_HANDLE_VALUE;
>> +gboolean err_valid = si.hStdError != INVALID_HANDLE_VALUE;
>> +
>> +/*
>> + * If not all output are redirected try to redirect to parent console.
>> + * If parent has no console (for instance as launched from GUI) just
>> + * rely on default (no output).
>> + */
>> +if ((!out_valid || !err_valid) && AttachConsole(ATTACH_PARENT_PROCESS))
>> {
>> +if (!out_valid) {
>> +freopen("CONOUT$", "w", stdout);
>> +dup2(fileno(stdout), STDOUT_FILENO);
>> +}
>> +if (!err_valid) {
>> +freopen("CONOUT$", "w", stderr);
>> +dup2(fileno(stderr), STDERR_FILENO);
>> +}
>> }
>>
>> /* Disable input method handling so that the Zenkaku_Hankaku can be
>> passed
>> --
>> 2.5.5
>>
>>
> ___
> 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] [spice-gtk 2/4] coverity: avoid string overflow
On Mon, Apr 25, 2016 at 10:59 AM, Frediano Ziglio wrote:
>>
>> On Mon, Apr 4, 2016 at 9:59 AM, Fabiano Fidêncio wrote:
>> > sockaddr_un.sun_path has 108 bytes, while pipe_name has
>> > PIPE_NAME_MAX_LEN (256 bytes)
>> > ---
>> > src/controller/test.c | 6 ++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/src/controller/test.c b/src/controller/test.c
>> > index 9a45581..649aca5 100644
>> > --- a/src/controller/test.c
>> > +++ b/src/controller/test.c
>> > @@ -233,6 +233,12 @@ int main (int argc, char *argv[])
>> > snprintf (pipe_name, PIPE_NAME_MAX_LEN, PIPE_NAME);
>
> So changing PIPE_NAME_MAX_LEN here to something like MIN(PIPE_NAME_MAX_LEN,
> sizeof(remote.sun_path))
> would work too.
>
>> > printf ("Creating a controller connection %s\n", pipe_name);
>> > struct sockaddr_un remote;
>> > +
>> > +if (strlen(pipe_name) + 1 > sizeof(remote.sun_path)) {
>> > +printf ("address is too long for unix socket_path: %s",
>> > pipe_name);
>> > +return -1;
>> > +}
>> > +
>> > if ((sock = socket (AF_UNIX, SOCK_STREAM, 0)) == -1) {
>> > printf ("Could not open socket, (%d) %s\n", errno,
>> > strerror(errno));
>> > return -1;
>> > --
>> > 2.7.3
>> >
>>
>> ping?
>
> By the way... code is only for a test and PIPE_NAME is "/tmp/test",
> IMHO the coverity report should be marked as "Intentional", "ignore"
Okay, taking your suggestion and dropping the patch!
>
> Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 1/4] coverity: fix unitialized use of mem.data
On Mon, Apr 25, 2016 at 11:04 AM, Frediano Ziglio wrote:
>>
>> ---
>> src/channel-main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/channel-main.c b/src/channel-main.c
>> index 4a1f58a..93537d2 100644
>> --- a/src/channel-main.c
>> +++ b/src/channel-main.c
>> @@ -1021,9 +1021,9 @@ static void agent_msg_queue_many(SpiceMainChannel
>> *channel, int type, const void
>> }
>> va_end(args);
>>
>> +memset(&msg, 0, sizeof(VDAgentMessage));
>> msg.protocol = VD_AGENT_PROTOCOL;
>> msg.type = type;
>> -msg.opaque = 0;
>> msg.size = size;
>>
>> paysize = MIN(VD_AGENT_MAX_DATA_SIZE, size + sizeof(VDAgentMessage));
>
> The definition is this
>
> typedef struct SPICE_ATTR_PACKED VDAgentMessage {
> uint32_t protocol;
> uint32_t type;
> uint64_t opaque;
> uint32_t size;
> uint8_t data[0];
> } VDAgentMessage;
>
> So I would say it's a false positive! data is 0 size the memset can't help.
Super, I'm dropping this patch!
>
> Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-common 8/8] Use g_getenv() instead of getenv()
On Mon, Apr 4, 2016 at 10:21 AM, Christophe Fergeau wrote:
> On Mon, Apr 04, 2016 at 10:03:39AM +0200, Fabiano Fidêncio wrote:
>> ---
>> common/log.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/log.c b/common/log.c
>> index 8d47cb6..91d3181 100644
>> --- a/common/log.c
>> +++ b/common/log.c
>> @@ -59,7 +59,7 @@ static GLogLevelFlags
>> spice_log_level_to_glib(SpiceLogLevel level)
>> static void spice_log_set_debug_level(void)
>> {
>> if (glib_debug_level == 0) {
>> -char *debug_str = getenv("SPICE_DEBUG_LEVEL");
>> +char *debug_str = (char *)g_getenv("SPICE_DEBUG_LEVEL");
>
> Looks like it could be
> const char *debug_str = g_getenv(...) ?
ACK with this change?
>
> Christophe
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-common 3/8] coverity: avoid out of bounds access
On Mon, Apr 4, 2016 at 10:29 AM, Christophe Fergeau wrote: > On Mon, Apr 04, 2016 at 10:03:34AM +0200, Fabiano Fidêncio wrote: >> We are allocating insufficient memory for the terminating null of the >> string. >> --- >> common/ssl_verify.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/common/ssl_verify.c b/common/ssl_verify.c >> index 601252e..4292ddf 100644 >> --- a/common/ssl_verify.c >> +++ b/common/ssl_verify.c >> @@ -283,8 +283,8 @@ static X509_NAME* subject_to_x509_name(const char >> *subject, int *nentries) >> spice_return_val_if_fail(subject != NULL, NULL); >> spice_return_val_if_fail(nentries != NULL, NULL); >> >> -key = (char*)alloca(strlen(subject)); >> -val = (char*)alloca(strlen(subject)); >> +key = (char*)alloca(strlen(subject) + 1); >> +val = (char*)alloca(strlen(subject) + 1); >> in_subject = X509_NAME_new(); > > Can try to write too many chars to the string in practice? We expect the > string to contain a '=', so key/state will be smaller than subject. If > there is no '=' in the string, we don't try to add a '\0' to 'key' (I > did not check the 'val' code path). Makes sense. I'll drop this patch. > > Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 2/4] coverity: avoid string overflow
On Mon, Apr 4, 2016 at 9:59 AM, Fabiano Fidêncio wrote:
> sockaddr_un.sun_path has 108 bytes, while pipe_name has
> PIPE_NAME_MAX_LEN (256 bytes)
> ---
> src/controller/test.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/controller/test.c b/src/controller/test.c
> index 9a45581..649aca5 100644
> --- a/src/controller/test.c
> +++ b/src/controller/test.c
> @@ -233,6 +233,12 @@ int main (int argc, char *argv[])
> snprintf (pipe_name, PIPE_NAME_MAX_LEN, PIPE_NAME);
> printf ("Creating a controller connection %s\n", pipe_name);
> struct sockaddr_un remote;
> +
> +if (strlen(pipe_name) + 1 > sizeof(remote.sun_path)) {
> +printf ("address is too long for unix socket_path: %s", pipe_name);
> +return -1;
> +}
> +
> if ((sock = socket (AF_UNIX, SOCK_STREAM, 0)) == -1) {
> printf ("Could not open socket, (%d) %s\n", errno, strerror(errno));
> return -1;
> --
> 2.7.3
>
ping?
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 1/4] coverity: fix unitialized use of mem.data
On Mon, Apr 4, 2016 at 11:08 AM, Christophe Fergeau wrote: > Where is the mem.data use you mention? In the memcpy? Exactly. mem.data use mentioned is in the memcpy. > > Christophe > > On Mon, Apr 04, 2016 at 09:59:50AM +0200, Fabiano Fidêncio wrote: >> --- >> src/channel-main.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/channel-main.c b/src/channel-main.c >> index 4a1f58a..93537d2 100644 >> --- a/src/channel-main.c >> +++ b/src/channel-main.c >> @@ -1021,9 +1021,9 @@ static void agent_msg_queue_many(SpiceMainChannel >> *channel, int type, const void >> } >> va_end(args); >> >> +memset(&msg, 0, sizeof(VDAgentMessage)); >> msg.protocol = VD_AGENT_PROTOCOL; >> msg.type = type; >> -msg.opaque = 0; >> msg.size = size; >> >> paysize = MIN(VD_AGENT_MAX_DATA_SIZE, size + sizeof(VDAgentMessage)); >> -- >> 2.7.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] Disable IME to allow receiving all keys
On Tue, Apr 19, 2016 at 4:11 PM, Fabiano Fidêncio wrote: > On Tue, Apr 19, 2016 at 4:00 PM, Frediano Ziglio wrote: >>> >>> On Tue, Apr 19, 2016 at 3:28 PM, Pavel Grunt wrote: >>> > Ack, I am going to push it. >>> >>> Do we really need this patch upstream? >>> It's a half-solution, at most, that doesn't work on newer Windows. >>> >> >> Are you referring to Window 11? It works on Windows 10. > > Hmm. I was basing my comment on > https://bugzilla.redhat.com/show_bug.cgi?id=1297640#c12 but I didn't > realize your comment > https://bugzilla.redhat.com/show_bug.cgi?id=1297640#c13 > > So, yeah, you have my ACK as well, but a better/more descriptive > commit message is needed. > Patch got applied already, so, nevermind. >> >> Frediano >> >>> Also, a better commit message would be more than appreciated in case >>> we really decide to have it upstream. >>> >>> > >>> > Thanks, >>> > Pavel >>> > >>> > On Mon, 2016-04-18 at 14:55 +0100, Frediano Ziglio wrote: >>> >> From: Christophe Fergeau >>> >> >>> >> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1297640 >>> >> >>> >> Acked-by: Frediano Ziglio >>> >> --- >>> >> src/virt-viewer-util.c | 7 +++ >>> >> 1 file changed, 7 insertions(+) >>> >> >>> >> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c >>> >> index 279f9a5..8cf52ec 100644 >>> >> --- a/src/virt-viewer-util.c >>> >> +++ b/src/virt-viewer-util.c >>> >> @@ -30,6 +30,7 @@ >>> >> #ifdef G_OS_WIN32 >>> >> #include >>> >> #include >>> >> +#include >>> >> #endif >>> >> >>> >> #include >>> >> @@ -272,6 +273,12 @@ void virt_viewer_util_init(const char *appname) >>> >> dup2(fileno(stdout), STDOUT_FILENO); >>> >> dup2(fileno(stderr), STDERR_FILENO); >>> >> } >>> >> + >>> >> +/* Disable input method handling so that the Zenkaku_Hankaku can >>> >> be passed >>> >> + * to VMs rather than being captured by Windows. >>> >> + * https://bugzilla.redhat.com/show_bug.cgi?id=1297640 >>> >> + */ >>> >> +ImmDisableIME(-1); >>> >> #endif >>> >> >>> >> setlocale(LC_ALL, ""); >>> > ___ >>> > Spice-devel mailing list >>> > Spice-devel@lists.freedesktop.org >>> > https://lists.freedesktop.org/mailman/listinfo/spice-devel >>> >>> >>> >>> -- >>> Fabiano Fidêncio >>> > > > > -- > Fabiano Fidêncio -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Disable IME to allow receiving all keys
On Tue, Apr 19, 2016 at 4:00 PM, Frediano Ziglio wrote: >> >> On Tue, Apr 19, 2016 at 3:28 PM, Pavel Grunt wrote: >> > Ack, I am going to push it. >> >> Do we really need this patch upstream? >> It's a half-solution, at most, that doesn't work on newer Windows. >> > > Are you referring to Window 11? It works on Windows 10. Hmm. I was basing my comment on https://bugzilla.redhat.com/show_bug.cgi?id=1297640#c12 but I didn't realize your comment https://bugzilla.redhat.com/show_bug.cgi?id=1297640#c13 So, yeah, you have my ACK as well, but a better/more descriptive commit message is needed. > > Frediano > >> Also, a better commit message would be more than appreciated in case >> we really decide to have it upstream. >> >> > >> > Thanks, >> > Pavel >> > >> > On Mon, 2016-04-18 at 14:55 +0100, Frediano Ziglio wrote: >> >> From: Christophe Fergeau >> >> >> >> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1297640 >> >> >> >> Acked-by: Frediano Ziglio >> >> --- >> >> src/virt-viewer-util.c | 7 +++ >> >> 1 file changed, 7 insertions(+) >> >> >> >> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c >> >> index 279f9a5..8cf52ec 100644 >> >> --- a/src/virt-viewer-util.c >> >> +++ b/src/virt-viewer-util.c >> >> @@ -30,6 +30,7 @@ >> >> #ifdef G_OS_WIN32 >> >> #include >> >> #include >> >> +#include >> >> #endif >> >> >> >> #include >> >> @@ -272,6 +273,12 @@ void virt_viewer_util_init(const char *appname) >> >> dup2(fileno(stdout), STDOUT_FILENO); >> >> dup2(fileno(stderr), STDERR_FILENO); >> >> } >> >> + >> >> +/* Disable input method handling so that the Zenkaku_Hankaku can >> >> be passed >> >> + * to VMs rather than being captured by Windows. >> >> + * https://bugzilla.redhat.com/show_bug.cgi?id=1297640 >> >> + */ >> >> +ImmDisableIME(-1); >> >> #endif >> >> >> >> setlocale(LC_ALL, ""); >> > ___ >> > Spice-devel mailing list >> > Spice-devel@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/spice-devel >> >> >> >> -- >> Fabiano Fidêncio >> -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Disable IME to allow receiving all keys
On Tue, Apr 19, 2016 at 3:28 PM, Pavel Grunt wrote: > Ack, I am going to push it. Do we really need this patch upstream? It's a half-solution, at most, that doesn't work on newer Windows. Also, a better commit message would be more than appreciated in case we really decide to have it upstream. > > Thanks, > Pavel > > On Mon, 2016-04-18 at 14:55 +0100, Frediano Ziglio wrote: >> From: Christophe Fergeau >> >> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1297640 >> >> Acked-by: Frediano Ziglio >> --- >> src/virt-viewer-util.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c >> index 279f9a5..8cf52ec 100644 >> --- a/src/virt-viewer-util.c >> +++ b/src/virt-viewer-util.c >> @@ -30,6 +30,7 @@ >> #ifdef G_OS_WIN32 >> #include >> #include >> +#include >> #endif >> >> #include >> @@ -272,6 +273,12 @@ void virt_viewer_util_init(const char *appname) >> dup2(fileno(stdout), STDOUT_FILENO); >> dup2(fileno(stderr), STDERR_FILENO); >> } >> + >> +/* Disable input method handling so that the Zenkaku_Hankaku can >> be passed >> + * to VMs rather than being captured by Windows. >> + * https://bugzilla.redhat.com/show_bug.cgi?id=1297640 >> + */ >> +ImmDisableIME(-1); >> #endif >> >> setlocale(LC_ALL, ""); > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] Coverity!
People, I've set up Coverity (https://scan.coverity.com/) for the spice related projects I've touched so far. Please, feel free to join and start taking care of the issues and/or do some integration with Github/Travis CI. Also, would be super nice to run a coverity check before every release. Here are the links: https://scan.coverity.com/projects/libcacard https://scan.coverity.com/projects/libgovirt https://scan.coverity.com/projects/spice-gtk https://scan.coverity.com/projects/spice-server https://scan.coverity.com/projects/usbredir https://scan.coverity.com/projects/vd_agent https://scan.coverity.com/projects/virt-viewer Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 4/4] coverity: identical code for different branches
On Mon, Apr 4, 2016 at 10:27 AM, Victor Toso wrote: > Hey, > > From coverity there are only 2 spice-gtk patches, right? There are 4, actually. I hope I ended up submitting all of them. > Both look good to me > Acked-by: Victor Toso > > On Mon, Apr 04, 2016 at 10:02:09AM +0200, Fabiano Fidêncio wrote: >> --- >> src/spice-client-glib-usb-acl-helper.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/src/spice-client-glib-usb-acl-helper.c >> b/src/spice-client-glib-usb-acl-helper.c >> index c2baeda..03a0317 100644 >> --- a/src/spice-client-glib-usb-acl-helper.c >> +++ b/src/spice-client-glib-usb-acl-helper.c >> @@ -139,8 +139,7 @@ update: >> /* update record */ >> acl_calc_mask(&acl); >> ret = acl_set_file(filename, ACL_TYPE_ACCESS, acl); >> -if (ret != 0) >> -goto out; >> + >> out: >> acl_free(acl); >> return ret; >> -- >> 2.7.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 -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 6/8] coverity: remove unused value
len is overwritten in the match label with the value from "ip - anchor". --- common/lz_compress_tmpl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/lz_compress_tmpl.c b/common/lz_compress_tmpl.c index 3162a96..e316c4b 100644 --- a/common/lz_compress_tmpl.c +++ b/common/lz_compress_tmpl.c @@ -214,9 +214,7 @@ static void FNAME(compress_seg)(Encoder *encoder, LzImageSegment *seg, PIXEL *fr ip += 3; ref = anchor + 2; ref_limit = (PIXEL *)(seg->lines_end); -#if defined(LZ_RGB16) || defined(LZ_RGB24) || defined(LZ_RGB32) -len = 3; -#endif + goto match; } } -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 1/8] coverity: avoid use after free
---
common/canvas_utils.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/common/canvas_utils.c b/common/canvas_utils.c
index c5813f4..0be761a 100644
--- a/common/canvas_utils.c
+++ b/common/canvas_utils.c
@@ -109,6 +109,7 @@ static inline pixman_image_t
*__surface_create_stride(pixman_format_code_t forma
if (surface == NULL) {
free(data);
+data = NULL;
spice_error("create surface failed, out of memory");
}
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 7/8] coverity: remove structurally dead code
The loop (for (;;)) will be executed only once, so, no reason for
keeping it.
---
common/lz_compress_tmpl.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/common/lz_compress_tmpl.c b/common/lz_compress_tmpl.c
index e316c4b..0305278 100644
--- a/common/lz_compress_tmpl.c
+++ b/common/lz_compress_tmpl.c
@@ -323,18 +323,15 @@ match:// RLE or dictionary (both are encoded by
distance from ref (-1) a
// TODO: maybe separate a run from the same seg or from different
ones in order
// to spare ref < ref_limit and that way we can also perform
8 calls of
// (ref++ != ip++) outside a loop
-for (;;) {
-while ((ip < ip_bound) && (ref < ref_limit)) {
-if (!SAME_PIXEL(*ref, *ip)) {
-ref++;
-ip++;
-break;
-} else {
-ref++;
-ip++;
-}
+while ((ip < ip_bound) && (ref < ref_limit)) {
+if (!SAME_PIXEL(*ref, *ip)) {
+ref++;
+ip++;
+break;
+} else {
+ref++;
+ip++;
}
-break;
}
}
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 2/8] coverity: avoid resource leak
--- common/canvas_base.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common/canvas_base.c b/common/canvas_base.c index fa4d373..650422b 100644 --- a/common/canvas_base.c +++ b/common/canvas_base.c @@ -769,6 +769,7 @@ static inline SpicePalette *canvas_get_localized_palette(CanvasBase *canvas, Spi case SPICE_SURFACE_FMT_16_565: default: spice_warn_if_reached(); +free(copy); return NULL; } *free_palette = TRUE; -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 4/8] coverity: avoid dereference after null check
All decompress functions used after this check take into account that
encoder->palette is not NULL. So, if we already detected that the
palette is NULL, let's just return early.
---
common/lz.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/common/lz.c b/common/lz.c
index d1c4033..2589223 100644
--- a/common/lz.c
+++ b/common/lz.c
@@ -647,6 +647,7 @@ void lz_decode(LzContext *lz, LzImageType to_type, uint8_t
*buf)
if (!encoder->palette) {
encoder->usr->error(encoder->usr,
"a palette is missing (for bpp to rgb
decoding)\n");
+return;
}
switch (encoder->type) {
case LZ_IMAGE_TYPE_PLT1_BE:
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 5/8] coverity: avoid division or modulo by zero
---
common/lines.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/lines.c b/common/lines.c
index 10ca318..e5097c4 100644
--- a/common/lines.c
+++ b/common/lines.c
@@ -412,7 +412,7 @@ miStepDash (int dist, /* distance to step */
totallen = 0;
for (i = 0; i < numInDashList; i++)
totallen += pDash[i];
-if (totallen <= dist)
+if (totallen > 0 && totallen <= dist)
dist = dist % totallen;
while (dist >= pDash[dashIndex]) {
dist -= pDash[dashIndex];
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 3/8] coverity: avoid out of bounds access
We are allocating insufficient memory for the terminating null of the
string.
---
common/ssl_verify.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/ssl_verify.c b/common/ssl_verify.c
index 601252e..4292ddf 100644
--- a/common/ssl_verify.c
+++ b/common/ssl_verify.c
@@ -283,8 +283,8 @@ static X509_NAME* subject_to_x509_name(const char *subject,
int *nentries)
spice_return_val_if_fail(subject != NULL, NULL);
spice_return_val_if_fail(nentries != NULL, NULL);
-key = (char*)alloca(strlen(subject));
-val = (char*)alloca(strlen(subject));
+key = (char*)alloca(strlen(subject) + 1);
+val = (char*)alloca(strlen(subject) + 1);
in_subject = X509_NAME_new();
if (!in_subject || !key || !val) {
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-common 8/8] Use g_getenv() instead of getenv()
---
common/log.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/log.c b/common/log.c
index 8d47cb6..91d3181 100644
--- a/common/log.c
+++ b/common/log.c
@@ -59,7 +59,7 @@ static GLogLevelFlags spice_log_level_to_glib(SpiceLogLevel
level)
static void spice_log_set_debug_level(void)
{
if (glib_debug_level == 0) {
-char *debug_str = getenv("SPICE_DEBUG_LEVEL");
+char *debug_str = (char *)g_getenv("SPICE_DEBUG_LEVEL");
if (debug_str != NULL) {
int debug_level;
char *debug_env;
@@ -93,7 +93,7 @@ static void spice_log_set_debug_level(void)
static void spice_log_set_abort_level(void)
{
if (abort_level == -1) {
-char *abort_str = getenv("SPICE_ABORT_LEVEL");
+char *abort_str = (char *)g_getenv("SPICE_ABORT_LEVEL");
if (abort_str != NULL) {
GLogLevelFlags glib_abort_level;
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 4/4] coverity: identical code for different branches
--- src/spice-client-glib-usb-acl-helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/spice-client-glib-usb-acl-helper.c b/src/spice-client-glib-usb-acl-helper.c index c2baeda..03a0317 100644 --- a/src/spice-client-glib-usb-acl-helper.c +++ b/src/spice-client-glib-usb-acl-helper.c @@ -139,8 +139,7 @@ update: /* update record */ acl_calc_mask(&acl); ret = acl_set_file(filename, ACL_TYPE_ACCESS, acl); -if (ret != 0) -goto out; + out: acl_free(acl); return ret; -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 3/4] coverity: avoid out of bounds access
--- src/controller/test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/test.c b/src/controller/test.c index 649aca5..2909b06 100644 --- a/src/controller/test.c +++ b/src/controller/test.c @@ -262,7 +262,7 @@ int main (int argc, char *argv[]) send_data (CONTROLLER_PASSWORD, (uint8_t*)PWD, strlen(PWD) + 1); send_data (CONTROLLER_SECURE_CHANNELS, (uint8_t*)SECURE_CHANNELS, strlen(SECURE_CHANNELS) + 1); send_data (CONTROLLER_DISABLE_CHANNELS, (uint8_t*)DISABLED_CHANNELS, strlen(DISABLED_CHANNELS) + 1); -send_data (CONTROLLER_TLS_CIPHERS, (uint8_t*)TLS_CIPHERS, sizeof(TLS_CIPHERS) + 1); +send_data (CONTROLLER_TLS_CIPHERS, (uint8_t*)TLS_CIPHERS, strlen(TLS_CIPHERS) + 1); send_data (CONTROLLER_CA_FILE, (uint8_t*)CA_FILE, strlen(CA_FILE) + 1); send_data (CONTROLLER_HOST_SUBJECT, (uint8_t*)HOST_SUBJECT, strlen(HOST_SUBJECT) + 1); send_data (CONTROLLER_SET_TITLE, (uint8_t*)TITLE, strlen(TITLE) + 1); -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 1/3] Send Zenkaku_Hankaku key in JP keyboard.
Ouch, sorry for resending this patch :-\
My intention was to submit another series completely not related to
this one, my bad :-\
On Mon, Apr 4, 2016 at 9:59 AM, Fabiano Fidêncio wrote:
> From: Takao Fujiwara
>
> MapVirtualKey() returns the scancode of the released Zenkaku_Hankaku key
> but does not pressed one and modifiered one. Any cases should be sent
> to use input method engines.
> ---
> src/spice-widget-priv.h | 1 +
> src/spice-widget.c | 37 +++--
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
> index 95bca8a..05f6a90 100644
> --- a/src/spice-widget-priv.h
> +++ b/src/spice-widget-priv.h
> @@ -119,6 +119,7 @@ struct _SpiceDisplayPrivate {
> HHOOK keyboard_hook;
> int win_mouse[3];
> int win_mouse_speed;
> +HKL keyboard_layout;
> #endif
> guint keypress_delay;
> gintzoom_level;
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index f605439..4e026f3 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -644,6 +644,9 @@ static void spice_display_init(SpiceDisplay *display)
> d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
> d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
> d->mouse_cursor = get_blank_cursor();
> +#ifdef G_OS_WIN32
> +d->keyboard_layout = GetKeyboardLayout(0);
> +#endif
> }
>
> static GObject *
> @@ -1418,6 +1421,7 @@ static gboolean key_event(GtkWidget *widget,
> GdkEventKey *key)
> SpiceDisplay *display = SPICE_DISPLAY(widget);
> SpiceDisplayPrivate *d = display->priv;
> int scancode;
> +int native_scancode;
>
> #ifdef G_OS_WIN32
> /* on windows, we ought to ignore the reserved key event? */
> @@ -1464,9 +1468,38 @@ static gboolean key_event(GtkWidget *widget,
> GdkEventKey *key)
> scancode = vnc_display_keymap_gdk2xtkbd(d->keycode_map,
> d->keycode_maplen,
> key->hardware_keycode);
> #ifdef G_OS_WIN32
> +native_scancode = MapVirtualKey(key->hardware_keycode, MAPVK_VK_TO_VSC);
> +
> +/* Some OEM virtual-key codes are missed in MapVirtualKey(). */
> +switch (HIWORD(d->keyboard_layout)) {
> +case 0x411: /* JP keyboard */
> +if (native_scancode == 0) {
> +switch (key->hardware_keycode) {
> +case VK_OEM_ENLW: /* from Pressed Zenkaku_Hankaku */
> +case VK_KANJI: /* from Alt + Zenkaku_Hankaku */
> +scancode = MapVirtualKey(VK_OEM_AUTO, MAPVK_VK_TO_VSC);
> +/* to Released Zenkaku_Hankaku */
> +goto got_scancode;
> +case VK_CAPITAL:/* from Alt + Eisu_toggle */
> +scancode = MapVirtualKey(VK_OEM_ATTN, MAPVK_VK_TO_VSC);
> +/* to Eisu_toggle */
> +goto got_scancode;
> +case VK_OEM_BACKTAB:/* from Alt + Hiragana_Katakana */
> +scancode = MapVirtualKey(VK_OEM_COPY, MAPVK_VK_TO_VSC);
> +/* to Hiragana_Katakana */
> +goto got_scancode;
> +default:;
> +}
> +}
> +break;
> +default:;
> +}
> +
> /* MapVirtualKey doesn't return scancode with needed higher byte */
> -scancode = MapVirtualKey(key->hardware_keycode, MAPVK_VK_TO_VSC) |
> -(scancode & 0xff00);
> +scancode = native_scancode | (scancode & 0xff00);
> +
> +got_scancode:
> +
> #endif
>
> switch (key->type) {
> --
> 2.7.3
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 0/3] Handle JP keyboard with spice-gtk from Windows client
Ouch, sorry for resending this patch :-\ My intention was to submit another series completely not related to this one, my bad :-\ On Mon, Apr 4, 2016 at 9:59 AM, Fabiano Fidêncio wrote: > In order to speed up the process I took the freedom to apply Takao's > patches locally, regenerate the patches (and cover letter) and submit > them upstream using git send-email. > > Please, for more info, take a look on: > https://lists.freedesktop.org/archives/spice-devel/2016-March/027878.html > > Takao Fujiwara (3): > Send Zenkaku_Hankaku key in JP keyboard. > Send key release event for some keys in JP keyboard. > Send key release event for some keys in JP keyboard. > > src/spice-widget-priv.h | 1 + > src/spice-widget.c | 64 > +++-- > 2 files changed, 63 insertions(+), 2 deletions(-) > > -- > 2.7.3 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 0/3] Handle JP keyboard with spice-gtk from Windows client
In order to speed up the process I took the freedom to apply Takao's patches locally, regenerate the patches (and cover letter) and submit them upstream using git send-email. Please, for more info, take a look on: https://lists.freedesktop.org/archives/spice-devel/2016-March/027878.html Takao Fujiwara (3): Send Zenkaku_Hankaku key in JP keyboard. Send key release event for some keys in JP keyboard. Send key release event for some keys in JP keyboard. src/spice-widget-priv.h | 1 + src/spice-widget.c | 64 +++-- 2 files changed, 63 insertions(+), 2 deletions(-) -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 2/4] coverity: avoid string overflow
sockaddr_un.sun_path has 108 bytes, while pipe_name has
PIPE_NAME_MAX_LEN (256 bytes)
---
src/controller/test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/controller/test.c b/src/controller/test.c
index 9a45581..649aca5 100644
--- a/src/controller/test.c
+++ b/src/controller/test.c
@@ -233,6 +233,12 @@ int main (int argc, char *argv[])
snprintf (pipe_name, PIPE_NAME_MAX_LEN, PIPE_NAME);
printf ("Creating a controller connection %s\n", pipe_name);
struct sockaddr_un remote;
+
+if (strlen(pipe_name) + 1 > sizeof(remote.sun_path)) {
+printf ("address is too long for unix socket_path: %s", pipe_name);
+return -1;
+}
+
if ((sock = socket (AF_UNIX, SOCK_STREAM, 0)) == -1) {
printf ("Could not open socket, (%d) %s\n", errno, strerror(errno));
return -1;
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 1/4] coverity: fix unitialized use of mem.data
--- src/channel-main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/channel-main.c b/src/channel-main.c index 4a1f58a..93537d2 100644 --- a/src/channel-main.c +++ b/src/channel-main.c @@ -1021,9 +1021,9 @@ static void agent_msg_queue_many(SpiceMainChannel *channel, int type, const void } va_end(args); +memset(&msg, 0, sizeof(VDAgentMessage)); msg.protocol = VD_AGENT_PROTOCOL; msg.type = type; -msg.opaque = 0; msg.size = size; paysize = MIN(VD_AGENT_MAX_DATA_SIZE, size + sizeof(VDAgentMessage)); -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 1/3] Send Zenkaku_Hankaku key in JP keyboard.
From: Takao Fujiwara
MapVirtualKey() returns the scancode of the released Zenkaku_Hankaku key
but does not pressed one and modifiered one. Any cases should be sent
to use input method engines.
---
src/spice-widget-priv.h | 1 +
src/spice-widget.c | 37 +++--
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
index 95bca8a..05f6a90 100644
--- a/src/spice-widget-priv.h
+++ b/src/spice-widget-priv.h
@@ -119,6 +119,7 @@ struct _SpiceDisplayPrivate {
HHOOK keyboard_hook;
int win_mouse[3];
int win_mouse_speed;
+HKL keyboard_layout;
#endif
guint keypress_delay;
gintzoom_level;
diff --git a/src/spice-widget.c b/src/spice-widget.c
index f605439..4e026f3 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -644,6 +644,9 @@ static void spice_display_init(SpiceDisplay *display)
d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
d->mouse_cursor = get_blank_cursor();
+#ifdef G_OS_WIN32
+d->keyboard_layout = GetKeyboardLayout(0);
+#endif
}
static GObject *
@@ -1418,6 +1421,7 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey
*key)
SpiceDisplay *display = SPICE_DISPLAY(widget);
SpiceDisplayPrivate *d = display->priv;
int scancode;
+int native_scancode;
#ifdef G_OS_WIN32
/* on windows, we ought to ignore the reserved key event? */
@@ -1464,9 +1468,38 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey
*key)
scancode = vnc_display_keymap_gdk2xtkbd(d->keycode_map, d->keycode_maplen,
key->hardware_keycode);
#ifdef G_OS_WIN32
+native_scancode = MapVirtualKey(key->hardware_keycode, MAPVK_VK_TO_VSC);
+
+/* Some OEM virtual-key codes are missed in MapVirtualKey(). */
+switch (HIWORD(d->keyboard_layout)) {
+case 0x411: /* JP keyboard */
+if (native_scancode == 0) {
+switch (key->hardware_keycode) {
+case VK_OEM_ENLW: /* from Pressed Zenkaku_Hankaku */
+case VK_KANJI: /* from Alt + Zenkaku_Hankaku */
+scancode = MapVirtualKey(VK_OEM_AUTO, MAPVK_VK_TO_VSC);
+/* to Released Zenkaku_Hankaku */
+goto got_scancode;
+case VK_CAPITAL:/* from Alt + Eisu_toggle */
+scancode = MapVirtualKey(VK_OEM_ATTN, MAPVK_VK_TO_VSC);
+/* to Eisu_toggle */
+goto got_scancode;
+case VK_OEM_BACKTAB:/* from Alt + Hiragana_Katakana */
+scancode = MapVirtualKey(VK_OEM_COPY, MAPVK_VK_TO_VSC);
+/* to Hiragana_Katakana */
+goto got_scancode;
+default:;
+}
+}
+break;
+default:;
+}
+
/* MapVirtualKey doesn't return scancode with needed higher byte */
-scancode = MapVirtualKey(key->hardware_keycode, MAPVK_VK_TO_VSC) |
-(scancode & 0xff00);
+scancode = native_scancode | (scancode & 0xff00);
+
+got_scancode:
+
#endif
switch (key->type) {
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Handle JP keyboard with spice-gtk from Windows client
On Thu, Mar 31, 2016 at 1:43 PM, Frediano Ziglio wrote: >> >> I noticed Linux GNOME desktop hangs up when Windows virt-viewer connects to >> the desktop and type some specific keys on JP keyboard. >> >> Server: GNOME desktop and IBus in RHEL 7 >> Client virt-viewer in Windows 7 with JP 106 keyboard >> >> When I see the key events with 'xev' command in GNOME desktop, I observed the >> unlimited too many key events of Zenkaku_Hankaku key and it caused the >> desktop freeze. >> The attached three patches resolves the desktop issues. >> > > Can you please send the patches as separate e-mails (like git format-patch > style) > so we can discuss them online? In order to speed up the process I took the freedom to apply Takao's patches locally, regenerate the patches (and cover letter) and submit them upstream using git send-email. Patches are on the mailing list at this point. > > Frediano > >> 1. spice-gtk-1311820-01-zkhk.patch >> Zenkaku_Hankaku key has the different virtual-key codes between WM_KEYDOWN >> and WM_KEYUP and MapVirtualKey() cannot get the scancode from virtual-key >> code of WM_KEYDOWN (VK_OEM_ENLW) and spice-gtk didn't send the key press >> events and caused the desktop freeze with unlimited key release events. >> >> The fix is to get the scancode from virtual-key code of WM_KEYUP >> (VK_OEM_AUTO) and Zenkaku_Hankaku key works fine. >> >> Alt + Zenkaku_Hankaku key also has the different virtual-key code and >> MapVirtualKey() cannot get the >> scancode from the virtual-key and spice-gtk didn't send the key press events >> and Alt+Zenkaku_Hankaku could not be used. >> >> The fix is to get the scancode from virtual-key code of Zenkaku_Hankaku key >> (VK_OEM_AUTO). >> >> VK_CAPITAL, VK_OEM_BACKTAB are also applied the similar fixes. >> >> 2. spice-gtk-1311820-02-freeze.patch >> After spice-gtk-1311820-01-zkhk.patch is applied, WM_KEYDOWN of >> Alt+Zenkaku_Hankaku (VK_KANJI) can be sent with spice-gtk but >> Alt+Zenkaku_Hankaku did >> not send the WM_KEYUP event in Windows and it caused Linux desktop freeze >> with unlimited key press events. >> >> The fix is to send the key release event in spice-gtk. >> >> VK_OEM_ATTN, VK_OEM_COPY, VK_OEM_BACKTAB are applied the similar fixes. >> >> 3. spice-gtk-1311820-03-hangul.patch >> This is an additional fix. Korean keyboard assigns Hangul key on the position >> of Right Alt and Left Alt and Hangul keys have the different scancodes >> but MapVirtualKey() returned the same scancode and could not use Hangul key >> on Linux desktop. >> >> The fix is to send the right scancode of VK_HANGUL. >> >> Thanks, >> Fujiwara >> > ___ > 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] [spice-gtk 2/3] Send key release event for some keys in JP keyboard.
From: Takao Fujiwara
Some of the keys in JP keyboard do no send WM_KEYUP and it causes
unlimited key events on Linux desktop. This sends the virtual
key release events to avoid the desktop hangup.
---
src/spice-widget.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 4e026f3..5e71d1b 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -1422,6 +1422,7 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey
*key)
SpiceDisplayPrivate *d = display->priv;
int scancode;
int native_scancode;
+gboolean no_key_release = FALSE;
#ifdef G_OS_WIN32
/* on windows, we ought to ignore the reserved key event? */
@@ -1500,11 +1501,28 @@ static gboolean key_event(GtkWidget *widget,
GdkEventKey *key)
got_scancode:
+/* Some keys do not send WM_KEYUP */
+switch (HIWORD(d->keyboard_layout)) {
+case 0x411: /* JP keyboard */
+switch (key->hardware_keycode) {
+case VK_KANJI: /* Alt + Zenkaku_Hankaku */
+case VK_OEM_ATTN: /* Eisu_toggle */
+case VK_OEM_COPY: /* Hiragana_Katakana */
+case VK_OEM_BACKTAB:/* Alt + Hiragana_Katakana */
+no_key_release = TRUE;
+break;
+default:;
+}
+break;
+default:;
+}
#endif
switch (key->type) {
case GDK_KEY_PRESS:
send_key(display, scancode, SEND_KEY_PRESS, !key->is_modifier);
+if (no_key_release)
+send_key(display, scancode, SEND_KEY_RELEASE, !key->is_modifier);
break;
case GDK_KEY_RELEASE:
send_key(display, scancode, SEND_KEY_RELEASE, !key->is_modifier);
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 3/3] Send key release event for some keys in JP keyboard.
From: Takao Fujiwara
Some of the keys in JP keyboard do no send WM_KEYUP and it causes
unlimited key events on Linux desktop. This sends the virtual
key release events to avoid the desktop hangup.
---
src/spice-widget.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 5e71d1b..c5a4530 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -1493,6 +1493,15 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey
*key)
}
}
break;
+case 0x412: /* KR keyboard */
+if (key->hardware_keycode == VK_HANGUL && native_scancode == 0x38) {
+/* Left Alt (VK_MENU) has the scancode 0x38 but Hangul (VK_HANGUL)
+ * has the scancode 0x138
+ */
+scancode = native_scancode | 0x100;
+goto got_scancode;
+}
+break;
default:;
}
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 1/3] Send Zenkaku_Hankaku key in JP keyboard.
From: Takao Fujiwara
MapVirtualKey() returns the scancode of the released Zenkaku_Hankaku key
but does not pressed one and modifiered one. Any cases should be sent
to use input method engines.
---
src/spice-widget-priv.h | 1 +
src/spice-widget.c | 37 +++--
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
index 95bca8a..05f6a90 100644
--- a/src/spice-widget-priv.h
+++ b/src/spice-widget-priv.h
@@ -119,6 +119,7 @@ struct _SpiceDisplayPrivate {
HHOOK keyboard_hook;
int win_mouse[3];
int win_mouse_speed;
+HKL keyboard_layout;
#endif
guint keypress_delay;
gintzoom_level;
diff --git a/src/spice-widget.c b/src/spice-widget.c
index f605439..4e026f3 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -644,6 +644,9 @@ static void spice_display_init(SpiceDisplay *display)
d->grabseq = spice_grab_sequence_new_from_string("Control_L+Alt_L");
d->activeseq = g_new0(gboolean, d->grabseq->nkeysyms);
d->mouse_cursor = get_blank_cursor();
+#ifdef G_OS_WIN32
+d->keyboard_layout = GetKeyboardLayout(0);
+#endif
}
static GObject *
@@ -1418,6 +1421,7 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey
*key)
SpiceDisplay *display = SPICE_DISPLAY(widget);
SpiceDisplayPrivate *d = display->priv;
int scancode;
+int native_scancode;
#ifdef G_OS_WIN32
/* on windows, we ought to ignore the reserved key event? */
@@ -1464,9 +1468,38 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey
*key)
scancode = vnc_display_keymap_gdk2xtkbd(d->keycode_map, d->keycode_maplen,
key->hardware_keycode);
#ifdef G_OS_WIN32
+native_scancode = MapVirtualKey(key->hardware_keycode, MAPVK_VK_TO_VSC);
+
+/* Some OEM virtual-key codes are missed in MapVirtualKey(). */
+switch (HIWORD(d->keyboard_layout)) {
+case 0x411: /* JP keyboard */
+if (native_scancode == 0) {
+switch (key->hardware_keycode) {
+case VK_OEM_ENLW: /* from Pressed Zenkaku_Hankaku */
+case VK_KANJI: /* from Alt + Zenkaku_Hankaku */
+scancode = MapVirtualKey(VK_OEM_AUTO, MAPVK_VK_TO_VSC);
+/* to Released Zenkaku_Hankaku */
+goto got_scancode;
+case VK_CAPITAL:/* from Alt + Eisu_toggle */
+scancode = MapVirtualKey(VK_OEM_ATTN, MAPVK_VK_TO_VSC);
+/* to Eisu_toggle */
+goto got_scancode;
+case VK_OEM_BACKTAB:/* from Alt + Hiragana_Katakana */
+scancode = MapVirtualKey(VK_OEM_COPY, MAPVK_VK_TO_VSC);
+/* to Hiragana_Katakana */
+goto got_scancode;
+default:;
+}
+}
+break;
+default:;
+}
+
/* MapVirtualKey doesn't return scancode with needed higher byte */
-scancode = MapVirtualKey(key->hardware_keycode, MAPVK_VK_TO_VSC) |
-(scancode & 0xff00);
+scancode = native_scancode | (scancode & 0xff00);
+
+got_scancode:
+
#endif
switch (key->type) {
--
2.7.3
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 0/3] Handle JP keyboard with spice-gtk from Windows client
In order to speed up the process I took the freedom to apply Takao's patches locally, regenerate the patches (and cover letter) and submit them upstream using git send-email. Please, for more info, take a look on: https://lists.freedesktop.org/archives/spice-devel/2016-March/027878.html Takao Fujiwara (3): Send Zenkaku_Hankaku key in JP keyboard. Send key release event for some keys in JP keyboard. Send key release event for some keys in JP keyboard. src/spice-widget-priv.h | 1 + src/spice-widget.c | 64 +++-- 2 files changed, 63 insertions(+), 2 deletions(-) -- 2.7.3 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] current git head spice-common fails to build on glib2 < 2.38
Brad, On Sat, Mar 26, 2016 at 2:53 PM, Brad Campbell wrote: > Checking the git logs, in Oct the required glib version was bumped to 2.22. > Since that point logging has been added using g_test_* functions which were > apparently added in 2.38. Seems that g_test_subprocess() was added in 2.38 :-\ > Trying to build on 2.33 (which I was) therefore > passes the configure tests, but fails to build. We need either bump the dep version or add a compat file for this. Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2] Introduce gtask-helper.[ch]
On Wed, Mar 23, 2016 at 2:45 PM, Victor Toso wrote:
> Hi,
>
> On Wed, Mar 23, 2016 at 02:32:06PM +0100, Christophe Fergeau wrote:
>> On Wed, Mar 23, 2016 at 01:04:03PM +0100, Fabiano Fidêncio wrote:
>> > gtask-helper provide methods that can easily be used for returning in
>> > idle, as a few issues have been found in the GTask code used in
>> > spice-gtk due to a immediately return when a return in idle was
>> > expected. As examples of these issues, you can take a look on commits
>> > 7774b8c and e81d97c.
>> >
>> > Not all the functions introduced in gtask-helper.h are being used, but I
>> > still decided to add them for completeness reasons.
>> >
>> > Also, all the functions called in idle are the same that were being
>> > called in idle when using GSimpleAsyncResult. So, no issues should be
>> > found after this change and no behavior change should noticed.
>>
>> This is indeed the safe way to go given the few crashes we had. However,
>> it would be nice to try to get rid of as much of these idle calls as
>> possible (when it's safe to do so) in the future so that the places
>> which need to be special are obvious.
>>
>> >
>>
>>
>> > diff --git a/src/gtask-helper.c b/src/gtask-helper.c
>> > new file mode 100644
>> > index 000..3c72396
>> > --- /dev/null
>> > +++ b/src/gtask-helper.c
>> > @@ -0,0 +1,153 @@
>> > +
>> > +
>> > +void __attribute__((format(gnu_printf, 4, 5)))
>>
>> glib provides G_GNUC_PRINTF()
>> Doesn't this belong in the header file though rather than in the .c
>> file?
>>
>> > +g_task_helper_return_new_error_in_idle(GTask *task,
>> > + GQuark domain,
>> > + gint code,
>> > + const char *format,
>> > + ...)
>> > +{
>> > +GTaskHelperData *data = g_task_helper_new();
>> > +va_list args;
>> > +
>> > +data->task = g_object_ref(task);
>> > +va_start(args, format);
>> > +data->error = g_error_new_valist(domain, code, format, args);
>> > +va_end(args);
>> > +
>> > +g_idle_add(complete_in_idle_error_cb, data);
>> > +}
>> > +
>> > +void g_task_helper_return_pointer_in_idle(GTask *task,
>> > + gpointer result,
>> > + GDestroyNotify result_destroy)
>> > +{
>> > +GTaskHelperData *data = g_task_helper_new();
>> > +data->task = g_object_ref(task);
>> > +data->pointer = result;
>> > +data->destroy_notify_cb = result_destroy;
>> > +
>> > +g_idle_add(complete_in_idle_pointer_cb, data);
>> > +}
>> > diff --git a/src/gtask-helper.h b/src/gtask-helper.h
>> > new file mode 100644
>> > index 000..81c041f
>> > --- /dev/null
>> > +++ b/src/gtask-helper.h
>> > @@ -0,0 +1,34 @@
>> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
>> > +/*
>> > + Copyright (C) 2016 Red Hat, Inc.
>> > +
>> > + This library is free software; you can redistribute it and/or
>> > + modify it under the terms of the GNU Lesser General Public
>> > + License as published by the Free Software Foundation; either
>> > + version 2.0 of the License, or (at your option) any later version.
>> > +
>> > + This library is distributed in the hope that it will be useful,
>> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> > + Lesser General Public License for more details.
>> > +
>> > + You should have received a copy of the GNU Lesser General Public
>> > + License along with this library; if not, write to the Free Software
>> > + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> > 02110-1301 USA
>> > +*/
>> > +#ifndef __G_TASK_HELPER_H__
>> > +#define __G_TASK_HELPER_H__
>> > +
>> > +#include
>> > +
>> > +G_BEGIN_DECLS
>> > +
>> > +void g_task_helper_return_boolean_in_idle(GTask *task, gboolean result);
>> > +void g_task_helper_return_int_in_idle(GTask *task, gssize result);
>> > +void g_task_helper_return_error_in_idle(GTask *task, GError *err
[Spice-devel] [spice-gtk v2] Introduce gtask-helper.[ch]
gtask-helper provide methods that can easily be used for returning in
idle, as a few issues have been found in the GTask code used in
spice-gtk due to a immediately return when a return in idle was
expected. As examples of these issues, you can take a look on commits
7774b8c and e81d97c.
Not all the functions introduced in gtask-helper.h are being used, but I
still decided to add them for completeness reasons.
Also, all the functions called in idle are the same that were being
called in idle when using GSimpleAsyncResult. So, no issues should be
found after this change and no behavior change should noticed.
Signed-off-by: Fabiano Fidêncio
---
src/Makefile.am | 2 +
src/channel-base.c | 4 +-
src/channel-main.c | 12 ++--
src/channel-usbredir.c | 12 ++--
src/gtask-helper.c | 153 +++
src/gtask-helper.h | 34 ++
src/spice-channel.c | 4 +-
src/spice-gstaudio.c | 6 +-
src/usb-acl-helper.c | 12 ++--
src/usb-device-manager.c | 18 ++---
src/vmcstream.c | 35 ++
src/win-usb-driver-install.c | 21 +++---
12 files changed, 246 insertions(+), 67 deletions(-)
create mode 100644 src/gtask-helper.c
create mode 100644 src/gtask-helper.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 66ba58b..783db37 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -235,6 +235,8 @@ libspice_client_glib_2_0_la_SOURCES =
\
coroutine.h \
gio-coroutine.c \
gio-coroutine.h \
+ gtask-helper.c \
+ gtask-helper.h \
\
channel-base.c \
channel-webdav.c\
diff --git a/src/channel-base.c b/src/channel-base.c
index de04b89..67352e5 100644
--- a/src/channel-base.c
+++ b/src/channel-base.c
@@ -17,6 +17,8 @@
*/
#include "config.h"
+#include "gtask-helper.h"
+
#include "spice-client.h"
#include "spice-common.h"
@@ -243,7 +245,7 @@ vmc_write_free_cb(uint8_t *data, void *user_data)
GTask *task = user_data;
gsize count = GPOINTER_TO_SIZE(g_task_get_task_data(task));
-g_task_return_int(task, count);
+g_task_helper_return_int_in_idle(task, count);
g_object_unref(task);
}
diff --git a/src/channel-main.c b/src/channel-main.c
index 1c19de1..3873c45 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -21,6 +21,8 @@
#include
#include
+#include "gtask-helper.h"
+
#include "spice-client.h"
#include "spice-common.h"
#include "spice-marshal.h"
@@ -923,7 +925,7 @@ static gboolean flush_foreach_remove(gpointer key
G_GNUC_UNUSED,
{
gboolean success = GPOINTER_TO_UINT(user_data);
GTask *result = value;
-g_task_return_boolean(result, success);
+g_task_helper_return_boolean_in_idle(result, success);
return TRUE;
}
@@ -946,7 +948,7 @@ static void file_xfer_flush_async(SpiceMainChannel
*channel, GCancellable *cance
was_empty = g_queue_is_empty(c->agent_msg_queue);
if (was_empty) {
-g_task_return_boolean(task, TRUE);
+g_task_helper_return_boolean_in_idle(task, TRUE);
g_object_unref(task);
return;
}
@@ -981,7 +983,7 @@ static void agent_send_msg_queue(SpiceMainChannel *channel)
task = g_hash_table_lookup(c->flushing, out);
if (task) {
/* if there's a flush task waiting for this message, finish it */
-g_task_return_boolean(task, TRUE);
+g_task_helper_return_boolean_in_idle(task, TRUE);
g_hash_table_remove(c->flushing, out);
}
}
@@ -1790,9 +1792,9 @@ static void file_xfer_close_cb(GObject *object,
self->priv->user_data);
if (self->priv->error) {
-g_task_return_error(task, self->priv->error);
+g_task_helper_return_error_in_idle(task, self->priv->error);
} else {
-g_task_return_boolean(task, TRUE);
+g_task_helper_return_boolean_in_idle(task, TRUE);
if (spice_util_get_debug()) {
gint64 now = g_get_monotonic_time();
gchar *basename = g_file_get_basename(self->priv->file);
diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index ab90800..9549763 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -32,6 +32,8 @@
#include "usbutil.h"
#endif
+#include "gtask-helper.h"
+
#include "spice-client.h"
#include "spice-common.h"
@@ -301,9 +303,9 @@ static void spice_usbredir_channel_open_acl_cb(
g_boxed_free(spice_usb_
Re: [Spice-devel] [spice-gtk 2/2] Introduce gtask-helper.[ch]
On Wed, Mar 23, 2016 at 11:34 AM, Christophe Fergeau wrote: > On Wed, Mar 23, 2016 at 10:48:31AM +0100, Fabiano Fidêncio wrote: >> As noticed, GTask's heurestic for return a task in idle or immediately >> doesn't work when using Coroutine > > s/heurestic/heuristic/ > > Imo we need a detailed description of what is not working well with > GTask + coroutine, and one concrete example when the right thing is not > done (either in this log, or as a comment in gtask-helper.[ch]). This > will be really helpful if we ever need to revisit this. Will be added in the v2 > > >> and that's okay, we just need to do >> the idle ourself. And in order to avoid code duplication, let's >> introduce and make usage of the new g_task_helper_return_* functions. >> >> These functions match exactly with the existing g_task_return_* >> functions and the only difference is that they return in idle instead of >> returning immediately. >> >> Signed-off-by: Fabiano Fidêncio >> --- >> m4/manywarnings.m4 | 1 - >> src/Makefile.am | 2 + >> src/channel-base.c | 4 +- >> src/channel-main.c | 12 ++-- >> src/channel-usbredir.c | 12 ++-- >> src/gtask-helper.c | 152 >> +++ >> src/gtask-helper.h | 43 >> src/spice-channel.c | 4 +- >> src/spice-gstaudio.c | 6 +- >> src/usb-acl-helper.c | 12 ++-- >> src/usb-device-manager.c | 18 ++--- >> src/vmcstream.c | 35 ++ >> src/win-usb-driver-install.c | 21 +++--- >> 13 files changed, 254 insertions(+), 68 deletions(-) >> create mode 100644 src/gtask-helper.c >> create mode 100644 src/gtask-helper.h > > Do we need to use the helper everywhere, or only in specific cases? Well, I've decided to use it in all places where a return in idle was being used before. We have seem some issues with coroutine context, some issues without coroutine context ... > >> >> diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4 >> index 3e6dd21..ead38a2 100644 >> --- a/m4/manywarnings.m4 >> +++ b/m4/manywarnings.m4 >> @@ -182,7 +182,6 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC], >> -Wstrict-overflow \ >> -Wstrict-prototypes \ >> -Wsuggest-attribute=const \ >> --Wsuggest-attribute=format \ > > Unrelated ? Actually, related, but shouldn't be here. I removed the warning because I was getting and ended up not fixing it: gtask-helper.c: In function ‘g_task_helper_return_new_error’: gtask-helper.c:136:5: error: function might be possible candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format] data->error = g_error_new_valist(domain, code, format, args); ^ > >> -Wsuggest-attribute=noreturn \ >> -Wsuggest-attribute=pure \ >> -Wswitch \ > > >> diff --git a/src/gtask-helper.h b/src/gtask-helper.h >> new file mode 100644 >> index 000..9020506 >> --- /dev/null >> +++ b/src/gtask-helper.h >> @@ -0,0 +1,43 @@ >> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ >> +/* >> + Copyright (C) 2016 Red Hat, Inc. >> + >> + This library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.0 of the License, or (at your option) any later version. >> + >> + This library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with this library; if not, write to the Free Software >> + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >> 02110-1301 USA >> +*/ >> +#ifndef __G_TASK_HELPER_H__ >> +#define __G_TASK_HELPER_H__ >> + >> +#include >> + >> +G_BEGIN_DECLS >> + >> +void g_task_helper_return_boolean(GTask *task, >> + gboolean boolean); > > In my opinion, there should be a 'idle' somewhere in the name of all > these helpers. Someone reading unrelated code would have to check what > these helpers are about with the current naming, with an 'idle' in the > name, it's much easier to guess what they are about and decide not to > care about the details. Sure, I can change it. > > >> +void g_task_helper_return_int(GTask *task, >> + gint integer); > > g_task_return_int takes a gssize, not a gint. Right. > > Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 2/2] Introduce gtask-helper.[ch]
As noticed, GTask's heurestic for return a task in idle or immediately
doesn't work when using Coroutine and that's okay, we just need to do
the idle ourself. And in order to avoid code duplication, let's
introduce and make usage of the new g_task_helper_return_* functions.
These functions match exactly with the existing g_task_return_*
functions and the only difference is that they return in idle instead of
returning immediately.
Signed-off-by: Fabiano Fidêncio
---
m4/manywarnings.m4 | 1 -
src/Makefile.am | 2 +
src/channel-base.c | 4 +-
src/channel-main.c | 12 ++--
src/channel-usbredir.c | 12 ++--
src/gtask-helper.c | 152 +++
src/gtask-helper.h | 43
src/spice-channel.c | 4 +-
src/spice-gstaudio.c | 6 +-
src/usb-acl-helper.c | 12 ++--
src/usb-device-manager.c | 18 ++---
src/vmcstream.c | 35 ++
src/win-usb-driver-install.c | 21 +++---
13 files changed, 254 insertions(+), 68 deletions(-)
create mode 100644 src/gtask-helper.c
create mode 100644 src/gtask-helper.h
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 3e6dd21..ead38a2 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -182,7 +182,6 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC],
-Wstrict-overflow \
-Wstrict-prototypes \
-Wsuggest-attribute=const \
--Wsuggest-attribute=format \
-Wsuggest-attribute=noreturn \
-Wsuggest-attribute=pure \
-Wswitch \
diff --git a/src/Makefile.am b/src/Makefile.am
index 66ba58b..783db37 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -235,6 +235,8 @@ libspice_client_glib_2_0_la_SOURCES =
\
coroutine.h \
gio-coroutine.c \
gio-coroutine.h \
+ gtask-helper.c \
+ gtask-helper.h \
\
channel-base.c \
channel-webdav.c\
diff --git a/src/channel-base.c b/src/channel-base.c
index de04b89..8648ac4 100644
--- a/src/channel-base.c
+++ b/src/channel-base.c
@@ -17,6 +17,8 @@
*/
#include "config.h"
+#include "gtask-helper.h"
+
#include "spice-client.h"
#include "spice-common.h"
@@ -243,7 +245,7 @@ vmc_write_free_cb(uint8_t *data, void *user_data)
GTask *task = user_data;
gsize count = GPOINTER_TO_SIZE(g_task_get_task_data(task));
-g_task_return_int(task, count);
+g_task_helper_return_int(task, count);
g_object_unref(task);
}
diff --git a/src/channel-main.c b/src/channel-main.c
index 1c19de1..9252777 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -21,6 +21,8 @@
#include
#include
+#include "gtask-helper.h"
+
#include "spice-client.h"
#include "spice-common.h"
#include "spice-marshal.h"
@@ -923,7 +925,7 @@ static gboolean flush_foreach_remove(gpointer key
G_GNUC_UNUSED,
{
gboolean success = GPOINTER_TO_UINT(user_data);
GTask *result = value;
-g_task_return_boolean(result, success);
+g_task_helper_return_boolean(result, success);
return TRUE;
}
@@ -946,7 +948,7 @@ static void file_xfer_flush_async(SpiceMainChannel
*channel, GCancellable *cance
was_empty = g_queue_is_empty(c->agent_msg_queue);
if (was_empty) {
-g_task_return_boolean(task, TRUE);
+g_task_helper_return_boolean(task, TRUE);
g_object_unref(task);
return;
}
@@ -981,7 +983,7 @@ static void agent_send_msg_queue(SpiceMainChannel *channel)
task = g_hash_table_lookup(c->flushing, out);
if (task) {
/* if there's a flush task waiting for this message, finish it */
-g_task_return_boolean(task, TRUE);
+g_task_helper_return_boolean(task, TRUE);
g_hash_table_remove(c->flushing, out);
}
}
@@ -1790,9 +1792,9 @@ static void file_xfer_close_cb(GObject *object,
self->priv->user_data);
if (self->priv->error) {
-g_task_return_error(task, self->priv->error);
+g_task_helper_return_error(task, self->priv->error);
} else {
-g_task_return_boolean(task, TRUE);
+g_task_helper_return_boolean(task, TRUE);
if (spice_util_get_debug()) {
gint64 now = g_get_monotonic_time();
gchar *basename = g_file_get_basename(self->priv->file);
diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index 0f76126..174f37a 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -32,6 +32,8 @@
#include "usbutil.h"
#endif
[Spice-devel] [spice-gtk 1/2] Revert "channel-usbredir: Fix crash due to a Task returning earlier than expected"
This reverts commit 7774b8c0dd85ce2bb311d8bbe1c25deb73970b6e.
The crash was fixed, but not properly. GTask shouldn't return
immediately in that scenario. Next patch brings a proper fix for the
issue.
Signed-off-by: Fabiano Fidêncio
---
src/channel-usbredir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index ab90800..0f76126 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -296,12 +296,12 @@ static void spice_usbredir_channel_open_acl_cb(
spice_usbredir_channel_open_device(channel, &err);
}
if (err) {
+g_task_return_error(priv->task, err);
libusb_unref_device(priv->device);
priv->device = NULL;
g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
priv->spice_device = NULL;
priv->state = STATE_DISCONNECTED;
-g_task_return_error(priv->task, err);
} else {
g_task_return_boolean(priv->task, TRUE);
}
--
2.5.5
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk 0/2] Introduce gtask-helper.[ch]
The first patch is only about reverting a patch that, even if it fixed the problem, it wasn't done in the proper way. The second patch introduces and uses new helpers for GTask, providing them functions that will explicitely return in idle instead of leaving the decision to the GTask heurestic (which doesn't work with Coroutine). Fabiano Fidêncio (2): Revert "channel-usbredir: Fix crash due to a Task returning earlier than expected" Introduce gtask-helper.[ch] m4/manywarnings.m4 | 1 - src/Makefile.am | 2 + src/channel-base.c | 4 +- src/channel-main.c | 12 ++-- src/channel-usbredir.c | 12 ++-- src/gtask-helper.c | 152 +++ src/gtask-helper.h | 43 src/spice-channel.c | 4 +- src/spice-gstaudio.c | 6 +- src/usb-acl-helper.c | 12 ++-- src/usb-device-manager.c | 18 ++--- src/vmcstream.c | 35 ++ src/win-usb-driver-install.c | 21 +++--- 13 files changed, 254 insertions(+), 68 deletions(-) create mode 100644 src/gtask-helper.c create mode 100644 src/gtask-helper.h -- 2.5.5 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] 答复: remote-viewer usb redirection
Please, On Wed, Mar 23, 2016 at 3:08 AM, lichanghua wrote: > Maybe it is, but I try to recompile the spice-gtk and virt-viewer, I can't > get correct result, Do anyone have the remote-viewer version supported > USB_redirection in windows OS. thanks very much! These kind of details are really important. So, you're building spice-gtk using mingw, is it? If yes, please, enable usbredir support in your build. > > best, > > -邮件原件- > 发件人: Fabiano Fidêncio [mailto:fabi...@fidencio.org] > 发送时间: 2016年3月23日 5:17 > 收件人: lichanghua > 抄送: spice-devel@lists.freedesktop.org > 主题: Re: [Spice-devel] remote-viewer usb redirection > > On Fri, Mar 18, 2016 at 3:24 PM, lichanghua wrote: >> Dear all, >> >> I setup the kvm virtual machine, which support the spice. I use >> remote-viewer to access it , it is ok! but when I click the usb redir icon, >> a pop window shows, it told me "the usb redirection support not compiled", >> where can I download the new version with the usb redirction function. thank >> you very much. > > Seems that your spice-gtk package is not build with support to > usb-redirection. > >> >> >> >> Best regards >> >> Edward Li >> >> >> _______ >> Spice-devel mailing list >> Spice-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/spice-devel >> > > > Best Regards, > -- > Fabiano Fidêncio > > -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v3] vmcstream, gtask: Do idle ourself instead of leaving it to GTask's heuristic
Pushed, thanks!
On Tue, Mar 22, 2016 at 11:01 PM, Fabiano Fidêncio wrote:
> On Tue, Mar 22, 2016 at 10:57 PM, Marc-André Lureau
> wrote:
>> ack
>>
>> - Original Message -
>>> Seems that GTask heuristic only makes sense in a non-coroutine case.
>>> After opening a bug[0] on spice-gtk and a few discussions in the mailing
>>> list, seems that is completely fine for coroutine code to deal with the
>>> idle explicitly.
>>>
>>> Signed-off-by: Fabiano Fidêncio
>>> ---
>>> Changes since v2, as per Marc-André review:
>>> - Change the commit log
>>> - Change the comment in the code
>>> - No need for a _free() function, just do the unref and free inside
>>> _idle_cb()
>>>
>>> Changes since v1, as per Marc-André review:
>>> - Use g_idle_add() instead of GSource ... which worries me a bit about the
>>> context used when using g_idle_add(), as previously the context used was
>>> the one from the GTask.
>>> ---
>>>
>>> src/vmcstream.c | 29 +++--
>>> 1 file changed, 27 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>>> index 5ebf15a..5dd2799 100644
>>> --- a/src/vmcstream.c
>>> +++ b/src/vmcstream.c
>>> @@ -97,6 +97,24 @@ spice_vmc_input_stream_new(void)
>>> return self;
>>> }
>>>
>>> +typedef struct _complete_in_idle_cb_data {
>>> +GTask *task;
>>> +gssize pos;
>>> +} complete_in_idle_cb_data;
>>> +
>>> +static gboolean
>>> +complete_in_idle_cb(gpointer user_data)
>>> +{
>>> +complete_in_idle_cb_data *data = user_data;
>>> +
>>> +g_task_return_int(data->task, data->pos);
>>> +
>>> +g_object_unref (data->task);
>>> +g_free (data);
>>> +
>>> +return FALSE;
>>> +}
>>> +
>>> /* coroutine */
>>> /**
>>> * Feed a SpiceVmc stream with new data from a coroutine
>>> @@ -116,6 +134,8 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
>>> *self,
>>> self->coroutine = coroutine_self();
>>>
>>> while (size > 0) {
>>> +complete_in_idle_cb_data *cb_data;
>>> +
>>> SPICE_DEBUG("spicevmc co_data %p", self->task);
>>> if (!self->task)
>>> coroutine_yield(NULL);
>>> @@ -137,10 +157,15 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
>>> *self,
>>> if (self->all && min > 0 && self->pos != self->count)
>>> continue;
>>>
>>> -g_task_return_int(self->task, self->pos);
>>> +/* Let's deal with the task complete in idle by ourselves, as GTask
>>> + * heuristic only makes sense in a non-coroutine case.
>>> + */
>>> +cb_data = g_new(complete_in_idle_cb_data , 1);
>>> + cb_data->task = g_object_ref(self->task);
>>> +cb_data->pos = self->pos;
>>> +g_idle_add(complete_in_idle_cb, cb_data);
>>>
>>> g_cancellable_disconnect(g_task_get_cancellable(self->task),
>>> self->cancel_id);
>>> -
>>> g_clear_object(&self->task);
>>> }
>>>
>>> --
>>> 2.5.0
>>>
>>> ___
>>> 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
>
>
>
> --
> Fabiano Fidêncio
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v3] vmcstream, gtask: Do idle ourself instead of leaving it to GTask's heuristic
On Tue, Mar 22, 2016 at 10:57 PM, Marc-André Lureau wrote:
> ack
>
> - Original Message -
>> Seems that GTask heuristic only makes sense in a non-coroutine case.
>> After opening a bug[0] on spice-gtk and a few discussions in the mailing
>> list, seems that is completely fine for coroutine code to deal with the
>> idle explicitly.
>>
>> Signed-off-by: Fabiano Fidêncio
>> ---
>> Changes since v2, as per Marc-André review:
>> - Change the commit log
>> - Change the comment in the code
>> - No need for a _free() function, just do the unref and free inside
>> _idle_cb()
>>
>> Changes since v1, as per Marc-André review:
>> - Use g_idle_add() instead of GSource ... which worries me a bit about the
>> context used when using g_idle_add(), as previously the context used was
>> the one from the GTask.
>> ---
>>
>> src/vmcstream.c | 29 +++--
>> 1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>> index 5ebf15a..5dd2799 100644
>> --- a/src/vmcstream.c
>> +++ b/src/vmcstream.c
>> @@ -97,6 +97,24 @@ spice_vmc_input_stream_new(void)
>> return self;
>> }
>>
>> +typedef struct _complete_in_idle_cb_data {
>> +GTask *task;
>> +gssize pos;
>> +} complete_in_idle_cb_data;
>> +
>> +static gboolean
>> +complete_in_idle_cb(gpointer user_data)
>> +{
>> +complete_in_idle_cb_data *data = user_data;
>> +
>> +g_task_return_int(data->task, data->pos);
>> +
>> +g_object_unref (data->task);
>> +g_free (data);
>> +
>> +return FALSE;
>> +}
>> +
>> /* coroutine */
>> /**
>> * Feed a SpiceVmc stream with new data from a coroutine
>> @@ -116,6 +134,8 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
>> self->coroutine = coroutine_self();
>>
>> while (size > 0) {
>> +complete_in_idle_cb_data *cb_data;
>> +
>> SPICE_DEBUG("spicevmc co_data %p", self->task);
>> if (!self->task)
>> coroutine_yield(NULL);
>> @@ -137,10 +157,15 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
>> *self,
>> if (self->all && min > 0 && self->pos != self->count)
>> continue;
>>
>> -g_task_return_int(self->task, self->pos);
>> +/* Let's deal with the task complete in idle by ourselves, as GTask
>> + * heuristic only makes sense in a non-coroutine case.
>> + */
>> +cb_data = g_new(complete_in_idle_cb_data , 1);
>> +cb_data->task = g_object_ref(self->task);
>> +cb_data->pos = self->pos;
>> +g_idle_add(complete_in_idle_cb, cb_data);
>>
>> g_cancellable_disconnect(g_task_get_cancellable(self->task),
>> self->cancel_id);
>> -
>> g_clear_object(&self->task);
>> }
>>
>> --
>> 2.5.0
>>
>> ___
>> 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
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v3] vmcstream, gtask: Do idle ourself instead of leaving it to GTask's heuristic
Seems that GTask heuristic only makes sense in a non-coroutine case.
After opening a bug[0] on spice-gtk and a few discussions in the mailing
list, seems that is completely fine for coroutine code to deal with the
idle explicitly.
Signed-off-by: Fabiano Fidêncio
---
Changes since v2, as per Marc-André review:
- Change the commit log
- Change the comment in the code
- No need for a _free() function, just do the unref and free inside _idle_cb()
Changes since v1, as per Marc-André review:
- Use g_idle_add() instead of GSource ... which worries me a bit about the
context used when using g_idle_add(), as previously the context used was
the one from the GTask.
---
src/vmcstream.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/vmcstream.c b/src/vmcstream.c
index 5ebf15a..5dd2799 100644
--- a/src/vmcstream.c
+++ b/src/vmcstream.c
@@ -97,6 +97,24 @@ spice_vmc_input_stream_new(void)
return self;
}
+typedef struct _complete_in_idle_cb_data {
+GTask *task;
+gssize pos;
+} complete_in_idle_cb_data;
+
+static gboolean
+complete_in_idle_cb(gpointer user_data)
+{
+complete_in_idle_cb_data *data = user_data;
+
+g_task_return_int(data->task, data->pos);
+
+g_object_unref (data->task);
+g_free (data);
+
+return FALSE;
+}
+
/* coroutine */
/**
* Feed a SpiceVmc stream with new data from a coroutine
@@ -116,6 +134,8 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
self->coroutine = coroutine_self();
while (size > 0) {
+complete_in_idle_cb_data *cb_data;
+
SPICE_DEBUG("spicevmc co_data %p", self->task);
if (!self->task)
coroutine_yield(NULL);
@@ -137,10 +157,15 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
if (self->all && min > 0 && self->pos != self->count)
continue;
-g_task_return_int(self->task, self->pos);
+/* Let's deal with the task complete in idle by ourselves, as GTask
+ * heuristic only makes sense in a non-coroutine case.
+ */
+cb_data = g_new(complete_in_idle_cb_data , 1);
+cb_data->task = g_object_ref(self->task);
+cb_data->pos = self->pos;
+g_idle_add(complete_in_idle_cb, cb_data);
g_cancellable_disconnect(g_task_get_cancellable(self->task),
self->cancel_id);
-
g_clear_object(&self->task);
}
--
2.5.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] remote-viewer usb redirection
On Fri, Mar 18, 2016 at 3:24 PM, lichanghua wrote: > Dear all, > > I setup the kvm virtual machine, which support the spice. I use > remote-viewer to access it , it is ok! but when I click the usb redir icon, > a pop window shows, it told me "the usb redirection support not compiled", > where can I download the new version with the usb redirction function. thank > you very much. Seems that your spice-gtk package is not build with support to usb-redirection. > > > > Best regards > > Edward Li > > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel > Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2] vmcstream, gtask: Do idle ourself as GTask's return gets messed up
Seems that our coroutines can mess up with GTask's return, ending up
in an early return from g_task_return_now() instead of doing a return
in idle.
As I still don't know if the bug is on spice-gtk or on GTask itself,
I've opened a bug[0] in spice-gtk's bugzilla and also made a reference
to the very same bug in the code.
[0]: https://bugs.freedesktop.org/show_bug.cgi?id=94662
Signed-off-by: Fabiano Fidêncio
---
Changes since v1, as per Marc-André review:
- Use g_idle_add() instead of GSource ... which worries me a bit about the
context used when using g_idle_add(), as previously the context used was
the one from the GTask.
---
src/vmcstream.c | 41 +++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/src/vmcstream.c b/src/vmcstream.c
index 5ebf15a..54bf4d9 100644
--- a/src/vmcstream.c
+++ b/src/vmcstream.c
@@ -97,6 +97,32 @@ spice_vmc_input_stream_new(void)
return self;
}
+typedef struct _complete_in_idle_cb_data {
+GTask *task;
+gssize pos;
+} complete_in_idle_cb_data;
+
+static void
+complete_in_idle_cb_data_free (gpointer user_data)
+{
+complete_in_idle_cb_data *data = user_data;
+
+g_object_unref (data->task);
+g_free (data);
+}
+
+static gboolean
+complete_in_idle_cb(gpointer user_data)
+{
+complete_in_idle_cb_data *data = user_data;
+
+g_task_return_int(data->task, data->pos);
+
+complete_in_idle_cb_data_free(data);
+
+return FALSE;
+}
+
/* coroutine */
/**
* Feed a SpiceVmc stream with new data from a coroutine
@@ -116,6 +142,8 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
self->coroutine = coroutine_self();
while (size > 0) {
+complete_in_idle_cb_data *cb_data;
+
SPICE_DEBUG("spicevmc co_data %p", self->task);
if (!self->task)
coroutine_yield(NULL);
@@ -137,10 +165,19 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
if (self->all && min > 0 && self->pos != self->count)
continue;
-g_task_return_int(self->task, self->pos);
+/* Somehow we are messing up with GTask return. As it's not known
+ * for now if it's a spice-gtk issue due to the coroutine usage or
+ * a GTask issue itself, let's do the idle ourself.
+ *
+ * For a future reference, see:
+ * https://bugs.freedesktop.org/show_bug.cgi?id=94662
+ */
+cb_data = g_new(complete_in_idle_cb_data , 1);
+cb_data->task = g_object_ref(self->task);
+cb_data->pos = self->pos;
+g_idle_add(complete_in_idle_cb, cb_data);
g_cancellable_disconnect(g_task_get_cancellable(self->task),
self->cancel_id);
-
g_clear_object(&self->task);
}
--
2.5.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] vmcstream, gtask: Do idle ourself as GTask's return gets messed up
On Tue, Mar 22, 2016 at 8:40 PM, Marc-André Lureau wrote:
> Hi
>
> - Original Message -
>> Seems that our coroutines can mess up with GTask's return, ending up
>> in an early return from g_task_return_now() instead of doing a return
>> in idle.
>> As I still don't know if the bug is on spice-gtk or on GTask itself,
>> I've opened a bug[0] in spice-gtk's bugzilla and also made a reference
>> to the very same bug in the code.
>>
>> [0]: https://bugs.freedesktop.org/show_bug.cgi?id=94662
>>
>> Signed-off-by: Fabiano Fidêncio
>
> Is it easy to reproduce the issue? Just use webdav channel? Is there a crash
> & backtrace?
There is no crash with the patch that I just pushed.
webdav channel is where it's really clear. Bur I'm pretty sure that
the fix done on 7774b8c0dd85ce2bb311d8bbe1c25deb73970b6e is for
exactly the same issue.
>
>> ---
>> src/vmcstream.c | 46 --
>> 1 file changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>> index 5ebf15a..4db94c7 100644
>> --- a/src/vmcstream.c
>> +++ b/src/vmcstream.c
>> @@ -97,6 +97,30 @@ spice_vmc_input_stream_new(void)
>> return self;
>> }
>>
>> +typedef struct _complete_in_idle_cb_data {
>> +GTask *task;
>> +gssize pos;
>> +} complete_in_idle_cb_data;
>> +
>> +static void
>> +complete_in_idle_cb_data_free (gpointer user_data)
>> +{
>> +complete_in_idle_cb_data *data = user_data;
>> +
>> +g_object_unref (data->task);
>> +g_free (data);
>> +}
>> +
>> +static gboolean
>> +complete_in_idle_cb(gpointer user_data)
>> +{
>> +complete_in_idle_cb_data *data = user_data;
>> +
>> +g_task_return_int(data->task, data->pos);
>> +
>> +return FALSE;
>> +}
>> +
>> /* coroutine */
>> /**
>> * Feed a SpiceVmc stream with new data from a coroutine
>> @@ -116,6 +140,9 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
>> self->coroutine = coroutine_self();
>>
>> while (size > 0) {
>> +complete_in_idle_cb_data *cb_data;
>> +GSource *source;
>> +
>> SPICE_DEBUG("spicevmc co_data %p", self->task);
>> if (!self->task)
>> coroutine_yield(NULL);
>> @@ -137,10 +164,25 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
>> *self,
>> if (self->all && min > 0 && self->pos != self->count)
>> continue;
>>
>> -g_task_return_int(self->task, self->pos);
>> +/* Somehow we are messing up with GTask return. As it's not known
>> + * for now if it's a spice-gtk issue due to the coroutine usage or
>> + * a GTask issue itself, let's do the idle ourself.
>> + *
>> + * For a future reference, see:
>> + * https://bugs.freedesktop.org/show_bug.cgi?id=94662
>> + */
>> +cb_data = g_new(complete_in_idle_cb_data , 1);
>> +cb_data->task = g_object_ref(self->task);
>> +cb_data->pos = self->pos;
>> +
>> +source = g_idle_source_new ();
>> +g_source_set_priority (source, G_PRIORITY_DEFAULT);
>> +g_source_set_callback (source, complete_in_idle_cb, cb_data,
>> complete_in_idle_cb_data_free);
>> +g_source_attach (source, g_task_get_context(self->task));
>> +g_source_set_name (source, "[spice-gtk] complete_in_idle_cb");
>> + g_source_unref (source);
>
> Why not use just g_idle_add()?
Hmm. Okay, works for me. I'll submit a v2.
>
>>
>> g_cancellable_disconnect(g_task_get_cancellable(self->task),
>> self->cancel_id);
>> -
>> g_clear_object(&self->task);
>> }
>>
>> --
>> 2.5.0
>>
>> ___
>> 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
--
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] vmcstream, gtask: Fix crash when trying to use webdav
On Tue, Mar 22, 2016 at 1:43 PM, Victor Toso wrote: > Interesting that I don't get the crash with remote-viewer, probably > because the shared-folder is not started by default as it is with spicy. > I thought it would be interesting to add here. > > Acked-by: Victor Toso Pushed, thanks! > > On Tue, Mar 22, 2016 at 01:01:30PM +0100, Pavel Grunt wrote: >> Ack. >> >> Pavel >> >> On Mon, 2016-03-21 at 16:02 +0100, Fabiano Fidêncio wrote: >> > Don't try to get the cancellable from a task that is, for sure >> > (ensured >> > by us), NULL. See the backtrace: >> > >> > #0 0x729cf250 in g_task_get_cancellable () at >> > /lib64/libgio-2.0.so.0 >> > #1 0x778a9012 in spice_vmc_input_stream_read_all_async >> > (stream=, buffer=0x7070f8, count=8, >> > io_priority=, cancellable=0x7fffcc00a470, >> > callback=0x7788c190 , user_data=0x707b30) at >> > vmcstream.c:187 >> > #2 0x7788c0ab in start_demux (self=0x707b30) at channel- >> > webdav.c:498 >> > #3 0x72700578 in g_closure_invoke () at /lib64/libgobject- >> > 2.0.so.0 >> > #4 0x727135f0 in signal_emit_unlocked_R () at >> > /lib64/libgobject-2.0.so.0 >> > #5 0x7271c43c in g_signal_emit_valist () at >> > /lib64/libgobject-2.0.so.0 >> > #6 0x7788acca in emit_main_context (opaque=0x7fffbfffe880) >> > at gio-coroutine.c:200 >> > #7 0x7242bab3 in g_main_context_dispatch () at >> > /lib64/libglib-2.0.so.0 >> > #8 0x7242be60 in g_main_context_iterate.isra () at >> > /lib64/libglib-2.0.so.0 >> > #9 0x7242c182 in g_main_loop_run () at /lib64/libglib- >> > 2.0.so.0 >> > #10 0x00405df6 in main (argc=, >> > argv=) at spicy.c:1897 >> > >> > It was pointed out during the review and, somehow, missed :-\ >> > >> > Signed-off-by: Fabiano Fidêncio >> > Signed-off-by: Pavel Grunt >> > --- >> > src/vmcstream.c | 2 -- >> > 1 file changed, 2 deletions(-) >> > >> > diff --git a/src/vmcstream.c b/src/vmcstream.c >> > index 48f9513..8a31a47 100644 >> > --- a/src/vmcstream.c >> > +++ b/src/vmcstream.c >> > @@ -184,7 +184,6 @@ >> > spice_vmc_input_stream_read_all_async(GInputStream*stream, >> > >> > /* no concurrent read permitted by ginputstream */ >> > g_return_if_fail(self->task == NULL); >> > -g_return_if_fail(g_task_get_cancellable(self->task) == NULL); >> > self->all = TRUE; >> > self->buffer = buffer; >> > self->count = count; >> > @@ -236,7 +235,6 @@ >> > spice_vmc_input_stream_read_async(GInputStream*stream, >> > >> > /* no concurrent read permitted by ginputstream */ >> > g_return_if_fail(self->task == NULL); >> > -g_return_if_fail(g_task_get_cancellable(self->task) == NULL); >> > self->all = FALSE; >> > self->buffer = buffer; >> > self->count = count; >> ___ >> 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 -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
