Change in vdsm[master]: jobs: Only report progress if set
nsoffer at redhat.com
nsoffer at redhat.com
Sat Dec 12 08:49:09 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: jobs: Only report progress if set
......................................................................
Patch Set 2: Code-Review-1
(2 comments)
We do not need the _progress instance variable.
https://gerrit.ovirt.org/#/c/50355/2/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:
Line 66: def __init__(self, job_id, description=''):
Line 67: self._id = job_id
Line 68: self._status = STATUS.RUNNING
Line 69: self._description = description
Line 70: self._progress = None
v2v jobs have more complicated progress:
393 @property
394 def progress(self):
395 '''
396 progress is part of multiple disk_progress its
397 flat and not 100% accurate - each disk take its
398 portion ie if we have 2 disks the first will take
399 0-50 and the second 50-100
400 '''
401 completed = (self._disk_count - 1) * 100
402 return (completed + self._disk_progress) / self._disk_count
So basing progress on an instance variable is not a good idea.
We should keep the concept of having to override progress, but the logic should be that job.progress should be None when you cannot report progress.
For example, job copying image will return while it is blocked, taking locks and validating the operation, and once qemu-img has started, it will start to return progress value.
So in copy image we would do something like this:
@property
def progress(self):
if self._convert_operation:
return self._convert_operation.progress
return None
So Job should have no _progress instance variable.
Line 71: self._error = None
Line 72:
Line 73: @property
Line 74: def id(self):
Line 83: return self._description
Line 84:
Line 85: @property
Line 86: def progress(self):
Line 87: return self._progress
Return None, so code which does not override this will get no progress. v2v ImportVm overrides this so it is a safe change.
Line 88:
Line 89: @property
Line 90: def job_type(self):
Line 91: return self._JOB_TYPE
--
To view, visit https://gerrit.ovirt.org/50355
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cc8b66d825676045fbcb8431bb3cf11885ff02b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list