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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
