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

[Markus Armbruster]

"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.

    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).

[...]