Change in vdsm[master]: v2v: Convert VM from external source to Data Domain

nsoffer at redhat.com nsoffer at redhat.com
Wed Mar 11 11:49:32 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: v2v: Convert VM from external source to Data Domain
......................................................................


Patch Set 8:

(2 comments)

https://gerrit.ovirt.org/#/c/37509/8/vdsm/v2v.py
File vdsm/v2v.py:

Line 114:             return {'status': doneCode}
Line 115:         elif _jobs[jobId].status != _STATUS_DONE:
Line 116:             return errCode['V2VJobNotDone']
Line 117:         else:
Line 118:             del _jobs[jobId]
> if the status is error there is no error we just need to delete the job sin
Do you mean that get_converted_vm will return success and no ovf?

The call should fail with the relevant error: no such ovf, error reading ovf, generic error (bug in the code).
Line 119: 
Line 120:     try:
Line 121:         file_name = os.path.join(_P_V2V_DIR, "%s.ovf" % jobId)
Line 122:         with open(file_name, 'r') as f:


Line 127:             return errCode['invalidV2VOvfPath']
Line 128:         else:
Line 129:             logging.error('error reading file "%s" error: %r message %r',
Line 130:                           file_name, exc.errno, exc.message)
Line 131:             raise
> I didn't put read_ovf in a method since it also check if the file exists an
Right, the method should not return an error, this belongs to the public function that has to provide output for the rpc layer.

Thats why we have exceptions - you read and return an ovf, or raise exceptions handled by the upper layers.
Line 132:     return {'status': doneCode, 'ovf': ovf}
Line 133: 
Line 134: 
Line 135: def abort_job(jobId):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I34bd86d5a87ea8c42113c4a732f87ddd4ceab9ea
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list