Change in vdsm[master]: v2v: Convert VM from external source to Data Domain
danken at redhat.com
danken at redhat.com
Tue Feb 17 11:47:34 UTC 2015
Dan Kenigsberg has posted comments on this change.
Change subject: v2v: Convert VM from external source to Data Domain
......................................................................
Patch Set 4: Code-Review-1
(3 comments)
partial review
http://gerrit.ovirt.org/#/c/37509/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 92: return errCode['invalidV2VJob']
Line 93:
Line 94: try:
Line 95: file_name = os.path.join(_P_V2V_DIR, "%s.ovf" % jobId)
Line 96: with open(file_name, 'r') as f:
this removes partially-written ovfs. We must first verify that the job at hand finished successfully.
We must first check if the virt-v2v command returned with a 0 return code.
Line 97: ovf = f.read()
Line 98: os.remove(file_name)
Line 99: except IOError as exc:
Line 100: if exc.errno == errno.ENOENT:
Line 206: self._vmProperties['vmName'])
Line 207:
Line 208: logging.info('import vm, (jobId %s) started, cmd: %r', self._job_id,
Line 209: cmd)
Line 210: proc = subprocess.Popen(cmd, env={'LIBGUESTFS_BACKEND': 'direct'},
> we'd probably need some input from infra here to make sure we are talking w
yes, you should use utils.execCmd(), and must never touch shell=True.
deathSingnal=15 would make sure that no stale processes remain after a vdsm crash.
Line 211: shell=True,
Line 212: stdout=subprocess.PIPE,
Line 213: stderr=subprocess.PIPE)
Line 214:
Line 313: 'domainID': self._vmProperties['domainID'],
Line 314: 'imageID': disk['imageID'],
Line 315: 'volumeID': disk['volumeID']}
Line 316:
Line 317: volPath = self._cif.getInstance().prepareVolumePath(drive)
if you pass ._cif all the way into here, you don't need getInstance.
I don't know if its any better, but you could
from clientIf import clientIf
clientIf.getInstance()....
Better call
cif.irs.prepareImage() directly, without the virt-only wrapper.
remember to teardown the image afterwards.
Line 318: volPath = volPath.split('/images/')[0]
Line 319: return volPath
Line 320:
Line 321:
--
To view, visit http://gerrit.ovirt.org/37509
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I34bd86d5a87ea8c42113c4a732f87ddd4ceab9ea
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken 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: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list