[Qemu-devel] [PATCH v3 00/11] spice: add -display spice-app, setup and launch a Spice client

[Qemu-devel] [PATCH v3 00/11] spice: add -display spice-app, setup and launch a Spice client

[Marc-André Lureau]

Hi,

The QEMU GTK UI is quite featurful. Yet, it doesn't provide many of
the functionalities one can expect from a VM desktop application (USB
redirection, shared folders, drag and drop, better multimonitor
support etc.).

Spice is able to export many of the VM controls to a client, including
QMP monitor, serial ports etc... It's thus possible for a Spice client
to provide a similar experience as the QEMU GTK UI (while keeping
similar display performances locally) and more.

This series provides "-display spice-app" to setup a Spice server and
launch a client (associated with the spice+unix:// URI). With
virt-viewer series "Add QEMU-like UI: VT console & basic VM state"
(under review), the resulting user experience is close to what qemu
GTK UI provides.

v3:
- Add hints on "default" selection in "qapi: document DisplayType enum"
- add tested-by tags
- rebased

v2:
- renamed -display app -> -display spice-app
- error if full-screen or window-close is provided (supports can be
  added later)
- use a stable URI when -name is provided
- print the URI at start
- added "qapi: document DisplayType enum"
- added "spice: use a default name for the server"
- commit message tweaks
- include previously reviewed "configure: bump spice-server required
  version to 0.12.5", to avoid conflicts
- rebased

Marc-André Lureau (11):
  char/spice: trigger HUP event
  char/spice: discard write() if backend is disconnected
  spice: avoid spice runtime assert
  spice: merge options lists
  spice: do not stop spice if VM is paused
  char: move SpiceChardev and open_spice_port() to spice.h header
  char: register spice ports after spice started
  build-sys: add gio-2.0 check
  qapi: document DisplayType enum
  spice: use a default name for the server
  display: add -display spice-app launching a Spice client

 qapi/ui.json            |  32 ++++++-
 include/chardev/spice.h |  27 ++++++
 chardev/spice.c         |  60 +++++++-----
 ui/spice-app.c          | 200 ++++++++++++++++++++++++++++++++++++++++
 ui/spice-core.c         |  13 ++-
 chardev/trace-events    |   1 +
 configure               |  13 +++
 qemu-options.hx         |   5 +
 ui/Makefile.objs        |   5 +
 9 files changed, 329 insertions(+), 27 deletions(-)
 create mode 100644 include/chardev/spice.h
 create mode 100644 ui/spice-app.c

-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 01/11] char/spice: trigger HUP event

[Marc-André Lureau]

Inform the front-end of disconnected state (spice client
disconnected).

This will wakeup the source handler immediately, so it can detect the
disconnection asap.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 chardev/spice.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/chardev/spice.c b/chardev/spice.c
index 173c257949..c2baeb5461 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -148,15 +148,25 @@ static void vmc_unregister_interface(SpiceChardev *scd)
 static gboolean spice_char_source_prepare(GSource *source, gint *timeout)
 {
     SpiceCharSource *src = (SpiceCharSource *)source;
+    Chardev *chr = CHARDEV(src->scd);
 
     *timeout = -1;
 
+    if (!chr->be_open) {
+        return true;
+    }
+
     return !src->scd->blocked;
 }
 
 static gboolean spice_char_source_check(GSource *source)
 {
     SpiceCharSource *src = (SpiceCharSource *)source;
+    Chardev *chr = CHARDEV(src->scd);
+
+    if (!chr->be_open) {
+        return true;
+    }
 
     return !src->scd->blocked;
 }
@@ -164,9 +174,12 @@ static gboolean spice_char_source_check(GSource *source)
 static gboolean spice_char_source_dispatch(GSource *source,
     GSourceFunc callback, gpointer user_data)
 {
+    SpiceCharSource *src = (SpiceCharSource *)source;
+    Chardev *chr = CHARDEV(src->scd);
     GIOFunc func = (GIOFunc)callback;
+    GIOCondition cond = chr->be_open ? G_IO_OUT : G_IO_HUP;
 
-    return func(NULL, G_IO_OUT, user_data);
+    return func(NULL, cond, user_data);
 }
 
 static GSourceFuncs SpiceCharSourceFuncs = {
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 02/11] char/spice: discard write() if backend is disconnected

[Marc-André Lureau]

Most chardev backend handle write() as discarded data if underlying
system is disconnected. For unknown historical reasons, the Spice
backend has "reliable" write: it will wait until the client end is
reconnected to do further successful write().

To decide whether it make sense to wait until the client is
reconnected (or queue the writes), let's review Spice chardev usage
and handling of a disconnected client:

 * spice vdagent
   The agents reopen the virtio port on disconnect. In qemu side,
   virtio_serial_close() will also discard pending data.

 * usb redirection
   A disconnect creates a device disconnection.

 * smartcard emulation
   Data is discarded in passthru_apdu_from_guest().

   (Spice doesn't explicitly open the smartcard char device until
   upcoming 0.14.2, commit 69a5cfc74131ec0459f2eb5a231139f5a69a8037)

 * spice webdavd
   The daemon will restart the service, and reopen the virtio port.

 * spice ports (serial console, qemu monitor..)
   Depends on the associated device or usage.

   - serial, may be throttled or discarded on write, depending on
     device

   - QMP/HMP monitor have some CLOSED event handling, but want to
     flush the write, which will finish when a new client connects.

On disconnect/reconnect, the client starts with fresh sessions. If it
is a seamless migration, the client disconnects after the source
migrated. The handling of source disconnect in qemu is thus irrelevant
for the Spice session migration.

For all these use cases, it is better to discard writes when the
client is disconnected, and require the vm-side device/agent to behave
correctly on CHR_EVENT_CLOSED, to stop reading and writing from
the spice chardev.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 chardev/spice.c      | 12 ++++++++++++
 chardev/trace-events |  1 +
 2 files changed, 13 insertions(+)

diff --git a/chardev/spice.c b/chardev/spice.c
index c2baeb5461..c68e60115b 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -208,6 +208,12 @@ static int spice_chr_write(Chardev *chr, const uint8_t *buf, int len)
     int read_bytes;
 
     assert(s->datalen == 0);
+
+    if (!chr->be_open) {
+        trace_spice_chr_discard_write(len);
+        return len;
+    }
+
     s->datapos = buf;
     s->datalen = len;
     spice_server_char_device_wakeup(&s->sin);
@@ -300,6 +306,12 @@ static void qemu_chr_open_spice_vmc(Chardev *chr,
     }
 
     *be_opened = false;
+#if SPICE_SERVER_VERSION < 0x000e02
+    /* Spice < 0.14.2 doesn't explicitly open smartcard chardev */
+    if (strcmp(type, "smartcard") == 0) {
+        *be_opened = true;
+    }
+#endif
     chr_open(chr, type);
 }
 
diff --git a/chardev/trace-events b/chardev/trace-events
index d0e5f3bbc1..b8a7596344 100644
--- a/chardev/trace-events
+++ b/chardev/trace-events
@@ -10,6 +10,7 @@ wct_cmd_other(const char *cmd) "%s"
 wct_speed(int speed) "%d"
 
 # chardev/spice.c
+spice_chr_discard_write(int len) "spice chr write discarded %d"
 spice_vmc_write(ssize_t out, int len) "spice wrote %zd of requested %d"
 spice_vmc_read(int bytes, int len) "spice read %d of requested %d"
 spice_vmc_register_interface(void *scd) "spice vmc registered interface %p"
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 03/11] spice: avoid spice runtime assert

[Marc-André Lureau]

The Spice server doesn't like to be started or stopped twice . It
aborts with:

(process:6191): Spice-ERROR **: 19:29:35.912: red-worker.c:623:handle_dev_start: assertion `!worker->running' failed

It's easy to avoid that situation since qemu spice_display_is_running
tracks the server state.

After the commit "spice: do not stop spice if VM is paused", it will
be possible to pause and resume the VM, and this will call
qemu_spice_display_start() twice. The easiest is to add a check for
spice_display_is_running with this patch to avoid the assert.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 ui/spice-core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index a40fb2c00d..94b31ce3ad 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -921,12 +921,20 @@ int qemu_spice_display_add_client(int csock, int skipauth, int tls)
 
 void qemu_spice_display_start(void)
 {
+    if (spice_display_is_running) {
+        return;
+    }
+
     spice_display_is_running = true;
     spice_server_vm_start(spice_server);
 }
 
 void qemu_spice_display_stop(void)
 {
+    if (!spice_display_is_running) {
+        return;
+    }
+
     spice_server_vm_stop(spice_server);
     spice_display_is_running = false;
 }
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 04/11] spice: merge options lists

[Marc-André Lureau]

Passing several -spice options to qemu command line, or calling
several time qemu_opts_set() will ignore all but the first option
list. Since the spice server is a singleton, it makes sense to merge
all the options, the last value being the one taken into account.

This changes the behaviour from, for ex:
$ qemu... -spice port=5900 -spice port=5901 -> port: 5900
to:
$ qemu... -spice port=5900 -spice port=5901 -> port: 5901

(if necessary we could instead produce an error when an option is
given twice, although this makes handling default values and such more
complicated)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 ui/spice-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 94b31ce3ad..91d0960522 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -397,6 +397,7 @@ static SpiceChannelList *qmp_query_spice_channels(void)
 static QemuOptsList qemu_spice_opts = {
     .name = "spice",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_spice_opts.head),
+    .merge_lists = true,
     .desc = {
         {
             .name = "port",
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 05/11] spice: do not stop spice if VM is paused

[Marc-André Lureau]

spice_server_vm_start/stop() was added to help migration state (commit
f5bb039c6d97ef3e664094eab3c9a4dc1824ed73).

However, a paused VM could keep running the spice server. This will
allow a Spice client to keep sending commands to a spice chardev. This
allows to stop/cont a VM from a Spice monitor port. Character
devices (vdagent/usb/smartcard/..) should not read from Spice when the
VM is paused.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 ui/spice-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 91d0960522..92eebe5f0e 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -627,7 +627,7 @@ static void vm_change_state_handler(void *opaque, int running,
 {
     if (running) {
         qemu_spice_display_start();
-    } else {
+    } else if (state != RUN_STATE_PAUSED) {
         qemu_spice_display_stop();
     }
 }
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 06/11] char: move SpiceChardev and open_spice_port() to spice.h header

[Marc-André Lureau]

This will allow easier subclassing of SpiceChardev, in upcoming
"display: add -display spice-app launching external application"
patch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 include/chardev/spice.h | 27 +++++++++++++++++++++++++++
 chardev/spice.c         | 28 +++++-----------------------
 2 files changed, 32 insertions(+), 23 deletions(-)
 create mode 100644 include/chardev/spice.h

diff --git a/include/chardev/spice.h b/include/chardev/spice.h
new file mode 100644
index 0000000000..6431da3205
--- /dev/null
+++ b/include/chardev/spice.h
@@ -0,0 +1,27 @@
+#ifndef CHARDEV_SPICE_H_
+#define CHARDEV_SPICE_H_
+
+#include <spice.h>
+#include "chardev/char-fe.h"
+
+typedef struct SpiceChardev {
+    Chardev               parent;
+
+    SpiceCharDeviceInstance sin;
+    bool                  active;
+    bool                  blocked;
+    const uint8_t         *datapos;
+    int                   datalen;
+    QLIST_ENTRY(SpiceChardev) next;
+} SpiceChardev;
+
+#define TYPE_CHARDEV_SPICE "chardev-spice"
+#define TYPE_CHARDEV_SPICEVMC "chardev-spicevmc"
+#define TYPE_CHARDEV_SPICEPORT "chardev-spiceport"
+
+#define SPICE_CHARDEV(obj) OBJECT_CHECK(SpiceChardev, (obj), TYPE_CHARDEV_SPICE)
+
+void qemu_chr_open_spice_port(Chardev *chr, ChardevBackend *backend,
+                              bool *be_opened, Error **errp);
+
+#endif
diff --git a/chardev/spice.c b/chardev/spice.c
index c68e60115b..2202d50eee 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -2,30 +2,12 @@
 #include "trace.h"
 #include "ui/qemu-spice.h"
 #include "chardev/char.h"
+#include "chardev/spice.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
-#include <spice.h>
 #include <spice/protocol.h>
 
-
-typedef struct SpiceChardev {
-    Chardev               parent;
-
-    SpiceCharDeviceInstance sin;
-    bool                  active;
-    bool                  blocked;
-    const uint8_t         *datapos;
-    int                   datalen;
-    QLIST_ENTRY(SpiceChardev) next;
-} SpiceChardev;
-
-#define TYPE_CHARDEV_SPICE "chardev-spice"
-#define TYPE_CHARDEV_SPICEVMC "chardev-spicevmc"
-#define TYPE_CHARDEV_SPICEPORT "chardev-spiceport"
-
-#define SPICE_CHARDEV(obj) OBJECT_CHECK(SpiceChardev, (obj), TYPE_CHARDEV_SPICE)
-
 typedef struct SpiceCharSource {
     GSource               source;
     SpiceChardev       *scd;
@@ -315,10 +297,10 @@ static void qemu_chr_open_spice_vmc(Chardev *chr,
     chr_open(chr, type);
 }
 
-static void qemu_chr_open_spice_port(Chardev *chr,
-                                     ChardevBackend *backend,
-                                     bool *be_opened,
-                                     Error **errp)
+void qemu_chr_open_spice_port(Chardev *chr,
+                              ChardevBackend *backend,
+                              bool *be_opened,
+                              Error **errp)
 {
     ChardevSpicePort *spiceport = backend->u.spiceport.data;
     const char *name = spiceport->fqdn;
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 07/11] char: register spice ports after spice started

[Marc-André Lureau]

Spice port registration is delayed until the server is started. But
ports created after are not being registered. If the server is already
started, do vmc_register_interface() to register it from
qemu_chr_open_spice_port().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 chardev/spice.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/chardev/spice.c b/chardev/spice.c
index 2202d50eee..22c30ae833 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -316,6 +316,11 @@ void qemu_chr_open_spice_port(Chardev *chr,
     *be_opened = false;
     s = SPICE_CHARDEV(chr);
     s->sin.portname = g_strdup(name);
+
+    if (using_spice) {
+        /* spice server already created */
+        vmc_register_interface(s);
+    }
 }
 
 void qemu_spice_register_ports(void)
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 08/11] build-sys: add gio-2.0 check

[Marc-André Lureau]

GIO is required for the "-display spice-app" backend.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 configure | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/configure b/configure
index a61682c3c7..05d72f1c56 100755
--- a/configure
+++ b/configure
@@ -3503,6 +3503,14 @@ for i in $glib_modules; do
     fi
 done
 
+if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then
+    gio=yes
+    gio_cflags=$($pkg_config --cflags gio-2.0)
+    gio_libs=$($pkg_config --libs gio-2.0)
+else
+    gio=no
+fi
+
 # Sanity check that the current size_t matches the
 # size that glib thinks it should be. This catches
 # problems on multi-arch where people try to build
@@ -6520,6 +6528,11 @@ if test "$gtk" = "yes" ; then
     echo "CONFIG_GTK_GL=y" >> $config_host_mak
   fi
 fi
+if test "$gio" = "yes" ; then
+    echo "CONFIG_GIO=y" >> $config_host_mak
+    echo "GIO_CFLAGS=$gio_cflags" >> $config_host_mak
+    echo "GIO_LIBS=$gio_libs" >> $config_host_mak
+fi
 echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
 if test "$gnutls" = "yes" ; then
   echo "CONFIG_GNUTLS=y" >> $config_host_mak
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 09/11] qapi: document DisplayType enum

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 qapi/ui.json | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/qapi/ui.json b/qapi/ui.json
index 7d9c4bddaf..7702ddf583 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1085,6 +1085,31 @@
 #
 # Display (user interface) type.
 #
+# @default: The default user interface, selecting from the first available
+#           of gtk, sdl, cocoa, and vnc.
+#
+# @none: No user interface or video output display. The guest will
+#        still see an emulated graphics card, but its output will not
+#        be displayed to the QEMU user.
+#
+# @gtk: The GTK user interface.
+#
+# @sdl: The SDL user interface.
+#
+# @egl-headless: No user interface, offload GL operations to a local
+#                DRI device. Graphical display need to be paired with
+#                VNC or Spice. (Since 3.1)
+#
+# @curses: Display video output via curses.  For graphics device
+#          models which support a text mode, QEMU can display this
+#          output using a curses/ncurses interface. Nothing is
+#          displayed when the graphics device is in graphical mode or
+#          if the graphics device does not support a text
+#          mode. Generally only the VGA device models support text
+#          mode.
+#
+# @cocoa: The Cocoa user interface.
+#
 # Since: 2.12
 #
 ##
-- 
2.21.0.rc1

Re: [Qemu-devel] [PATCH v3 09/11] qapi: document DisplayType enum

[Eric Blake]

On 2/21/19 5:07 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Tested-by: Victor Toso <victortoso@redhat.com>
> ---
>  qapi/ui.json | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 7d9c4bddaf..7702ddf583 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1085,6 +1085,31 @@
>  #
>  # Display (user interface) type.
>  #
> +# @default: The default user interface, selecting from the first available
> +#           of gtk, sdl, cocoa, and vnc.
> +#
> +# @none: No user interface or video output display. The guest will
> +#        still see an emulated graphics card, but its output will not
> +#        be displayed to the QEMU user.
> +#
> +# @gtk: The GTK user interface.
> +#
> +# @sdl: The SDL user interface.
> +#
> +# @egl-headless: No user interface, offload GL operations to a local
> +#                DRI device. Graphical display need to be paired with

s/need/needs/

> +#                VNC or Spice. (Since 3.1)
> +#

Are these retroactive documentation for things that existed in 3.1 or
2.12, or should any of them have '(since 4.0)' tags?  The commit message
should probably mention that.

> +# @curses: Display video output via curses.  For graphics device
> +#          models which support a text mode, QEMU can display this
> +#          output using a curses/ncurses interface. Nothing is
> +#          displayed when the graphics device is in graphical mode or
> +#          if the graphics device does not support a text
> +#          mode. Generally only the VGA device models support text
> +#          mode.
> +#
> +# @cocoa: The Cocoa user interface.
> +#
>  # Since: 2.12
>  #
>  ##
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH v3 10/11] spice: use a default name for the server

[Marc-André Lureau]

If no -name is given, let's use a friendly "QEMU version" server
name. This is sometime exposed on spice client side, for example on
remote-viewer title.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 ui/spice-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 92eebe5f0e..2ac81129e5 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -784,7 +784,7 @@ void qemu_spice_init(void)
 
     qemu_opt_foreach(opts, add_channel, &tls_port, &error_fatal);
 
-    spice_server_set_name(spice_server, qemu_name);
+    spice_server_set_name(spice_server, qemu_name ?: "QEMU " QEMU_VERSION);
     spice_server_set_uuid(spice_server, (unsigned char *)&qemu_uuid);
 
     seamless_migration = qemu_opt_get_bool(opts, "seamless-migration", 0);
-- 
2.21.0.rc1

[Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Marc-André Lureau]

Add a new display backend that will configure Spice to allow a remote
client to control QEMU in a similar fashion as other QEMU display
backend/UI like GTK.

For this to work, it will set up Spice server with a unix socket, and
register a VC chardev that will be exposed as Spice ports. A QMP
monitor is also exposed as a Spice port, this allows the remote client
fuller qemu control and state handling.

- doesn't handle VC set_echo() - this doesn't seem a strong
  requirement, very few front-end use it
- spice options can be tweaked with other -spice arguments
- Windows support shouldn't be hard to do, but will probably use a TCP
  port instead
- we may want to watch the child process to quit automatically if it
  crashed

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
---
 qapi/ui.json     |   7 +-
 ui/spice-app.c   | 200 +++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx  |   5 ++
 ui/Makefile.objs |   5 ++
 4 files changed, 216 insertions(+), 1 deletion(-)
 create mode 100644 ui/spice-app.c

diff --git a/qapi/ui.json b/qapi/ui.json
index 7702ddf583..c5d1d7f099 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1110,12 +1110,17 @@
 #
 # @cocoa: The Cocoa user interface.
 #
+# @spice-app: Set up a Spice server and run the default associated
+#             application to connect to it. The server will redirect
+#             the serial console and QEMU monitors. (Since 4.0)
+#
 # Since: 2.12
 #
 ##
 { 'enum'    : 'DisplayType',
   'data'    : [ 'default', 'none', 'gtk', 'sdl',
-                'egl-headless', 'curses', 'cocoa' ] }
+                'egl-headless', 'curses', 'cocoa',
+                'spice-app'] }
 
 ##
 # @DisplayOptions:
diff --git a/ui/spice-app.c b/ui/spice-app.c
new file mode 100644
index 0000000000..4f5229f3ee
--- /dev/null
+++ b/ui/spice-app.c
@@ -0,0 +1,200 @@
+/*
+ * QEMU external Spice client display driver
+ *
+ * Copyright (c) 2018 Marc-André Lureau <marcandre.lureau@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+
+#include <gio/gio.h>
+
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "io/channel-command.h"
+#include "chardev/spice.h"
+#include "sysemu/sysemu.h"
+
+static const char *tmp_dir;
+static char *app_dir;
+static char *sock_path;
+
+typedef struct VCChardev {
+    SpiceChardev parent;
+} VCChardev;
+
+#define TYPE_CHARDEV_VC "chardev-vc"
+#define VC_CHARDEV(obj) OBJECT_CHECK(VCChardev, (obj), TYPE_CHARDEV_VC)
+
+static ChardevBackend *
+chr_spice_backend_new(void)
+{
+    ChardevBackend *be = g_new0(ChardevBackend, 1);
+
+    be->type = CHARDEV_BACKEND_KIND_SPICEPORT;
+    be->u.spiceport.data = g_new0(ChardevSpicePort, 1);
+
+    return be;
+}
+
+static void vc_chr_open(Chardev *chr,
+                        ChardevBackend *backend,
+                        bool *be_opened,
+                        Error **errp)
+{
+    ChardevBackend *be;
+    const char *fqdn = NULL;
+
+    if (strstart(chr->label, "serial", NULL)) {
+        fqdn = "org.qemu.console.serial.0";
+    } else if (strstart(chr->label, "parallel", NULL)) {
+        fqdn = "org.qemu.console.parallel.0";
+    } else if (strstart(chr->label, "compat_monitor", NULL)) {
+        fqdn = "org.qemu.monitor.hmp.0";
+    }
+
+    be = chr_spice_backend_new();
+    be->u.spiceport.data->fqdn = fqdn ?
+        g_strdup(fqdn) : g_strdup_printf("org.qemu.console.%s", chr->label);
+    qemu_chr_open_spice_port(chr, be, be_opened, errp);
+    qapi_free_ChardevBackend(be);
+}
+
+static void vc_chr_set_echo(Chardev *chr, bool echo)
+{
+    /* TODO: set echo for frontends QMP and qtest */
+}
+
+static void char_vc_class_init(ObjectClass *oc, void *data)
+{
+    ChardevClass *cc = CHARDEV_CLASS(oc);
+
+    cc->parse = qemu_chr_parse_vc;
+    cc->open = vc_chr_open;
+    cc->chr_set_echo = vc_chr_set_echo;
+}
+
+static const TypeInfo char_vc_type_info = {
+    .name = TYPE_CHARDEV_VC,
+    .parent = TYPE_CHARDEV_SPICEPORT,
+    .instance_size = sizeof(VCChardev),
+    .class_init = char_vc_class_init,
+};
+
+static void spice_app_atexit(void)
+{
+    if (sock_path) {
+        unlink(sock_path);
+    }
+    if (tmp_dir) {
+        rmdir(tmp_dir);
+    }
+    g_free(sock_path);
+    g_free(app_dir);
+}
+
+static void spice_app_display_early_init(DisplayOptions *opts)
+{
+    QemuOpts *qopts;
+    ChardevBackend *be = chr_spice_backend_new();
+    GError *err = NULL;
+
+    if (opts->has_full_screen) {
+        error_report("spice-app full-screen isn't supported yet.");
+        exit(1);
+    }
+    if (opts->has_window_close) {
+        error_report("spice-app window-close isn't supported yet.");
+        exit(1);
+    }
+
+    atexit(spice_app_atexit);
+
+    if (qemu_name) {
+        app_dir = g_build_filename(g_get_user_runtime_dir(),
+                                   "qemu", qemu_name, NULL);
+        if (g_mkdir_with_parents(app_dir, S_IRWXU) < -1) {
+            error_report("Failed to create directory %s: %s",
+                         app_dir, strerror(errno));
+            exit(1);
+        }
+    } else {
+        app_dir = g_dir_make_tmp(NULL, &err);
+        tmp_dir = app_dir;
+        if (err) {
+            error_report("Failed to create temporary directory: %s",
+                         err->message);
+            exit(1);
+        }
+    }
+
+    type_register(&char_vc_type_info);
+
+    sock_path = g_strjoin("", app_dir, "/", "spice.sock", NULL);
+    qopts = qemu_opts_create(qemu_find_opts("spice"), NULL, 0, &error_abort);
+    qemu_opt_set(qopts, "disable-ticketing", "on", &error_abort);
+    qemu_opt_set(qopts, "unix", "on", &error_abort);
+    qemu_opt_set(qopts, "addr", sock_path, &error_abort);
+    qemu_opt_set(qopts, "image-compression", "off", &error_abort);
+    qemu_opt_set(qopts, "streaming-video", "off", &error_abort);
+    qemu_opt_set(qopts, "gl", opts->has_gl ? "on" : "off", &error_abort);
+    display_opengl = opts->has_gl;
+
+    be->u.spiceport.data->fqdn = g_strdup("org.qemu.monitor.qmp.0");
+    qemu_chardev_new("org.qemu.monitor.qmp", TYPE_CHARDEV_SPICEPORT,
+                     be, NULL, &error_abort);
+    qopts = qemu_opts_create(qemu_find_opts("mon"),
+                             NULL, 0, &error_fatal);
+    qemu_opt_set(qopts, "chardev", "org.qemu.monitor.qmp", &error_abort);
+    qemu_opt_set(qopts, "mode", "control", &error_abort);
+
+    qapi_free_ChardevBackend(be);
+}
+
+static void spice_app_display_init(DisplayState *ds, DisplayOptions *opts)
+{
+    GError *err = NULL;
+    gchar *uri;
+
+    uri = g_strjoin("", "spice+unix://", app_dir, "/", "spice.sock", NULL);
+    info_report("Launching display with URI: %s", uri);
+    g_app_info_launch_default_for_uri(uri, NULL, &err);
+    if (err) {
+        error_report("Failed to launch %s URI: %s", uri, err->message);
+        exit(1);
+    }
+    g_free(uri);
+}
+
+static QemuDisplay qemu_display_spice_app = {
+    .type       = DISPLAY_TYPE_SPICE_APP,
+    .early_init = spice_app_display_early_init,
+    .init       = spice_app_display_init,
+};
+
+static void register_spice_app(void)
+{
+    qemu_display_register(&qemu_display_spice_app);
+}
+
+type_init(register_spice_app);
diff --git a/qemu-options.hx b/qemu-options.hx
index 77bd98e20b..c843126ebd 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1211,6 +1211,7 @@ STEXI
 ETEXI
 
 DEF("display", HAS_ARG, QEMU_OPTION_display,
+    "-display spice-app[,gl=on|off]\n"
     "-display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n"
     "            [,window_close=on|off][,gl=on|core|es|off]\n"
     "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n"
@@ -1262,6 +1263,10 @@ Start a VNC server on display <arg>
 @item egl-headless
 Offload all OpenGL operations to a local DRI device. For any graphical display,
 this display needs to be paired with either VNC or SPICE displays.
+@item spice-app
+Start QEMU as a Spice server and launch the default Spice client
+application. The Spice server will redirect the serial consoles and
+QEMU monitors. (Since 4.0)
 @end table
 ETEXI
 
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 7f8b3da791..fe1a7aed97 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -49,6 +49,11 @@ curses.mo-objs := curses.o
 curses.mo-cflags := $(CURSES_CFLAGS)
 curses.mo-libs := $(CURSES_LIBS)
 
+common-obj-$(call land,$(CONFIG_SPICE),$(CONFIG_GIO)) += spice-app.mo
+spice-app.mo-objs := spice-app.o
+spice-app.mo-cflags := $(GIO_CFLAGS)
+spice-app.mo-libs := $(GIO_LIBS)
+
 common-obj-$(CONFIG_OPENGL) += shader.o
 common-obj-$(CONFIG_OPENGL) += console-gl.o
 common-obj-$(CONFIG_OPENGL) += egl-helpers.o
-- 
2.21.0.rc1

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Gerd Hoffmann]

On Thu, Feb 21, 2019 at 12:07:03PM +0100, Marc-André Lureau wrote:
> Add a new display backend that will configure Spice to allow a remote
> client to control QEMU in a similar fashion as other QEMU display
> backend/UI like GTK.
> 
> For this to work, it will set up Spice server with a unix socket, and
> register a VC chardev that will be exposed as Spice ports. A QMP
> monitor is also exposed as a Spice port, this allows the remote client
> fuller qemu control and state handling.
> 
> - doesn't handle VC set_echo() - this doesn't seem a strong
>   requirement, very few front-end use it
> - spice options can be tweaked with other -spice arguments
> - Windows support shouldn't be hard to do, but will probably use a TCP
>   port instead
> - we may want to watch the child process to quit automatically if it
>   crashed

Hmm, not working on RHEL-7:

  qemu-system-x86_64: Failed to launch spice+unix:///tmp/.QON7WZ/spice.sock URI: The specified location is not supported

virt-viewer + remote-viewer are installed.

any clue?

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Marc-André Lureau]

Hi

On Thu, Feb 21, 2019 at 2:24 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Thu, Feb 21, 2019 at 12:07:03PM +0100, Marc-André Lureau wrote:
> > Add a new display backend that will configure Spice to allow a remote
> > client to control QEMU in a similar fashion as other QEMU display
> > backend/UI like GTK.
> >
> > For this to work, it will set up Spice server with a unix socket, and
> > register a VC chardev that will be exposed as Spice ports. A QMP
> > monitor is also exposed as a Spice port, this allows the remote client
> > fuller qemu control and state handling.
> >
> > - doesn't handle VC set_echo() - this doesn't seem a strong
> >   requirement, very few front-end use it
> > - spice options can be tweaked with other -spice arguments
> > - Windows support shouldn't be hard to do, but will probably use a TCP
> >   port instead
> > - we may want to watch the child process to quit automatically if it
> >   crashed
>
> Hmm, not working on RHEL-7:
>
>   qemu-system-x86_64: Failed to launch spice+unix:///tmp/.QON7WZ/spice.sock URI: The specified location is not supported
>
> virt-viewer + remote-viewer are installed.
>
> any clue?

5 months old patch, not event released yet :( but virt-viewer should
have a release very soon hopefully

https://pagure.io/virt-viewer/c/c4f6142f15c4e51cbf427f5f1bf1fc6ac0e10d88?branch=master

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Gerd Hoffmann]

  Hi,

> > Hmm, not working on RHEL-7:
> >
> >   qemu-system-x86_64: Failed to launch spice+unix:///tmp/.QON7WZ/spice.sock URI: The specified location is not supported
> >
> > virt-viewer + remote-viewer are installed.
> >
> > any clue?
> 
> 5 months old patch, not event released yet :( but virt-viewer should
> have a release very soon hopefully
> 
> https://pagure.io/virt-viewer/c/c4f6142f15c4e51cbf427f5f1bf1fc6ac0e10d88?branch=master
> 
> thanks

Too bad.  So broken for pretty much everyone I guess?

I think we should have a fallback which exec's remote-viewer then, for
the next 2-3 years, until a fixed virt-viewer is in widespread use ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Marc-André Lureau]

Hi

On Thu, Feb 21, 2019 at 3:39 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > Hmm, not working on RHEL-7:
> > >
> > >   qemu-system-x86_64: Failed to launch spice+unix:///tmp/.QON7WZ/spice.sock URI: The specified location is not supported
> > >
> > > virt-viewer + remote-viewer are installed.
> > >
> > > any clue?
> >
> > 5 months old patch, not event released yet :( but virt-viewer should
> > have a release very soon hopefully
> >
> > https://pagure.io/virt-viewer/c/c4f6142f15c4e51cbf427f5f1bf1fc6ac0e10d88?branch=master
> >
> > thanks
>
> Too bad.  So broken for pretty much everyone I guess?

It should be easy to backport the patch for older virt-viewer.

But to get the "complete" experience, I would recommend you use the
upcoming virt-viewer. Without it, you won't have the QMP monitor
handling, VTE serial console, etc..

> I think we should have a fallback which exec's remote-viewer then, for
> the next 2-3 years, until a fixed virt-viewer is in widespread use ...

I am not sure, I think it make sense to require the upcoming
virt-viewer instead.

After all, if you use a bleeding edge qemu, you can use bleeding edge
virt-viewer, no?


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Gerd Hoffmann]

  Hi,

> > Too bad.  So broken for pretty much everyone I guess?
> 
> It should be easy to backport the patch for older virt-viewer.
> 
> But to get the "complete" experience, I would recommend you use the
> upcoming virt-viewer. Without it, you won't have the QMP monitor
> handling, VTE serial console, etc..
> 
> > I think we should have a fallback which exec's remote-viewer then, for
> > the next 2-3 years, until a fixed virt-viewer is in widespread use ...
> 
> I am not sure, I think it make sense to require the upcoming
> virt-viewer instead.
> 
> After all, if you use a bleeding edge qemu, you can use bleeding edge
> virt-viewer, no?

Fair enough, but please add a hint to the error message then so users
have a clue how to deal with it.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Marc-André Lureau]

Hi

On Thu, Feb 21, 2019 at 4:19 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > Too bad.  So broken for pretty much everyone I guess?
> >
> > It should be easy to backport the patch for older virt-viewer.
> >
> > But to get the "complete" experience, I would recommend you use the
> > upcoming virt-viewer. Without it, you won't have the QMP monitor
> > handling, VTE serial console, etc..
> >
> > > I think we should have a fallback which exec's remote-viewer then, for
> > > the next 2-3 years, until a fixed virt-viewer is in widespread use ...
> >
> > I am not sure, I think it make sense to require the upcoming
> > virt-viewer instead.
> >
> > After all, if you use a bleeding edge qemu, you can use bleeding edge
> > virt-viewer, no?
>
> Fair enough, but please add a hint to the error message then so users
> have a clue how to deal with it.
>

Would something like that be helpful?

diff --git a/ui/spice-app.c b/ui/spice-app.c
index 4f5229f3ee..69f92e440c 100644
--- a/ui/spice-app.c
+++ b/ui/spice-app.c
@@ -181,6 +181,7 @@ static void spice_app_display_init(DisplayState
*ds, DisplayOptions *opts)
     g_app_info_launch_default_for_uri(uri, NULL, &err);
     if (err) {
         error_report("Failed to launch %s URI: %s", uri, err->message);
+        error_report("You need a capable Spice client, such as
virt-viewer 8.0");
         exit(1);
     }
     g_free(uri);

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 11/11] display: add -display spice-app launching a Spice client

[Gerd Hoffmann]

  Hi,

> Would something like that be helpful?
> 
> diff --git a/ui/spice-app.c b/ui/spice-app.c
> index 4f5229f3ee..69f92e440c 100644
> --- a/ui/spice-app.c
> +++ b/ui/spice-app.c
> @@ -181,6 +181,7 @@ static void spice_app_display_init(DisplayState
> *ds, DisplayOptions *opts)
>      g_app_info_launch_default_for_uri(uri, NULL, &err);
>      if (err) {
>          error_report("Failed to launch %s URI: %s", uri, err->message);
> +        error_report("You need a capable Spice client, such as
> virt-viewer 8.0");

Yep, looks good to me.  Can you send that as incremental patch?

thanks,
  Gerd