Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm

mzamazal at redhat.com mzamazal at redhat.com
Tue Mar 22 08:34:35 UTC 2016


Milan Zamazal 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])
> I don't like them because updating the conf/devices from xml should not be 
> I don't like them because updating the conf/devices from xml should not be the responsibility of the Device class.

I see.  I think it's desirable to group code by devices.  Most device classes just create and process domain XML and it wouldn't be good to perform those two transformations in separate places.  Properties of a given device should be handled in a single place, as a form of encapsulation.

> Also in this design the code for handling the xml is spread all over the place, instead of one module handling this task.

We should generally improve XML handling, it's mess.  If we can do that then this shouldn't be a problem.
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 <mzamazal at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list