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
