Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

[Dr. David Alan Gilbert]

* Markus Armbruster (address@hidden) wrote:
> "Dr. David Alan Gilbert" <address@hidden> writes:
> 
> > * Marc-André Lureau (address@hidden) wrote:
> >> Add #if defined(CONFIG_VNC) in generated code, and adjust the
> >> qmp/hmp code accordingly.
> >> 
> >> Signed-off-by: Marc-André Lureau <address@hidden>
> >
> >> diff --git a/hmp.c b/hmp.c
> >> index fd80dce758..9454c634bd 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -605,6 +605,7 @@ void hmp_info_blockstats(Monitor *mon, const QDict 
> >> *qdict)
> >>      qapi_free_BlockStatsList(stats_list);
> >>  }
> >>  
> >> +#ifdef CONFIG_VNC
> >>  /* Helper for hmp_info_vnc_clients, _servers */
> >>  static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
> >>                                    const char *name)
> >> @@ -692,6 +693,12 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >>      qapi_free_VncInfo2List(info2l);
> >>  
> >>  }
> >> +#else
> >> +void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >> +{
> >> +    warn_report("VNC support is disabled");
> 
> error_report(), please (see below).
> 
> >> +}
> >> +#endif
> >
> > I'm OK with this, so
> >
> > Acked-by: Dr. David Alan Gilbert <address@hidden>
> >
> > although you might just be able to add a #ifdef in hmp-commands-info.hx
> > and avoid the is disabled function, or you might find that with the QMP
> > returning an error the HMP just passes that error on.
> 
> Let's compare failures when !CONFIG_VNC:
> 
> (a) Marc-André's patch as is:
> 
>         (qemu) info vnc
>         warning: VNC support is disabled
> 
>     Drop the "warning: " (because it ain't; the command failed), and this
>     is fine.
> 
> (b) Compiling them out completely (#ifdef in hmp-commands*.hx):
> 
>         unknown command: 'vnc'
> 
>     HMP bug; should be something like
> 
>         Unknown command: 'info vnc'
> 
>     but that's not this series' problem.

I'll fix that missing 'info'

Dave

>     Good enough for me.
> 
> (c) Forwarding the QMP error verbatim
> 
>         The command query-vnc has not been found
> 
>     No good.
> 
> (d) Handling CommandNotFound
> 
>     More work than (a) for the same result.
> 
> As far as I'm concerned, feel free to do (a) or (b).
> 
> [...]
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK