Change in vdsm[master]: v2v: extract specific classes for libvirt and ova

shavivi at redhat.com shavivi at redhat.com
Wed Dec 23 12:47:57 UTC 2015


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 <shavivi at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list