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

shavivi at redhat.com shavivi at redhat.com
Sun Mar 15 07:42:02 UTC 2015


Shahar Havivi has posted comments on this change.

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


Patch Set 9:

(5 comments)

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

Line 219:     def _import(self):
Line 220:         self._create_passwd_file()
Line 221:         self._status = _STATUS_STARTING
Line 222:         # TODO: use the process handling http://gerrit.ovirt.org/#/c/33909/
Line 223:         cmd = [str(_VIRT_V2V), '-ic', self._uri, '-o', 'vdsm', '-of', 'raw']
> _VIRT_V2V.path is cleaner
Done
Line 224:         cmd.extend(self._generate_disk_parameters())
Line 225:         cmd.extend(['--password-file', self._pass_file, '--vdsm-vm-uuid',
Line 226:                     self._job_id, '--vdsm-ovf-output', _P_V2V_DIR,
Line 227:                     '--machine-readable', '-os', self._get_domain_path(),


Line 309:             os.remove(self._pass_file)
Line 310: 
Line 311:     def abort_job(self):
Line 312:         self._abort = True
Line 313:         self._status = _STATUS_ABORT
> If the v2v process is stuck, we would never notice this flag-waving.
Done
Line 314:         logging.info('aborting job id: %r', self._job_id)
Line 315: 
Line 316:     def _abort_msg(self, msg):
Line 317:         self._status = _STATUS_ERROR


Line 334:         ''' method prepare the images and return storage domain mounted path
Line 335:             since all images are in the same domain we return arbitrary image
Line 336:             for the return path (res['path']) '''
Line 337:         for disk in self._vmProperties['disks']:
Line 338:             res = self._cif.irs.prepareImage(self._vmProperties['domainID'],
> we should check the returned status, collect successful preperations, and t
Done
Line 339:                                              self._vmProperties['poolID'],
Line 340:                                              disk['imageID'], disk['volumeID'])
Line 341:         volPath = res['path']
Line 342:         return volPath.split('/images/')[0]


Line 338:             res = self._cif.irs.prepareImage(self._vmProperties['domainID'],
Line 339:                                              self._vmProperties['poolID'],
Line 340:                                              disk['imageID'], disk['volumeID'])
Line 341:         volPath = res['path']
Line 342:         return volPath.split('/images/')[0]
> I think that using os.path.split() three times is cleaner. It may deserve i
Done
Line 343: 
Line 344:     def _teardownVolumes(self):
Line 345:         try:
Line 346:             drive = {'device': 'disk',


Line 348:                      'domainID': self._vmProperties['domainID']}
Line 349:             for disk in self._vmProperties['disks']:
Line 350:                 drive['imageID'] = disk['imageID']
Line 351:                 drive['volumeID'] = disk['volumeID']
Line 352:                 self._cif.teardownVolumePath(drive)
> please use _cif.irs.teardownImage()
Done
Line 353:         except Exception as e:
Line 354:             logging.error('error while trying to teardownVolumePath: %r', e)
Line 355: 
Line 356: 


-- 
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: 9
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