[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/3] vnc: trivial problem and memory leak fix
From: |
Gonglei |
Subject: |
Re: [Qemu-devel] [PATCH 0/3] vnc: trivial problem and memory leak fix |
Date: |
Thu, 23 Oct 2014 20:27:23 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 |
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
- [Qemu-devel] [PATCH 1/3] vnc: remove superfluous vnc_display_close(), (continued)