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