From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=33463 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Py2d3-0006UO-4U
	for qemu-devel@nongnu.org; Fri, 11 Mar 2011 08:43:45 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1Py2d2-0006TP-4h
	for qemu-devel@nongnu.org; Fri, 11 Mar 2011 08:43:41 -0500
Received: from mx1.redhat.com ([209.132.183.28]:60122)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1Py2d1-0006TJ-ND
	for qemu-devel@nongnu.org; Fri, 11 Mar 2011 08:43:40 -0500
Message-ID: <4D7A26F7.1010901@redhat.com>
Date: Fri, 11 Mar 2011 14:43:19 +0100
From: Jes Sorensen <Jes.Sorensen@redhat.com>
MIME-Version: 1.0
References: <1299847134-5854-1-git-send-email-Jes.Sorensen@redhat.com>
	<4D7A255B.4000409@codemonkey.ws>
In-Reply-To: <4D7A255B.4000409@codemonkey.ws>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH] Make VNC support optional
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org

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