Change in vdsm[master]: virt: Switch from minidom to etree in DomainDescriptor

mzamazal at redhat.com mzamazal at redhat.com
Fri Jan 22 10:33:28 UTC 2016


Milan Zamazal has posted comments on this change.

Change subject: virt: Switch from minidom to etree in DomainDescriptor
......................................................................


Patch Set 2:

(1 comment)

The size of the patch worries me too as it makes checking the code a bit more difficult to reviewers and they are not going to be happy about that.  I think we could reduce it by changing the order of the initial steps: 

1. Define helper methods for common XML patterns.
2. Actually use the helper methods for XML processing in Vm.py.  This can be made as a series of patches, split by device groups or by teams (to not be overly obsessive about the splitting).
3. Switch to etree by changing the helper methods.

The question is where to define the XML helper methods, especially in order to avoid their duplication.  Hopefully they could fit to DomainDescriptor.

https://gerrit.ovirt.org/#/c/51034/2/tests/vmOperationsTests.py
File tests/vmOperationsTests.py:

Line 128: 
Line 129:     def testUpdateSingleDeviceGraphics(self):
Line 130:         devXmls = (
Line 131:             '<graphics connected="disconnect" passwd="12345678"'
Line 132:             ' port="5900" type="spice" />',
> no big deal, but why is this needed?
etree inserts the space so it must be adjusted accordingly to make the tests pass.
Line 133:             '<graphics passwd="12345678" port="5900" type="vnc" />')
Line 134:         for device, devXml in zip(self.GRAPHIC_DEVICES, devXmls):
Line 135:             domXml = '''
Line 136:                 <devices>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4185f3b46e8034c594bd28af71bbcb1978039a15
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list