Re: [Qemu-devel] [PATCH] ui: fix regression in x509verify parameter for VNC server

[Gonglei]

On 2015/3/11 17:45, Daniel P. Berrange wrote:
> On Wed, Mar 11, 2015 at 09:48:46AM +0800, Gonglei wrote:
>> On 2015/3/11 0:27, Daniel P. Berrange wrote:
>>> The 'x509verify' parameter is documented as taking a path to the
>>> x509 certificates, ie the same syntax as the 'x509' parameter.
>>>
>>>   commit 4db14629c38611061fc19ec6927405923de84f08
>>>   Author: Gerd Hoffmann <address@hidden>
>>>   Date:   Tue Sep 16 12:33:03 2014 +0200
>>>
>>>     vnc: switch to QemuOpts, allow multiple servers
>>>
>>> caused a regression by turning 'x509verify' into a boolean
>>> parameter instead. This breaks setup from libvirt and is not
>>> consistent with the docs.
>>>
>>> Signed-off-by: Daniel P. Berrange <address@hidden>
>>> ---
>>>  ui/vnc.c | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ui/vnc.c b/ui/vnc.c
>>> index 10a2724..37290e7 100644
>>> --- a/ui/vnc.c
>>> +++ b/ui/vnc.c
>>> @@ -3304,7 +3304,7 @@ static QemuOptsList qemu_vnc_opts = {
>>>              .type = QEMU_OPT_BOOL,
>>>          },{
>>>              .name = "x509verify",
>>> -            .type = QEMU_OPT_BOOL,
>>> +            .type = QEMU_OPT_STRING,
>>>          },{
>>>              .name = "acl",
>>>              .type = QEMU_OPT_BOOL,
>>> @@ -3391,9 +3391,14 @@ void vnc_display_open(const char *id, Error **errp)
>>>  #ifdef CONFIG_VNC_TLS
>>>      tls  = qemu_opt_get_bool(opts, "tls", false);
>>>      path = qemu_opt_get(opts, "x509");
>>> +    if (!path) {
>>> +        path = qemu_opt_get(opts, "x509verify");
>>> +        if (path) {
>>> +            vs->tls.x509verify = true;
>>> +        }
>>> +    }
>>>      if (path) {
>>>          x509 = true;
>>> -        vs->tls.x509verify = qemu_opt_get_bool(opts, "x509verify", false);
>>
>> We still need to get the x509verify value, while both 'x509' and 'x509verify'
>> are configured, isn't it?
> 
> Err, the code 7 lines earlier gets the x509verify value. The x509 and 
> x509verify
> parameters are never to be specified at the same time - either one or the 
> other
> is used.
> 
I noticed that there are several places that check x509verify (or 
vs->tls.x509verify)
value:

./ui/vnc-auth-vencrypt.c:85:    if (vs->vd->tls.x509verify) {
./ui/vnc-tls.c:260:            if (vs->vd->tls.x509verify) {
./ui/vnc-tls.c:388:            if (vs->vd->tls.x509verify) {
./ui/vnc.c:3212:    vs->tls.x509verify = 0;
./ui/vnc.c:3306:            .name = "x509verify",
./ui/vnc.c:3396:        vs->tls.x509verify = qemu_opt_get_bool(opts, 
"x509verify", false);
./ui/vnc.c:3456:    if (acl && x509 && vs->tls.x509verify) {

If only 'x509' parameter is specified, can vnc-tls work after applying this 
patch?
Am I missing something?

Regards,
-Gonglei