Nir Soffer has posted comments on this change. Change subject: virt: vm: Update time on VM after resume ......................................................................
Patch Set 4: (4 comments) https://gerrit.ovirt.org/#/c/48860/4/tests/vmTests.py File tests/vmTests.py: Line 987: Line 988: def test_sync_time(self): Line 989: # No easy check it works, so let's just test it doesn't crash. Line 990: with fake.VM() as fakevm: Line 991: fakevm._syncTime() This does not test anything. You should check that this invokes the proper libvirt api, using mock libvirt connection. See freeze and thaw tests for example. Line 992: Line 993: Line 994: VM_EXITS = tuple(product((define.NORMAL, define.ERROR), Line 995: vmexitreason.exitReasons.keys())) https://gerrit.ovirt.org/#/c/48860/4/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1190: finally: Line 1191: if not guestCpuLocked: Line 1192: self._guestCpuLock.release() Line 1193: Line 1194: def _syncTime(self): maybe _syncGuestTime? Line 1195: """ Line 1196: Try to set VM time to the current value. This is typically useful when Line 1197: clock wasn't running on the VM for some time (e.g. during suspension or Line 1198: migration), especially if the time delay exceeds NTP tolerance. Line 1202: very precise (NTP in the guest should take care of it if needed). Line 1203: """ Line 1204: t = time.time() Line 1205: seconds = int(t) Line 1206: nseconds = int((t - seconds) * 10**9) Use NSEC_PER_SEC? Line 1207: try: Line 1208: self._dom.setTime(time={'seconds': seconds, 'nseconds': nseconds}) Line 1209: except libvirt.libvirtError: Line 1210: # It may fail for various reasons, e.g. QEMU guest agent not Line 1208: self._dom.setTime(time={'seconds': seconds, 'nseconds': nseconds}) Line 1209: except libvirt.libvirtError: Line 1210: # It may fail for various reasons, e.g. QEMU guest agent not Line 1211: # running on the VM. Line 1212: self.log.exception("Failed to set time") If this depends on qemu-guest-agent, it should use the error handling used in freeze() and thaw(). Expected errors such as libvirt.VIR_ERR_AGENT_UNRESPONSIVE or libvirt.VIR_ERR_NO_SUPPORT should be logged in debug level. Only other errors should logged using log.execption, or maybe log.error, since the traceback here is not very useful. Line 1213: except virdomain.NotConnectedError: Line 1214: self.log.debug("Failed to set time: not connected") Line 1215: else: Line 1216: self.log.debug('Time updated to: %d.%09d', seconds, nseconds) -- To view, visit https://gerrit.ovirt.org/48860 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieb583cd5d21e56d7730b0ba21d75ed93b9d34025 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
