On 12/9/19 4:53 PM, Michael Weiser wrote: > Hello Cole, > > this is the third iteration of implementing guest time sync as per > your suggestions. The only change compared to v2 is in the way the last > patch blocks the toggled signal. > > These patches do: > * add method _set_time() calling the libvirt setTime API > * wire it up in resume from paused and saved state as well as revert to > a snapshot containing a running domain state > * only run the API if the connection is of type qemu or test > * only run the API if a guest agent channel is defined for a qemu > connection > * wait for the guest agent channel state to become connected, i.e. the > guest agent coming online inside the guest > * add global and per-vm option to disable time sync > * try to fix up some peculiar behaviour of checkable vm window menu > items > Nice work. I pushed the first 4 patches. > If you drop me some quick pointers how to go about dispatching the set > time operation to a separate or already existing background thread so it > doesn't block the UI, I'll be happy to give that a whirl as well. As it > stands now, the attempts at syncing time cause a five second delay if > the agent does not come online. It would be nice to avoid that. > Does it make the progress dialog spin for 5 seconds, or does it actually block any interaction with the app? All the actions that you added _set_time to should be invoked via threads in the UI portions of the code, if they aren't then that's a bug in itself. But if you wanted to avoid the sleep spin in _set_time, the way to do it is to use timeout_add, which will invoke the passed function from the main UI thread when the timeout is hit. Everywhere you have a time.sleep(X) in the current code, you'd do self.timeout_add(X * 1000, self._set_time, args) In args, you'd pass how many iterations the loop has already done, so you can track when to kill the timeout loop. As for for the configuration piece, I'd prefer not to add it at all unless there's a really good reason. Yes hardcoding timeouts will always mean we get it wrong in some case, but I don't think this is important enough to justify 100 lines of code and UI to match, and the testing and maintenance burden that entails. Plus I just don't expect many people will ever change the setting unless I'm missing some specific case. There is possible ways to make it not need a timeout, like watching for the domain event when the agent is connected. It's probably tricky to get it right. Generally I'd rather jack up the timeout and use the timeout_add behavior to hide it from the UI, then make this configurable. Thanks, Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
