Shahar Havivi has posted comments on this change. Change subject: v2v: extract specific classes for libvirt and ova ......................................................................
Patch Set 7: (5 comments) https://gerrit.ovirt.org/#/c/49951/7/vdsm/v2v.py File vdsm/v2v.py: Line 284: self._uri = uri Line 285: self._vminfo = vminfo Line 286: self._vmid = vmid Line 287: self._irs = irs Line 288: > Remove the blank line Done Line 289: self._prepared_volumes = [] Line 290: Line 291: def execute(self): Line 292: """ Line 290: Line 291: def execute(self): Line 292: """ Line 293: implement in subclasses Line 294: """ > To make it clear that this is abstract class, and subclass must implement t Done Line 295: Line 296: def _get_disk_format(self): Line 297: fmt = self._vminfo.get('format', 'raw').lower() Line 298: if fmt == 'cow': Line 296: def _get_disk_format(self): Line 297: fmt = self._vminfo.get('format', 'raw').lower() Line 298: if fmt == 'cow': Line 299: return 'qcow2' Line 300: return fmt > Can simplified to: Done Line 301: Line 302: def _generate_disk_parameters(self): Line 303: parameters = [] Line 304: for disk in self._vminfo['disks']: Line 298: if fmt == 'cow': Line 299: return 'qcow2' Line 300: return fmt Line 301: Line 302: def _generate_disk_parameters(self): > I would call it _disk_parameters() Done Line 303: parameters = [] Line 304: for disk in self._vminfo['disks']: Line 305: try: Line 306: parameters.append('--vdsm-image-uuid') Line 312: % (self._vmid, e)) Line 313: return parameters Line 314: Line 315: @contextmanager Line 316: def _volumes_handler(self): > Why _handler? Why not call it _volumes()? Done Line 317: self._prepare_volumes() Line 318: try: Line 319: yield Line 320: finally: -- To view, visit https://gerrit.ovirt.org/49951 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a9ecd4a2cde6f379188da647c3a6f8874c41abd Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches