On Thu, 2018-02-01 at 16:40 +0000, Frediano Ziglio wrote:
> This is required so the daemon can manage the various sessions.
At first glance, this looks like a really big hack. At second glance, I
understand why you did it... (doesn't mean I like it now ;)) But in any
case this deserves a much better description... well, preferably in the
code, not (only) commit message.
Also, I would really like to think of a better way of doing this. Seems
the streaming agent is starting to have quite a lot of moving parts...
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> data/spice-streaming.desktop.in | 3 +--
> spice-streaming-agent.spec.in | 1 +
> src/Makefile.am | 5 +++++
> src/spice-streaming-helper | 24 ++++++++++++++++++++++++
> 4 files changed, 31 insertions(+), 2 deletions(-)
> create mode 100755 src/spice-streaming-helper
>
> diff --git a/data/spice-streaming.desktop.in b/data/spice-streaming.desktop.in
> index dcb30a3..5e4e863 100644
> --- a/data/spice-streaming.desktop.in
> +++ b/data/spice-streaming.desktop.in
> @@ -1,8 +1,7 @@
> [Desktop Entry]
> Name=SPICE Streaming Agent
> Comment=Agent for Streaming the framebuffer to Spice
> -Exec=@BINDIR@/spice-streaming-agent
> -#TryExec=/usr/bin/spice-streaming-agent
> +Exec=@BINDIR@/spice-streaming-helper
> Terminal=false
> Type=Application
> Categories=
> diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in
> index eef8f90..54a3598 100644
> --- a/spice-streaming-agent.spec.in
> +++ b/spice-streaming-agent.spec.in
> @@ -54,6 +54,7 @@ fi
> %doc COPYING ChangeLog README
> %{_udevrulesdir}/90-spice-guest-streaming.rules
> %{_bindir}/spice-streaming-agent
> +%{_bindir}/spice-streaming-helper
> %{_sysconfdir}/xdg/autostart/spice-streaming.desktop
> %{_datadir}/gdm/greeter/autostart/spice-streaming.desktop
> /usr/lib/systemd/system/spice-streaming-agent.socket
Does this add dependency on Python 3? Should it be added somewhere in
this file?
> diff --git a/src/Makefile.am b/src/Makefile.am
> index d2f8a47..9e9c7e7 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -5,6 +5,7 @@
>
> NULL =
> SUBDIRS = . unittests
> +EXTRA_DIST =
>
> AM_CPPFLAGS = \
> -DSPICE_STREAMING_AGENT_PROGRAM \
> @@ -61,3 +62,7 @@ spice_streaming_agent_SOURCES = \
> daemon.cpp \
> daemon.h \
> $(NULL)
> +
> +helperdir = $(bindir)
> +helper_SCRIPTS = spice-streaming-helper
> +EXTRA_DIST += spice-streaming-helper
> diff --git a/src/spice-streaming-helper b/src/spice-streaming-helper
> new file mode 100755
> index 0000000..beba559
> --- /dev/null
> +++ b/src/spice-streaming-helper
> @@ -0,0 +1,24 @@
> +#!/usr/bin/env python3
> +import os
> +import socket
> +import sys
> +import struct
> +
I am not sure how mandatory this actually is in Python, but I would
prefer to have this here:
if __name__ == "__main__":
main()
> +sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
> +
> +server_address = "\0spice-streaming-agent-daemon"
> +def code(var, type):
Like... it's pretty obvious and the code is local here, but still...
name the function properly? :)
> + val = os.environ.get(var, '').encode('utf-8')
> + return struct.pack('BB', type, len(val)) + val
> +
> +display = code('DISPLAY', 1)
> +xauthority = code('XAUTHORITY', 2)
> +
> +try:
> + sock.connect(server_address)
> + data = struct.pack('B', 1) + display + xauthority
> + sock.send(data)
> + sock.recv(10)
What's the recv(10) for, actually?
> +except socket.error as msg:
> + print(msg, file=sys.stderr)
> + sys.exit(1)
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel