Re: [Qemu-devel] [PATCH v2] hmp/qxl: info spice: add qxl info

[Alon Levy]

On Fri, May 25, 2012 at 11:43:11AM -0300, Luiz Capitulino wrote:
> On Thu, 24 May 2012 19:22:52 +0300
> Alon Levy <address@hidden> wrote:
> 
> > For all devices print id, mode and guest_bug status.
> 
> Is qxl really tied to spice? In the meaning that it's impossible to use it
> without spice? Wouldn't it be better to have 'info display' instead?

Would a patch implementing 'info display' require me to implement it for
all displays?

> 
> > Known problems: Prints devices from highest id to lowest.
> 
> That's because qapi lists usually inserts new items at the head. You could
> add them at the tail instead. Requires more code, but works.

Right, I didn't think it was a real enough problem to fix.

> 
> > 
> > Signed-off-by: Alon Levy <address@hidden>
> > ---
> > This one builds. Fixed qapi-schema to say additions are for 1.2
> > 
> >  hmp.c            |   11 +++++++++++
> >  hw/qxl.c         |   22 ++++++++++++++++++++++
> >  qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++--
> >  ui/qemu-spice.h  |    5 +++++
> >  ui/spice-core.c  |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  5 files changed, 130 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hmp.c b/hmp.c
> > index bb0952e..5126921 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -331,6 +331,7 @@ void hmp_info_spice(Monitor *mon)
> >  {
> >      SpiceChannelList *chan;
> >      SpiceInfo *info;
> > +    QXLInfoList *qxl;
> >  
> >      info = qmp_query_spice(NULL);
> >  
> > @@ -353,6 +354,16 @@ void hmp_info_spice(Monitor *mon)
> >      monitor_printf(mon, "  mouse-mode: %s\n",
> >                     SpiceQueryMouseMode_lookup[info->mouse_mode]);
> >  
> > +    for (qxl = info->qxl; qxl; qxl = qxl->next) {
> > +        if (qxl->value->guest_bug == -1 || qxl->value->mode == -1) {
> > +            continue;
> > +        }
> > +        monitor_printf(mon, "qxl-%"PRId64":\n", qxl->value->id);
> > +        monitor_printf(mon, "        mode: %s\n",
> > +                SpiceQueryQXLMode_lookup[qxl->value->mode]);
> > +        monitor_printf(mon, "   guest_bug: %"PRIu64"\n", 
> > qxl->value->guest_bug);
> > +    }
> > +
> >      if (!info->has_channels || info->channels == NULL) {
> >          monitor_printf(mon, "Channels: none\n");
> >      } else {
> > diff --git a/hw/qxl.c b/hw/qxl.c
> > index b5e53ce..21c825c 100644
> > --- a/hw/qxl.c
> > +++ b/hw/qxl.c
> > @@ -1711,6 +1711,28 @@ static DisplayChangeListener display_listener = {
> >      .dpy_refresh = display_refresh,
> >  };
> >  
> > +/* helpers for spice_info */
> > +int qxl_get_guest_bug(DeviceState *dev)
> > +{
> > +    PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci.qdev, dev);
> > +
> > +    return qxl->guest_bug;
> > +}
> > +
> > +int qxl_get_mode(DeviceState *dev)
> > +{
> > +    PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci.qdev, dev);
> > +
> > +    return qxl->mode;
> > +}
> > +
> > +int qxl_get_id(DeviceState *dev)
> > +{
> > +    PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci.qdev, dev);
> > +
> > +    return qxl->id;
> > +}
> > +
> >  static void qxl_init_ramsize(PCIQXLDevice *qxl, uint32_t ram_min_mb)
> >  {
> >      /* vga ram (bar 0) */
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 2ca7195..af1fac2 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -638,7 +638,7 @@
> >  ##
> >  # @SpiceQueryMouseMode
> >  #
> > -# An enumation of Spice mouse states.
> > +# An enumeration of Spice mouse states.
> >  #
> >  # @client: Mouse cursor position is determined by the client.
> >  #
> > @@ -655,6 +655,44 @@
> >    'data': [ 'client', 'server', 'unknown' ] }
> >  
> >  ##
> > +# @SpiceQueryQXLMode
> > +#
> > +# An enumeration of QXL States.
> > +#
> > +# @undefined: guest driver in control but no primary device. Reached after 
> > a destroy primary IO
> > +#             from native mode.
> > +#
> > +# @vga: no device driver in control. default mode, returns to it after any 
> > vga port access.
> > +#
> > +# @compat: No information is available about mouse mode used by
> > +#           the spice server.
> > +#
> > +# @native: guest driver in control of device. Reached after a create 
> > primary IO.
> > +#
> > +# Note: hw/qxl.h has a qxl_mode enum, name chose to not confuse the two.
> > +#
> > +# Since: 1.2
> > +##
> > +{ 'enum': 'SpiceQueryQXLMode',
> > +  'data': [ 'undefined', 'vga', 'compat', 'native' ] }
> > +
> > +##
> > +# @QXLInfo
> > +#
> > +# Information about a QXL device.
> > +#
> > +# @id: qxl id, non negative integer, 0 for primary device.
> > +#
> > +# @guest_bug: Has a guest error been detected.
> > +#
> > +# @mode: Mode of device, based on guest activity.
> > +#
> > +# Since: 1.2
> > +##
> > +{ 'type': 'QXLInfo',
> > +  'data': {'id': 'int', 'guest_bug': 'int', 'mode': 'SpiceQueryQXLMode'} }
> > +
> > +##
> >  # @SpiceInfo
> >  #
> >  # Information about the SPICE session.
> > @@ -683,12 +721,17 @@
> >  #
> >  # @channels: a list of @SpiceChannel for each active spice channel
> >  #
> > +# @qxl: a list of @QXLInfo for each qxl device
> > +#
> > +#              Since: 1.2
> > +#
> >  # Since: 0.14.0
> >  ##
> >  { 'type': 'SpiceInfo',
> >    'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
> >             '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
> > -           'mouse-mode': 'SpiceQueryMouseMode', '*channels': 
> > ['SpiceChannel']} }
> > +           'mouse-mode': 'SpiceQueryMouseMode', '*channels': 
> > ['SpiceChannel'],
> > +           'qxl': ['QXLInfo']} }
> >  
> >  ##
> >  # @query-spice
> > diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
> > index 3299da8..edcf3a5 100644
> > --- a/ui/qemu-spice.h
> > +++ b/ui/qemu-spice.h
> > @@ -47,6 +47,11 @@ void do_info_spice(Monitor *mon, QObject **ret_data);
> >  
> >  CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
> >  
> > +/* implemented in hw/qxl.c */
> > +int qxl_get_guest_bug(DeviceState *dev);
> > +int qxl_get_mode(DeviceState *dev);
> > +int qxl_get_id(DeviceState *dev);
> > +
> >  #else  /* CONFIG_SPICE */
> >  #include "monitor.h"
> >  
> > diff --git a/ui/spice-core.c b/ui/spice-core.c
> > index 4fc48f8..25833e5 100644
> > --- a/ui/spice-core.c
> > +++ b/ui/spice-core.c
> > @@ -22,7 +22,6 @@
> >  #include "sysemu.h"
> >  
> >  #include "qemu-common.h"
> > -#include "qemu-spice.h"
> >  #include "qemu-thread.h"
> >  #include "qemu-timer.h"
> >  #include "qemu-queue.h"
> > @@ -37,6 +36,8 @@
> >  #include "migration.h"
> >  #include "monitor.h"
> >  #include "hw/hw.h"
> > +#include "hw/qdev.h"
> > +#include "qemu-spice.h"
> >  
> >  /* core bits */
> >  
> > @@ -419,6 +420,50 @@ static SpiceChannelList *qmp_query_spice_channels(void)
> >      return head;
> >  }
> >  
> > +static int qdev_walk_qxl(DeviceState *dev, void *opaque)
> > +{
> > +    QXLInfoList **cur = opaque;
> > +    QXLInfoList *qxl_info;
> > +    int first = 0;
> > +    const char *class_name = object_get_typename(OBJECT(dev));
> > +
> > +    if (strcmp(class_name, "qxl") != 0 &&
> > +        strcmp(class_name, "qxl-vga") != 0) {
> > +        return 0;
> > +    }
> > +    if ((*cur)->value == NULL) {
> > +        first = 1;
> > +        qxl_info = *cur;
> > +    } else {
> > +        qxl_info = g_malloc(sizeof(*qxl_info));
> > +    }
> > +    qxl_info->next = NULL;
> > +    qxl_info->value = g_malloc(sizeof(*qxl_info->value));
> > +    qxl_info->value->id = qxl_get_id(dev);
> > +    qxl_info->value->guest_bug = qxl_get_guest_bug(dev);
> > +    qxl_info->value->mode = qxl_get_mode(dev);
> > +    if (!first) {
> > +        (*cur)->next = qxl_info;
> > +        *cur = qxl_info;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int qbus_walk_all(BusState *bus, void *opaque)
> > +{
> > +    return 0;
> > +}
> > +
> > +static QXLInfoList *qmp_query_qxl(void)
> > +{
> > +    QXLInfoList *root = g_malloc0(sizeof(*root));
> > +    QXLInfoList *cur = root;
> > +    BusState *default_bus = sysbus_get_default();
> > +
> > +    qbus_walk_children(default_bus, qdev_walk_qxl, qbus_walk_all, &cur);
> > +    return root;
> > +}
> > +
> >  SpiceInfo *qmp_query_spice(Error **errp)
> >  {
> >      QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
> > @@ -461,6 +506,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
> >          info->has_tls_port = true;
> >          info->tls_port = tls_port;
> >      }
> > +    info->qxl = qmp_query_qxl();
> >  
> >  #if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
> >      info->mouse_mode = spice_server_is_server_mouse(spice_server) ?
>