[virt-tools-list] [virt-viewer] spice: avoid crashing when using multi-seat

[Jonathon Jongsma]

On Tue, 2016-03-15 at 16:51 +0100, Fabiano Fidêncio wrote:

> As multi-seat configuration is not supported by spice, the best we can
> do for now is avoid crashing on this setup.

Could you expand this commit log to make it more obvious how this change relates
to "multi-seat"? Actually, I think that "multi-seat" is not really the correct
term here. What you really mean is "more than one multi-head graphics device", I
think.

Also (this is unrelated to your change):
I wonder if we should change virt_viewer_display_spice_new() to not use
g_return_val_if_fail() for this situation. The fact that g_return_*if_fail()
functions can be disabled by setting G_DISABLE_CHECKS indicates that they should
only be used for programming errors, whereas this is a situation caused by data
received over the spice protocol. Maybe something like:


diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
index 5114833..c657047 100644
--- a/src/virt-viewer-display-spice.c
+++ b/src/virt-viewer-display-spice.c
@@ -286,8 +286,10 @@ virt_viewer_display_spice_new(VirtViewerSessionSpice
*session,
     g_return_val_if_fail(SPICE_IS_DISPLAY_CHANNEL(channel), NULL);
 
     g_object_get(channel, "channel-id", &channelid, NULL);
-    // We don't allow monitorid != 0 && channelid != 0
-    g_return_val_if_fail(channelid == 0 || monitorid == 0, NULL);
+    if (channelid == 0 || monitorid == 0) {
+        g_warning("Unsupported graphics configuration. spice-gtk only supports
multiple graphics channels if they are single-head");
+        return NULL;
+    }
 
     self = g_object_new(VIRT_VIEWER_TYPE_DISPLAY_SPICE,
                         "session", session,


Probably that comment could be worded better though...

Jonathon


> 
> Resolves: rhbz#1250820
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  src/virt-viewer-session-spice.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index dc0c1ff..a3014a8 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -836,8 +836,11 @@ static void
>  destroy_display(gpointer data)
>  {
>      VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(data);
> -    VirtViewerSession *session = virt_viewer_display_get_session(display);
> +    VirtViewerSession *session;
>  
> +    g_return_if_fail (display != NULL);
> +
> +    session = virt_viewer_display_get_session(display);
>      g_debug("Destroying spice display %p", display);
>      virt_viewer_session_remove_display(session, display);
>      g_object_unref(display);
> @@ -886,6 +889,9 @@ virt_viewer_session_spice_display_monitors(SpiceChannel
> *channel,
>          display = g_ptr_array_index(displays, i);
>          if (display == NULL) {
>              display = virt_viewer_display_spice_new(self, channel, i);
> +            if (display == NULL)
> +                continue;
> +
>              g_debug("creating spice display (#:%d)",
>                     
>  virt_viewer_display_get_nth(VIRT_VIEWER_DISPLAY(display)));
>              g_ptr_array_index(displays, i) = g_object_ref_sink(display);