From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44924) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVeVd-00038P-Uv for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:09:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVeVY-0002AH-Fr for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:09:05 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:34363) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVeVX-00029m-Lb for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:09:00 -0400 Message-ID: <55002205.2040509@huawei.com> Date: Wed, 11 Mar 2015 19:07:49 +0800 From: Gonglei MIME-Version: 1.0 References: <1426004854-4869-1-git-send-email-berrange@redhat.com> <54FF9EFE.8000706@huawei.com> <20150311094529.GB22609@redhat.com> In-Reply-To: <20150311094529.GB22609@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] ui: fix regression in x509verify parameter for VNC server List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, Gerd Hoffmann 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 >>> 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 >>> --- >>> 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