Re: [Spice-devel] [PATCH spice-gtk v2] build-sys: Make git-version-gen work again with Meson
>
> d0cbd9618f0b removed the ability to use git-version-gen to generate
> proper version string.
> Generate .tarball-version file in the distribution file to allow
> building from tarball.
> Do not use MESON_SOURCE_ROOT when calling git-version-gen command as
> this won't be expanded.
> Change directory in git-version-gen script to allow the script to be
> called from a different directory.
Sorry, removed this last sentence.
>
> Signed-off-by: Frediano Ziglio
> ---
> meson.build | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Chnges since v1:
> - do not change git-version-gen, not necessary
> - typo in commit message
>
> diff --git a/meson.build b/meson.build
> index 5f39ff4c..8e540c92 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2,13 +2,15 @@
> # project definition
> #
> project('spice-gtk', 'c',
> - version : '0.36',
> + version : run_command('build-aux/git-version-gen',
> '@0@/.tarball-version'.format(meson.source_root()), check :
> true).stdout().strip(),
> license : 'LGPLv2.1',
> meson_version : '>= 0.49')
>
> message('Updating submodules')
> run_command('build-aux/meson/check-spice-common', check : true)
>
> +meson.add_dist_script('sh', '-c', 'echo
> @0@>"$MESON_DIST_ROOT/.tarball-version"'.format(meson.project_version()))
> +
> #
> # global C defines
> #
> --
> 2.20.1
>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-gtk v2] build-sys: Make git-version-gen work again with Meson
d0cbd9618f0b removed the ability to use git-version-gen to generate
proper version string.
Generate .tarball-version file in the distribution file to allow
building from tarball.
Do not use MESON_SOURCE_ROOT when calling git-version-gen command as
this won't be expanded.
Change directory in git-version-gen script to allow the script to be
called from a different directory.
Signed-off-by: Frediano Ziglio
---
meson.build | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Chnges since v1:
- do not change git-version-gen, not necessary
- typo in commit message
diff --git a/meson.build b/meson.build
index 5f39ff4c..8e540c92 100644
--- a/meson.build
+++ b/meson.build
@@ -2,13 +2,15 @@
# project definition
#
project('spice-gtk', 'c',
- version : '0.36',
+ version : run_command('build-aux/git-version-gen',
'@0@/.tarball-version'.format(meson.source_root()), check :
true).stdout().strip(),
license : 'LGPLv2.1',
meson_version : '>= 0.49')
message('Updating submodules')
run_command('build-aux/meson/check-spice-common', check : true)
+meson.add_dist_script('sh', '-c', 'echo
@0@>"$MESON_DIST_ROOT/.tarball-version"'.format(meson.project_version()))
+
#
# global C defines
#
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk] build-sys: Make git-version-gen work again with Meson
> On Wed, Jan 23, 2019 at 09:48:43AM +, Frediano Ziglio wrote:
> > d0cbd9618f0b removed the ability to use git-version-gen to
> > generate proper version string.
> > Generate .tarball-version file in the distribution file to
> > allow building from tarball.
> > Do not use MESON_SOURCE_ROOT calling git-version-gen command
> > as this won't be expanded.
>
> "when calling" I think.
>
> > Change directory in git-version-gen script to allow the
> > script to be called from a different directory.
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > build-aux/git-version-gen | 1 +
> > meson.build | 4 +++-
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
> > index 5617eb8d..00c4a55a 100755
> > --- a/build-aux/git-version-gen
> > +++ b/build-aux/git-version-gen
> > @@ -95,6 +95,7 @@ then
> > && echo "$0: WARNING: $tarball_version_file seems to be damaged" 1>&2
> > fi
> >
> > +cd "`dirname $0`/.."
> > if test -n "$v"
> > then
> > : # use $v
>
> I don't know for sure what the canonical source is for git-version-gen,
> but we should avoid making local changes to our copy.
> I found
> https://git.savannah.gnu.org/cgit/gnulib.git/tree/build-aux/git-version-gen
> which seems much newer than our copy.
>
Just tested, the patch works even without changing git-version-gen script,
I'll send a new one
>
> I did not test it, but the changes look reasonable.
>
> Christophe
>
> > diff --git a/meson.build b/meson.build
> > index 70dd3188..dcfa4763 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2,13 +2,15 @@
> > # project definition
> > #
> > project('spice-gtk', 'c',
> > - version : '0.36',
> > + version : run_command('build-aux/git-version-gen',
> > '@0@/.tarball-version'.format(meson.source_root()), check :
> > true).stdout().strip(),
> > license : 'LGPLv2.1',
> > meson_version : '>= 0.49')
> >
> > message('Updating submodules')
> > run_command('build-aux/meson/check-spice-common', check : true)
> >
> > +meson.add_dist_script('sh', '-c', 'echo
> > @0@>"$MESON_DIST_ROOT/.tarball-version"'.format(meson.project_version()))
> > +
> > #
> > # global C defines
> > #
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server v2] image-encoders: Initialize Zlib lazily
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
---
server/image-encoders.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
Changes since v1:
- log a warning if zlib encoding cannot be initialized
diff --git a/server/image-encoders.c b/server/image-encoders.c
index 88073a3e..f35621ff 100644
--- a/server/image-encoders.c
+++ b/server/image-encoders.c
@@ -451,12 +451,6 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
{
enc->zlib_data.usr.more_space = zlib_usr_more_space;
enc->zlib_data.usr.more_input = zlib_usr_more_input;
-
-enc->zlib = zlib_encoder_create(&enc->zlib_data.usr,
ZLIB_DEFAULT_COMPRESSION_LEVEL);
-
-if (!enc->zlib) {
-spice_critical("create zlib encoder failed");
-}
}
void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
*shared_data)
@@ -494,8 +488,10 @@ void image_encoders_free(ImageEncoders *enc)
lz4_encoder_destroy(enc->lz4);
enc->lz4 = NULL;
#endif
-zlib_encoder_destroy(enc->zlib);
-enc->zlib = NULL;
+if (enc->zlib) {
+zlib_encoder_destroy(enc->zlib);
+enc->zlib = NULL;
+}
pthread_mutex_destroy(&enc->glz_drawables_inst_to_free_lock);
}
@@ -1261,6 +1257,13 @@ bool image_encoders_compress_glz(ImageEncoders *enc,
if (!enable_zlib_glz_wrap || (glz_size < MIN_GLZ_SIZE_FOR_ZLIB)) {
goto glz;
}
+if (!enc->zlib) {
+enc->zlib = zlib_encoder_create(&enc->zlib_data.usr,
ZLIB_DEFAULT_COMPRESSION_LEVEL);
+if (!enc->zlib) {
+g_warning("create zlib encoder failed");
+goto glz;
+}
+}
stat_start_time_init(&start_time, &enc->shared_data->zlib_glz_stat);
zlib_data = &enc->zlib_data;
--
2.20.1
___
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
> 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__) would be fine? > Looks good otherwise, > Acked-by: Christophe Fergeau > > Christophe > > > + > > static int verbose = usbredirparser_info; > > static int client_fd, running = 1; > > static libusb_context *ctx; Frediano ___ 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] gtk-session: clipboard: add some more debugs
Hi,
On Mon, Jan 28, 2019 at 1:23 PM Victor Toso wrote:
>
> From: Victor Toso
>
> To help track race conditions and bad/unexpected behavior in general.
Awesome! I was planning on sending something similar too.
>
> Signed-off-by: Victor Toso
> ---
> src/spice-gtk-session.c | 24
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index ac2ba0b..bdb1d9d 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -544,7 +544,7 @@ static void clipboard_handler_get_targets_cb(GtkClipboard
> *clipboard,
> {
> SpiceGtkSession *self = free_weak_ref(user_data);
>
> -SPICE_DEBUG("%s:", __FUNCTION__);
> +SPICE_DEBUG("%s natoms=%d from %p", __func__, n_atoms, clipboard);
Printing the address clipboard points to is rather useless imho. The
corresponding selection id is what we're interested in.
>
> if (self == NULL)
> return;
> @@ -704,9 +704,10 @@ client_clipboard_received_data(SpiceMainChannel *main,
> SpiceGtkSessionPrivate *s = ri->self->priv;
> gchar *conv = NULL;
>
> -g_return_if_fail(selection == ri->selection);
> +SPICE_DEBUG("%s %u bytes (%p) for sel=%u type=%u",
> +__func__, size, data, selection, type);
>
> -SPICE_DEBUG("clipboard got data");
> +g_return_if_fail(selection == ri->selection);
>
> if (atom2agent[ri->info].vdagent == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
> /* on windows, gtk+ would already convert to LF endings, but
> @@ -751,12 +752,11 @@ client_clipboard_request_data(GtkClipboard *clipboard,
> gboolean agent_connected = FALSE;
> gulong clipboard_handler;
> gulong agent_handler;
> -int selection;
>
> -SPICE_DEBUG("clipboard get");
> -
> -selection = get_selection_from_clipboard(s, clipboard);
> +int selection = get_selection_from_clipboard(s, clipboard);
> g_return_if_fail(selection != -1);
> +SPICE_DEBUG("%s for sel=%d (%p)", __func__, selection, clipboard);
Maybe also print the type that is being requested (info,
atom2agent[info].vdagent) ?
> +
> g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent));
> g_return_if_fail(s->main != NULL);
>
> @@ -801,7 +801,7 @@ cleanup:
> static void
> clipboard_handler_clear(GtkClipboard *clipboard, gpointer user_data)
> {
> -SPICE_DEBUG("clipboard_clear");
> +SPICE_DEBUG("%s on %p", __func__, clipboard);
Same as above: print selection instead of pointer's address
> /* We watch for clipboard ownership changes and act on those, so we
> don't need to do anything here */
> }
> @@ -846,6 +846,9 @@ guest_clipboard_grab(SpiceMainChannel *main,
> }
> }
>
> +SPICE_DEBUG("%s selection=%u (%p), might request %d/%u types",
> +__func__, selection, cb, num_targets, ntypes);
When we send a grab to vdagent, we print each target (atom's name).
Maybe we could do that here as well?
> +
> g_free(s->clip_targets[selection]);
> s->nclip_targets[selection] = num_targets;
> s->clip_targets[selection] = g_memdup(targets, sizeof(GtkTargetEntry) *
> num_targets);
> @@ -1014,6 +1017,7 @@ guest_clipboard_request_send_data(GtkClipboard
> *clipboard,
> */
> g_warn_if_fail(type != VD_AGENT_CLIPBOARD_UTF8_TEXT);
>
> +SPICE_DEBUG("%s %d bytes for selection=%d (%p) type=%u", __func__, len,
> selection, clipboard, type);
Split into two lines?
> spice_main_channel_clipboard_selection_notify(s->main, selection, type,
> data, len);
> }
>
> @@ -1034,6 +1038,8 @@ guest_clipboard_request_data(SpiceMainChannel *main,
>
> cb = get_clipboard_from_selection(s, selection);
> g_return_val_if_fail(cb != NULL, FALSE);
> +SPICE_DEBUG("%s selection=%u (%p) type=%u", __func__, selection, cb,
> type);
> +
> g_return_val_if_fail(s->clipboard_by_guest[selection] == FALSE, FALSE);
> g_return_val_if_fail(s->clip_grabbed[selection], FALSE);
>
> @@ -1072,6 +1078,8 @@ guest_clipboard_release(SpiceMainChannel *main,
> SpiceGtkSessionPrivate *s = self->priv;
> GtkClipboard* clipboard = get_clipboard_from_selection(s, selection);
>
> +SPICE_DEBUG("%s selection=%u (%p)", __func__, selection, clipboard);
> +
> if (!clipboard)
> return;
>
> --
> 2.20.1
>
In some logs, you use "sel=" while in others "selection=".
I think owner-change deserves a log too.
Logs in clipboard_handler_received_text_cb() could contain the
concerned selection.
Regards,
Jakub
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] QXL interface: improve the spice_qxl_set_device_info documentation
Acked-by: Jonathon Jongsma
On Mon, 2019-01-28 at 11:14 +0100, Lukáš Hrázký wrote:
> Instead of one unsupported example, present two real world examples.
>
> Signed-off-by: Lukáš Hrázký
> ---
> server/spice-qxl.h | 32 ++--
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/server/spice-qxl.h b/server/spice-qxl.h
> index e7af5e5e..2f47910b 100644
> --- a/server/spice-qxl.h
> +++ b/server/spice-qxl.h
> @@ -122,7 +122,7 @@ void spice_qxl_gl_draw_async(QXLInstance
> *instance,
> * @instance the QXL instance to set the path to
> * @device_address the path of the device this QXL instance belongs
> to
> * @device_display_id_start the starting device display ID of this
> interface,
> - * i.e. the one monitor ID 0 maps to
> + * i.e. the device display ID of monitor ID
> 0
> * @device_display_id_count the total number of device display IDs
> on this
> * interface
> *
> @@ -145,16 +145,28 @@ void spice_qxl_gl_draw_async(QXLInstance
> *instance,
> *
> * The device_display_id_{start,count} denotes the sequence of
> device display
> * IDs that map to the zero-based sequence of monitor IDs provided
> by monitors
> - * config on this interface. For example with
> device_display_id_start = 2 and
> - * device_display_id_count = 3 you get the following mapping:
> - * monitor_id -> device_display_id
> - * 0 -> 2
> - * 1 -> 3
> - * 2 -> 4
> + * config on this interface.
> *
> - * Note this example is unsupported in practice. The only supported
> cases are
> - * either a single device display ID (count = 1) or multiple device
> display IDs
> - * in a sequence starting from 0.
> + * Example 1:
> + * A QXL graphics device with 3 heads (monitors).
> + *
> + * device_display_id_start = 0
> + * device_display_id_count = 3
> + *
> + * Results in the following mapping of monitor_id -
> > device_display_id:
> + * 0 -> 0
> + * 1 -> 1
> + * 2 -> 2
> + *
> + * Example 2:
> + * A virtio graphics device, multiple monitors, a QXL interface
> for each
> + * monitor. On the QXL interface for the third monitor:
> + *
> + * device_display_id_start = 2
> + * device_display_id_count = 1
> + *
> + * Results in the following mapping of monitor_id -
> > device_display_id:
> + * 0 -> 2
> */
> void spice_qxl_set_device_info(QXLInstance *instance,
> const char *device_address,
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-streaming-agent v4 5/6] Send the GraphicsDeviceInfo to the server
On Mon, 2019-01-28 at 15:09 +0100, Lukáš Hrázký wrote:
> Adds serialization of the GraphicsDeviceInfo message and sends it to
> the
> server when it starts to stream.
>
> Signed-off-by: Lukáš Hrázký
> Reviewed-by: Jonathon Jongsma
> ---
> configure.ac | 2 +-
> src/spice-streaming-agent.cpp | 60 +--
>
> 2 files changed, 52 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..a9fde2a 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 STREAM_TYPE_DEVICE_DISPLAY_INFO>
> +{
> +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,25 @@ 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) {
> +stream_port.send(display_info[
> 0]);
Personally, I'd still prefer a warning here where there is more than 1
device display info.
something like
"Warning: the Frame Capture plugin returned information about more than
one display device, but we currently only support a single device.
Sending information for first device to the server."
but, let's get it in
Acked-by: Jonathon Jongsma
> +} else {
> +syslog(LOG_ERR, "Empty device display info from the
> plugin");
> }
>
> while (!quit_requested && streaming_requested) {
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedeskt
Re: [Spice-devel] [spice-gtk v1 1/2] gtk-session: clipboard: rename functions
Hi,
On Mon, Jan 28, 2019 at 1:23 PM Victor Toso wrote:
>
> From: Victor Toso
>
> * Attaching the 'guest_' prefix for actions that were started from
> guest agent, those renames are:
>
> clipboard_grab -> guest_clipboard_grab
> clipboard_release -> guest_clipboard_release
> clipboard_request -> guest_clipboard_request_data
> clipboard_request_send_data -> guest_clipboard_request_send_data
>
> * Attaching the 'client_' prefix for actions that were triggered by
> client and sent/received by guest agent, those renames are:
>
> clipboard_get -> client_clipboard_request_data
> clipboard_got_from_guest -> client_clipboard_received_data
>
> * Changed from 'clipboard_' to 'clipboard_handler' prefix for
> callbacks related to GtkClipboard, those renames are:
>
> clipboard_clear -> clipboard_handler_clear
> clipboard_owner_change -> clipboard_handler_owner_change_cb
> clipboard_received_text_cb -> clipboard_handler_received_text_cb
> clipboard_get_targets -> clipboard_handler_get_targets_cb
These 4 seem a bit weird.
I would either change them all to "clipboard_*_handler", or "clipboard_*_cb".
I wouldn't use "handler" together with "cb".
They could also bear the "client_" prefix as they're all concerning
clipboard on the client side.
>
> Signed-off-by: Victor Toso
> ---
> src/spice-gtk-session.c | 123
> 1 file changed, 75 insertions(+), 48 deletions(-)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 1a19bca..ac2ba0b 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -95,9 +95,9 @@ struct _SpiceGtkSessionPrivate {
>
> /* -- */
> /* Prototypes for private functions */
> -static void clipboard_owner_change(GtkClipboard *clipboard,
> - GdkEventOwnerChange *event,
> - gpointer user_data);
> +static void clipboard_handler_owner_change_cb(GtkClipboard *clipboard,
> + GdkEventOwnerChange *event,
> + gpointer user_data);
> static void channel_new(SpiceSession *session, SpiceChannel *channel,
> gpointer user_data);
> static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
> @@ -182,10 +182,10 @@ static void spice_gtk_session_init(SpiceGtkSession
> *self)
>
> s->clipboard = gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
> g_signal_connect(G_OBJECT(s->clipboard), "owner-change",
> - G_CALLBACK(clipboard_owner_change), self);
> + G_CALLBACK(clipboard_handler_owner_change_cb), self);
> s->clipboard_primary = gtk_clipboard_get(GDK_SELECTION_PRIMARY);
> g_signal_connect(G_OBJECT(s->clipboard_primary), "owner-change",
> - G_CALLBACK(clipboard_owner_change), self);
> + G_CALLBACK(clipboard_handler_owner_change_cb), self);
> spice_g_signal_connect_object(keymap, "state-changed",
>G_CALLBACK(keymap_modifiers_changed),
> self, 0);
> }
> @@ -222,13 +222,13 @@ static void spice_gtk_session_dispose(GObject *gobject)
> /* release stuff */
> if (s->clipboard) {
> g_signal_handlers_disconnect_by_func(s->clipboard,
> -G_CALLBACK(clipboard_owner_change), self);
> +G_CALLBACK(clipboard_handler_owner_change_cb), self);
> s->clipboard = NULL;
> }
>
> if (s->clipboard_primary) {
> g_signal_handlers_disconnect_by_func(s->clipboard_primary,
> -G_CALLBACK(clipboard_owner_change), self);
> +G_CALLBACK(clipboard_handler_owner_change_cb), self);
> s->clipboard_primary = NULL;
> }
>
> @@ -537,10 +537,10 @@ static gpointer free_weak_ref(gpointer data)
> return object;
> }
>
> -static void clipboard_get_targets(GtkClipboard *clipboard,
> - GdkAtom *atoms,
> - gint n_atoms,
> - gpointer user_data)
> +static void clipboard_handler_get_targets_cb(GtkClipboard *clipboard,
> + GdkAtom *atoms,
> + gint n_atoms,
> + gpointer user_data)
> {
> SpiceGtkSession *self = free_weak_ref(user_data);
>
> @@ -635,9 +635,10 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
> * emits owner-change event; On Wayland that does not happen as spice-gtk
> still is
> * the owner of the clipboard.
> */
> -static void clipboard_owner_change(GtkClipboard*clipboard,
> - GdkEventOwnerChange *event,
> - gpointeruser_data)
> +static void
> +clipboard_handler_owner_change_cb(GtkClipboard *clipboard,
> +
Re: [Spice-devel] [PATCH spice v4 2/6] Receive the GraphicsDeviceInfo message from the streaming agent
Acked-by: Jonathon Jongsma
On Mon, 2019-01-28 at 15:09 +0100, Lukáš Hrázký wrote:
> Receives the GraphicsDeviceInfo message from the streaming agent and
> stores the data in a list on the streaming device.
>
> Signed-off-by: Lukáš Hrázký
> ---
> server/display-limits.h| 3 ++
> server/red-qxl.c | 2 +-
> server/red-stream-device.c | 78
> +-
> server/red-stream-device.h | 8
> 4 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/server/display-limits.h b/server/display-limits.h
> index e875149b..d79d3211 100644
> --- a/server/display-limits.h
> +++ b/server/display-limits.h
> @@ -25,4 +25,7 @@
> /** Maximum number of streams created by spice-server */
> #define NUM_STREAMS 50
>
> +/** Maximum length of the device address string */
> +#define MAX_DEVICE_ADDRESS_LEN 256
> +
> #endif /* DISPLAY_LIMITS_H_ */
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 7f8ac35d..37f3d9c8 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -37,11 +37,11 @@
> #include "dispatcher.h"
> #include "red-parse-qxl.h"
> #include "red-channel-client.h"
> +#include "display-limits.h"
>
> #include "red-qxl.h"
>
>
> -#define MAX_DEVICE_ADDRESS_LEN 256
> #define MAX_MONITORS_COUNT 16
>
> struct QXLState {
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index 3b553510..38df8e4c 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -39,6 +39,7 @@ struct StreamDevice {
> StreamMsgCapabilities capabilities;
> StreamMsgCursorSet cursor_set;
> StreamMsgCursorMove cursor_move;
> +StreamMsgDeviceDisplayInfo device_display_info;
> uint8_t buf[STREAM_MSG_CAPABILITIES_MAX_BYTES];
> } *msg;
> uint32_t msg_pos;
> @@ -51,6 +52,7 @@ struct StreamDevice {
> CursorChannel *cursor_channel;
> SpiceTimer *close_timer;
> uint32_t frame_mmtime;
> +StreamDeviceDisplayInfo device_display_info;
> };
>
> struct StreamDeviceClass {
> @@ -66,8 +68,8 @@ static void char_device_set_state(RedCharDevice
> *char_dev, int state);
> typedef bool StreamMsgHandler(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> SPICE_GNUC_WARN_UNUSED_RESULT;
>
> -static StreamMsgHandler handle_msg_format, handle_msg_data,
> handle_msg_cursor_set,
> -handle_msg_cursor_move, handle_msg_capabilities;
> +static StreamMsgHandler handle_msg_format,
> handle_msg_device_display_info, handle_msg_data,
> +handle_msg_cursor_set, handle_msg_cursor_move,
> handle_msg_capabilities;
>
> static bool handle_msg_invalid(StreamDevice *dev,
> SpiceCharDeviceInstance *sin,
> const char *error_msg)
> SPICE_GNUC_WARN_UNUSED_RESULT;
> @@ -150,6 +152,13 @@ stream_device_partial_read(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> handled = handle_msg_format(dev, sin);
> }
> break;
> +case STREAM_TYPE_DEVICE_DISPLAY_INFO:
> +if (dev->hdr.size > sizeof(StreamMsgDeviceDisplayInfo) +
> MAX_DEVICE_ADDRESS_LEN) {
> +handled = handle_msg_invalid(dev, sin,
> "StreamMsgDeviceDisplayInfo too large");
> +} else {
> +handled = handle_msg_device_display_info(dev, sin);
> +}
> +break;
> case STREAM_TYPE_DATA:
> if (dev->hdr.size > 32*1024*1024) {
> handled = handle_msg_invalid(dev, sin, "STREAM_DATA too
> large");
> @@ -275,6 +284,71 @@ handle_msg_format(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> return true;
> }
>
> +static bool
> +handle_msg_device_display_info(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> +{
> +SpiceCharDeviceInterface *sif =
> spice_char_device_get_interface(sin);
> +
> +if (spice_extra_checks) {
> +spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> +spice_assert(dev->hdr.type ==
> STREAM_TYPE_DEVICE_DISPLAY_INFO);
> +}
> +
> +if (dev->msg_len < dev->hdr.size) {
> +dev->msg = g_realloc(dev->msg, dev->hdr.size);
> +dev->msg_len = dev->hdr.size;
> +}
> +
> +/* read from device */
> +ssize_t n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev-
> >hdr.size - dev->msg_pos);
> +if (n <= 0) {
> +return dev->msg_pos == dev->hdr.size;
> +}
> +
> +dev->msg_pos += n;
> +if (dev->msg_pos != dev->hdr.size) { /* some bytes are still
> missing */
> +return false;
> +}
> +
> +StreamMsgDeviceDisplayInfo *display_info_msg = &dev->msg-
> >device_display_info;
> +
> +size_t device_address_len = GUINT32_FROM_LE(display_info_msg-
> >device_address_len);
> +if (device_address_len > MAX_DEVICE_ADDRESS_LEN) {
> +g_warning("Received a device address longer than %u (%zu), "
> + "will be truncated!", MAX_DEVICE_ADDRESS_LEN,
> device_address_len);
> +device_address_len = sizeof(dev-
> >device_display_info.device_address);
> +}
> +
> +if (d
[Spice-devel] [PATCH spice-streaming-agent v4 4/6] Interface + implementation of getting device display info
Adds an interface method to the FrameCapture class to get the device
display info (device address and device display id) for each display of
the graphics device that is captured.
Also adds functions to the API implementing this functionality for X11
in variants with and without DRM (the non-DRM version is rather limited
and may not work for more complex setups) as well as some helper
functions to make it easier for plugins to implement this and avoid code
duplication.
Implements the new interface method for the two built-in plugins
(mjpeg-fallback and gst-plugin).
Signed-off-by: Lukáš Hrázký
Acked-by: Jonathon Jongsma
---
configure.ac | 2 +
include/spice-streaming-agent/Makefile.am | 2 +
.../spice-streaming-agent/display-info.hpp| 52 +++
.../spice-streaming-agent/frame-capture.hpp | 13 +
.../x11-display-info.hpp | 57
src/Makefile.am | 7 +
src/display-info.cpp | 101 ++
src/gst-plugin.cpp| 14 +
src/mjpeg-fallback.cpp| 17 +-
src/spice-streaming-agent.cpp | 13 +
src/unittests/Makefile.am | 6 +
src/utils.cpp | 46 +++
src/utils.hpp | 4 +
src/x11-display-info.cpp | 302 ++
14 files changed, 634 insertions(+), 2 deletions(-)
create mode 100644 include/spice-streaming-agent/display-info.hpp
create mode 100644 include/spice-streaming-agent/x11-display-info.hpp
create mode 100644 src/display-info.cpp
create mode 100644 src/utils.cpp
create mode 100644 src/x11-display-info.cpp
diff --git a/configure.ac b/configure.ac
index e66e6d8..fd18efe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -34,8 +34,10 @@ SPICE_PROTOCOL_MIN_VER=0.12.14
PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >=
$SPICE_PROTOCOL_MIN_VER])
AC_SUBST([SPICE_PROTOCOL_MIN_VER])
+PKG_CHECK_MODULES(DRM, libdrm)
PKG_CHECK_MODULES(X11, x11)
PKG_CHECK_MODULES(XFIXES, xfixes)
+PKG_CHECK_MODULES(XRANDR, xrandr)
PKG_CHECK_MODULES(JPEG, libjpeg, , [
AC_CHECK_LIB(jpeg, jpeg_destroy_decompress,
diff --git a/include/spice-streaming-agent/Makefile.am
b/include/spice-streaming-agent/Makefile.am
index bcd679b..96c1a57 100644
--- a/include/spice-streaming-agent/Makefile.am
+++ b/include/spice-streaming-agent/Makefile.am
@@ -1,8 +1,10 @@
NULL =
public_includedir = $(includedir)/spice-streaming-agent
public_include_HEADERS = \
+ display-info.hpp \
error.hpp \
frame-capture.hpp \
plugin.hpp \
+ x11-display-info.hpp \
$(NULL)
diff --git a/include/spice-streaming-agent/display-info.hpp
b/include/spice-streaming-agent/display-info.hpp
new file mode 100644
index 000..f16212b
--- /dev/null
+++ b/include/spice-streaming-agent/display-info.hpp
@@ -0,0 +1,52 @@
+/* \copyright
+ * Copyright 2018 Red Hat Inc. All rights reserved.
+ */
+
+#ifndef SPICE_STREAMING_AGENT_DISPLAY_INFO_HPP
+#define SPICE_STREAMING_AGENT_DISPLAY_INFO_HPP
+
+#include
+#include
+
+
+namespace spice __attribute__ ((visibility ("default"))) {
+namespace streaming_agent {
+
+/**
+ * Lists graphics cards listed in the DRM sybsystem in /sys/class/drm.
+ * Throws an instance of Error in case of an I/O error.
+ *
+ * @return a vector of paths of all graphics cards present in /sys/class/drm
+ */
+std::vector list_cards();
+
+/**
+ * Reads a single number in hex format from a file.
+ * Throws an instance of Error in case of an I/O or parsing error.
+ *
+ * @param path the path to the file
+ * @return the number parsed from the file
+ */
+uint32_t read_hex_number_from_file(const std::string &path);
+
+/**
+ * Resolves any symlinks and then extracts the PCI path from the canonical path
+ * to a card. Returns the path in the following format:
+ * "pci//./.../."
+ *
+ * is the PCI domain, followed by . of any PCI bridges
+ * in the chain leading to the device. The last . is the
+ * graphics device. All of , , are hexadecimal numbers
+ * with the following number of digits:
+ * : 4
+ * : 2
+ * : 1
+ *
+ * @param device_path the path to the card
+ * @return the device address
+ */
+std::string get_device_address(const std::string &card_path);
+
+}} // namespace spice::streaming_agent
+
+#endif // SPICE_STREAMING_AGENT_DISPLAY_INFO_HPP
diff --git a/include/spice-streaming-agent/frame-capture.hpp
b/include/spice-streaming-agent/frame-capture.hpp
index 51a2987..c244fb9 100644
--- a/include/spice-streaming-agent/frame-capture.hpp
+++ b/include/spice-streaming-agent/frame-capture.hpp
@@ -6,7 +6,11 @@
*/
#ifndef SPICE_STREAMING_AGENT_FRAME_CAPTURE_HPP
#define SPICE_STREAMING_AGENT_FRAME_CAPTURE_HPP
+
+#include
#include
+#include
+#include
#include
@@ -29,6 +33,13 @@ struct FrameInfo
bool stream_start;
};
+struct DeviceDisplayInfo
+{
+uint32_t stream_id;
+std::st
[Spice-devel] [PATCH spice v4 2/6] Receive the GraphicsDeviceInfo message from the streaming agent
Receives the GraphicsDeviceInfo message from the streaming agent and
stores the data in a list on the streaming device.
Signed-off-by: Lukáš Hrázký
---
server/display-limits.h| 3 ++
server/red-qxl.c | 2 +-
server/red-stream-device.c | 78 +-
server/red-stream-device.h | 8
4 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/server/display-limits.h b/server/display-limits.h
index e875149b..d79d3211 100644
--- a/server/display-limits.h
+++ b/server/display-limits.h
@@ -25,4 +25,7 @@
/** Maximum number of streams created by spice-server */
#define NUM_STREAMS 50
+/** Maximum length of the device address string */
+#define MAX_DEVICE_ADDRESS_LEN 256
+
#endif /* DISPLAY_LIMITS_H_ */
diff --git a/server/red-qxl.c b/server/red-qxl.c
index 7f8ac35d..37f3d9c8 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -37,11 +37,11 @@
#include "dispatcher.h"
#include "red-parse-qxl.h"
#include "red-channel-client.h"
+#include "display-limits.h"
#include "red-qxl.h"
-#define MAX_DEVICE_ADDRESS_LEN 256
#define MAX_MONITORS_COUNT 16
struct QXLState {
diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index 3b553510..38df8e4c 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -39,6 +39,7 @@ struct StreamDevice {
StreamMsgCapabilities capabilities;
StreamMsgCursorSet cursor_set;
StreamMsgCursorMove cursor_move;
+StreamMsgDeviceDisplayInfo device_display_info;
uint8_t buf[STREAM_MSG_CAPABILITIES_MAX_BYTES];
} *msg;
uint32_t msg_pos;
@@ -51,6 +52,7 @@ struct StreamDevice {
CursorChannel *cursor_channel;
SpiceTimer *close_timer;
uint32_t frame_mmtime;
+StreamDeviceDisplayInfo device_display_info;
};
struct StreamDeviceClass {
@@ -66,8 +68,8 @@ static void char_device_set_state(RedCharDevice *char_dev,
int state);
typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance *sin)
SPICE_GNUC_WARN_UNUSED_RESULT;
-static StreamMsgHandler handle_msg_format, handle_msg_data,
handle_msg_cursor_set,
-handle_msg_cursor_move, handle_msg_capabilities;
+static StreamMsgHandler handle_msg_format, handle_msg_device_display_info,
handle_msg_data,
+handle_msg_cursor_set, handle_msg_cursor_move, handle_msg_capabilities;
static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin,
const char *error_msg)
SPICE_GNUC_WARN_UNUSED_RESULT;
@@ -150,6 +152,13 @@ stream_device_partial_read(StreamDevice *dev,
SpiceCharDeviceInstance *sin)
handled = handle_msg_format(dev, sin);
}
break;
+case STREAM_TYPE_DEVICE_DISPLAY_INFO:
+if (dev->hdr.size > sizeof(StreamMsgDeviceDisplayInfo) +
MAX_DEVICE_ADDRESS_LEN) {
+handled = handle_msg_invalid(dev, sin, "StreamMsgDeviceDisplayInfo
too large");
+} else {
+handled = handle_msg_device_display_info(dev, sin);
+}
+break;
case STREAM_TYPE_DATA:
if (dev->hdr.size > 32*1024*1024) {
handled = handle_msg_invalid(dev, sin, "STREAM_DATA too large");
@@ -275,6 +284,71 @@ handle_msg_format(StreamDevice *dev,
SpiceCharDeviceInstance *sin)
return true;
}
+static bool
+handle_msg_device_display_info(StreamDevice *dev, SpiceCharDeviceInstance *sin)
+{
+SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
+
+if (spice_extra_checks) {
+spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
+spice_assert(dev->hdr.type == STREAM_TYPE_DEVICE_DISPLAY_INFO);
+}
+
+if (dev->msg_len < dev->hdr.size) {
+dev->msg = g_realloc(dev->msg, dev->hdr.size);
+dev->msg_len = dev->hdr.size;
+}
+
+/* read from device */
+ssize_t n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size -
dev->msg_pos);
+if (n <= 0) {
+return dev->msg_pos == dev->hdr.size;
+}
+
+dev->msg_pos += n;
+if (dev->msg_pos != dev->hdr.size) { /* some bytes are still missing */
+return false;
+}
+
+StreamMsgDeviceDisplayInfo *display_info_msg =
&dev->msg->device_display_info;
+
+size_t device_address_len =
GUINT32_FROM_LE(display_info_msg->device_address_len);
+if (device_address_len > MAX_DEVICE_ADDRESS_LEN) {
+g_warning("Received a device address longer than %u (%zu), "
+ "will be truncated!", MAX_DEVICE_ADDRESS_LEN,
device_address_len);
+device_address_len = sizeof(dev->device_display_info.device_address);
+}
+
+if (device_address_len == 0) {
+g_warning("Zero length device_address in DeviceDisplayInfo message,
ignoring.");
+return true;
+}
+
+if (display_info_msg->device_address + device_address_len > (uint8_t*)
dev->msg + dev->hdr.size) {
+g_warning("Malformed DeviceDisplayInfo message, device_address length
(%zu) "
+ "goes beyo
[Spice-devel] [PATCH vd_agent v4 6/6] Receive the graphics_device_info message
The graphics_device_info message contains the device display ID
information (device address and device display ID). Stores the data in a
hash table in vdagent.
Signed-off-by: Lukáš Hrázký
Acked-by: Jonathon Jongsma
---
configure.ac | 2 +-
src/vdagent/vdagent.c| 3 ++
src/vdagent/x11-priv.h | 1 +
src/vdagent/x11-randr.c | 65
src/vdagent/x11.c| 13
src/vdagent/x11.h| 1 +
src/vdagentd-proto-strings.h | 1 +
src/vdagentd-proto.h | 1 +
src/vdagentd/vdagentd.c | 16 +
9 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 7cb44db..7faebfd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,7 +102,7 @@ AC_ARG_ENABLE([static-uinput],
PKG_CHECK_MODULES([GLIB2], [glib-2.0 >= 2.34])
PKG_CHECK_MODULES(X, [xfixes xrandr >= 1.3 xinerama x11])
-PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.13])
+PKG_CHECK_MODULES(SPICE, [spice-protocol >= 0.12.16])
PKG_CHECK_MODULES(ALSA, [alsa >= 1.0.22])
PKG_CHECK_MODULES([DBUS], [dbus-1])
diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
index 90247f9..7cc6287 100644
--- a/src/vdagent/vdagent.c
+++ b/src/vdagent/vdagent.c
@@ -240,6 +240,9 @@ static void daemon_read_complete(struct udscs_connection
**connp,
((VDAgentFileXferDataMessage
*)data)->id);
}
break;
+case VDAGENTD_GRAPHICS_DEVICE_INFO:
+vdagent_x11_handle_graphics_device_info(agent->x11, data,
header->size);
+break;
case VDAGENTD_CLIENT_DISCONNECTED:
vdagent_clipboards_release_all(agent->clipboards);
if (vdagent_finalize_file_xfer(agent)) {
diff --git a/src/vdagent/x11-priv.h b/src/vdagent/x11-priv.h
index b31b0a5..0e954cf 100644
--- a/src/vdagent/x11-priv.h
+++ b/src/vdagent/x11-priv.h
@@ -139,6 +139,7 @@ struct vdagent_x11 {
int xrandr_minor;
int has_xinerama;
int dont_send_guest_xorg_res;
+GHashTable *graphics_display_infos;
};
extern int (*vdagent_x11_prev_error_handler)(Display *, XErrorEvent *);
diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
index 192b888..405fca9 100644
--- a/src/vdagent/x11-randr.c
+++ b/src/vdagent/x11-randr.c
@@ -727,6 +727,71 @@ static void dump_monitors_config(struct vdagent_x11 *x11,
}
}
+typedef struct GraphicsDisplayInfo {
+char device_address[256];
+uint32_t device_display_id;
+} GraphicsDisplayInfo;
+
+// handle the device info message from the server. This will allow us to
+// maintain a mapping from spice display id to xrandr output
+void vdagent_x11_handle_graphics_device_info(struct vdagent_x11 *x11, uint8_t
*data, size_t size)
+{
+VDAgentGraphicsDeviceInfo *graphics_device_info =
(VDAgentGraphicsDeviceInfo *)data;
+VDAgentDeviceDisplayInfo *device_display_info =
graphics_device_info->display_info;
+
+void *buffer_end = data + size;
+
+syslog(LOG_INFO, "Received Graphics Device Info:");
+
+for (size_t i = 0; i < graphics_device_info->count; ++i) {
+if ((void*) device_display_info > buffer_end ||
+(void*) (&device_display_info->device_address +
+device_display_info->device_address_len) > buffer_end) {
+syslog(LOG_ERR, "Malformed graphics_display_info message, "
+ "extends beyond the end of the buffer");
+break;
+}
+
+GraphicsDisplayInfo *value = g_malloc(sizeof(GraphicsDisplayInfo));
+value->device_address[0] = '\0';
+
+size_t device_address_len = device_display_info->device_address_len;
+if (device_address_len > sizeof(value->device_address)) {
+syslog(LOG_ERR, "Received a device address longer than %lu, "
+ "will be truncated!", device_address_len);
+device_address_len = sizeof(value->device_address);
+}
+
+strncpy(value->device_address,
+(char*) device_display_info->device_address,
+device_address_len);
+
+if (device_address_len > 0) {
+value->device_address[device_address_len - 1] = '\0'; // make
sure the string is terminated
+} else {
+syslog(LOG_WARNING, "Zero length device_address received for
channel_id: %u, monitor_id: %u",
+ device_display_info->channel_id,
device_display_info->monitor_id);
+}
+
+value->device_display_id = device_display_info->device_display_id;
+
+syslog(LOG_INFO, " channel_id: %u monitor_id: %u device_address: %s,
"
+ "device_display_id: %u",
+ device_display_info->channel_id,
+ device_display_info->monitor_id,
+ value->device_address,
+ value->device_display_id);
+
+g_hash_table_insert(x11->graphics_display_infos,
+GUINT_TO_POINTER(device_display_info->channel_id +
device_display_in
[Spice-devel] [PATCH spice v4 1/6] Send the graphics device info to the vd_agent
Sends the device address and device display IDs to the vdagent. The
message is sent either in reaction to the SPICE_MSGC_MAIN_AGENT_START
message or when the graphics device info changes.
Signed-off-by: Lukáš Hrázký
Acked-by: Jonathon Jongsma
---
configure.ac | 2 +-
meson.build | 2 +-
server/red-qxl.c | 20 +
server/red-qxl.h | 3 ++
server/reds-private.h | 1 +
server/reds.c | 67 +++
server/reds.h | 1 +
7 files changed, 94 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index 903993a1..fa79af7f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -161,7 +161,7 @@ AS_IF([test x"$have_smartcard" = "xyes"], [
AS_VAR_APPEND([SPICE_REQUIRES], [" libcacard >= 0.1.2"])
])
-SPICE_PROTOCOL_MIN_VER=0.12.15
+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/meson.build b/meson.build
index 3184a6f5..5f402a5f 100644
--- a/meson.build
+++ b/meson.build
@@ -82,7 +82,7 @@ endif
#
# check for mandatory dependencies
#
-spice_protocol_version='0.12.15'
+spice_protocol_version='0.12.16'
glib_version = '2.38'
glib_version_info = '>= @0@'.format(glib_version)
diff --git a/server/red-qxl.c b/server/red-qxl.c
index 907f78d6..7f8ac35d 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -889,6 +889,26 @@ void spice_qxl_set_device_info(QXLInstance *instance,
instance->st->monitors_count = device_display_id_count;
instance->st->max_monitors = device_display_id_count;
+
+reds_send_device_display_info(red_qxl_get_server(instance->st));
+}
+
+const char* red_qxl_get_device_address(const QXLInstance *qxl)
+{
+const QXLState *qxl_state = qxl->st;
+return qxl_state->device_address;
+}
+
+const uint32_t* red_qxl_get_device_display_ids(const QXLInstance *qxl)
+{
+const QXLState *qxl_state = qxl->st;
+return qxl_state->device_display_ids;
+}
+
+size_t red_qxl_get_monitors_count(const QXLInstance *qxl)
+{
+const QXLState *qxl_state = qxl->st;
+return qxl_state->monitors_count;
}
void red_qxl_init(RedsState *reds, QXLInstance *qxl)
diff --git a/server/red-qxl.h b/server/red-qxl.h
index 6014d32a..94753948 100644
--- a/server/red-qxl.h
+++ b/server/red-qxl.h
@@ -40,6 +40,9 @@ void red_qxl_put_gl_scanout(QXLInstance *qxl,
SpiceMsgDisplayGlScanoutUnix *scan
void red_qxl_gl_draw_async_complete(QXLInstance *qxl);
int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int minor);
SpiceServer* red_qxl_get_server(QXLState *qxl);
+const char* red_qxl_get_device_address(const QXLInstance *qxl);
+const uint32_t* red_qxl_get_device_display_ids(const QXLInstance *qxl);
+size_t red_qxl_get_monitors_count(const QXLInstance *qxl);
/* Wrappers around QXLInterface vfuncs */
void red_qxl_get_init_info(QXLInstance *qxl, QXLDevInitInfo *info);
diff --git a/server/reds-private.h b/server/reds-private.h
index 920edc5c..9dbc7fa9 100644
--- a/server/reds-private.h
+++ b/server/reds-private.h
@@ -81,6 +81,7 @@ struct RedsState {
SpiceWatch *secure_listen_watch;
RedCharDeviceVDIPort *agent_dev;
int pending_mouse_event;
+bool pending_device_display_info_message;
GList *clients;
MainChannel *main_channel;
InputsChannel *inputs_channel;
diff --git a/server/reds.c b/server/reds.c
index e95c62d5..c4ec2a3f 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -257,6 +257,7 @@ typedef struct __attribute__ ((__packed__)) VDInternalBuf {
VDAgentMessage header;
union {
VDAgentMouseState mouse_state;
+VDAgentGraphicsDeviceInfo graphics_device_info;
}
u;
} VDInternalBuf;
@@ -894,6 +895,65 @@ static RedPipeItem
*vdi_port_read_one_msg_from_device(RedCharDevice *self,
return NULL;
}
+void reds_send_device_display_info(RedsState *reds)
+{
+if (!reds->agent_dev->priv->agent_attached) {
+return;
+}
+g_debug("Sending device display info to the agent:");
+
+size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
+QXLInstance *qxl;
+FOREACH_QXL_INSTANCE(reds, qxl) {
+message_size +=
+(sizeof(VDAgentDeviceDisplayInfo) +
strlen(red_qxl_get_device_address(qxl)) + 1) *
+red_qxl_get_monitors_count(qxl);
+}
+
+RedCharDeviceWriteBuffer *char_dev_buf =
vdagent_new_write_buffer(reds->agent_dev,
+ VD_AGENT_GRAPHICS_DEVICE_INFO,
+ message_size,
+ true);
+
+if (!char_dev_buf) {
+reds->pending_device_display_info_message = true;
+return;
+}
+
+VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf->buf;
+VDAgentGraphicsDeviceInfo *graphics_device_info =
&internal_buf->u.graphics_device_info;
+graphics_device_info->count = 0;
+
+VDAgentDeviceDisplayInfo *device_display_info =
gra
[Spice-devel] [PATCH spice-streaming-agent v4 5/6] Send the GraphicsDeviceInfo to the server
Adds serialization of the GraphicsDeviceInfo message and sends it to the
server when it starts to stream.
Signed-off-by: Lukáš Hrázký
Reviewed-by: Jonathon Jongsma
---
configure.ac | 2 +-
src/spice-streaming-agent.cpp | 60 +--
2 files changed, 52 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..a9fde2a 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,25 @@ 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) {
+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
[Spice-devel] [PATCH v4 0/6] Monitor ID between host and guest
Hello, this series introduces a new mechanism to identify monitors across the host and guest boundary, using the pair (device_address and device_display_id). device_address is a HW address of a graphics device (a PCI address to be specific, though other address domains can be used as well), device_display_id is an ID of the monitor on that device. This information is gathered from QEMU (regular spice) or spice-streaming-agent (for the streaming channels) and a mapping of (channel_id, monitor_id) -> (device_address, device_display_id) is sent from spice server to the vd_agent. vd_agent uses this mapping to translate the former ID in incoming messages to the latter. This series follows up on the "QXL interface to set monitor ID" series [1] and then Jonathon Jongsma's "Use the PCI addr and display ID" [2] follows up on this to make a complete solution. You can also find the code in the following branches: https://gitlab.freedesktop.org/lukash/spice/commits/monitor-id-v3-4 https://gitlab.freedesktop.org/lukash/spice-streaming-agent/commits/monitor-id-v3-4 https://gitlab.freedesktop.org/lukash/vd_agent/commits/monitor-id-v3-4 Cheers, Lukas Changes since v3: * spice-protocol patches were merged. * Bumped minimum required version of spice-protocol to 0.12.16 for spice, spice-streaming-agent and vd_agent. * spice-streaming-agent: Include cstdlib.h in display-info.cpp. * spice-streaming-agent: use a constant for maximum device address length instead of a number literal. * spice-streaming-agent: explicitly compare with zero for std::string::compare() * spice-streaming-agent: rename local variable output to xoutput. [1] https://lists.freedesktop.org/archives/spice-devel/2019-January/046997.html [2] https://lists.freedesktop.org/archives/spice-devel/2019-January/047547.html -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice v4 3/6] Send the graphics device info from streaming agent to the vd_agent
Adds the graphics device info from the streaming device(s) to the
VDAgentGraphicsDeviceInfo message sent to the vd_agent.
Signed-off-by: Lukáš Hrázký
Acked-by: Jonathon Jongsma
---
server/red-stream-device.c | 18 +++
server/red-stream-device.h | 7 +
server/reds.c | 63 --
3 files changed, 86 insertions(+), 2 deletions(-)
diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index 38df8e4c..440b2689 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -346,6 +346,8 @@ handle_msg_device_display_info(StreamDevice *dev,
SpiceCharDeviceInstance *sin)
dev->device_display_info.device_address,
dev->device_display_info.device_display_id);
+
reds_send_device_display_info(red_char_device_get_server(RED_CHAR_DEVICE(dev)));
+
return true;
}
@@ -805,6 +807,22 @@ stream_device_init(StreamDevice *dev)
dev->msg_len = sizeof(*dev->msg);
}
+StreamDeviceDisplayInfo *stream_device_get_device_display_info(StreamDevice
*dev)
+{
+return &dev->device_display_info;
+}
+
+int32_t stream_device_get_stream_channel_id(StreamDevice *dev)
+{
+if (dev->stream_channel == NULL) {
+return -1;
+}
+
+int32_t channel_id;
+g_object_get(dev->stream_channel, "id", &channel_id, NULL);
+return channel_id;
+}
+
static StreamDevice *
stream_device_new(SpiceCharDeviceInstance *sin, RedsState *reds)
{
diff --git a/server/red-stream-device.h b/server/red-stream-device.h
index 996be016..d7ab5e41 100644
--- a/server/red-stream-device.h
+++ b/server/red-stream-device.h
@@ -56,6 +56,13 @@ StreamDevice *stream_device_connect(RedsState *reds,
SpiceCharDeviceInstance *si
*/
void stream_device_create_channel(StreamDevice *dev);
+StreamDeviceDisplayInfo *stream_device_get_device_display_info(StreamDevice
*dev);
+
+/**
+ * Returns -1 if the StreamDevice doesn't have a channel yet.
+ */
+int32_t stream_device_get_stream_channel_id(StreamDevice *dev);
+
G_END_DECLS
#endif /* STREAM_DEVICE_H */
diff --git a/server/reds.c b/server/reds.c
index c4ec2a3f..d7a71dc1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -902,14 +902,33 @@ void reds_send_device_display_info(RedsState *reds)
}
g_debug("Sending device display info to the agent:");
-size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
QXLInstance *qxl;
+RedCharDevice *dev;
+
+size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
+
+// size for the qxl device info
FOREACH_QXL_INSTANCE(reds, qxl) {
message_size +=
(sizeof(VDAgentDeviceDisplayInfo) +
strlen(red_qxl_get_device_address(qxl)) + 1) *
red_qxl_get_monitors_count(qxl);
}
+// size for the stream device info
+GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
+if (IS_STREAM_DEVICE(dev)) {
+size_t device_address_len =
+
strlen(stream_device_get_device_display_info(STREAM_DEVICE(dev))->device_address);
+
+if (device_address_len == 0) {
+// the device info wasn't set (yet), don't send it
+continue;
+}
+
+message_size += (sizeof(VDAgentDeviceDisplayInfo) +
device_address_len + 1);
+}
+}
+
RedCharDeviceWriteBuffer *char_dev_buf =
vdagent_new_write_buffer(reds->agent_dev,
VD_AGENT_GRAPHICS_DEVICE_INFO,
message_size,
@@ -925,6 +944,8 @@ void reds_send_device_display_info(RedsState *reds)
graphics_device_info->count = 0;
VDAgentDeviceDisplayInfo *device_display_info =
graphics_device_info->display_info;
+
+// add the qxl devices to the message
FOREACH_QXL_INSTANCE(reds, qxl) {
for (size_t i = 0; i < red_qxl_get_monitors_count(qxl); ++i) {
device_display_info->channel_id = qxl->id;
@@ -936,7 +957,45 @@ void reds_send_device_display_info(RedsState *reds)
device_display_info->device_address_len =
strlen((char*) device_display_info->device_address) + 1;
-g_debug(" channel_id: %u monitor_id: %u, device_address: %s,
device_display_id: %u",
+g_debug(" (qxl)channel_id: %u monitor_id: %u,
device_address: %s, device_display_id: %u",
+device_display_info->channel_id,
+device_display_info->monitor_id,
+device_display_info->device_address,
+device_display_info->device_display_id);
+
+device_display_info = (VDAgentDeviceDisplayInfo*) ((char*)
device_display_info +
+sizeof(VDAgentDeviceDisplayInfo) +
device_display_info->device_address_len);
+
+graphics_device_info->count++;
+}
+}
+
+// add the stream devices to the message
+GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
+if (IS_STREAM_DEVICE(dev)) {
+
[Spice-devel] [vdagent-linux] vdagent: Silently ignore missing spicevmc device
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
---
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);
+return 0;
}
if (do_daemonize)
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk] build-sys: Make git-version-gen work again with Meson
On Wed, Jan 23, 2019 at 09:48:43AM +, Frediano Ziglio wrote:
> d0cbd9618f0b removed the ability to use git-version-gen to
> generate proper version string.
> Generate .tarball-version file in the distribution file to
> allow building from tarball.
> Do not use MESON_SOURCE_ROOT calling git-version-gen command
> as this won't be expanded.
"when calling" I think.
> Change directory in git-version-gen script to allow the
> script to be called from a different directory.
>
> Signed-off-by: Frediano Ziglio
> ---
> build-aux/git-version-gen | 1 +
> meson.build | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
> index 5617eb8d..00c4a55a 100755
> --- a/build-aux/git-version-gen
> +++ b/build-aux/git-version-gen
> @@ -95,6 +95,7 @@ then
> && echo "$0: WARNING: $tarball_version_file seems to be damaged" 1>&2
> fi
>
> +cd "`dirname $0`/.."
> if test -n "$v"
> then
> : # use $v
I don't know for sure what the canonical source is for git-version-gen,
but we should avoid making local changes to our copy.
I found
https://git.savannah.gnu.org/cgit/gnulib.git/tree/build-aux/git-version-gen
which seems much newer than our copy.
I did not test it, but the changes look reasonable.
Christophe
> diff --git a/meson.build b/meson.build
> index 70dd3188..dcfa4763 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2,13 +2,15 @@
> # project definition
> #
> project('spice-gtk', 'c',
> - version : '0.36',
> + version : run_command('build-aux/git-version-gen',
> '@0@/.tarball-version'.format(meson.source_root()), check :
> true).stdout().strip(),
> license : 'LGPLv2.1',
> meson_version : '>= 0.49')
>
> message('Updating submodules')
> run_command('build-aux/meson/check-spice-common', check : true)
>
> +meson.add_dist_script('sh', '-c', 'echo
> @0@>"$MESON_DIST_ROOT/.tarball-version"'.format(meson.project_version()))
> +
> #
> # global C defines
> #
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
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 spice-gtk] spice-marshal: Remove unused marshalling closure
Acked-by: Christophe Fergeau On Wed, Jan 23, 2019 at 10:04:19AM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > src/spice-marshal.txt | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/spice-marshal.txt b/src/spice-marshal.txt > index 92087c58..cf35790e 100644 > --- a/src/spice-marshal.txt > +++ b/src/spice-marshal.txt > @@ -11,6 +11,5 @@ VOID:UINT,POINTER,UINT > VOID:UINT,UINT,POINTER,UINT > BOOLEAN:UINT,POINTER,UINT > BOOLEAN:UINT,UINT > -VOID:OBJECT,OBJECT > VOID:BOXED,BOXED > BOOLEAN:POINTER > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel 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] usbredirserver: Make compile under MacOS
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? Looks good otherwise, Acked-by: Christophe Fergeau Christophe > + > static int verbose = usbredirparser_info; > static int client_fd, running = 1; > static libusb_context *ctx; > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel 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] qxl-wddm-dod: Load best know defaults for video mode at driver's start
On Mon, Jan 28, 2019 at 2:27 PM Christophe Fergeau
wrote:
> Hey,
>
> On Fri, Jan 25, 2019 at 09:48:52AM +0200, Yuri Benditovich wrote:
> > Even if initial display resolution is not available at driver start, try
> > to find it in the registry.
>
> > Then the driver can prevent black screen
> > on uninstall/disable also when it was installed on UEFI machine after
> > the production driver 0.18 which did not report valid video mode.
>
> I'm not sure I understand the problem you are trying to describe here.
> "Then the driver can prevent black screen on uninstall/disable" -> is
> this saying on non-UEFI system, we still have a black screen issue with
> the driver in git master?
>
> "also when it was installed on UEFI machine after the production driver
> 0.18 which did not report valid video mode." -> I assume this part is
> related to the commit "Fix for black screen on driver uninstall on ovmf
> platform"? If yes, isn't the issue that you describe already fixed by
> that commit?
>
>
All the scenarious below are related to ovmf platform
Flow 1: the driver from current master branch installed on the machine
where no qxl-wddm-dod driver installed
(or previous one uninstalled and reboot done)
The driver can be uninstalled without problem (this what was fixed by
previous commit).
Flow 2: there is current production driver (0.18) installed. Driver update
0.18->current master works OK.
(the driver does not know video mode on startup but it can live without it)
Uninstalling the driver still causes black screen (or broken screen), as
the driver does not know which mode to report on exit.
If it even reports some default, it might be wrong due to various reasons
(will cause broken screen, which is not better than black).
This is what current commit fixes.
> >
> > Signed-off-by: Yuri Benditovich
> > ---
> > qxldod/QxlDod.cpp | 37 -
> > 1 file changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index dea78e2..525cdc3 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -5169,6 +5169,37 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
> > }
> > }
> >
> > +// Width and Height values for initial video mode are populated by
> > +// display class driver upon switch from boot display to operational
> one.
> > +// This is not documented and can be changed in future, but
> > +// present under the same key in Win8.1/Win10/2016/2019
> > +static void RetrieveDisplayDefaults(DXGK_DISPLAY_INFORMATION& DispInfo)
> > +{
> > +PAGED_CODE();
> > +RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {};
> > +QueryTable[0].Flags = QueryTable[1].Flags =
> RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_TYPECHECK |
> RTL_QUERY_REGISTRY_REQUIRED;
> > +QueryTable[0].DefaultType = QueryTable[1].DefaultType = REG_DWORD
> << 24;
> > +QueryTable[0].Name = L"Height";
> > +QueryTable[0].EntryContext = &DispInfo.Height;
> > +QueryTable[1].Name = L"Width";
> > +QueryTable[1].EntryContext = &DispInfo.Width;
> > +
> > +NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_CONTROL,
> L"BGFX", QueryTable, NULL, NULL);
> > +if (NT_SUCCESS(status))
> > +{
> > +DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: %dx%d\n", __FUNCTION__,
> DispInfo.Width, DispInfo.Height));
> > +}
> > +else
> > +{
> > +DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: status = %X\n",
> __FUNCTION__, status));
> > +DispInfo.Width = INITIAL_WIDTH;
> > +DispInfo.Height = INITIAL_HEIGHT;
> > +}
> > +DispInfo.ColorFormat = D3DDDIFMT_X8R8G8B8;
>
> By any chance, is this stored in the registry as well?
>
Proper video mode is stored in the registry by the OS. Undocumented,
however.
This does not help to store it again, the point is to query _proper_ video
mode.
>
> Apart from this, looks good to me.
>
> Christophe
>
> > +DispInfo.Pitch = DispInfo.Width *
> BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE;
> > +DispInfo.TargetId = DispInfo.AcpiId = 0;
> > +}
> > +
> > NTSTATUS
> HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo)
> > {
> > PAGED_CODE();
> > @@ -5188,11 +5219,7 @@ NTSTATUS
> HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf
> > if (DispInfo.Width == 0)
> > {
> > DbgPrint(TRACE_LEVEL_WARNING, ("QxlDod::AcquireDisplayInfo has
> zero width!\n"));
> > -DispInfo.ColorFormat = D3DDDIFMT_A8R8G8B8;
> > -DispInfo.Width = INITIAL_WIDTH;
> > -DispInfo.Height = INITIAL_HEIGHT;
> > -DispInfo.Pitch = DispInfo.Width *
> BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE;
> > -DispInfo.TargetId = 0;
> > +RetrieveDisplayDefaults(DispInfo);
> > }
> > return Status;
> > }
> > --
> > 2.16.1.windows.4
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mail
Re: [Spice-devel] [PATCH spice-server] image-encoders: Initialize Zlib lazily
On Wed, Jan 23, 2019 at 07:46:16PM +, Frediano Ziglio wrote:
> 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
> ---
> server/image-encoders.c | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/server/image-encoders.c b/server/image-encoders.c
> index 88073a3e..6af1dc94 100644
> --- a/server/image-encoders.c
> +++ b/server/image-encoders.c
> @@ -451,12 +451,6 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
> {
> enc->zlib_data.usr.more_space = zlib_usr_more_space;
> enc->zlib_data.usr.more_input = zlib_usr_more_input;
> -
> -enc->zlib = zlib_encoder_create(&enc->zlib_data.usr,
> ZLIB_DEFAULT_COMPRESSION_LEVEL);
> -
> -if (!enc->zlib) {
> -spice_critical("create zlib encoder failed");
> -}
> }
>
> void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData
> *shared_data)
> @@ -494,8 +488,10 @@ void image_encoders_free(ImageEncoders *enc)
> lz4_encoder_destroy(enc->lz4);
> enc->lz4 = NULL;
> #endif
> -zlib_encoder_destroy(enc->zlib);
> -enc->zlib = NULL;
> +if (enc->zlib) {
> +zlib_encoder_destroy(enc->zlib);
> +enc->zlib = NULL;
> +}
> pthread_mutex_destroy(&enc->glz_drawables_inst_to_free_lock);
> }
>
> @@ -1261,6 +1257,12 @@ bool image_encoders_compress_glz(ImageEncoders *enc,
> if (!enable_zlib_glz_wrap || (glz_size < MIN_GLZ_SIZE_FOR_ZLIB)) {
> goto glz;
> }
> +if (!enc->zlib) {
> +enc->zlib = zlib_encoder_create(&enc->zlib_data.usr,
> ZLIB_DEFAULT_COMPRESSION_LEVEL);
> +if (!enc->zlib) {
This used to be a spice_critical(), I'd add some kind of log here to
make it easier to spot something went wrong.
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] qxl-wddm-dod: Load best know defaults for video mode at driver's start
Hey,
On Fri, Jan 25, 2019 at 09:48:52AM +0200, Yuri Benditovich wrote:
> Even if initial display resolution is not available at driver start, try
> to find it in the registry.
> Then the driver can prevent black screen
> on uninstall/disable also when it was installed on UEFI machine after
> the production driver 0.18 which did not report valid video mode.
I'm not sure I understand the problem you are trying to describe here.
"Then the driver can prevent black screen on uninstall/disable" -> is
this saying on non-UEFI system, we still have a black screen issue with
the driver in git master?
"also when it was installed on UEFI machine after the production driver
0.18 which did not report valid video mode." -> I assume this part is
related to the commit "Fix for black screen on driver uninstall on ovmf
platform"? If yes, isn't the issue that you describe already fixed by
that commit?
>
> Signed-off-by: Yuri Benditovich
> ---
> qxldod/QxlDod.cpp | 37 -
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index dea78e2..525cdc3 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -5169,6 +5169,37 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
> }
> }
>
> +// Width and Height values for initial video mode are populated by
> +// display class driver upon switch from boot display to operational one.
> +// This is not documented and can be changed in future, but
> +// present under the same key in Win8.1/Win10/2016/2019
> +static void RetrieveDisplayDefaults(DXGK_DISPLAY_INFORMATION& DispInfo)
> +{
> +PAGED_CODE();
> +RTL_QUERY_REGISTRY_TABLE QueryTable[3] = {};
> +QueryTable[0].Flags = QueryTable[1].Flags = RTL_QUERY_REGISTRY_DIRECT |
> RTL_QUERY_REGISTRY_TYPECHECK | RTL_QUERY_REGISTRY_REQUIRED;
> +QueryTable[0].DefaultType = QueryTable[1].DefaultType = REG_DWORD << 24;
> +QueryTable[0].Name = L"Height";
> +QueryTable[0].EntryContext = &DispInfo.Height;
> +QueryTable[1].Name = L"Width";
> +QueryTable[1].EntryContext = &DispInfo.Width;
> +
> +NTSTATUS status = RtlQueryRegistryValues(RTL_REGISTRY_CONTROL, L"BGFX",
> QueryTable, NULL, NULL);
> +if (NT_SUCCESS(status))
> +{
> +DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: %dx%d\n", __FUNCTION__,
> DispInfo.Width, DispInfo.Height));
> +}
> +else
> +{
> +DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: status = %X\n",
> __FUNCTION__, status));
> +DispInfo.Width = INITIAL_WIDTH;
> +DispInfo.Height = INITIAL_HEIGHT;
> +}
> +DispInfo.ColorFormat = D3DDDIFMT_X8R8G8B8;
By any chance, is this stored in the registry as well?
Apart from this, looks good to me.
Christophe
> +DispInfo.Pitch = DispInfo.Width *
> BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE;
> +DispInfo.TargetId = DispInfo.AcpiId = 0;
> +}
> +
> NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION&
> DispInfo)
> {
> PAGED_CODE();
> @@ -5188,11 +5219,7 @@ NTSTATUS
> HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf
> if (DispInfo.Width == 0)
> {
> DbgPrint(TRACE_LEVEL_WARNING, ("QxlDod::AcquireDisplayInfo has zero
> width!\n"));
> -DispInfo.ColorFormat = D3DDDIFMT_A8R8G8B8;
> -DispInfo.Width = INITIAL_WIDTH;
> -DispInfo.Height = INITIAL_HEIGHT;
> -DispInfo.Pitch = DispInfo.Width *
> BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE;
> -DispInfo.TargetId = 0;
> +RetrieveDisplayDefaults(DispInfo);
> }
> return Status;
> }
> --
> 2.16.1.windows.4
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v1 1/2] gtk-session: clipboard: rename functions
From: Victor Toso
* Attaching the 'guest_' prefix for actions that were started from
guest agent, those renames are:
clipboard_grab -> guest_clipboard_grab
clipboard_release -> guest_clipboard_release
clipboard_request -> guest_clipboard_request_data
clipboard_request_send_data -> guest_clipboard_request_send_data
* Attaching the 'client_' prefix for actions that were triggered by
client and sent/received by guest agent, those renames are:
clipboard_get -> client_clipboard_request_data
clipboard_got_from_guest -> client_clipboard_received_data
* Changed from 'clipboard_' to 'clipboard_handler' prefix for
callbacks related to GtkClipboard, those renames are:
clipboard_clear -> clipboard_handler_clear
clipboard_owner_change -> clipboard_handler_owner_change_cb
clipboard_received_text_cb -> clipboard_handler_received_text_cb
clipboard_get_targets -> clipboard_handler_get_targets_cb
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 123
1 file changed, 75 insertions(+), 48 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 1a19bca..ac2ba0b 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -95,9 +95,9 @@ struct _SpiceGtkSessionPrivate {
/* -- */
/* Prototypes for private functions */
-static void clipboard_owner_change(GtkClipboard *clipboard,
- GdkEventOwnerChange *event,
- gpointer user_data);
+static void clipboard_handler_owner_change_cb(GtkClipboard *clipboard,
+ GdkEventOwnerChange *event,
+ gpointer user_data);
static void channel_new(SpiceSession *session, SpiceChannel *channel,
gpointer user_data);
static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
@@ -182,10 +182,10 @@ static void spice_gtk_session_init(SpiceGtkSession *self)
s->clipboard = gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
g_signal_connect(G_OBJECT(s->clipboard), "owner-change",
- G_CALLBACK(clipboard_owner_change), self);
+ G_CALLBACK(clipboard_handler_owner_change_cb), self);
s->clipboard_primary = gtk_clipboard_get(GDK_SELECTION_PRIMARY);
g_signal_connect(G_OBJECT(s->clipboard_primary), "owner-change",
- G_CALLBACK(clipboard_owner_change), self);
+ G_CALLBACK(clipboard_handler_owner_change_cb), self);
spice_g_signal_connect_object(keymap, "state-changed",
G_CALLBACK(keymap_modifiers_changed), self,
0);
}
@@ -222,13 +222,13 @@ static void spice_gtk_session_dispose(GObject *gobject)
/* release stuff */
if (s->clipboard) {
g_signal_handlers_disconnect_by_func(s->clipboard,
-G_CALLBACK(clipboard_owner_change), self);
+G_CALLBACK(clipboard_handler_owner_change_cb), self);
s->clipboard = NULL;
}
if (s->clipboard_primary) {
g_signal_handlers_disconnect_by_func(s->clipboard_primary,
-G_CALLBACK(clipboard_owner_change), self);
+G_CALLBACK(clipboard_handler_owner_change_cb), self);
s->clipboard_primary = NULL;
}
@@ -537,10 +537,10 @@ static gpointer free_weak_ref(gpointer data)
return object;
}
-static void clipboard_get_targets(GtkClipboard *clipboard,
- GdkAtom *atoms,
- gint n_atoms,
- gpointer user_data)
+static void clipboard_handler_get_targets_cb(GtkClipboard *clipboard,
+ GdkAtom *atoms,
+ gint n_atoms,
+ gpointer user_data)
{
SpiceGtkSession *self = free_weak_ref(user_data);
@@ -635,9 +635,10 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
* emits owner-change event; On Wayland that does not happen as spice-gtk
still is
* the owner of the clipboard.
*/
-static void clipboard_owner_change(GtkClipboard*clipboard,
- GdkEventOwnerChange *event,
- gpointeruser_data)
+static void
+clipboard_handler_owner_change_cb(GtkClipboard *clipboard,
+ GdkEventOwnerChange *event,
+ gpointer user_data)
{
g_return_if_fail(SPICE_IS_GTK_SESSION(user_data));
@@ -676,7 +677,8 @@ static void clipboard_owner_change(GtkClipboard
*clipboard,
s->clipboard_by_guest[selection] = FALSE;
s->clip_hasdata[selection] = TRUE;
if (s->auto_clipboard_enable && !read_only(self))
-gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
+gtk_clipboard_request_t
[Spice-devel] [spice-gtk v1 2/2] gtk-session: clipboard: add some more debugs
From: Victor Toso
To help track race conditions and bad/unexpected behavior in general.
Signed-off-by: Victor Toso
---
src/spice-gtk-session.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index ac2ba0b..bdb1d9d 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -544,7 +544,7 @@ static void clipboard_handler_get_targets_cb(GtkClipboard
*clipboard,
{
SpiceGtkSession *self = free_weak_ref(user_data);
-SPICE_DEBUG("%s:", __FUNCTION__);
+SPICE_DEBUG("%s natoms=%d from %p", __func__, n_atoms, clipboard);
if (self == NULL)
return;
@@ -704,9 +704,10 @@ client_clipboard_received_data(SpiceMainChannel *main,
SpiceGtkSessionPrivate *s = ri->self->priv;
gchar *conv = NULL;
-g_return_if_fail(selection == ri->selection);
+SPICE_DEBUG("%s %u bytes (%p) for sel=%u type=%u",
+__func__, size, data, selection, type);
-SPICE_DEBUG("clipboard got data");
+g_return_if_fail(selection == ri->selection);
if (atom2agent[ri->info].vdagent == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
/* on windows, gtk+ would already convert to LF endings, but
@@ -751,12 +752,11 @@ client_clipboard_request_data(GtkClipboard *clipboard,
gboolean agent_connected = FALSE;
gulong clipboard_handler;
gulong agent_handler;
-int selection;
-SPICE_DEBUG("clipboard get");
-
-selection = get_selection_from_clipboard(s, clipboard);
+int selection = get_selection_from_clipboard(s, clipboard);
g_return_if_fail(selection != -1);
+SPICE_DEBUG("%s for sel=%d (%p)", __func__, selection, clipboard);
+
g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent));
g_return_if_fail(s->main != NULL);
@@ -801,7 +801,7 @@ cleanup:
static void
clipboard_handler_clear(GtkClipboard *clipboard, gpointer user_data)
{
-SPICE_DEBUG("clipboard_clear");
+SPICE_DEBUG("%s on %p", __func__, clipboard);
/* We watch for clipboard ownership changes and act on those, so we
don't need to do anything here */
}
@@ -846,6 +846,9 @@ guest_clipboard_grab(SpiceMainChannel *main,
}
}
+SPICE_DEBUG("%s selection=%u (%p), might request %d/%u types",
+__func__, selection, cb, num_targets, ntypes);
+
g_free(s->clip_targets[selection]);
s->nclip_targets[selection] = num_targets;
s->clip_targets[selection] = g_memdup(targets, sizeof(GtkTargetEntry) *
num_targets);
@@ -1014,6 +1017,7 @@ guest_clipboard_request_send_data(GtkClipboard *clipboard,
*/
g_warn_if_fail(type != VD_AGENT_CLIPBOARD_UTF8_TEXT);
+SPICE_DEBUG("%s %d bytes for selection=%d (%p) type=%u", __func__, len,
selection, clipboard, type);
spice_main_channel_clipboard_selection_notify(s->main, selection, type,
data, len);
}
@@ -1034,6 +1038,8 @@ guest_clipboard_request_data(SpiceMainChannel *main,
cb = get_clipboard_from_selection(s, selection);
g_return_val_if_fail(cb != NULL, FALSE);
+SPICE_DEBUG("%s selection=%u (%p) type=%u", __func__, selection, cb, type);
+
g_return_val_if_fail(s->clipboard_by_guest[selection] == FALSE, FALSE);
g_return_val_if_fail(s->clip_grabbed[selection], FALSE);
@@ -1072,6 +1078,8 @@ guest_clipboard_release(SpiceMainChannel *main,
SpiceGtkSessionPrivate *s = self->priv;
GtkClipboard* clipboard = get_clipboard_from_selection(s, selection);
+SPICE_DEBUG("%s selection=%u (%p)", __func__, selection, clipboard);
+
if (!clipboard)
return;
--
2.20.1
___
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] Require GStreamer 1.10 and above
On Wed, Jan 23, 2019 at 10:30:21PM +0100, Victor Toso wrote: > From: Victor Toso > > Odd minor versions are for development version, which means that > checks on 1.9.x should be considered for development while normal > environments should be running the stable. > > Some timeline of stable releases: > > 1.14.4: Tue Oct 2 22:53:01 2018 +0100 > 1.14.0: Mon Mar 19 20:09:51 2018 + > 1.12.0: Thu May 4 15:36:55 2017 +0300 > 1.10.0: Tue Nov 1 17:50:24 2016 +0200 > > This patch reduces a bit the code paths in channel-display-gst.c > > CentOS 7.6 : 1.10.4 For what it's worth, this is even true for el7.4 releases. 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 v3 10/23] drm/qxl: move qxl_primary_apply_cursor to correct place
Hi, > > If the above explains things better to you I should probably replace the > > commit message with that. > > This is actually my first review of a driver that I'm not familiar with. > I'm not quite sure how much in depth understanding that is required to > put my ack on it. Usually I try to show that by picking either Reviewed-by (I'm confident the changes are fine) or Acked-by (Changes look sane, but I don't know the code base and/or hardware good enough to be sure). > Going further into the patchset I realised that > there's no way that I can verify the logic without being intimate with > the driver. Yep. Same for me when looking at patches for other drivers. I think this is one reason why it isn't that easy to get patches for drivers reviewed where effectively only a single maintainer/developer is working on. > I liked your expanded explanation better. Updated the commit message. cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 17/23] drm/qxl: use generic fbdev emulation
Den 28.01.2019 09.59, skrev Gerd Hoffmann:
> On Fri, Jan 25, 2019 at 06:25:27PM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 18.01.2019 13.20, skrev Gerd Hoffmann:
>>> Switch qxl over to the new generic fbdev emulation.
>>>
>>> Signed-off-by: Gerd Hoffmann
>>> ---
>>> drivers/gpu/drm/qxl/qxl_display.c | 7 ---
>>> drivers/gpu/drm/qxl/qxl_drv.c | 2 ++
>>> 2 files changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
>>> b/drivers/gpu/drm/qxl/qxl_display.c
>>> index ef832f98ab..9c751f01e3 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_display.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_display.c
>>> @@ -1221,18 +1221,11 @@ int qxl_modeset_init(struct qxl_device *qdev)
>>> qxl_display_read_client_monitors_config(qdev);
>>>
>>> drm_mode_config_reset(&qdev->ddev);
>>> -
>>> - /* primary surface must be created by this point, to allow
>>> -* issuing command queue commands and having them read by
>>> -* spice server. */
>>> - qxl_fbdev_init(qdev);
>>> return 0;
>>> }
>>>
>>> void qxl_modeset_fini(struct qxl_device *qdev)
>>> {
>>> - qxl_fbdev_fini(qdev);
>>> -
>>> qxl_destroy_monitors_object(qdev);
>>> drm_mode_config_cleanup(&qdev->ddev);
>>> }
>>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>>> index 13c8a662f9..3fce7d16df 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>>> @@ -93,6 +93,8 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
>>> pci_device_id *ent)
>>> if (ret)
>>> goto modeset_cleanup;
>>>
>>> + drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
>>
>> I couldn't find that this was part of old fbdev code, so it would be
>> nice to mention it in the commit message.
>
> It actually is, but a bit hidden because it doesn't use a helper you can
> easily grep for. Instead sets fb_info->apertures->ranges[0] in
> qxlfb_create(), which has the same effect.
>
Indeed,
Acked-by: Noralf Trønnes
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 10/23] drm/qxl: move qxl_primary_apply_cursor to correct place
Den 28.01.2019 09.10, skrev Gerd Hoffmann:
>>> The cursor must be set again after creating the primary surface.
>>> Also drop the error message.
>
>>> if (!bo->is_primary) {
>>> - if (!same_shadow)
>>> + if (!same_shadow) {
>>> qxl_io_create_primary(qdev, 0, bo);
>>> + qxl_primary_apply_cursor(plane);
>>> + }
>>> bo->is_primary = true;
>>> }
>>>
>>>
>>
>> I don't see how the commit message matches what you're doing. It gives
>> the impression that it must be applied under yet another condition, but
>> the condition for applying the cursor is changed from bo_old->is_primary
>> to !bo->is_primary.
>
> The qxl device ties the cursor to the primary surface. Therefore
> calling qxl_io_destroy_primary() and qxl_io_create_primary() to switch
> the framebuffer causes the cursor information being lost and the driver
> must re-apply it.
>
> The correct call order to do that is qxl_io_destroy_primary() +
> qxl_io_create_primary() + qxl_primary_apply_cursor().
>
> The old code did qxl_io_destroy_primary() + qxl_primary_apply_cursor() +
> qxl_io_create_primary(). Due to qxl_primary_apply_cursor request being
> queued in a ringbuffer and qxl_io_create_primary() trapping to the
> hypervisor instantly there is a high chance that qxl_io_create_primary()
> is processed first even with the wrong call order. But it's racy and
> thus not reliable.
>
>> It probably makes sense to someone that knows the driver.
>
> If the above explains things better to you I should probably replace the
> commit message with that.
>
This is actually my first review of a driver that I'm not familiar with.
I'm not quite sure how much in depth understanding that is required to
put my ack on it. Going further into the patchset I realised that
there's no way that I can verify the logic without being intimate with
the driver. So I have tried to verify things from a kms point of view.
I liked your expanded explanation better.
Noralf.
>> Acked-by: Noralf Trønnes
>
> thanks,
> Gerd
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] spice: set device address and device display ID in QXL interface
Calls the new SPICE QXL interface function spice_qxl_set_device_info to
set the hardware address of the graphics device represented by the QXL
interface (e.g. a PCI path) and the device display IDs (the IDs of the
device's monitors that belong to this QXL interface).
Also stops using the deprecated spice_qxl_set_max_monitors, the new
interface function replaces it.
Signed-off-by: Lukáš Hrázký
---
Hello,
the spice part of the interface was merged to spice server, so this is
ready to merge now too.
The original series:
https://lists.freedesktop.org/archives/spice-devel/2019-January/046997.html
Cheers,
Lukas
hw/display/qxl.c | 14 -
include/ui/spice-display.h | 2 ++
ui/spice-core.c| 42 ++
ui/spice-display.c | 11 ++
4 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 8e9a65e75b..434a2be7a4 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -276,7 +276,8 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice
*qxl, int replay)
QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
0));
} else {
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
+#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */ && \
+SPICE_SERVER_VERSION < 0x000e02
if (qxl->max_outputs) {
spice_qxl_set_max_monitors(&qxl->ssd.qxl, qxl->max_outputs);
}
@@ -2182,6 +2183,17 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error
**errp)
SPICE_INTERFACE_QXL_MAJOR, SPICE_INTERFACE_QXL_MINOR);
return;
}
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+char device_address[256] = "";
+if (qemu_spice_fill_device_address(qxl->vga.con, device_address, 256)) {
+spice_qxl_set_device_info(&qxl->ssd.qxl,
+ device_address,
+ 0,
+ qxl->max_outputs);
+}
+#endif
+
qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 87a84a59d4..7608fa7ebd 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -179,3 +179,5 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
void qemu_spice_display_start(void);
void qemu_spice_display_stop(void);
int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd);
+
+bool qemu_spice_fill_device_address(QemuConsole *con, char *device_address,
size_t size);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a40fb2c00d..65ad66ea53 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -34,6 +34,8 @@
#include "qemu/option.h"
#include "migration/misc.h"
#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
#include "ui/spice-display.h"
/* core bits */
@@ -863,6 +865,46 @@ bool qemu_spice_have_display_interface(QemuConsole *con)
return false;
}
+/*
+ * Recursively (in reverse order) appends addresses of PCI devices as it moves
+ * up in the PCI hierarchy.
+ *
+ * @returns true on success, false when the buffer wasn't large enough
+ */
+static bool append_pci_address(char *buf, size_t buf_size, const PCIDevice
*pci)
+{
+PCIBus *bus = pci_get_bus(pci);
+if (!pci_bus_is_root(bus)) {
+append_pci_address(buf, buf_size, bus->parent_dev);
+}
+
+size_t len = strlen(buf);
+ssize_t written = snprintf(buf + len, buf_size - len, "/%02x.%x",
+PCI_SLOT(pci->devfn), PCI_FUNC(pci->devfn));
+
+return written > 0 && written < buf_size - len;
+}
+
+bool qemu_spice_fill_device_address(QemuConsole *con, char *device_address,
size_t size)
+{
+DeviceState *dev = DEVICE(object_property_get_link(OBJECT(con), "device",
&error_abort));
+PCIDevice *pci = (PCIDevice *) object_dynamic_cast(OBJECT(dev),
TYPE_PCI_DEVICE);
+
+if (pci == NULL) {
+warn_report("Setting device address of a display device to SPICE: Not
a PCI device.");
+return false;
+}
+
+strncpy(device_address, "pci/", size);
+if (!append_pci_address(device_address, size, pci)) {
+warn_report("Setting device address of a display device to SPICE: "
+"Too many PCI devices in the chain.");
+return false;
+}
+
+return true;
+}
+
int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con)
{
if (g_slist_find(spice_consoles, con)) {
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 52f8cb5ae1..c1605b3bc9 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -1147,6 +1147,17 @@ static void qemu_spice_display_init_one(QemuConsole *con)
ssd->qxl.base.sif = &dpy_interface.base;
qemu_spice_add_display_interface(&ssd->qxl, con);
+
+#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
+char device_address[256] = "";
+if (qe
Re: [Spice-devel] [spice-gtk] clipboard: don't request targets without owner on X11
Hi,
On Sun, Jan 27, 2019 at 06:14:20PM +0100, Jakub Janků wrote:
> On X11, if the owner in GdkEventOwnerChange is set to NULL,
> it means there's no data in the clipboard, so it's pointless to
> request targets as the request will fail anyway.
>
> On Wayland, owner is always NULL, so don't do anything there.
Patch seems fine. I did similar but using the focus; Marc-André
did similar but for all instead of targeting x11 on NULL owner.
CC'ing Marc-André.
> Signed-off-by: Jakub Janků
> ---
> src/spice-gtk-session.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 1a19bca..b48f92a 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -674,6 +674,14 @@ static void clipboard_owner_change(GtkClipboard
> *clipboard,
> }
>
> s->clipboard_by_guest[selection] = FALSE;
> +
> +#ifdef GDK_WINDOWING_X11
> +if (!event->owner && GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
> +s->clip_hasdata[selection] = FALSE;
> +return;
> +}
> +#endif
> +
> s->clip_hasdata[selection] = TRUE;
> if (s->auto_clipboard_enable && !read_only(self))
> gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] QXL interface: improve the spice_qxl_set_device_info documentation
Instead of one unsupported example, present two real world examples.
Signed-off-by: Lukáš Hrázký
---
server/spice-qxl.h | 32 ++--
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/server/spice-qxl.h b/server/spice-qxl.h
index e7af5e5e..2f47910b 100644
--- a/server/spice-qxl.h
+++ b/server/spice-qxl.h
@@ -122,7 +122,7 @@ void spice_qxl_gl_draw_async(QXLInstance *instance,
* @instance the QXL instance to set the path to
* @device_address the path of the device this QXL instance belongs to
* @device_display_id_start the starting device display ID of this interface,
- * i.e. the one monitor ID 0 maps to
+ * i.e. the device display ID of monitor ID 0
* @device_display_id_count the total number of device display IDs on this
* interface
*
@@ -145,16 +145,28 @@ void spice_qxl_gl_draw_async(QXLInstance *instance,
*
* The device_display_id_{start,count} denotes the sequence of device display
* IDs that map to the zero-based sequence of monitor IDs provided by monitors
- * config on this interface. For example with device_display_id_start = 2 and
- * device_display_id_count = 3 you get the following mapping:
- * monitor_id -> device_display_id
- * 0 -> 2
- * 1 -> 3
- * 2 -> 4
+ * config on this interface.
*
- * Note this example is unsupported in practice. The only supported cases are
- * either a single device display ID (count = 1) or multiple device display IDs
- * in a sequence starting from 0.
+ * Example 1:
+ * A QXL graphics device with 3 heads (monitors).
+ *
+ * device_display_id_start = 0
+ * device_display_id_count = 3
+ *
+ * Results in the following mapping of monitor_id -> device_display_id:
+ * 0 -> 0
+ * 1 -> 1
+ * 2 -> 2
+ *
+ * Example 2:
+ * A virtio graphics device, multiple monitors, a QXL interface for each
+ * monitor. On the QXL interface for the third monitor:
+ *
+ * device_display_id_start = 2
+ * device_display_id_count = 1
+ *
+ * Results in the following mapping of monitor_id -> device_display_id:
+ * 0 -> 2
*/
void spice_qxl_set_device_info(QXLInstance *instance,
const char *device_address,
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v4 2/2] gtk-session: clipboard: x11: owner-change improvement
Hi,
I tried to fix this bug in a less radical way, but my patch unfortunately
did not cover all the cases.
I obtained some logs from James Harvey which make the situation clearer -
it can be found here:
https://termbin.com/40un
So I'll try to explain what's happening:
James uses KDE which has a clipboard manager integrated in (klipper).
(1) user copies something in the guest, grab is sent to the spice-gtk
(2) clipboard manager immediately requests the data, data is retrieved from
the vdagent
(3) user pastes the copied data in guest, this causes a quick re-grab in
the guest = a clipboard release message is sent to spice-gtk and it is
immediately followed by a new grab
(4) spice-gtk receives clipboard release, so it clears the clipboard
(5) clipboard manager detects that the clipboard has no owner, so it grabs
it itself, advertising the data it originally obtained from us in step (2)
(this normally enables the user to paste the data even after the source app
has been closed)
(6) spice-gtk receives "owner-change" signal caused by the grab in step
(5), requests clipboard targets and sends a grab to vdagent
(7) spice-gtk finally receives the grab from step (3), so it sets the
clipboard using gtk_clipboard_set_with_owner()
(8) vdagent receives grab from step (6), so it too sets the clipboard using
gtk_clipboard_set_with_owner()
This is clearly a race. We ended up in a state where both spice-gtk and
vdagent thinks the other component can provide the data, but in reality
none of them can.
(This does not happen always, as can be seen in the log, the first paste
succeeds.)
Given current spice protocol, I don't see a way to detect the race on
either side, but I'd love to be wrong. So I'd update the commit log and
comment in the code to explain the situation and then it's ack from me.
Cheers,
Jakub
On Mon, Jan 14, 2019, 1:34 PM Victor Toso From: Victor Toso
>
> On X11, the release-grab message might end up clearing the
> GtkClipboard which triggers the owner-changed callback having the
> event owner as NULL.
>
> We should not be calling gtk_clipboard_request_targets() in this
> situation as is prone to errors as the intention here is request
> clipboard information from changes made by client OS.
>
> The fix is to avoid any such request while spice client is holding the
> keyboard grab.
>
> Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
> Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876
>
> Changed in v4:
> - Updated commit log
>
> Signed-off-by: Victor Toso
> Tested-by: James Harvey @jamespharvey20
> ---
> src/spice-gtk-session.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index abce43f..20df70d 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -674,6 +674,19 @@ static void clipboard_owner_change(GtkClipboard
> *clipboard,
> return;
> }
>
> +#ifdef GDK_WINDOWING_X11
> +/* In X11, while holding the keyboard-grab we are not interested in
> this
> + * event as it either came by grab or release messages from agent. */
> +if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> +spice_gtk_session_get_keyboard_has_focus(self)) {
> +SPICE_DEBUG("clipboard: owner-changed event: not requesting
> client's target "
> +"while holding focus");
> +return;
> +}
> +#endif
> +SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> +spice_gtk_session_get_keyboard_has_focus(self));
> +
> s->clipboard_by_guest[selection] = FALSE;
> s->clip_hasdata[selection] = TRUE;
> if (s->auto_clipboard_enable && !read_only(self))
> --
> 2.20.1
>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 17/23] drm/qxl: use generic fbdev emulation
On Fri, Jan 25, 2019 at 06:25:27PM +0100, Noralf Trønnes wrote:
>
>
> Den 18.01.2019 13.20, skrev Gerd Hoffmann:
> > Switch qxl over to the new generic fbdev emulation.
> >
> > Signed-off-by: Gerd Hoffmann
> > ---
> > drivers/gpu/drm/qxl/qxl_display.c | 7 ---
> > drivers/gpu/drm/qxl/qxl_drv.c | 2 ++
> > 2 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> > b/drivers/gpu/drm/qxl/qxl_display.c
> > index ef832f98ab..9c751f01e3 100644
> > --- a/drivers/gpu/drm/qxl/qxl_display.c
> > +++ b/drivers/gpu/drm/qxl/qxl_display.c
> > @@ -1221,18 +1221,11 @@ int qxl_modeset_init(struct qxl_device *qdev)
> > qxl_display_read_client_monitors_config(qdev);
> >
> > drm_mode_config_reset(&qdev->ddev);
> > -
> > - /* primary surface must be created by this point, to allow
> > -* issuing command queue commands and having them read by
> > -* spice server. */
> > - qxl_fbdev_init(qdev);
> > return 0;
> > }
> >
> > void qxl_modeset_fini(struct qxl_device *qdev)
> > {
> > - qxl_fbdev_fini(qdev);
> > -
> > qxl_destroy_monitors_object(qdev);
> > drm_mode_config_cleanup(&qdev->ddev);
> > }
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 13c8a662f9..3fce7d16df 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -93,6 +93,8 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > if (ret)
> > goto modeset_cleanup;
> >
> > + drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
>
> I couldn't find that this was part of old fbdev code, so it would be
> nice to mention it in the commit message.
It actually is, but a bit hidden because it doesn't use a helper you can
easily grep for. Instead sets fb_info->apertures->ranges[0] in
qxlfb_create(), which has the same effect.
cheers,
Gerd
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 10/23] drm/qxl: move qxl_primary_apply_cursor to correct place
> > The cursor must be set again after creating the primary surface.
> > Also drop the error message.
> > if (!bo->is_primary) {
> > - if (!same_shadow)
> > + if (!same_shadow) {
> > qxl_io_create_primary(qdev, 0, bo);
> > + qxl_primary_apply_cursor(plane);
> > + }
> > bo->is_primary = true;
> > }
> >
> >
>
> I don't see how the commit message matches what you're doing. It gives
> the impression that it must be applied under yet another condition, but
> the condition for applying the cursor is changed from bo_old->is_primary
> to !bo->is_primary.
The qxl device ties the cursor to the primary surface. Therefore
calling qxl_io_destroy_primary() and qxl_io_create_primary() to switch
the framebuffer causes the cursor information being lost and the driver
must re-apply it.
The correct call order to do that is qxl_io_destroy_primary() +
qxl_io_create_primary() + qxl_primary_apply_cursor().
The old code did qxl_io_destroy_primary() + qxl_primary_apply_cursor() +
qxl_io_create_primary(). Due to qxl_primary_apply_cursor request being
queued in a ringbuffer and qxl_io_create_primary() trapping to the
hypervisor instantly there is a high chance that qxl_io_create_primary()
is processed first even with the wrong call order. But it's racy and
thus not reliable.
> It probably makes sense to someone that knows the driver.
If the above explains things better to you I should probably replace the
commit message with that.
> Acked-by: Noralf Trønnes
thanks,
Gerd
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
