[Qemu-devel] Re: [PATCH] Make VNC support optional

[Jes Sorensen]

On 03/11/11 14:36, Anthony Liguori wrote:
>> diff --git a/monitor.c b/monitor.c
>> index 22ae3bb..4425315 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -441,6 +441,7 @@ void monitor_protocol_event(MonitorEvent event,
>> QObject *data)
>>           case QEVENT_RESUME:
>>               event_name = "RESUME";
>>               break;
>> +#ifdef CONFIG_VNC
>>           case QEVENT_VNC_CONNECTED:
>>               event_name = "VNC_CONNECTED";
>>               break;
>> @@ -450,6 +451,7 @@ void monitor_protocol_event(MonitorEvent event,
>> QObject *data)
>>           case QEVENT_VNC_DISCONNECTED:
>>               event_name = "VNC_DISCONNECTED";
>>               break;
>> +#endif
> 
> No need to if this out.

I realized that, but I figured it would reduce the code a bit more. I'll
leave it in.

>> @@ -1073,11 +1077,15 @@ static int do_change(Monitor *mon, const QDict
>> *qdict, QObject **ret_data)
>>       const char *arg = qdict_get_try_str(qdict, "arg");
>>       int ret;
>>
>> +#ifdef CONFIG_VNC
>>       if (strcmp(device, "vnc") == 0) {
>>           ret = do_change_vnc(mon, target, arg);
>>       } else {
>> +#endif
>>           ret = do_change_block(mon, device, target, arg);
>> +#ifdef CONFIG_VNC
>>       }
>> +#endif
>>
>>       return ret;
>>   }
> 
> Or this stuff.
> 
> Provide a stub function for changing the VNC password and have it return
> a failure.  Then this function can check that failure and throw a proper
> QError indicating that VNC is not present.

Ok, I can do that.

>> @@ -3002,6 +3014,7 @@ static const mon_cmd_t info_cmds[] = {
>>           .user_print = do_info_mice_print,
>>           .mhandler.info_new = do_info_mice,
>>       },
>> +#ifdef CONFIG_VNC
>>       {
>>           .name       = "vnc",
>>           .args_type  = "",
>> @@ -3010,6 +3023,7 @@ static const mon_cmd_t info_cmds[] = {
>>           .user_print = do_info_vnc_print,
>>           .mhandler.info_new = do_info_vnc,
>>       },
>> +#endif
> 
> We don't want to hide commands based on compile settings.
> 
> Otherwise, looks good.

I am not sure I follow you here, you prefer to have the commands visible
even though they are disabled? There are other commands which get
disabled like this as well.

Cheers,
Jes