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! 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. 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. Oh, and the switch to QemuOpts for vnc parameter parsing in the branch might already improve things. cheers, Gerd
