[Spice-devel] [PATCH spice-gtk v2] glib: add SpiceQmpPort helper
Victor Toso
victortoso at redhat.com
Fri Sep 21 11:25:29 UTC 2018
Hi,
On Fri, Sep 21, 2018 at 02:31:40PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Fri, Sep 21, 2018 at 1:24 PM Victor Toso <victortoso at redhat.com> wrote:
> >
> > Hi,
> >
> > On Thu, Sep 20, 2018 at 12:01:37AM +0400, marcandre.lureau at redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > >
> > > Add a few helper functions to deal with a QMP port channel, in order
> > > to ease json handling, and wrapping a few commands.
> > >
> > > (by convention, the port should have the name
> > > "org.qemu.monitor.qmp.0", but it's not strictly required)
> > >
> > > This helper is put into use in the virt-viewer "Add QEMU-like UI: VT
> > > console & basic VM status" series.
> > >
> > > Note: this adds a strong dependency on json-glib for
> > > spice-client-glib, a widely available and fairly small
> > > library.
> > >
> > > QMP specification is:
> > > https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/qmp-spec.txt
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > ---
> > > configure.ac | 2 +
> > > doc/reference/spice-gtk-docs.xml | 1 +
> > > doc/reference/spice-gtk-sections.txt | 27 ++
> > > meson.build | 2 +
> > > src/Makefile.am | 6 +
> > > src/map-file | 9 +
> > > src/meson.build | 4 +
> > > src/qmp-port.c | 575 +++++++++++++++++++++++++++
> > > src/qmp-port.h | 122 ++++++
> > > src/spice-client.h | 1 +
> > > src/spice-glib-sym-file | 9 +
> > > 11 files changed, 758 insertions(+)
> > > create mode 100644 src/qmp-port.c
> > > create mode 100644 src/qmp-port.h
> > >
> > > v2: after Victor review
> > > - add meson build
> >
> > Thanks
> >
> > > - print only corresponding message when dispatch failed
> > > - warn/critical on unknown task/id
> > > - fix PortChannel/QmpPort weak ref relationship
> > > - some renaming
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 7b32e29..e686d76 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -173,6 +173,8 @@ PKG_CHECK_MODULES(CAIRO, cairo >= 1.2.0)
> > >
> > > PKG_CHECK_MODULES(GTHREAD, gthread-2.0)
> > >
> > > +PKG_CHECK_MODULES(JSON, json-glib-1.0)
> > > +
> > > AC_ARG_ENABLE([webdav],
> > > AS_HELP_STRING([--enable-webdav=@<:@auto/yes/no@:>@],
> > > [Enable webdav support @<:@default=auto@:>@]),
> > > diff --git a/doc/reference/spice-gtk-docs.xml b/doc/reference/spice-gtk-docs.xml
> > > index db5dd3d..696c375 100644
> > > --- a/doc/reference/spice-gtk-docs.xml
> > > +++ b/doc/reference/spice-gtk-docs.xml
> > > @@ -52,6 +52,7 @@
> > > <xi:include href="xml/spice-audio.xml"/>
> > > <xi:include href="xml/smartcard-manager.xml"/>
> > > <xi:include href="xml/usb-device-manager.xml"/>
> > > + <xi:include href="xml/qmp-port.xml"/>
> > > <xi:include href="xml/spice-util.xml"/>
> > > <xi:include href="xml/spice-version.xml"/>
> > > <xi:include href="xml/spice-uri.xml"/>
> > > diff --git a/doc/reference/spice-gtk-sections.txt b/doc/reference/spice-gtk-sections.txt
> > > index a85df7a..a0336aa 100644
> > > --- a/doc/reference/spice-gtk-sections.txt
> > > +++ b/doc/reference/spice-gtk-sections.txt
> > > @@ -494,6 +494,33 @@ SPICE_PORT_CHANNEL_GET_CLASS
> > > SpicePortChannelPrivate
> > > </SECTION>
> > >
> > > +<SECTION>
> > > +<FILE>qmp-port</FILE>
> > > +<TITLE>SpiceQmpPort</TITLE>
> > > +
> > > +<SUBSECTION>
> > > +SpiceQmpPort
> > > +SpiceQmpPortVmAction
> > > +SpiceQmpStatus
> > > +<SUBSECTION>
> > > +spice_qmp_port_get
> > > +spice_qmp_port_vm_action_async
> > > +spice_qmp_port_vm_action_finish
> > > +spice_qmp_port_query_status_async
> > > +spice_qmp_port_query_status_finish
> > > +spice_qmp_status_ref
> > > +spice_qmp_status_unref
> > > +<SUBSECTION Standard>
> > > +SPICE_IS_QMP_PORT
> > > +SPICE_IS_QMP_PORT_CLASS
> > > +SPICE_QMP_PORT
> > > +SPICE_QMP_PORT_CLASS
> > > +SPICE_QMP_PORT_GET_CLASS
> > > +SPICE_TYPE_QMP_PORT
> > > +spice_qmp_port_get_type
> > > +spice_qmp_status_get_type
> > > +</SECTION>
> > > +
> > > <SECTION>
> > > <FILE>spice-uri</FILE>
> > > spice_uri_get_scheme
> > > diff --git a/meson.build b/meson.build
> > > index 3c20401..875e07f 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -98,6 +98,8 @@ foreach dep, version : deps
> > > spice_gtk_deps += dependency(dep, version : version)
> > > endforeach
> > >
> > > +spice_gtk_deps += dependency('json-glib-1.0')
> > > +
> > > # TODO: specify minimum version for cairo, jpeg and zlib?
> > > deps = ['cairo', 'libjpeg', 'zlib']
> > > if spice_gtk_host_system == 'windows'
> > > diff --git a/src/Makefile.am b/src/Makefile.am
> > > index 4dd657d..1bb6f9b 100644
> > > --- a/src/Makefile.am
> > > +++ b/src/Makefile.am
> > > @@ -78,6 +78,7 @@ SPICE_COMMON_CPPFLAGS = \
> > > $(GLIB2_CFLAGS) \
> > > $(GIO_CFLAGS) \
> > > $(GOBJECT2_CFLAGS) \
> > > + $(JSON_CFLAGS) \
> > > $(SSL_CFLAGS) \
> > > $(SASL_CFLAGS) \
> > > $(GSTAUDIO_CFLAGS) \
> > > @@ -182,6 +183,7 @@ libspice_client_glib_2_0_la_LIBADD = \
> > > $(GIO_LIBS) \
> > > $(GOBJECT2_LIBS) \
> > > $(JPEG_LIBS) \
> > > + $(JSON_LIBS) \
> > > $(Z_LIBS) \
> > > $(LZ4_LIBS) \
> > > $(PIXMAN_LIBS) \
> > > @@ -243,6 +245,8 @@ libspice_client_glib_2_0_la_SOURCES = \
> > > channel-smartcard.c \
> > > channel-usbredir.c \
> > > channel-usbredir-priv.h \
> > > + qmp-port.c \
> > > + qmp-port.h \
> > > smartcard-manager.c \
> > > smartcard-manager-priv.h \
> > > spice-uri.c \
> > > @@ -293,6 +297,7 @@ libspice_client_glibinclude_HEADERS = \
> > > channel-smartcard.h \
> > > channel-usbredir.h \
> > > channel-webdav.h \
> > > + qmp-port.h \
> > > usb-device-manager.h \
> > > smartcard-manager.h \
> > > spice-file-transfer-task.h \
> > > @@ -526,6 +531,7 @@ glib_introspection_files = \
> > > channel-record.c \
> > > channel-smartcard.c \
> > > channel-usbredir.c \
> > > + qmp-port.c \
> > > smartcard-manager.c \
> > > usb-device-manager.c \
> > > $(NULL)
> > > diff --git a/src/map-file b/src/map-file
> > > index cdb81c3..500683c 100644
> > > --- a/src/map-file
> > > +++ b/src/map-file
> > > @@ -117,6 +117,15 @@ spice_port_channel_write_finish;
> > > spice_port_event;
> > > spice_port_write_async;
> > > spice_port_write_finish;
> > > +spice_qmp_port_get;
> > > +spice_qmp_port_get_type;
> > > +spice_qmp_port_query_status_async;
> > > +spice_qmp_port_query_status_finish;
> > > +spice_qmp_port_vm_action_async;
> > > +spice_qmp_port_vm_action_finish;
> > > +spice_qmp_status_get_type;
> > > +spice_qmp_status_ref;
> > > +spice_qmp_status_unref;
> > > spice_record_channel_get_type;
> > > spice_record_channel_send_data;
> > > spice_record_send_data;
> > > diff --git a/src/meson.build b/src/meson.build
> > > index 8c9199e..80e777d 100644
> > > --- a/src/meson.build
> > > +++ b/src/meson.build
> > > @@ -32,6 +32,7 @@ spice_client_glib_headers = [
> > > 'channel-smartcard.h',
> > > 'channel-usbredir.h',
> > > 'channel-webdav.h',
> > > + 'qmp-port.h',
> > > 'smartcard-manager.h',
> > > 'spice-audio.h',
> > > 'spice-channel.h',
> > > @@ -70,6 +71,7 @@ spice_client_glib_introspection_sources = [
> > > 'channel-smartcard.c',
> > > 'channel-usbredir.c',
> > > 'channel-webdav.c',
> > > + 'qmp-port.c',
> > > 'smartcard-manager.c',
> > > 'spice-audio.c',
> > > 'spice-channel.c',
> > > @@ -98,6 +100,8 @@ spice_client_glib_sources = [
> > > 'decode-zlib.c',
> > > 'gio-coroutine.c',
> > > 'gio-coroutine.h',
> > > + 'qmp-port.c',
> > > + 'qmp-port.h',
> > > 'smartcard-manager-priv.h',
> > > 'spice-audio-priv.h',
> > > 'spice-channel-cache.h',
> > > diff --git a/src/qmp-port.c b/src/qmp-port.c
> > > new file mode 100644
> > > index 0000000..c2e5c03
> > > --- /dev/null
> > > +++ b/src/qmp-port.c
> > > @@ -0,0 +1,575 @@
> > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > +/*
> > > + Copyright (C) 2018 Red Hat, Inc.
> > > +
> > > + This library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later version.
> > > +
> > > + This library is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General Public
> > > + License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > > +*/
> > > +#include "config.h"
> > > +
> > > +#define _GNU_SOURCE
> > > +#include <string.h>
> > > +#include <json-glib/json-glib.h>
> > > +#include "spice-client.h"
> > > +
> > > +/**
> > > + * SECTION:qmp-port
> > > + * @short_description: QMP port helper
> > > + * @title: QMP port channel helper
> > > + * @section_id:
> > > + * @see_also: #SpicePortChannel
> > > + * @stability: Stable
> > > + * @include: spice-client.h
> > > + *
> > > + * A helper to handle QMP messages over a %SpicePortChannel.
> > > + *
> > > + * Since: 0.36
> > > + */
> > > +
> > > +typedef struct _SpiceQmpPortPrivate SpiceQmpPortPrivate;
> > > +
> > > +struct _SpiceQmpPortPrivate
> > > +{
> > > + SpicePortChannel *channel;
> > > + gboolean ready;
> > > +
> > > + gint id_counter;
> > > + GString *qmp_data;
> > > + JsonParser *qmp_parser;
> > > + GHashTable *qmp_tasks;
> > > +};
> > > +
> > > +struct _SpiceQmpPort
> > > +{
> > > + GObject parent;
> > > +
> > > + SpiceQmpPortPrivate *priv;
> > > +};
> > > +
> > > +struct _SpiceQmpPortClass
> > > +{
> > > + GObjectClass parent_class;
> > > +};
> > > +
> > > +enum {
> > > + PROP_CHANNEL = 1,
> > > + PROP_READY,
> > > +
> > > + PROP_LAST,
> > > +};
> > > +
> > > +enum {
> > > + SIGNAL_EVENT,
> > > +
> > > + SIGNAL_LAST,
> > > +};
> > > +
> > > +static guint signals[SIGNAL_LAST];
> > > +static GParamSpec *props[PROP_LAST] = { NULL, };
> > > +
> > > +G_DEFINE_TYPE_WITH_PRIVATE(SpiceQmpPort, spice_qmp_port, G_TYPE_OBJECT)
> > > +
> > > +G_DEFINE_BOXED_TYPE(SpiceQmpStatus, spice_qmp_status, spice_qmp_status_ref, spice_qmp_status_unref)
> > > +
> > > +typedef void (QMPCb)(GTask *task, JsonNode *node);
> > > +
> > > +static void
> > > +qmp_error_return(GTask *task, const gchar *desc)
> > > +{
> > > + g_task_return_new_error(task, SPICE_CLIENT_ERROR,
> > > + SPICE_CLIENT_ERROR_FAILED, "%s", desc);
> > > + g_object_unref(task);
> > > +}
> > > +
> > > +static gboolean
> > > +spice_qmp_dispatch_message(SpiceQmpPort *self)
> > > +{
> > > + JsonObject *obj = json_node_get_object(json_parser_get_root(self->priv->qmp_parser));
> > > + JsonNode *node;
> > > + GTask *task;
> > > + const gchar *event;
> > > +
> > > + if (!self->priv->ready && json_object_get_member(obj, "QMP")) {
> > > + g_debug("QMP greeting received");
> >
> > As mentioned earlier, self->priv->ready can (should?) be moved
> > inside the if, likely to have this check in g_warn_if_fail() like
> > it is done to others below.
>
> Ah sorry, I missed that.
>
> So QMP greeting isn't that useful to us at this point.
>
> I can just remove the self->priv->ready from there:
>
> if (json_object_get_member(obj, "QMP")) {
> g_debug("QMP greeting received");
> return:
> }
>
> Would you prefer that?
Either way is fine to me ;)
Cheers,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180921/05363e26/attachment.sig>
More information about the Spice-devel
mailing list