[virt-tools-list] Guest time sync on snapshot resume

[Cole Robinson]

On 11/14/19 10:30 AM, Michael Weiser wrote:
> Hello,
> 
> I expect this pops up every couple of months and apologise if indeed it
> does. But I couldn't find a discussion specific to virt-manager and so
> it goes:
> 
> When working with snapshots, guests will have outdated date and time
> upon resume. Although the (virtual) hardware clock seems to get updated
> upon resume, it can take quite a long time until the guest OS resyncs
> its current time from either that or via NTP. I've seen workarounds
> employing hwclock --hctosys in once-a-minute cron jobs or very
> aggressive NTP client configs.
> 
> Now I have come across an agent based solution in Nova[1] and ported it
> PoC-style to virt-manager. It works very well for me.
> 
> Would this be something that could be considered for inclusion?
> Or is there another mechanism that should already take care of this and
> just doesn't work in my case?
> 

Yes it's definitely worth fixing, we are overdue for it in fact. There's
a virt-manager bug about it, it doesn't have much more info though:

https://bugzilla.redhat.com/show_bug.cgi?id=1222069

> [1] https://github.com/openstack/nova/commit/414df1e56ea9df700756a1732125e06c5d97d792.
> 
> PoC code:
> 
> --- virt-manager/virtManager/object/domain.py.orig	2019-10-31 17:35:04.829950245 +0100
> +++ virt-manager/virtManager/object/domain.py	2019-10-31 18:19:39.834899951 +0100
> @@ -1276,7 +1276,28 @@
>  
>      @vmmLibvirtObject.lifecycle_action
>      def startup(self):
> +        sync_time = self.has_managed_save()
>          self._backend.create()
> +        if sync_time:
> +            dry = 1
> +            while dry < 5:
> +                t = time.time()
> +                seconds = int(t)
> +                nseconds = int((t - seconds) * 10 ** 9)
> +                try:
> +                    self._backend.setTime(time = {'seconds': seconds, 'nseconds': nseconds})
> +                except libvirt.libvirtError as e:
> +                    code = e.get_error_code()
> +                    if code == libvirt.VIR_ERR_AGENT_UNRESPONSIVE:
> +                        log.debug('Failed to set time: QEMU agent unresponsive')
> +                    elif code == libvirt.VIR_ERR_NO_SUPPORT:
> +                        log.debug('Failed to set time: not supported')
> +                    elif code == libvirt.VIR_ERR_ARGUMENT_UNSUPPORTED:
> +                        log.debug('Failed to set time: agent not configured')
> +                    else:
> +                        log.warning('Failed to set time: %s', e)
> +
> +                dry += 1
> 

I like the idea of using nova's code, that's a good starting point. Add
a comment in the code that that this is inspired by nova code, with a
link to that commit you mentioned, it's a good reference.

Everything under sync_time can be moved to its own function, def
_set_time(self), so we can easily reuse it from other code paths like
snapshot restore and possibly resume from pause.

The different log levels aren't interesting for virt-manager, you can
just do:

except Exception as e:
    log.debug("Failed to set time: %s", e)

With those changes it will be commitable IMO. After that, some ideas are:

* Only run the API for qemu and test drivers, they are the only ones
that support it. This will save spamming logs with error output
* For qemu, only run the API if the VM has the qemu guest agent in the XML.
* Wire it up for snapshot restore, and possibly after unpause (need to
confirm it's desired there)
* Maybe dispatch the whole thing in its old thread, so it doesn't appear
in the UI like the VM start process is blocked by the time API completing.

I can help with the extra bits but I'm happy to provide guidance if
you'd like to work on them too.

Thanks,
Cole