Change in vdsm[master]: Update v2v to use new jobs infrastructure

nsoffer at redhat.com nsoffer at redhat.com
Wed Aug 26 16:37:44 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: Update v2v to use new jobs infrastructure
......................................................................


Patch Set 1: Code-Review-1

(11 comments)

https://gerrit.ovirt.org/#/c/45382/1/vdsm/v2v.py
File vdsm/v2v.py:

Line 97
Line 98
Line 99
Line 100
Line 101
We don't need this, just use jobs.JobExistsError


Line 101
Line 102
Line 103
Line 104
Line 105
This error needs a name - looks like a bug in the current code. ClientError should have consistent interface. I would fix this in a separate patch.


Line 106
Line 107
Line 108
Line 109
Line 110
We don't need this, just use jobs.NoSuchJob


Line 111
Line 112
Line 113
Line 114
Line 115
Use jobs.JobNotDone


Line 116
Line 117
Line 118
Line 119
Line 120
Should rename err_name to name to match jobs.ClientError.


Line 124
Line 125
Line 126
Line 127
Line 128
Needs an error name.


Line 202
Line 203
Line 204
Line 205
Line 206
ClientError does not have err_name now, use e.name


Line 259:         super(ImportVm, self).__init__(job_id)
Line 260:         self._vminfo = vminfo
Line 261:         self._irs = irs
Line 262: 
Line 263:         self._status = V2VSTATUS.STARTING
We can make the patch smaller by keeping v2v.STATUS (it can still inherit from jobs.STATUS)
Line 264:         self._disk_progress = 0
Line 265:         self._disk_count = 1
Line 266:         self._current_disk = 1
Line 267:         self._aborted = False


Line 325:             if self._aborted:
Line 326:                 logging.debug("Job %r was aborted", self._id)
Line 327:             else:
Line 328:                 logging.exception("Job %r failed", self._id)
Line 329:                 self._status = V2VSTATUS.FAILED
Can be avoided
Line 330:                 self._description = ex.message
Line 331:                 try:
Line 332:                     self._abort()
Line 333:                 except Exception as e:


Line 355:                                   (self._id, self._proc.returncode,
Line 356:                                    self._proc.stderr.read(1024)))
Line 357: 
Line 358:         if self._status != V2VSTATUS.ABORTED:
Line 359:             self._status = V2VSTATUS.DONE
Can be avoided
Line 360:             logging.info('Job %r finished import successfully', self._id)
Line 361: 
Line 362:     def _execution_environments(self):
Line 363:         env = {'LIBGUESTFS_BACKEND': 'direct'}


Line 376:     def _watch_process_output(self):
Line 377:         parser = OutputParser()
Line 378:         for event in parser.parse(self._proc.stdout):
Line 379:             if isinstance(event, ImportProgress):
Line 380:                 self._status = V2VSTATUS.COPYING_DISK
Can be avoided
Line 381:                 logging.info("Job %r copying disk %d/%d",
Line 382:                              self._id, event.current_disk, event.disk_count)
Line 383:                 self._disk_progress = 0
Line 384:                 self._current_disk = event.current_disk


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9118e0fe4aaeceab2109afa393dd45fbd97e070f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list