Change in vdsm[master]: virt: Prevent monitoring of VMs in post-copy

Milan Zamazal has uploaded a new change for review.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................

virt: Prevent monitoring of VMs in post-copy

Migrating VMs in post-copy mode on the source end are paused.  It makes
no sense to monitor them and we don't want to get error messages in the
log or experience other problems.  So we skip VMs in that state in
periodic operations.

We also skip the VMs that just successfully initiated switching to
post-copy mode.  Under normal circumstances, they should enter post-copy
mode shortly afterwards, so it's better to skip them immediately.

We still can't guarantee (without excessive locking) that VM doesn't
switch to post-copy (and gets paused), so we handle the corresponding
contingent failure as well.

Note that we still gather stats, since we need to know the migration
progress.

Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com&gt;
Bug-Url: https://bugzilla.redhat.com/1354343
---
M lib/vdsm/virt/periodic.py
M vdsm/virt/vm.py
2 files changed, 10 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/64146/7

diff --git a/lib/vdsm/virt/periodic.py b/lib/vdsm/virt/periodic.py
index 132fe93..959ff1b 100644
--- a/lib/vdsm/virt/periodic.py
+++ b/lib/vdsm/virt/periodic.py
@@ -290,7 +290,8 @@
     def required(self):
         # Disable everything until the migration destination VM
         # is fully started, to avoid false positives log spam.
-        return not self._vm.incomingMigrationPending()
+        return (not self._vm.incomingMigrationPending() and
+                not self._vm.post_copy_initiated)
 
     @property
     def runnable(self):
@@ -307,6 +308,9 @@
             self._vm.log.warning('could not run on %s: domain not connected',
                                  self._vm.id)
         except libvirt.libvirtError as e:
+            if self._vm.in_post_copy:
+                # race on entering post-copy, VM paused now
+                return
             if e.get_error_code() in (
                 # race on shutdown/migration completion
                 libvirt.VIR_ERR_NO_DOMAIN,
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index f806c45..2fb498a 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -323,6 +323,11 @@
     def in_post_copy(self):
         return self._in_post_copy
 
+    @property
+    def post_copy_initiated(self):
+        return (self._in_post_copy or
+                self._migrationSourceThread.post_copy_initiated)
+
     def _get_lastStatus(self):
         # note that we don't use _statusLock here. One of the reasons is the
         # non-obvious recursive locking in the following flow:


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 8:

* Update Tracker::#1354343::OK, status: POST
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1354343::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 9:

* Update Tracker::#1354343::OK, status: POST
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1354343::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Milan Zamazal has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 9:

Test breakage fixed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 9: Code-Review+1

Most likely https://gerrit.ovirt.org/#/c/65590/ will be merged earlier than this patch,
because 65590 wants to fix one bug.
You may want to rebase this on top of 65590. Let's keep it as-is until that moment.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 10:

* Update Tracker::#1354343::OK, status: POST
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1354343::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Milan Zamazal has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 10:

Just rebase + updates.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


Patch Set 10: Code-Review+1

(1 comment)

minor comment inside

https://gerrit.ovirt.org/#/c/64146/10/lib/vdsm/virt/periodic.py
File lib/vdsm/virt/periodic.py:

PS10, Line 311: if self._vm.post_copy:
again, not sure about using this in a boolean context, especially because you used it as
enumerated above.
I'd say using here as enumerated is a bit cleaner.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has submitted this change and it was merged.

Change subject: virt: Prevent monitoring of VMs in post-copy
......................................................................


virt: Prevent monitoring of VMs in post-copy

Migrating VMs in post-copy mode on the source end are paused.  It makes
no sense to monitor them and we don't want to get error messages in the
log or experience other problems.  So we skip VMs in that state in
periodic operations.

We also skip the VMs that just successfully initiated switching to
post-copy mode.  Under normal circumstances, they should enter post-copy
mode shortly afterwards, so it's better to skip them immediately.

We still can't guarantee (without excessive locking) that VM doesn't
switch to post-copy (and gets paused), so we handle the corresponding
contingent failure as well.

Note that we still gather stats, since we need to know the migration
progress.

Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com&gt;
Bug-Url: https://bugzilla.redhat.com/1354343
---
M lib/vdsm/virt/periodic.py
M tests/periodicTests.py
M vdsm/virt/vm.py
3 files changed, 9 insertions(+), 1 deletion(-)

Approvals:
  Jenkins CI: Passed CI tests
  Francesco Romani: Looks good to me, approved
  Milan Zamazal: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I010d3311ce497081720a871e2a3c715f53dcb299
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;