From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhHUv-0007Ph-Cg for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:28:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhHUq-0007Gh-JD for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:28:09 -0400 Received: from szxga01-in.huawei.com ([119.145.14.64]:60504) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhHUp-0007F0-6x for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:28:04 -0400 Message-ID: <5448F42B.10907@huawei.com> Date: Thu, 23 Oct 2014 20:27:23 +0800 From: Gonglei MIME-Version: 1.0 References: <1414042783-2180-1-git-send-email-arei.gonglei@huawei.com> <1414051702.30724.6.camel@nilsson.home.kraxel.org> <5448BA00.4010608@huawei.com> <1414066616.30724.10.camel@nilsson.home.kraxel.org> In-Reply-To: <1414066616.30724.10.camel@nilsson.home.kraxel.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/3] vnc: trivial problem and memory leak fix List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: "Huangweidong (C)" , "qemu-devel@nongnu.org" , "Huangpeng (Peter)" On 2014/10/23 20:16, Gerd Hoffmann wrote: > Hi, > >>>> Though we call qmp_change_vnc() failed, the content of global variable vnc_display >>>> still will change, such as 'vs->auth = VNC_AUTH_NONE' now. I think this is not >>>> reasonable, but I have not good idea to address this issue. >>> >>> I think the current behavior is fine. Better be on the safe side (no >>> connects possible) in case "change vnc $display" failed. You can fix >>> that using "change vnc 0.0.0.0:10,password,sasl" >> >> Yes. we can do this. But is it reasonable that an API return false, >> but its content changed? > > We have: Call failed -> vnc server not working. I think that is ok. > > You want: Call failed -> vnc server has previous state. Reasonable > too, but that is a hard problem. You can try store everything in > temporary variables while initializing. On success cleanup VncDisplay > struct and fill with the new data. On failure leave VncDisplay > untouched. But note that there are tricky corner cases such as binding > the new listening socket will fail when it is still in use by the old > listening socket. Or goes on try IPv4 for the new in case the IPv6 > address is in use by the old. Have fun debugging your connection > problems then! > Indeed. > There is some middle ground -- you can try to do parameter parsing > first. Any failures there are easily catched. When that succeeds go > call vnc_display_close + reinitialize. This avoids the problems > outlined above at the cost of not catching all possible failure modes. > Yes. > If you want try solving this nevertheless I'd suggest to do it on top of > https://www.kraxel.org/cgit/qemu/log/?h=rebase/ui-vnc-next to avoid > conflicts. > OK. > Oh, and the switch to QemuOpts for vnc parameter parsing in the branch > might already improve things. > Hmm. That's very useful, Thanks a lot :) Best regards, -Gonglei