Martin Polednik has posted comments on this change. Change subject: hostdev: use specific device classes in HostDevice ......................................................................
Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/57963/7/vdsm/virt/vmdevices/hostdevice.py File vdsm/virt/vmdevices/hostdevice.py: PS7, Line 314: def __new__(cls, conf, log, **kwargs): : device_params = get_device_params(kwargs['device']) : device = cls._DEVICE_MAPPING[ : device_params['capability']](conf, log, **kwargs) : return device > This is a massive improvement over the delegation approach. I'm working with 2 conditions: 1) no changes in device creation code and 2) no interface-like object. To satisfy 1), we must be able to construct the proper device object from within HostDevice object. Using a more java-ish approach, except for being just bad, is therefore not possible. Same for traditional inheritance. On top of that, if we add 2) to the mix, we are out of possible composition (the previous version with delegation). I'm still trying to figure out a way of not creating too clever code - that is the reason for this patch series, after all. I like from_device_params idea, but as of now it would create additional complexity instead of reducing it. Devices are created in _devMapFromDevSpecMap with following code: dev_map[dev_type].append(dev_class(self.conf, self.log, **dev)) It can be argued that the code itself is a bit too clever at the moment. To avoid creating a device specific exception, I still prefer __new__. We could discuss adding similar API to other devices and using the added API to initialize the object. Such approach assumes that there is nontrivial benefit of such change for any non-host device. -- To view, visit https://gerrit.ovirt.org/57963 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2734b7ec789c0ce57b64d7699cbe967c774bb608 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
