Re: [Qemu-devel] [PATCH] hmp: Update info vnc

[Dr. David Alan Gilbert]

* Marc-André Lureau (address@hidden) wrote:
> 
> 
> ----- Original Message -----
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > The QMP query-vnc interfaces have gained a lot more information that
> > the HMP interfaces hasn't got yet. Update it.
> > 
> > Note the output format has changed, but this is HMP so that's OK.
> > 
> > In particular, this now includes client information for reverse
> > connections:
> > 
> > -vnc :0
> > (qemu) info vnc
> > default:
> >   Server: 0.0.0.0:5900 (ipv4)
> >     Auth: none (Sub: none)
> >   Auth: none (Sub: none)
> > 
> >   (Now connect a client)
> > 
> > (qemu) info vnc
> > default:
> >   Server: 0.0.0.0:5900 (ipv4)
> >     Auth: none (Sub: none)
> >   Client: 127.0.0.1:51828 (ipv4)
> >     x509_dname: none
> >     username: none
> >   Auth: none (Sub: none)
> > 
> > -vnc localhost:7000,reverse
> > (qemu) info vnc
> > default:
> >   Client: ::1:7000 (ipv6)
> >     x509_dname: none
> >     username: none
> >   Auth: none (Sub: none)
> > 
> > -vnc :1,password,id=rev -vnc localhost:7000,reverse
> > (qemu) info vnc
> > default:
> >   Client: ::1:7000 (ipv6)
> >     x509_dname: none
> >     username: none
> >   Auth: none (Sub: none)
> > rev:
> >   Server: 0.0.0.0:5901 (ipv4)
> >     Auth: vnc (Sub: none)
> >   Client: 127.0.0.1:53616 (ipv4)
> >     x509_dname: none
> >     username: none
> >   Auth: vnc (Sub: none)
> > 
> > This was originally RH bz 1461682
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  hmp.c | 98
> >  ++++++++++++++++++++++++++++++++++++++++++++++---------------------
> >  1 file changed, 67 insertions(+), 31 deletions(-)
> > 
> > diff --git a/hmp.c b/hmp.c
> > index dee40284c1..c11a5fe2c6 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict
> > *qdict)
> >      qapi_free_BlockStatsList(stats_list);
> >  }
> >  
> > +/* Helper for hmp_info_vnc_clients, _servers */
> > +static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
> > +                                  const char *name)
> > +{
> > +    monitor_printf(mon, "  %s: %s:%s (%s%s)\n",
> > +                   name,
> > +                   info->host,
> > +                   info->service,
> > +                   NetworkAddressFamily_lookup[info->family],
> > +                   info->websocket ? " (Websocket)" : "");
> > +}
> > +
> > +/* Helper displaying and euth and crypt info */
> > +static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
> > +                                   VncPrimaryAuth auth,
> > +                                   VncVencryptSubAuth *vencrypt)
> > +{
> > +    monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
> > +                   VncPrimaryAuth_lookup[auth],
> > +                   vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] :
> > "none");
> > +}
> > +
> > +static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
> > +{
> > +    while (client) {
> > +        VncClientInfo *cinfo = client->value;
> > +
> > +        hmp_info_VncBasicInfo(mon, (VncBasicInfo *)cinfo, "Client");
> > +        monitor_printf(mon, "    x509_dname: %s\n",
> > +                       cinfo->x509_dname ?
> 
> Why not use has_x509_dname ?

Interesting; that line I just moved from the old code;  fixed.

> > +                       cinfo->x509_dname : "none");
> > +        monitor_printf(mon, "    username: %s\n",
> 
> sasl_username would be more clear

Also from the old code; also fixed.

Thanks,

Dave

> > +                       cinfo->has_sasl_username ?
> > +                       cinfo->sasl_username : "none");
> > +
> > +        client = client->next;
> > +    }
> > +}
> > +
> > +static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
> > +{
> > +    while (server) {
> > +        VncServerInfo2 *sinfo = server->value;
> > +        hmp_info_VncBasicInfo(mon, (VncBasicInfo *)sinfo, "Server");
> > +        hmp_info_vnc_authcrypt(mon, "    ", sinfo->auth,
> > +                               sinfo->has_vencrypt ? &sinfo->vencrypt :
> > NULL);
> > +        server = server->next;
> > +    }
> > +}
> > +
> >  void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >  {
> > -    VncInfo *info;
> > +    VncInfo2List *info2l;
> >      Error *err = NULL;
> > -    VncClientInfoList *client;
> >  
> > -    info = qmp_query_vnc(&err);
> > +    info2l = qmp_query_vnc_servers(&err);
> >      if (err) {
> >          error_report_err(err);
> >          return;
> >      }
> > -
> > -    if (!info->enabled) {
> > -        monitor_printf(mon, "Server: disabled\n");
> > -        goto out;
> > -    }
> > -
> > -    monitor_printf(mon, "Server:\n");
> > -    if (info->has_host && info->has_service) {
> > -        monitor_printf(mon, "     address: %s:%s\n", info->host,
> > info->service);
> > -    }
> > -    if (info->has_auth) {
> > -        monitor_printf(mon, "        auth: %s\n", info->auth);
> > +    if (!info2l) {
> > +        monitor_printf(mon, "None\n");
> > +        return;
> >      }
> >  
> > -    if (!info->has_clients || info->clients == NULL) {
> > -        monitor_printf(mon, "Client: none\n");
> > -    } else {
> > -        for (client = info->clients; client; client = client->next) {
> > -            monitor_printf(mon, "Client:\n");
> > -            monitor_printf(mon, "     address: %s:%s\n",
> > -                           client->value->host,
> > -                           client->value->service);
> > -            monitor_printf(mon, "  x509_dname: %s\n",
> > -                           client->value->x509_dname ?
> > -                           client->value->x509_dname : "none");
> > -            monitor_printf(mon, "    username: %s\n",
> > -                           client->value->has_sasl_username ?
> > -                           client->value->sasl_username : "none");
> > +    while (info2l) {
> > +        VncInfo2 *info = info2l->value;
> > +        monitor_printf(mon, "%s:\n", info->id);
> > +        hmp_info_vnc_servers(mon, info->server);
> > +        hmp_info_vnc_clients(mon, info->clients);
> > +        hmp_info_vnc_authcrypt(mon, "  ", info->auth,
> > +                          info->has_vencrypt ? &info->vencrypt : NULL);
> > +        if (info->has_display) {
> > +            monitor_printf(mon, "  Display: %s\n", info->display);
> >          }
> > +        info2l = info2l->next;
> >      }
> >  
> > -out:
> > -    qapi_free_VncInfo(info);
> > +    qapi_free_VncInfo2List(info2l);
> > +
> >  }
> >  
> >  #ifdef CONFIG_SPICE
> > --
> > 2.13.0
> 
> Looks good to me otherwise
> Reviewed-by: Marc-André Lureau <address@hidden>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK