Re: [Spice-devel] [PATCH] spec: call semanage in posttrans not in post

2019-01-29 Thread Daniel P . Berrangé 
On Tue, Jan 29, 2019 at 06:40:32PM +0200, Uri Lublin wrote:
> It can happen that selinux-policy (targeted) is installed only after
> spice-streaming-agent (upon system installation). In that case
> running semanage in post scriptlet will fail.
> 
> In posttrans all packages are already installed, so it should be
> safe to call semanage at that point.
> 
> rhbz#1647789
> 
> Signed-off-by: Uri Lublin 
> ---
> 
> In a first patch I wrote I also added a condition that
> checks if selinuxenabled. If people feel it's better
> I'll send a V2 with it.
> 
> ---
>  spice-streaming-agent.spec.in | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in
> index 5a06e89..6b5ac22 100644
> --- a/spice-streaming-agent.spec.in
> +++ b/spice-streaming-agent.spec.in
> @@ -13,7 +13,7 @@ BuildRequires:  catch-devel
>  BuildRequires:  pkgconfig(udev)
>  # we need /usr/sbin/semanage program which is available on different
>  # packages depending on distribution
> -Requires(post): /usr/sbin/semanage
> +Requires(posttrans): /usr/sbin/semanage
>  Requires(postun): /usr/sbin/semanage
>  
>  %description
> @@ -45,7 +45,9 @@ if test -d "%{buildroot}/%{_libdir}/%{name}/plugins"; then
>  find %{buildroot}/%{_libdir}/%{name}/plugins -name '*.la' -delete
>  fi
>  
> -%post
> +# See rhbz#1647789 - call semanage in posttrans, not in post
> +# and https://fedoraproject.org/wiki/Packaging:Scriptlets
> +%posttrans
>  semanage fcontext -a -t xserver_exec_t %{_bindir}/spice-streaming-agent 
> 2>/dev/null || :
>  restorecon %{_bindir}/spice-streaming-agent || :

I'm curious why these commands are present at all ? The normal way to deal
with this would be to file a bug against the SELinux policy to explicitly
add the spice-streaming-agent binary to the default policy, so that RPM
will set the correct context at install time.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH] spec: call semanage in posttrans not in post

2019-01-29 Thread Uri Lublin 
It can happen that selinux-policy (targeted) is installed only after
spice-streaming-agent (upon system installation). In that case
running semanage in post scriptlet will fail.

In posttrans all packages are already installed, so it should be
safe to call semanage at that point.

rhbz#1647789

Signed-off-by: Uri Lublin 
---

In a first patch I wrote I also added a condition that
checks if selinuxenabled. If people feel it's better
I'll send a V2 with it.

---
 spice-streaming-agent.spec.in | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spice-streaming-agent.spec.in b/spice-streaming-agent.spec.in
index 5a06e89..6b5ac22 100644
--- a/spice-streaming-agent.spec.in
+++ b/spice-streaming-agent.spec.in
@@ -13,7 +13,7 @@ BuildRequires:  catch-devel
 BuildRequires:  pkgconfig(udev)
 # we need /usr/sbin/semanage program which is available on different
 # packages depending on distribution
-Requires(post): /usr/sbin/semanage
+Requires(posttrans): /usr/sbin/semanage
 Requires(postun): /usr/sbin/semanage
 
 %description
@@ -45,7 +45,9 @@ if test -d "%{buildroot}/%{_libdir}/%{name}/plugins"; then
 find %{buildroot}/%{_libdir}/%{name}/plugins -name '*.la' -delete
 fi
 
-%post
+# See rhbz#1647789 - call semanage in posttrans, not in post
+# and https://fedoraproject.org/wiki/Packaging:Scriptlets
+%posttrans
 semanage fcontext -a -t xserver_exec_t %{_bindir}/spice-streaming-agent 
2>/dev/null || :
 restorecon %{_bindir}/spice-streaming-agent || :
 
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH linux vdagent] Fix coding style

2019-01-29 Thread Jonathon Jongsma 
Use brackets everywhere.
---
 src/vdagent/device-info.c | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/vdagent/device-info.c b/src/vdagent/device-info.c
index 7c0f615..4983543 100644
--- a/src/vdagent/device-info.c
+++ b/src/vdagent/device-info.c
@@ -84,8 +84,9 @@ static int read_next_hex_number(const char *input, char 
delim, char **endptr)
 n = strtol(input, &endpos, 16);
 
 // check if we read all characters until the delimiter
-if (endpos != pos)
+if (endpos != pos) {
 endpos = NULL;
+}
 
 *endptr = endpos;
 return n;
@@ -95,15 +96,19 @@ static int read_next_hex_number(const char *input, char 
delim, char **endptr)
 // see https://wiki.xen.org/wiki/Bus:Device.Function_(BDF)_Notation
 static bool parse_pci_device(const char *bdf, const char *end, PciDevice 
*device)
 {
-if (!end) end = strchr(bdf, 0);
+if (!end) {
+end = strchr(bdf, 0);
+}
 
 int endpos = -1;
 int domain, bus, slot, function;
 sscanf(bdf, "%x:%x:%x.%x%n", &domain, &bus, &slot, &function, &endpos);
-if (!device || endpos < 0 || bdf + endpos != end)
+if (!device || endpos < 0 || bdf + endpos != end) {
 return false;
-if (domain < 0 || bus < 0 || slot < 0 || function < 0)
+}
+if (domain < 0 || bus < 0 || slot < 0 || function < 0) {
 return false;
+}
 
 device->domain = domain;
 device->bus = bus;
@@ -121,8 +126,9 @@ static bool parse_pci_device(const char *bdf, const char 
*end, PciDevice *device
 static PciAddress* parse_pci_address_from_sysfs_path(const char* addr)
 {
 char *pos = strstr(addr, "/pci");
-if (!pos)
+if (!pos) {
 return NULL;
+}
 
 // advance to the numbers in pci:00
 pos += 4;
@@ -157,8 +163,9 @@ static PciAddress* parse_pci_address_from_sysfs_path(const 
char* addr)
 static PciAddress* parse_pci_address_from_spice(char *input)
 {
 static const char prefix[] = "pci/";
-if (strncmp(input, prefix, strlen(prefix)) != 0)
+if (strncmp(input, prefix, strlen(prefix)) != 0) {
 return NULL;
+}
 
 char *pos = input + strlen(prefix);
 int domain = read_next_hex_number(pos, '/', &pos);
@@ -187,8 +194,9 @@ static PciAddress* parse_pci_address_from_spice(char *input)
 
 address->devices = g_list_append(address->devices, dev);
 pos = next;
-if (!pos)
+if (!pos) {
 break;
+}
 }
 return address;
 }
@@ -298,12 +306,14 @@ static void drm_conn_name_modesetting(drmModeConnector 
*conn, char *dest, size_t
 
 static bool read_hex_value_from_file(const char *path, int* value)
 {
-if (value == NULL || path == NULL)
+if (value == NULL || path == NULL) {
 return false;
+}
 
 FILE *f = fopen(path, "r");
-if (f == NULL)
+if (f == NULL) {
 return false;
+}
 
 int endpos = -1;
 bool result = (fscanf(f, "%x\n%n", value, &endpos) > 0 && endpos >= 0);
-- 
2.17.2

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH linux vdagent v5 1/7] Add lookup_xrand_output_for_device_info()

2019-01-29 Thread Jonathon Jongsma 
On Tue, 2019-01-29 at 10:17 -0500, Frediano Ziglio wrote:
> Can you use the coding style at least for new code?
> In particular always use brackets, code is half with and half without

Oh, I already pushed after Lukas's ACK. I'll send a follow-up patch
fixing some bracket usage. Not sure how I missed so many...


> 
> > 
> > Add a function to look up an xrandr output for a given device
> > display
> > id. This uses sysfs and the drm subsystem to lookup information
> > about a
> > graphics device output. It then compares the drm output name to
> > xrandr
> > output names to try to match that device output to an xrandr
> > output.
> > This is necesary for guests that have multiple graphics devices.
> > ---
> >  Makefile.am   |  14 ++
> >  configure.ac  |   1 +
> >  src/vdagent/device-info.c | 506
> > ++
> >  src/vdagent/device-info.h |  30 +++
> >  tests/test-device-info.c  | 262 
> >  5 files changed, 813 insertions(+)
> >  create mode 100644 src/vdagent/device-info.c
> >  create mode 100644 src/vdagent/device-info.h
> >  create mode 100644 tests/test-device-info.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 97b8bf0..c2e9668 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -14,6 +14,7 @@ common_sources =  \
> > $(NULL)
> >  
> >  src_spice_vdagent_CFLAGS = \
> > +   $(DRM_CFLAGS)   \
> > $(X_CFLAGS) \
> > $(SPICE_CFLAGS) \
> > $(GLIB2_CFLAGS) \
> > @@ -24,6 +25,7 @@ src_spice_vdagent_CFLAGS =
> > \
> > $(NULL)
> >  
> >  src_spice_vdagent_LDADD =  \
> > +   $(DRM_LIBS) \
> > $(X_LIBS)   \
> > $(SPICE_LIBS)   \
> > $(GLIB2_LIBS)   \
> > @@ -37,6 +39,8 @@ src_spice_vdagent_SOURCES =   
> > \
> > src/vdagent/audio.h \
> > src/vdagent/clipboard.c \
> > src/vdagent/clipboard.h \
> > +   src/vdagent/device-info.c   \
> > +   src/vdagent/device-info.h   \
> > src/vdagent/file-xfers.c\
> > src/vdagent/file-xfers.h\
> > src/vdagent/x11-priv.h  \
> > @@ -159,3 +163,13 @@ EXTRA_DIST =   
> > \
> >  DISTCHECK_CONFIGURE_FLAGS =\
> > --with-init-script=redhat   \
> > $(NULL)
> > +
> > +tests_test_device_info_LDADD = $(src_spice_vdagent_LDADD)
> > +tests_test_device_info_CFLAGS = $(src_spice_vdagent_CFLAGS)
> > +
> > +check_PROGRAMS =   \
> > +   tests/test-device-info  \
> > +   $(NULL)
> > +
> > +TESTS = $(check_PROGRAMS)
> > +
> > diff --git a/configure.ac b/configure.ac
> > index 7cb44db..55b031e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -105,6 +105,7 @@ PKG_CHECK_MODULES(X, [xfixes xrandr >= 1.3
> > xinerama x11])
> >  PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.13])
> >  PKG_CHECK_MODULES(ALSA, [alsa >= 1.0.22])
> >  PKG_CHECK_MODULES([DBUS], [dbus-1])
> > +PKG_CHECK_MODULES([DRM], [libdrm])
> >  
> >  if test "$with_session_info" = "auto" || test "$with_session_info"
> > =
> >  "systemd"; then
> >  PKG_CHECK_MODULES([LIBSYSTEMD_LOGIN],
> > diff --git a/src/vdagent/device-info.c b/src/vdagent/device-info.c
> > new file mode 100644
> > index 000..7c0f615
> > --- /dev/null
> > +++ b/src/vdagent/device-info.c
> > @@ -0,0 +1,506 @@
> > +/*  device-info.c utility function for looking up the xrandr
> > output id for a
> > + *  given device address and display id
> > + *
> > + * Copyright 2018 Red Hat, Inc.
> > + *
> > + * Red Hat Authors:
> > + * Jonathon Jongsma 
> > + *
> > + * This program is free software: you can redistribute it and/or
> > modify
> > + * it under the terms of the GNU General Public License as
> > published by
> > + * the Free Software Foundation, either version 3 of the License,
> > or
> > + * (at your option) any later version.
> > + *
> > + * This program 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 General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public
> > License
> > + * along with this program.  If not, see <
> > http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "device-info.h"
> > +
> > +#define PCI_VENDOR_ID_REDHAT 0x1b36
> > +#define PCI_VENDO

Re: [Spice-devel] [PATCH] catch: directory is now catch2

2019-01-29 Thread Frediano Ziglio 
> Upstream and since Fedora 27
> 
> Signed-off-by: Uri Lublin 

Does it work on RHEL7 ?

> ---
> 
> Another option is to check both catch/ and catch2/
> and pick the one that is installed on the system, if any
> 
> ---
>  configure.ac  | 4 ++--
>  src/unittests/test-mjpeg-fallback.cpp | 2 +-
>  src/unittests/test-stream-port.cpp| 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index c259f7e..321dea3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -119,9 +119,9 @@ case "$enable_tests" in
>*) AC_MSG_ERROR([bad value ${enable_tests} for enable-tests option]) ;;
>  esac
>  AS_IF([test "x$enable_tests" != "xno"],
> -  [AC_CHECK_HEADER([catch/catch.hpp],have_check="yes",)])
> +  [AC_CHECK_HEADER([catch2/catch.hpp],have_check="yes",)])
>  AS_IF([test "x$enable_tests" = "xyes" && test "x$have_check" != "xyes"],
> -  [AC_MSG_ERROR([Could not find Catch dependency header
> (catch/catch.hpp)])])
> +  [AC_MSG_ERROR([Could not find Catch dependency header
> (catch2/catch.hpp)])])
>  AM_CONDITIONAL([ENABLE_TESTS], [test "x$have_check" = "xyes"])
>  
>  AC_DEFINE_DIR([BINDIR], [bindir], [Where binaries are installed.])
> diff --git a/src/unittests/test-mjpeg-fallback.cpp
> b/src/unittests/test-mjpeg-fallback.cpp
> index e39dc49..6a73dbe 100644
> --- a/src/unittests/test-mjpeg-fallback.cpp
> +++ b/src/unittests/test-mjpeg-fallback.cpp
> @@ -1,5 +1,5 @@
>  #define CATCH_CONFIG_MAIN
> -#include "catch/catch.hpp"
> +#include "catch2/catch.hpp"
>  
>  #include "mjpeg-fallback.hpp"
>  
> diff --git a/src/unittests/test-stream-port.cpp
> b/src/unittests/test-stream-port.cpp
> index e7b7b89..6bae7fa 100644
> --- a/src/unittests/test-stream-port.cpp
> +++ b/src/unittests/test-stream-port.cpp
> @@ -5,7 +5,7 @@
>   */
>  
>  #define CATCH_CONFIG_MAIN
> -#include 
> +#include 
>  #include 
>  #include 
>  

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH] catch: directory is now catch2

2019-01-29 Thread Uri Lublin 
Upstream and since Fedora 27

Signed-off-by: Uri Lublin 
---

Another option is to check both catch/ and catch2/
and pick the one that is installed on the system, if any

---
 configure.ac  | 4 ++--
 src/unittests/test-mjpeg-fallback.cpp | 2 +-
 src/unittests/test-stream-port.cpp| 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index c259f7e..321dea3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -119,9 +119,9 @@ case "$enable_tests" in
   *) AC_MSG_ERROR([bad value ${enable_tests} for enable-tests option]) ;;
 esac
 AS_IF([test "x$enable_tests" != "xno"],
-  [AC_CHECK_HEADER([catch/catch.hpp],have_check="yes",)])
+  [AC_CHECK_HEADER([catch2/catch.hpp],have_check="yes",)])
 AS_IF([test "x$enable_tests" = "xyes" && test "x$have_check" != "xyes"],
-  [AC_MSG_ERROR([Could not find Catch dependency header 
(catch/catch.hpp)])])
+  [AC_MSG_ERROR([Could not find Catch dependency header 
(catch2/catch.hpp)])])
 AM_CONDITIONAL([ENABLE_TESTS], [test "x$have_check" = "xyes"])
 
 AC_DEFINE_DIR([BINDIR], [bindir], [Where binaries are installed.])
diff --git a/src/unittests/test-mjpeg-fallback.cpp 
b/src/unittests/test-mjpeg-fallback.cpp
index e39dc49..6a73dbe 100644
--- a/src/unittests/test-mjpeg-fallback.cpp
+++ b/src/unittests/test-mjpeg-fallback.cpp
@@ -1,5 +1,5 @@
 #define CATCH_CONFIG_MAIN
-#include "catch/catch.hpp"
+#include "catch2/catch.hpp"
 
 #include "mjpeg-fallback.hpp"
 
diff --git a/src/unittests/test-stream-port.cpp 
b/src/unittests/test-stream-port.cpp
index e7b7b89..6bae7fa 100644
--- a/src/unittests/test-stream-port.cpp
+++ b/src/unittests/test-stream-port.cpp
@@ -5,7 +5,7 @@
  */
 
 #define CATCH_CONFIG_MAIN
-#include 
+#include 
 #include 
 #include 
 
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH linux vdagent v5 1/7] Add lookup_xrand_output_for_device_info()

2019-01-29 Thread Frediano Ziglio 
Can you use the coding style at least for new code?
In particular always use brackets, code is half with and half without

> 
> Add a function to look up an xrandr output for a given device display
> id. This uses sysfs and the drm subsystem to lookup information about a
> graphics device output. It then compares the drm output name to xrandr
> output names to try to match that device output to an xrandr output.
> This is necesary for guests that have multiple graphics devices.
> ---
>  Makefile.am   |  14 ++
>  configure.ac  |   1 +
>  src/vdagent/device-info.c | 506 ++
>  src/vdagent/device-info.h |  30 +++
>  tests/test-device-info.c  | 262 
>  5 files changed, 813 insertions(+)
>  create mode 100644 src/vdagent/device-info.c
>  create mode 100644 src/vdagent/device-info.h
>  create mode 100644 tests/test-device-info.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 97b8bf0..c2e9668 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -14,6 +14,7 @@ common_sources =\
>   $(NULL)
>  
>  src_spice_vdagent_CFLAGS =   \
> + $(DRM_CFLAGS)   \
>   $(X_CFLAGS) \
>   $(SPICE_CFLAGS) \
>   $(GLIB2_CFLAGS) \
> @@ -24,6 +25,7 @@ src_spice_vdagent_CFLAGS =  \
>   $(NULL)
>  
>  src_spice_vdagent_LDADD =\
> + $(DRM_LIBS) \
>   $(X_LIBS)   \
>   $(SPICE_LIBS)   \
>   $(GLIB2_LIBS)   \
> @@ -37,6 +39,8 @@ src_spice_vdagent_SOURCES = \
>   src/vdagent/audio.h \
>   src/vdagent/clipboard.c \
>   src/vdagent/clipboard.h \
> + src/vdagent/device-info.c   \
> + src/vdagent/device-info.h   \
>   src/vdagent/file-xfers.c\
>   src/vdagent/file-xfers.h\
>   src/vdagent/x11-priv.h  \
> @@ -159,3 +163,13 @@ EXTRA_DIST = \
>  DISTCHECK_CONFIGURE_FLAGS =  \
>   --with-init-script=redhat   \
>   $(NULL)
> +
> +tests_test_device_info_LDADD = $(src_spice_vdagent_LDADD)
> +tests_test_device_info_CFLAGS = $(src_spice_vdagent_CFLAGS)
> +
> +check_PROGRAMS = \
> + tests/test-device-info  \
> + $(NULL)
> +
> +TESTS = $(check_PROGRAMS)
> +
> diff --git a/configure.ac b/configure.ac
> index 7cb44db..55b031e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -105,6 +105,7 @@ PKG_CHECK_MODULES(X, [xfixes xrandr >= 1.3 xinerama x11])
>  PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.13])
>  PKG_CHECK_MODULES(ALSA, [alsa >= 1.0.22])
>  PKG_CHECK_MODULES([DBUS], [dbus-1])
> +PKG_CHECK_MODULES([DRM], [libdrm])
>  
>  if test "$with_session_info" = "auto" || test "$with_session_info" =
>  "systemd"; then
>  PKG_CHECK_MODULES([LIBSYSTEMD_LOGIN],
> diff --git a/src/vdagent/device-info.c b/src/vdagent/device-info.c
> new file mode 100644
> index 000..7c0f615
> --- /dev/null
> +++ b/src/vdagent/device-info.c
> @@ -0,0 +1,506 @@
> +/*  device-info.c utility function for looking up the xrandr output id for a
> + *  given device address and display id
> + *
> + * Copyright 2018 Red Hat, Inc.
> + *
> + * Red Hat Authors:
> + * Jonathon Jongsma 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "device-info.h"
> +
> +#define PCI_VENDOR_ID_REDHAT 0x1b36
> +#define PCI_VENDOR_ID_REDHAT_QUMRANET 0x1af4 // virtio-gpu
> +#define PCI_VENDOR_ID_INTEL 0x8086
> +#define PCI_VENDOR_ID_NVIDIA 0x10de
> +
> +#define PCI_DEVICE_ID_QXL 0x0100
> +#define PCI_DEVICE_ID_VIRTIO_GPU 0x1050
> +
> +typedef struct PciDevice {
> +int domain;
> +uint8_t bus;
> +uint8_t slot;
> +uint8_t function;
> +} PciDevice;
> +
> +typedef struct PciAddress {
> +int domain;
> +GList *devices; /* PciDevice */
> +} PciAddress;
> +
> +static PciA

Re: [Spice-devel] [PATCH linux vdagent v5 0/7] Use the PCI addr and display ID

2019-01-29 Thread Lukáš Hrázký 
For the series:

Acked-by: Lukáš Hrázký 


On Wed, 2019-01-23 at 16:18 -0600, Jonathon Jongsma wrote:
> This is a patch set that handles the PCI address and device dispay ID
> sent down to the agent by the server, and uses that to maintain a map
> for looking up a particular xrandr output for a given spice display id.
> 
> This patch series builds on the patch from Lukas titled "Receive the
> graphics_device_info message".
> 
> Changes in v5:
>  - more line length changes
>  - squashed last several patches into one
> 
> Changes in v4:
>  - rebased on Lukas's latest patch
>  - Added a new commit that fixes the cursor when using with mjpeg streaming.
>  - various small fixes from previous reviews (e.g. line length, etc)
> 
> Changes in v3:
>  - Fixed a bug where the X Display was not set properly and caused a
>crash
>  - moved call to vdagent_x11_send_daemon_guest_xorg_res() to outside the
>loop to avoid sending multiple times when more than one device is
>received. (Patch 9)
> 
> Changes in v2:
>  - Removed some ACKed and merged patches
>- Fix typo in comment
>- Fix confusion between output index and crtc index
>  - added a patch to send new display IDs when device info changes
>  - multiple changes from review
>- factored out function find_device_at_pci_address()
>- No more initializing X/XRandr
>- removed goto
>- improved logging
>- etc.
> 
> 
> Jonathon Jongsma (7):
>   Add lookup_xrand_output_for_device_info()
>   Look up and store xrandr output in display map
>   Make clearer distinctions between output ids
>   Use guest output map to determine xrandr output
>   Factor a function out of get_current_mon_config()
>   Use new function in vdagent_x11_send_daemon_guest_xorg_res()
>   Send display_id down to the vdagentd daemon
> 
>  Makefile.am   |  14 ++
>  configure.ac  |   1 +
>  src/vdagent/device-info.c | 506 ++
>  src/vdagent/device-info.h |  30 +++
>  src/vdagent/x11-priv.h|   2 +-
>  src/vdagent/x11-randr.c   | 348 +-
>  src/vdagent/x11.c |  20 +-
>  src/vdagentd-proto.h  |   1 +
>  src/vdagentd/uinput.c |  23 +-
>  tests/test-device-info.c  | 262 
>  10 files changed, 1073 insertions(+), 134 deletions(-)
>  create mode 100644 src/vdagent/device-info.c
>  create mode 100644 src/vdagent/device-info.h
>  create mode 100644 tests/test-device-info.c
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-streaming-agent v5 5/6] Send the GraphicsDeviceInfo to the server

2019-01-29 Thread Lukáš Hrázký 
Adds serialization of the GraphicsDeviceInfo message and sends it to the
server when it starts to stream.

Signed-off-by: Lukáš Hrázký 
Acked-by: Jonathon Jongsma 
---
 configure.ac  |  2 +-
 src/spice-streaming-agent.cpp | 65 ++-
 2 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index fd18efe..c259f7e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -30,7 +30,7 @@ PKG_PROG_PKG_CONFIG
 dnl =
 dnl Check deps
 
-SPICE_PROTOCOL_MIN_VER=0.12.14
+SPICE_PROTOCOL_MIN_VER=0.12.16
 PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 
$SPICE_PROTOCOL_MIN_VER])
 AC_SUBST([SPICE_PROTOCOL_MIN_VER])
 
diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index cd23111..9507a54 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -93,6 +94,39 @@ public:
 }
 };
 
+class DeviceDisplayInfoMessage : public 
OutboundMessage
+{
+public:
+DeviceDisplayInfoMessage(const DeviceDisplayInfo &info) : 
OutboundMessage(info) {}
+
+static size_t size(const DeviceDisplayInfo &info)
+{
+return sizeof(PayloadType) +
+   std::min(info.device_address.length(), 
static_cast(max_device_address_len)) +
+   1;
+}
+
+void write_message_body(StreamPort &stream_port, const DeviceDisplayInfo 
&info)
+{
+std::string device_address = info.device_address;
+if (device_address.length() > max_device_address_len) {
+syslog(LOG_WARNING,
+   "device address of stream id %u is longer than %u bytes, 
trimming.",
+   info.stream_id, max_device_address_len);
+device_address = device_address.substr(0, max_device_address_len);
+}
+StreamMsgDeviceDisplayInfo strm_msg_info{};
+strm_msg_info.stream_id = info.stream_id;
+strm_msg_info.device_display_id = info.device_display_id;
+strm_msg_info.device_address_len = device_address.length() + 1;
+stream_port.write(&strm_msg_info, sizeof(strm_msg_info));
+stream_port.write(device_address.c_str(), device_address.length() + 1);
+}
+
+private:
+static constexpr uint32_t max_device_address_len = 255;
+};
+
 static bool streaming_requested = false;
 static bool quit_requested = false;
 static std::set client_codecs;
@@ -217,17 +251,30 @@ do_capture(StreamPort &stream_port, FrameLog &frame_log)
 throw std::runtime_error("cannot find a suitable capture system");
 }
 
+std::vector display_info;
 try {
-std::vector display_info = 
capture->get_device_display_info();
-syslog(LOG_DEBUG, "Got device info of %lu devices from the 
plugin", display_info.size());
-for (const auto &info : display_info) {
-syslog(LOG_DEBUG, "   id %u: device address %s, device display 
id: %u",
-   info.stream_id,
-   info.device_address.c_str(),
-   info.device_display_id);
-}
+display_info = capture->get_device_display_info();
 } catch (const Error &e) {
-syslog(LOG_ERR, "Error while getting device info: %s", e.what());
+syslog(LOG_ERR, "Error while getting device display info: %s", 
e.what());
+}
+
+syslog(LOG_DEBUG, "Got device info of %zu devices from the plugin", 
display_info.size());
+for (const auto &info : display_info) {
+syslog(LOG_DEBUG, "   stream id %u: device address: %s, device 
display id: %u",
+   info.stream_id,
+   info.device_address.c_str(),
+   info.device_display_id);
+}
+
+if (display_info.size() > 0) {
+if (display_info.size() > 1) {
+syslog(LOG_WARNING, "Warning: the Frame Capture plugin 
returned device display "
+   "info for more than one display device, but we 
currently only support "
+   "a single device. Sending information for first device 
to the server.");
+}
+stream_port.send(display_info[0]);
+} else {
+syslog(LOG_ERR, "Empty device display info from the plugin");
 }
 
 while (!quit_requested && streaming_requested) {
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] smartcard: do not keep weak ref when device is NULL

2019-01-29 Thread Frediano Ziglio 
> 
> When a client disconnects, smartcard_channel_client_set_char_device
> is called with a NULL "device" argument. In that case there is
> no need to take a weak reference to the device.
> 
> Without this patch the server complains:
>   g_object_add_weak_pointer: assertion 'G_IS_OBJECT (object)' failed
> 
> and aborts when a second client attempts to connect.
> 
> Signed-off-by: Uri Lublin 

Yes, not much sense to call g_object_add_weak_pointer on NULL.

Acked-by: Frediano Ziglio 

> ---
>  server/smartcard-channel-client.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/server/smartcard-channel-client.c
> b/server/smartcard-channel-client.c
> index 3b3fc27cc..49e765b7c 100644
> --- a/server/smartcard-channel-client.c
> +++ b/server/smartcard-channel-client.c
> @@ -389,8 +389,10 @@ void
> smartcard_channel_client_set_char_device(SmartCardChannelClient *scc,
>  }
>  
>  scc->priv->smartcard = device;
> -g_object_add_weak_pointer(G_OBJECT(scc->priv->smartcard),
> -  (gpointer*)&scc->priv->smartcard);
> +if (scc->priv->smartcard) {
> +g_object_add_weak_pointer(G_OBJECT(scc->priv->smartcard),
> +  (gpointer*)&scc->priv->smartcard);
> +}
>  }
>  
>  RedCharDeviceSmartcard*
>  smartcard_channel_client_get_char_device(SmartCardChannelClient *scc)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH 1/3] drm/irq: Don't check for DRIVER_HAVE_IRQ in drm_irq_(un)install

2019-01-29 Thread Daniel Vetter 
If a non-legacy driver calls these it's valid to assume there is
interrupt support. The flag is really only needed for legacy drivers,
which control IRQ enabling/disabling through the DRM_IOCTL_CONTROL
legacy IOCTL.

Also remove all the flag usage from non-legacy drivers.

v2: Review from Emil:
- improve commit message
- I forgot hibmc, fix that

Cc: linux-arm-ker...@lists.infradead.org
Cc: intel-...@lists.freedesktop.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
Reviewed-by: Emil Velikov 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
 drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 2 +-
 drivers/gpu/drm/drm_irq.c   | 6 --
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 2 +-
 drivers/gpu/drm/gma500/psb_drv.c| 2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 drivers/gpu/drm/meson/meson_drv.c   | 2 +-
 drivers/gpu/drm/msm/msm_drv.c   | 3 +--
 drivers/gpu/drm/mxsfb/mxsfb_drv.c   | 3 +--
 drivers/gpu/drm/qxl/qxl_drv.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
 drivers/gpu/drm/shmobile/shmob_drm_drv.c| 2 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
 drivers/gpu/drm/vc4/vc4_drv.c   | 1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
 drivers/staging/vboxvideo/vbox_drv.c| 2 +-
 18 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0c22bae0c736..22502417c18c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1189,7 +1189,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, 
unsigned int pipe,
 static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
-   DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
+   DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index e68935b80917..8fc0b884c428 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -229,7 +229,7 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
 DEFINE_DRM_GEM_CMA_FOPS(fops);
 
 static struct drm_driver hdlcd_driver = {
-   .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+   .driver_features = DRIVER_GEM |
   DRIVER_MODESET | DRIVER_PRIME |
   DRIVER_ATOMIC,
.irq_handler = hdlcd_irq,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 034a91112098..0be13eceedba 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -720,7 +720,7 @@ static void atmel_hlcdc_dc_irq_uninstall(struct drm_device 
*dev)
 DEFINE_DRM_GEM_CMA_FOPS(fops);
 
 static struct drm_driver atmel_hlcdc_dc_driver = {
-   .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+   .driver_features = DRIVER_GEM |
   DRIVER_MODESET | DRIVER_PRIME |
   DRIVER_ATOMIC,
.irq_handler = atmel_hlcdc_dc_irq_handler,
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 45a07652fa00..c5babb3e4752 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -103,9 +103,6 @@ int drm_irq_install(struct drm_device *dev, int irq)
int ret;
unsigned long sh_flags = 0;
 
-   if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
-   return -EOPNOTSUPP;
-
if (irq == 0)
return -EINVAL;
 
@@ -174,9 +171,6 @@ int drm_irq_uninstall(struct drm_device *dev)
bool irq_enabled;
int i;
 
-   if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
-   return -EOPNOTSUPP;
-
irq_enabled = dev->irq_enabled;
dev->irq_enabled = false;
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c 
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 54ace3436605..dfc73aade325 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -137,7 +137,7 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
 DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
 
 static struct drm_driver fsl_dcu_drm_driver = {
-   .driver_features= DRIVER_HAVE_IRQ | DRIVER_

Re: [Spice-devel] [vdagent-linux] vdagent: Silently ignore missing spicevmc device

2019-01-29 Thread Christophe Fergeau 
On Tue, Jan 29, 2019 at 06:05:35AM -0500, Frediano Ziglio wrote:
> > 
> > On most distros, spice-vdagent will be autostarted as part of the
> > startup of the desktop environment session. This is done by
> > spice-vdagent.desktop, which has no way of checking if we are in a virt
> > environment with the needed devices present.
> > 
> > Currently, if /dev/virtio-ports/com.redhat.spice.0 is missing, we log an
> > error in syslog, and exit with an error exit code. This is too noisy
> > when autostarting it on a bare metal machine which have no use for
> > spice-vdagent. This reverts 0159111b to get rid of these warnings in the
> > session's logs
> > 
> > https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/12
> > 
> > Signed-off-by: Christophe Fergeau 
> 
> Sounds reasonable.
> 
> > ---
> >  src/vdagent/vdagent.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > index 90247f9..dd89aa4 100644
> > --- a/src/vdagent/vdagent.c
> > +++ b/src/vdagent/vdagent.c
> > @@ -451,8 +451,8 @@ int main(int argc, char *argv[])
> >  LOG_USER);
> >  
> >  if (file_test(portdev) != 0) {
> > -syslog(LOG_ERR, "Cannot access vdagent virtio channel %s", 
> > portdev);
> > -return 1;
> > +g_print("vdagent virtio channel %s is not available, exiting\n",
> > portdev);
> 
> The "is not available" is weird. The file_test (quite misleading name too,
> what is testing? existence? access?) returns if we can get statistics
> information, more or less I assume we are checking the existence of the
> file (can also returns failure if we don't have directory access),
> maybe "vdagent virtio channel %s does not exist, exiting\n" ?

Fine with me.

> 
> OT: why not using "access" function?

I was wondering exactly the same, I'll look into it.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [vdagent-linux] vdagent: Silently ignore missing spicevmc device

2019-01-29 Thread Frediano Ziglio 
> 
> On most distros, spice-vdagent will be autostarted as part of the
> startup of the desktop environment session. This is done by
> spice-vdagent.desktop, which has no way of checking if we are in a virt
> environment with the needed devices present.
> 
> Currently, if /dev/virtio-ports/com.redhat.spice.0 is missing, we log an
> error in syslog, and exit with an error exit code. This is too noisy
> when autostarting it on a bare metal machine which have no use for
> spice-vdagent. This reverts 0159111b to get rid of these warnings in the
> session's logs
> 
> https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/12
> 
> Signed-off-by: Christophe Fergeau 

Sounds reasonable.

> ---
>  src/vdagent/vdagent.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index 90247f9..dd89aa4 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -451,8 +451,8 @@ int main(int argc, char *argv[])
>  LOG_USER);
>  
>  if (file_test(portdev) != 0) {
> -syslog(LOG_ERR, "Cannot access vdagent virtio channel %s", portdev);
> -return 1;
> +g_print("vdagent virtio channel %s is not available, exiting\n",
> portdev);

The "is not available" is weird. The file_test (quite misleading name too,
what is testing? existence? access?) returns if we can get statistics
information, more or less I assume we are checking the existence of the
file (can also returns failure if we don't have directory access),
maybe "vdagent virtio channel %s does not exist, exiting\n" ?

OT: why not using "access" function?

> +return 0;
>  }
>  
>  if (do_daemonize)

Otherwise,
 Acked-by: Frediano Ziglio 

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH] smartcard: do not keep weak ref when device is NULL

2019-01-29 Thread Uri Lublin 
When a client disconnects, smartcard_channel_client_set_char_device
is called with a NULL "device" argument. In that case there is
no need to take a weak reference to the device.

Without this patch the server complains:
  g_object_add_weak_pointer: assertion 'G_IS_OBJECT (object)' failed

and aborts when a second client attempts to connect.

Signed-off-by: Uri Lublin 
---
 server/smartcard-channel-client.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/server/smartcard-channel-client.c 
b/server/smartcard-channel-client.c
index 3b3fc27cc..49e765b7c 100644
--- a/server/smartcard-channel-client.c
+++ b/server/smartcard-channel-client.c
@@ -389,8 +389,10 @@ void 
smartcard_channel_client_set_char_device(SmartCardChannelClient *scc,
 }
 
 scc->priv->smartcard = device;
-g_object_add_weak_pointer(G_OBJECT(scc->priv->smartcard),
-  (gpointer*)&scc->priv->smartcard);
+if (scc->priv->smartcard) {
+g_object_add_weak_pointer(G_OBJECT(scc->priv->smartcard),
+  (gpointer*)&scc->priv->smartcard);
+}
 }
 
 RedCharDeviceSmartcard* 
smartcard_channel_client_get_char_device(SmartCardChannelClient *scc)
-- 
2.20.1

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-common 2/2] log: remove deprecated SPICE_DEBUG_LEVEL support

2019-01-29 Thread Frediano Ziglio 
This feature was marked obsolete by efd1d3cb4d8eee more than
three years ago.

Signed-off-by: Frediano Ziglio 
---
 common/log.c | 86 
 tests/test-logging.c | 67 --
 2 files changed, 153 deletions(-)

diff --git a/common/log.c b/common/log.c
index 5819974..837930a 100644
--- a/common/log.c
+++ b/common/log.c
@@ -32,94 +32,12 @@
 #include "log.h"
 #include "backtrace.h"
 
-static int glib_debug_level = INT_MAX;
 static const int abort_mask = G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR;
 
 #define G_LOG_DOMAIN "Spice"
 
-typedef enum {
-SPICE_LOG_LEVEL_ERROR,
-SPICE_LOG_LEVEL_CRITICAL,
-SPICE_LOG_LEVEL_WARNING,
-SPICE_LOG_LEVEL_INFO,
-SPICE_LOG_LEVEL_DEBUG,
-} SpiceLogLevel;
-
-static GLogLevelFlags spice_log_level_to_glib(SpiceLogLevel level)
-{
-static const GLogLevelFlags glib_levels[] = {
-[ SPICE_LOG_LEVEL_ERROR ] = G_LOG_LEVEL_ERROR,
-[ SPICE_LOG_LEVEL_CRITICAL ] = G_LOG_LEVEL_CRITICAL,
-[ SPICE_LOG_LEVEL_WARNING ] = G_LOG_LEVEL_WARNING,
-[ SPICE_LOG_LEVEL_INFO ] = G_LOG_LEVEL_INFO,
-[ SPICE_LOG_LEVEL_DEBUG ] = G_LOG_LEVEL_DEBUG,
-};
-g_return_val_if_fail (level >= 0, G_LOG_LEVEL_ERROR);
-g_return_val_if_fail (level < G_N_ELEMENTS(glib_levels), 
G_LOG_LEVEL_DEBUG);
-
-return glib_levels[level];
-}
-
-static void spice_log_set_debug_level(void)
-{
-if (glib_debug_level == INT_MAX) {
-const char *debug_str = g_getenv("SPICE_DEBUG_LEVEL");
-if (debug_str != NULL) {
-int debug_level;
-char *debug_env;
-
-/* FIXME: To be removed after enough deprecation time */
-g_warning("Setting SPICE_DEBUG_LEVEL is deprecated, use 
G_MESSAGES_DEBUG instead");
-debug_level = atoi(debug_str);
-if (debug_level > SPICE_LOG_LEVEL_DEBUG) {
-debug_level = SPICE_LOG_LEVEL_DEBUG;
-}
-glib_debug_level = spice_log_level_to_glib(debug_level);
-
-/* If the debug level is too high, make sure we don't try to enable
- * display of glib debug logs */
-if (debug_level < SPICE_LOG_LEVEL_INFO)
-return;
-
-/* Make sure GLib default log handler will show the debug 
messages. Messing with
- * environment variables like this is ugly, but this only happens 
when the legacy
- * SPICE_DEBUG_LEVEL is used
- */
-debug_env = (char *)g_getenv("G_MESSAGES_DEBUG");
-if (debug_env == NULL) {
-g_setenv("G_MESSAGES_DEBUG", G_LOG_DOMAIN, FALSE);
-} else {
-debug_env = g_strconcat(debug_env, " ", G_LOG_DOMAIN, NULL);
-g_setenv("G_MESSAGES_DEBUG", G_LOG_DOMAIN, FALSE);
-g_free(debug_env);
-}
-}
-}
-}
-
-static void spice_logger(const gchar *log_domain,
- GLogLevelFlags log_level,
- const gchar *message,
- gpointer user_data G_GNUC_UNUSED)
-{
-if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
-return; // do not print anything
-}
-g_log_default_handler(log_domain, log_level, message, NULL);
-}
-
 SPICE_CONSTRUCTOR_FUNC(spice_log_init)
 {
-
-spice_log_set_debug_level();
-if (glib_debug_level != INT_MAX) {
-/* If SPICE_DEBUG_LEVEL is set, we need a custom handler, which is
- * going to break use of g_log_set_default_handler() by apps
- */
-g_log_set_handler(G_LOG_DOMAIN,
-  G_LOG_LEVEL_MASK | G_LOG_FLAG_FATAL | 
G_LOG_FLAG_RECURSION,
-  spice_logger, NULL);
-}
 /* Threading is always enabled from 2.31.0 onwards */
 /* Our logging is potentially used from different threads.
  * Older glibs require that g_thread_init() is called when
@@ -140,10 +58,6 @@ static void spice_logv(const char *log_domain,
 {
 GString *log_msg;
 
-if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level) {
-return; // do not print anything
-}
-
 log_msg = g_string_new(NULL);
 if (strloc && function) {
 g_string_append_printf(log_msg, "%s:%s: ", strloc, function);
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 3b17f44..ff2d8bd 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -217,71 +217,6 @@ static void test_log_levels(void)
 g_test_trap_assert_stdout_unmatched("*other_debug*");
 }
 
-/* Checks that SPICE_DEBUG_LEVEL impacts spice_debug(), g_debug() but not 
other_debug() */
-static void test_spice_debug_level(void)
-{
-if (g_test_subprocess()) {
-/* g_test_expected_message only checks whether the appropriate 
messages got up to g_log()
- * The following calls will be caught by the parent process to check 
what was (not) printed
- * to stdout/stderr
- */
-spice_info("spice_i

[Spice-devel] [PATCH spice-common 1/2] log: remove deprecated SPICE_ABORT_LEVEL support

2019-01-29 Thread Frediano Ziglio 
This feature was marked obsolete by efd1d3cb4d8eee more than
three years ago.

Signed-off-by: Frediano Ziglio 
---
 common/log.c | 30 +-
 tests/test-logging.c | 39 +--
 2 files changed, 2 insertions(+), 67 deletions(-)

diff --git a/common/log.c b/common/log.c
index a7806c5..5819974 100644
--- a/common/log.c
+++ b/common/log.c
@@ -33,11 +33,7 @@
 #include "backtrace.h"
 
 static int glib_debug_level = INT_MAX;
-static int abort_mask = 0;
-
-#ifndef SPICE_ABORT_MASK_DEFAULT
-#define SPICE_ABORT_MASK_DEFAULT (G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR)
-#endif
+static const int abort_mask = G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_ERROR;
 
 #define G_LOG_DOMAIN "Spice"
 
@@ -101,29 +97,6 @@ static void spice_log_set_debug_level(void)
 }
 }
 
-static void spice_log_set_abort_level(void)
-{
-if (abort_mask == 0) {
-const char *abort_str = g_getenv("SPICE_ABORT_LEVEL");
-if (abort_str != NULL) {
-GLogLevelFlags glib_abort_level;
-
-/* FIXME: To be removed after enough deprecation time */
-g_warning("Setting SPICE_ABORT_LEVEL is deprecated, use G_DEBUG 
instead");
-glib_abort_level = spice_log_level_to_glib(atoi(abort_str));
-unsigned int fatal_mask = G_LOG_FATAL_MASK;
-while (glib_abort_level >= G_LOG_LEVEL_ERROR) {
-fatal_mask |= glib_abort_level;
-glib_abort_level >>= 1;
-}
-abort_mask = fatal_mask;
-g_log_set_fatal_mask(G_LOG_DOMAIN, fatal_mask);
-} else {
-abort_mask = SPICE_ABORT_MASK_DEFAULT;
-}
-}
-}
-
 static void spice_logger(const gchar *log_domain,
  GLogLevelFlags log_level,
  const gchar *message,
@@ -139,7 +112,6 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
 {
 
 spice_log_set_debug_level();
-spice_log_set_abort_level();
 if (glib_debug_level != INT_MAX) {
 /* If SPICE_DEBUG_LEVEL is set, we need a custom handler, which is
  * going to break use of g_log_set_default_handler() by apps
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 559d656..3b17f44 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -44,37 +44,6 @@ LOG_OTHER_HELPER(warning, WARNING)
 LOG_OTHER_HELPER(critical, CRITICAL)
 
 #if GLIB_CHECK_VERSION(2,38,0)
-/* Checks that spice_warning() aborts after changing SPICE_ABORT_LEVEL */
-static void test_spice_abort_level(void)
-{
-if (g_test_subprocess()) {
-spice_warning("spice_warning");
-return;
-}
-/* 2 = SPICE_LOG_LEVEL_WARNING  */
-g_setenv("SPICE_ABORT_LEVEL", "2", TRUE);
-g_test_trap_subprocess(NULL, 0, 0);
-g_unsetenv("SPICE_ABORT_LEVEL");
-g_test_trap_assert_failed();
-g_test_trap_assert_stderr("*SPICE_ABORT_LEVEL*deprecated*");
-g_test_trap_assert_stderr("*spice_warning*");
-}
-
-/* Checks that g_warning() aborts after changing SPICE_ABORT_LEVEL */
-static void test_spice_abort_level_g_warning(void)
-{
-if (g_test_subprocess()) {
-g_warning("g_warning");
-return;
-}
-g_setenv("SPICE_ABORT_LEVEL", "2", TRUE);
-g_test_trap_subprocess(NULL, 0, 0);
-g_unsetenv("SPICE_ABORT_LEVEL");
-g_test_trap_assert_failed();
-g_test_trap_assert_stderr("*SPICE_ABORT_LEVEL*deprecated*");
-g_test_trap_assert_stderr("*g_warning*");
-}
-
 /* Checks that spice_warning() aborts after setting G_DEBUG=fatal-warnings */
 static void test_spice_fatal_warning(void)
 {
@@ -283,7 +252,6 @@ static void test_spice_debug_level_warning(void)
 spice_info("spice_info");
 spice_debug("spice_debug");
 spice_warning("spice_warning");
-spice_critical("spice_critical");
 g_debug("g_debug");
 g_info("g_info");
 g_message("g_message");
@@ -298,15 +266,12 @@ static void test_spice_debug_level_warning(void)
 return;
 }
 
-g_setenv("SPICE_ABORT_LEVEL", "0", TRUE);
 g_setenv("SPICE_DEBUG_LEVEL", "1", TRUE);
 g_test_trap_subprocess(NULL, 0, 0);
-g_unsetenv("SPICE_ABORT_LEVEL");
 g_unsetenv("SPICE_DEBUG_LEVEL");
 g_test_trap_assert_passed();
 g_test_trap_assert_stderr("*SPICE_DEBUG_LEVEL*deprecated*");
-g_test_trap_assert_stderr("*SPICE_ABORT_LEVEL*deprecated*");
-
g_test_trap_assert_stderr("*spice_critical\n*g_critical\n*other_message\n*other_warning\n*other_critical\n");
+
g_test_trap_assert_stderr("*g_critical\n*other_message\n*other_warning\n*other_critical\n");
 g_test_trap_assert_stdout_unmatched("*spice_info*");
 g_test_trap_assert_stdout_unmatched("*spice_debug*");
 g_test_trap_assert_stderr_unmatched("*spice_warning*");
@@ -393,8 +358,6 @@ int main(int argc, char **argv)
 g_log_set_always_fatal(fatal_mask & G_LOG_LEVEL_MASK);
 
 #if GLIB_CHECK_VERSION(2,38,0)
-g_test_add_func("/spice-common/spice-abort-level", test_spice_abort_level);
-g_test_a

[Spice-devel] Compression efficiency (was: image-encoders: Initialize Zlib lazily)

2019-01-29 Thread Frediano Ziglio 
About these compression and encodings.

How is ZLib compared to Glz. Glz is really demanding in terms of memory
resources (keep drawable to be freed and the hash table is really huge too).
Is it worth having it?
Is also quite tricky/complicated as is shared amongst different DisplayChannel
connection which could lead to issues.
But if a more simpler Zlib have the same gains without all the complication
maybe would be worth removing Glz.

> 
> Zlib structure take up more than 1MB and it is rarely used nowadays
> as it is not much effective.
> Initialise it only when necessary saving some memory in the normal
> case.
> 
> Signed-off-by: Frediano Ziglio 

...
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH] usbredirserver: Make compile under MacOS

2019-01-29 Thread Christophe Fergeau 
On Mon, Jan 28, 2019 at 02:28:36PM -0500, Frediano Ziglio wrote:
> > On Wed, Jan 23, 2019 at 10:09:25AM +, Frediano Ziglio wrote:
> > > This fixes Gitlab issue #9
> > > (cfr https://gitlab.freedesktop.org/spice/usbredir/issues/9).
> > > 
> > > Signed-off-by: Frediano Ziglio 
> > > ---
> > >  usbredirserver/usbredirserver.c | 7 +++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/usbredirserver/usbredirserver.c
> > > b/usbredirserver/usbredirserver.c
> > > index 6aa2740..429985a 100644
> > > --- a/usbredirserver/usbredirserver.c
> > > +++ b/usbredirserver/usbredirserver.c
> > > @@ -43,6 +43,13 @@
> > >  
> > >  #define SERVER_VERSION "usbredirserver " PACKAGE_VERSION
> > >  
> > > +#if !defined(SOL_TCP) && defined(IPPROTO_TCP)
> > > +#define SOL_TCP IPPROTO_TCP
> > > +#endif
> > > +#if !defined(TCP_KEEPIDLE) && defined(TCP_KEEPALIVE)
> > > +#define TCP_KEEPIDLE TCP_KEEPALIVE
> > > +#endif
> > 
> > Might be better to restrict this to OSX in case a system decides to
> > use TCP_KEEPALIVE, but with a different meaning?
> > 
> 
> So
> 
> #if !defined(TCP_KEEPIDLE) && defined(TCP_KEEPALIVE) && defined(__APPLE__)

Yep, or even just #if defined(__APPLE__)

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v2] drm/qxl: use ttm_tt

2019-01-29 Thread Noralf Trønnes 


Den 29.01.2019 09.25, skrev Gerd Hoffmann:
> qxl device will not dma, so we don't need ttm_dma_tt.  Go use ttm_tt
> instead, to avoid wasting resources (swiotlb bounce buffers for
> example).
> 
> Signed-off-by: Gerd Hoffmann 
> ---

Acked-by: Noralf Trønnes 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH v2] drm/qxl: use ttm_tt

2019-01-29 Thread Gerd Hoffmann 
qxl device will not dma, so we don't need ttm_dma_tt.  Go use ttm_tt
instead, to avoid wasting resources (swiotlb bounce buffers for
example).

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 36ea993aac..92f5db5b29 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -204,7 +204,7 @@ static void qxl_ttm_io_mem_free(struct ttm_bo_device *bdev,
  * TTM backend functions.
  */
 struct qxl_ttm_tt {
-   struct ttm_dma_tt   ttm;
+   struct ttm_tt   ttm;
struct qxl_device   *qdev;
u64 offset;
 };
@@ -233,7 +233,7 @@ static void qxl_ttm_backend_destroy(struct ttm_tt *ttm)
 {
struct qxl_ttm_tt *gtt = (void *)ttm;
 
-   ttm_dma_tt_fini(>t->ttm);
+   ttm_tt_fini(>t->ttm);
kfree(gtt);
 }
 
@@ -253,13 +253,13 @@ static struct ttm_tt *qxl_ttm_tt_create(struct 
ttm_buffer_object *bo,
gtt = kzalloc(sizeof(struct qxl_ttm_tt), GFP_KERNEL);
if (gtt == NULL)
return NULL;
-   gtt->ttm.ttm.func = &qxl_backend_func;
+   gtt->ttm.func = &qxl_backend_func;
gtt->qdev = qdev;
-   if (ttm_dma_tt_init(>t->ttm, bo, page_flags)) {
+   if (ttm_tt_init(>t->ttm, bo, page_flags)) {
kfree(gtt);
return NULL;
}
-   return >t->ttm.ttm;
+   return >t->ttm;
 }
 
 static void qxl_move_null(struct ttm_buffer_object *bo,
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel