Change in vdsm[master]: virt: factor out _getUnderlying* methods to another class.

danken at redhat.com danken at redhat.com
Fri Mar 13 13:37:04 UTC 2015


Dan Kenigsberg has posted comments on this change.

Change subject: virt: factor out _getUnderlying* methods to another class.
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/38704/2//COMMIT_MSG
Commit Message:

Line 19: a. Add unit tests for this class.
Line 20: b. The technique in which getUnderlyingVmDevicesInfo changes the 'conf'
Line 21:    reference instead of returning data is cumbersome and hard to read. It
Line 22:    should return an updated copy of of the conf dictionary to the caller.
Line 23:    it will ease testing and debugging.
Right.

However, the price of having a new object with no state of its own is quite hight (architecutre-wise).

I would prefer to see a function that eats a parsed xml and converts it back to conf-style list of dicts.

Another function can update the VM.conf with information extracted from xml.
Line 24: 
Line 25: Change-Id: Ia04b6ee17da207f772f77059e45be97594e6be78


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia04b6ee17da207f772f77059e45be97594e6be78
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list