Change in vdsm[master]: virt: vm: Update time on VM after migration

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3:

Thinking about it more and more, my opinion is we shouldn't update time after 
live migration. The time change may cause effects like unnecessary timeouts, 
logging SQL commands on the VM db as time excessive etc., which is probably not 
what one expects after "live" migration. It's better to leave the time change 
to ntp.
(But the qemu-guest-agent issue is still relevant in other situations.)

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[fromani]

Francesco Romani has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3:

I share your doubts (that was the reason I had in the first place to split off 
this change).
If you feel this patch is no longer needed, feel free to abandon it.

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[michal . skrivanek]

Michal Skrivanek has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3: Code-Review-1

I concur, based on the fact that the migration downtime is 500ms max by 
default, even in customized environment it's in the range of what NTP can 
handle easily

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3:

* #1156194::Update tracker: OK

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[mzamazal]

Milan Zamazal has abandoned this change.

Change subject: virt: vm: Update time on VM after migration
..


Abandoned

As explained in the comments, we shouldn't merge this. See 
https://gerrit.ovirt.org/50499 for related source code documentation update.

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3:

According to a libvirt developer, there should be no delay -- libvirt should be 
aware whether the agent is available or not. So either the functionality is not 
present in older libvirt versions or there is a bug. I was instructed what to 
look for, I'm going to debug it to see what's wrong.

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3: Code-Review-1

We should wait with this until qemu-guest-agent issues are resolved, as 
discussed in https://gerrit.ovirt.org/48860.

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[fromani]

Francesco Romani has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3:

agreed. Do we have an ETA of any form for qemu-guest-agent? I guess this will 
take quite some time to get fixed.

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 2:

* #1156194::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1156194::OK, public bug
* Check Product::#1156194::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 3:

* #1156194::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1156194::OK, public bug
* Check Product::#1156194::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[automation]

gerrit-hooks has posted comments on this change.

Change subject: virt: vm: Update time on VM after migration
..


Patch Set 1:

* #1156194::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1156194::OK, public bug
* Check Product::#1156194::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm: Update time on VM after migration

[mzamazal]

Milan Zamazal has uploaded a new change for review.

Change subject: virt: vm: Update time on VM after migration
..

virt: vm: Update time on VM after migration

When a VM is resumed from suspension and/or migrated, its clock
continues from the time of suspension, i.e. it's delayed.  We already
addressed the problem by introducing VM._syncTime() method.  But for
safety reasons we call the method only in the restoreState path, see
https://gerrit.ovirt.org/48860.

This patch calls _syncTime in migrationDest path as well.  It serves the
originally intended purpose of restoring the clock also after migration,
just separates it from the original solution in case something bad could
happen with migration due to it.

Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Bug-Url: https://bugzilla.redhat.com/1156194
Signed-off-by: Milan Zamazal 
---
M vdsm/virt/vm.py
1 file changed, 2 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/49212/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 9aa08bb..7f33c8d 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2801,10 +2801,6 @@
 fromSnapshot = self.conf.pop('restoreFromSnapshot', False)
 hooks.after_vm_dehibernate(self._dom.XMLDesc(0), self.conf,
{'FROM_SNAPSHOT': fromSnapshot})
-# TODO: _syncTime() should probably be called for both the
-# restore_state and migration_dest paths, but let's handle just the
-# safer case first.
-self._syncTime()
 elif 'migrationDest' in self.conf:
 if self._needToWaitForMigrationToComplete():
 usedTimeout = self._waitForUnderlyingMigration()
@@ -2819,6 +2815,8 @@
 hooks.after_device_migrate_destination(
 dev._deviceXML, self.conf, dev.custom)
 
+self._syncTime()
+
 if 'guestIPs' in self.conf:
 del self.conf['guestIPs']
 if 'guestFQDN' in self.conf:


-- 
To view, visit https://gerrit.ovirt.org/49212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I21ebeecf52bc3f72a2cb7fe9cd9da7b6fd1a86d7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches