Milan Zamazal has posted comments on this change.

Change subject: hostdev: use specific device classes in HostDevice
......................................................................


Patch Set 8:

(2 comments)

https://gerrit.ovirt.org/#/c/57963/8/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 320:     @classmethod
Line 321:     def update_device_info(cls, vm, device_conf):
Line 322:         for device_xml in vm.domain.get_device_elements('hostdev'):
Line 323:             device_type = device_xml.getAttribute('type')
Line 324:             cls._DEVICE_MAPPING[device_type].update_device_info(
As I've stated in a preceding patch, the called update_device_info should be 
renamed. Let's not forget about it here.


PS8, Line 320:     @classmethod
             :     def update_device_info(cls, vm, device_conf):
             :         for device_xml in 
vm.domain.get_device_elements('hostdev'):
             :             device_type = device_xml.getAttribute('type')
             :             cls._DEVICE_MAPPING[device_type].update_device_info(
             :                 vm, device_conf, device_xml)
> That is slightly confusing. I don't understand how we'd create the device c
I also prefer not having special cases when creating device classes. Using 
__new__ here is completely fine from my point of view.


-- 
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: 8
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]

Reply via email to