Change in vdsm[master]: periodic: use name for the work sent to executor

Francesco Romani has uploaded a new change for review.

Change subject: periodic: use name for the work sent to executor
......................................................................

periodic: use name for the work sent to executor

WIP
to improve debuggability

Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
---
M tests/periodicTests.py
M vdsm/virt/periodic.py
2 files changed, 6 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/48193/1

diff --git a/tests/periodicTests.py b/tests/periodicTests.py
index 5320223..1fee0b3 100644
--- a/tests/periodicTests.py
+++ b/tests/periodicTests.py
@@ -266,7 +266,7 @@
     def __init__(self, fail=False):
         self._fail = fail
 
-    def dispatch(self, func, timeout):
+    def dispatch(self, func, timeout, name):
         if self._fail:
             raise executor.TooManyTasks()
         else:
diff --git a/vdsm/virt/periodic.py b/vdsm/virt/periodic.py
index 8e4012a..ead51d8 100644
--- a/vdsm/virt/periodic.py
+++ b/vdsm/virt/periodic.py
@@ -167,6 +167,7 @@
         self._lock = threading.Lock()
         self._running = False
         self._call = None
+        self._name = str(self._func)
 
     def start(self):
         with self._lock:
@@ -213,7 +214,7 @@
         Send `func' to Executor to be run as soon as possible.
         """
         self._call = None
-        self._executor.dispatch(self, self._timeout)
+        self._executor.dispatch(self, self._timeout, self._name)
         self._step()
 
 
@@ -250,6 +251,7 @@
         skipped = []
 
         for vm_id, vm_obj in vms.iteritems():
+            name = '%s on %s' % (self._create, vm_id)
             try:
                 if self._busy_doms.get(vm_id, False):
                     skipped.append(vm_id)
@@ -270,11 +272,10 @@
 
             except Exception:
                 # we want to make sure to have VM UUID logged
-                self._log.exception("while dispatching %s to VM '%s'",
-                                    self._create, vm_id)
+                self._log.exception("while dispatching %s", name)
             else:
                 try:
-                    self._executor.dispatch(op, self._timeout)
+                    self._executor.dispatch(op, self._timeout, name)
                 except executor.TooManyTasks:
                     skipped.append(vm_id)
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


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.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


Patch Set 2:

* 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.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


Patch Set 3:

* 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.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


Patch Set 4:

* 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.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


Patch Set 5:

* #1250839::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1250839::OK, public bug
* Check Product::#1250839::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


Patch Set 5: Verified+1

verified together: 48191, 48193, 48192, 48333
verified with 48332 using steps provided in
https://bugzilla.redhat.com/show_bug.cgi?id=1250839#c7

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: periodic: use name for the work sent to executor
......................................................................


Patch Set 5: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: automation(a)ovirt.org
Gerrit-HasComments: No

Change in vdsm[master]: periodic: add executor-compatible naming

automation(a)ovirt.org has posted comments on this change.

Change subject: periodic: add executor-compatible naming
......................................................................


Patch Set 6:

* #1250839::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1250839::OK, public bug
* Check Product::#1250839::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: automation(a)ovirt.org
Gerrit-HasComments: No

Change in vdsm[master]: periodic: add executor-compatible naming

Francesco Romani has posted comments on this change.

Change subject: periodic: add executor-compatible naming
......................................................................


Patch Set 6: Verified+1

verified - actually on branch 3.6, while checking https://gerrit.ovirt.org/#/c/48348/

logs as expected.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: automation(a)ovirt.org
Gerrit-HasComments: No

Change in vdsm[master]: periodic: add executor-compatible naming

Milan Zamazal has posted comments on this change.

Change subject: periodic: add executor-compatible naming
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/48193/6//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: We want to make as easy as possible to debug
Line 10: thread leaks from executor, so we make the periodic
Line 11: framework support names for Operations, using the
Line 12: way Executor expects them.
What's the executor-compatible naming? Is NAME utilized anywhere outside periodic.py,
perhaps in another patch (I can't see it in the current source code)?
Line 13: 
Line 14: Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Line 15: Bug-Url: https://bugzilla.redhat.com/1250839
Line 16: Backport-To: 3.6


https://gerrit.ovirt.org/#/c/48193/6/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 262:         return '<VmDispatcher operation=%s at 0x%x>' % (
Line 263:             self._create, id(self)
Line 264:         )
Line 265: 
Line 266:     __repr__ = __str__
Why renaming __repr__ to __str__ and then defining this alias here? Wasn't having just
__repr__ method enough?
Line 267: 
Line 268: 
Line 269: class _RunnableOnVm(object):
Line 270: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Change in vdsm[master]: periodic: add executor-compatible naming

Francesco Romani has posted comments on this change.

Change subject: periodic: add executor-compatible naming
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/48193/6//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: We want to make as easy as possible to debug
Line 10: thread leaks from executor, so we make the periodic
Line 11: framework support names for Operations, using the
Line 12: way Executor expects them.
>  What's the executor-compatible naming? Is NAME utilized anywhere
> outside pe 
It is a relic of versions past. Now we just use the standard python tools
(__str__ magic method), so I can rephrase this commit message.
Line 13: 
Line 14: Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Line 15: Bug-Url: https://bugzilla.redhat.com/1250839
Line 16: Backport-To: 3.6


https://gerrit.ovirt.org/#/c/48193/6/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 262:         return '<VmDispatcher operation=%s at 0x%x>' % (
Line 263:             self._create, id(self)
Line 264:         )
Line 265: 
Line 266:     __repr__ = __str__
>  Why renaming __repr__ to __str__ and then defining this alias here?
> Wasn't  
Back in time I added __repr__ to have something nicer output in the
logs, but without much thought. executor now uses str(), hence the newly added __str__.

I decided to keep __repr__ here, hence the alias.
Line 267: 
Line 268: 
Line 269: class _RunnableOnVm(object):
Line 270: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Change in vdsm[master]: periodic: add executor-compatible naming

Milan Zamazal has posted comments on this change.

Change subject: periodic: add executor-compatible naming
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/48193/6/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 262:         return '<VmDispatcher operation=%s at 0x%x>' % (
Line 263:             self._create, id(self)
Line 264:         )
Line 265: 
Line 266:     __repr__ = __str__
>  Back in time I added __repr__ to have something nicer output in the
> logs, b 
Just thought about retaining __repr__ here (as `str' falls back to it),
but if you think using __str__ here (and in other classes here) is more appropriate then
it's fine for me.
Line 267: 
Line 268: 
Line 269: class _RunnableOnVm(object):
Line 270: 


Line 292: 
Line 293:     def __str__(self):
Line 294:         return '<%s vm=%s at 0x%x>' % (
Line 295:             self.NAME, self._vm.id, id(self)
Line 296:         )
Why not simply using self.__class__.__name__ here and introducing NAME?
Line 297: 
Line 298: 
Line 299: class UpdateVolumes(_RunnableOnVm):
Line 300: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Change in vdsm[master]: periodic: add executor-compatible naming

Francesco Romani has posted comments on this change.

Change subject: periodic: add executor-compatible naming
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/48193/6/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 262:         return '<VmDispatcher operation=%s at 0x%x>' % (
Line 263:             self._create, id(self)
Line 264:         )
Line 265: 
Line 266:     __repr__ = __str__
>  Just thought about retaining __repr__ here (as `str' falls back
> to it), but 
You are right, as the doc -why I should have refreshed earlier- reminds
us:
https://docs.python.org/2/reference/datamodel.html#object.__repr__

However, here I abused __repr__ in the old code, because this representation definitely
leans toward "informal", and not even attempts to be a valid python expression.
So it always was more akin to __str__ than __repr__ in the intentions.

For this reason I'll keep the rename and drop the alias, and maybe readd back __repr__
in a later patch, with better context and rationale.
Line 267: 
Line 268: 
Line 269: class _RunnableOnVm(object):
Line 270: 


Line 292: 
Line 293:     def __str__(self):
Line 294:         return '<%s vm=%s at 0x%x>' % (
Line 295:             self.NAME, self._vm.id, id(self)
Line 296:         )
>  Why not simply using self.__class__.__name__ here and introducing
> NAME? 
no good reason. Changing - thanks!
Line 297: 
Line 298: 
Line 299: class UpdateVolumes(_RunnableOnVm):
Line 300: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Change in vdsm[master]: periodic: add __str__ methods

gerrit-hooks has posted comments on this change.

Change subject: periodic: add __str__ methods
......................................................................


Patch Set 7:

* #1250839::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1250839::OK, public bug
* Check Product::#1250839::OK, Correct classification oVirt
* 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/48193
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: periodic: add __str__ methods

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: periodic: add __str__ methods
......................................................................


Patch Set 7:

(1 comment)

https://gerrit.ovirt.org/#/c/48193/7/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 314: 
Line 315: 
Line 316: class NumaInfoMonitor(_RunnableOnVm):
Line 317: 
Line 318:     NAME = "NumaInfoMonitor"
why are these any better than self.__class__.__name__ ?
Line 319: 
Line 320:     @property
Line 321:     def required(self):
Line 322:         return self._vm.hasGuestNumaNode


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: periodic: add __str__ methods

Francesco Romani has posted comments on this change.

Change subject: periodic: add __str__ methods
......................................................................


Patch Set 7: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/48193/7/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 314: 
Line 315: 
Line 316: class NumaInfoMonitor(_RunnableOnVm):
Line 317: 
Line 318:     NAME = "NumaInfoMonitor"
>  why are these any better than self.__class__.__name__ ? 
They
are not at all, Milan already pointed out and I just missed to update the patch.
Line 319: 
Line 320:     @property
Line 321:     def required(self):
Line 322:         return self._vm.hasGuestNumaNode


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: periodic: add __str__ methods

gerrit-hooks has posted comments on this change.

Change subject: periodic: add __str__ methods
......................................................................


Patch Set 8:

* #1250839::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1250839::OK, public bug
* Check Product::#1250839::OK, Correct classification oVirt
* 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/48193
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: periodic: add __str__ methods

Milan Zamazal has posted comments on this change.

Change subject: periodic: add __str__ methods
......................................................................


Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8d1180b727571cce34e8304ac48390ed2135f79
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(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: Martin Polednik <mpolednik(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No