Francesco Romani has posted comments on this change.

Change subject: hostdev: move pci device code to separate class
......................................................................


Patch Set 9: Code-Review-1

(1 comment)

actually -1 for visibility, attaching the comment to this patch even though is 
arbitrary (please check inline comment) :\

https://gerrit.ovirt.org/#/c/57956/9/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

PS9, Line 290:         known_device = False
             :         for dev in vm.conf['devices']:
             :             if dev['device'] == device:
             :                 dev['alias'] = alias
             :                 dev['address'] = address
             : 
             :         for dev in device_conf:
             :             if dev.device == device:
             :                 dev.alias = alias
             :                 dev.address = address
             :                 known_device = True
not related to this patch, rather than the series:
please keep the order consistent across the HostDevice classes (PciDevice, 
UsbDevice...)
e.g. always update first device_conf than vm.conf or the other way around. Any 
ordering is fine as long as is consistent. I prefer device_conf, vm.conf but I 
don't really mind.


-- 
To view, visit https://gerrit.ovirt.org/57956
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb93c2434ff827406bbf4ee0af30f5726f6af
Gerrit-PatchSet: 9
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 Betak <[email protected]>
Gerrit-Reviewer: Martin Polednik <[email protected]>
Gerrit-Reviewer: Milan Zamazal <[email protected]>
Gerrit-Reviewer: Tomas Golembiovsky <[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