[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema |
Date: |
Thu, 17 Aug 2017 10:33:05 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
* 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