Dan Kenigsberg has posted comments on this change.
Change subject: Qos feature: Adds bandwidth definition at configuration level.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(7 inline comments)
Thanks.
....................................................
File tests/netconfTests.py
Line 76: Compare two xml artifacts for equality.
Line 77: """
Line 78: contents_a = ElementTree.tostring(ElementTree.fromstring(a))
Line 79: contents_b = ElementTree.tostring(ElementTree.fromstring(b))
Line 80: return contents_a == contents_b
Assert methods should raise, not return.
You could use assertEqual. (And please add a msg arg for conformity.)
Line 81:
Line 82: def setUp(self):
Line 83: self.cw = ifcfg.ConfigWriter()
Line 84:
Line 79: contents_b = ElementTree.tostring(ElementTree.fromstring(b))
Line 80: return contents_a == contents_b
Line 81:
Line 82: def setUp(self):
Line 83: self.cw = ifcfg.ConfigWriter()
If you make this a data member, make it private _cw.
Line 84:
Line 85: @MonkeyPatch(subprocess, 'Popen', lambda x: None)
Line 86: def testAtomicRestore(self):
Line 87: self._createFiles()
Line 138: actualDoc = self.cw.createNetXml(netName, network, True, None)
Line 139:
Line 140: self.assertEqualXml(expectedDoc, actualDoc)
Line 141:
Line 142: def testCreateNetXml(self):
Can any user run this with no monkey-patching?
Line 143: netName = "awesome_net"
Line 144: iface = "dummy"
Line 145: expectedDoc = ("""<network>
Line 146: <name>%s</name>
Line 146: <name>%s</name>
Line 147: <forward mode='passthrough'/>
Line 148: <interface dev='%s'/>
Line 149: </network>""" % (netName, iface))
Line 150: actualDoc = self.cw.createNetXml(netName, None, False, iface)
You should clean after your "create"s with "restore".
Line 151:
Line 152: self.assertEqualXml(expectedDoc, actualDoc)
Line 153:
Line 154: def testCreateNetXmlBridgedQos(self):
....................................................
File vdsm_api/vdsmapi-schema.json
Line 71: ##
Line 72: {'type': 'NetworkOptions',
Line 73: 'data': {'*ipaddr': 'str', '*netmask': 'str', '*gateway': 'str',
Line 74: '*bootproto': 'str', '*delay': 'uint', '*onboot': 'str',
Line 75: '*bondingOptions', 'str', '*qosInbound':
'VmInterfaceDeviceBandwidthParams',
Too long line.
Line 76: '*qosOutbound': 'VmInterfaceDeviceBandwidthParams'}}
Line 77:
Line 78: ##
Line 79: # @Host.addNetwork:
Line 79: # @Host.addNetwork:
Line 80: #
Line 81: # Add a new network to this host.
Line 82: #
Line 83: # @bridge: The name of the bridge device
Why are these indentation changes happening now?
Line 84: #
Line 85: # @vlan: #optional The name of a VLAN to create on the device
Line 86: #
Line 87: # @bond: #optional The name of a bond device to create from
@nics
Line 167: # @onboot: #optional Start the network device automatically
during boot
Line 168: #
Line 169: # @remove: #optional If True, remove existing network only
Line 170: #
Line 171: # @qosInbound: #optional VmInterfaceDeviceBandwidthParams for
incoming traffic.
VmInterface...Bandwidth should probably be renamed, as it now affects networks,
too.
Line 172: #
Line 173: # @qosOutbound: #optional VmInterfaceDeviceBandwidthParams for
outgoing traffic.
Line 174: #
Line 175: # Since: 4.10.0
--
To view, visit http://gerrit.ovirt.org/15724
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c3b4b9a6fd5eb53579fb2e157b5325caa88d04
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
