Change in vdsm[master]: vm: reformat setUnresponsiveIfTimeout

Francesco Romani has uploaded a new change for review.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

vm: reformat setUnresponsiveIfTimeout

Reformat the order of operations to make clear
on which cases we don't care about timeouts.

Change-Id: I53e9c284962f7ebe3987e460e4d138f2a08704dc
Backport-To: 4.0
Bug-Url: https://bugzilla.redhat.com/1382578
Signed-off-by: Francesco Romani 
---
M vdsm/virt/vm.py
1 file changed, 12 insertions(+), 7 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/65504/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index e3974e3..dc6ffe9 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -4667,13 +4667,18 @@
                       reason)

def _setUnresponsiveIfTimeout(self, stats, statsAge):
-        if (not self.isMigrating()
-                and statsAge > config.getint('vars', 'vm_command_timeout')
-                and stats['monitorResponse'] != '-1'):
-            self.log.warning('monitor become unresponsive'
-                             ' (command timeout, age=%s)',
-                             statsAge)
-            stats['monitorResponse'] = '-1'
+        if self.isMigrating():
+            return
+        # we don't care about decimals here
+        if statsAge < config.getint('vars', 'vm_command_timeout'):
+            return
+        if stats['monitorResponse') == '-1':
+            return
+
+        self.log.warning('monitor become unresponsive'
+                         ' (command timeout, age=%s)',
+                         statsAge)
+        stats['monitorResponse'] = '-1'

def updateNumaInfo(self):
         self._numaInfo = numa.getVmNumaNodeRuntimeInfo(self)

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 1:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::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'])

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 2:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::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'])

Francesco Romani has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 2: Verified+1

verified using the existing and the new tests (see followup patches)

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 3:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::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'])

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 4:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::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'])

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 5:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::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'])

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 6:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::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'])

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 7:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::OK, public bug
* Check Public Bug::#1382583::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'])

Francesco Romani has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 7: Verified+1

verified with 65590

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 8:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1382578::OK, public bug
* Check Public Bug::#1382583::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'])

Milan Zamazal has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 8: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/65504/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS8, Line 4687: become
When we are on it: "became"?

[Martin Polednik]

Martin Polednik has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 8: Code-Review+1

I don't think there is really a need for this, but it doesn't hurt either. Agree with Milan's comment, not critical though.

Francesco Romani has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 8:

(1 comment)

https://gerrit.ovirt.org/#/c/65504/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS8, Line 4687: become
> When we are on it: "became"?

yes, will fix

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 9:

* Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification
* Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification
* Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification
* Check Public Bug::#1382578::OK, public bug
* Check Public Bug::#1382583::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'])

Francesco Romani has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 9: Verified+1

fixed comment, if jenkins is happy so we are

[Martin Polednik]

Martin Polednik has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 9: Code-Review+1

Milan Zamazal has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 9: Code-Review+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 9: Code-Review+2

raising scores

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

vm: reformat setUnresponsiveIfTimeout

Reformat the order of operations to make clear
on which cases we don't care about timeouts.

Change-Id: I53e9c284962f7ebe3987e460e4d138f2a08704dc
Backport-To: 4.0
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1382578
Bug-Url: https://bugzilla.redhat.com/1382583
Signed-off-by: Francesco Romani 
Reviewed-on: https://gerrit.ovirt.org/65504
Continuous-Integration: Jenkins CI
Reviewed-by: Martin Polednik 
Reviewed-by: Milan Zamazal 
Reviewed-by: Dan Kenigsberg 
---
M vdsm/virt/vm.py
1 file changed, 12 insertions(+), 7 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified
  Milan Zamazal: Looks good to me, but someone else must approve
  Martin Polednik: Looks good to me, but someone else must approve

gerrit-hooks has posted comments on this change.

Change subject: vm: reformat setUnresponsiveIfTimeout
......................................................................

Patch Set 10:

* update_tracker: OK
* Set MODIFIED::bug 1382578::::#1382578::IGNORE, skipping for branch 'master'
* Set MODIFIED::bug 1382578::::bug 1382583::::#1382583::IGNORE, skipping for branch 'master'