Nir Soffer has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm ......................................................................
Patch Set 11: (1 comment) https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1723: devices = self._devices Line 1724: vmdevices.network.Interface.update_device_info( Line 1725: self, devices[hwclass.NIC]) Line 1726: vmdevices.storage.Drive.update_device_info( Line 1727: self, devices[hwclass.DISK]) > Why don't you like them? Because they are class methods or for another reas I don't like them because updating the conf/devices from xml should not be the responsibility of the Device class. Classes should be responsible for creating instances. Also in this design the code for handling the xml is spread all over the place, instead of one module handling this task. Line 1728: vmdevices.core.Sound.update_device_info( Line 1729: self, devices[hwclass.SOUND]) Line 1730: vmdevices.core.Video.update_device_info( Line 1731: self, devices[hwclass.VIDEO]) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@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
