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