Dan Kenigsberg has posted comments on this change.
Change subject: bugfix: UpdateVmDevice QoS
......................................................................
Patch Set 1: Code-Review-1
(3 comments)
....................................................
File vdsm/vm.py
Line 1302: if self.sndbufParam:
Line 1303: tune = iface.appendChildWithArgs('tune')
Line 1304: tune.appendChildWithArgs('sndbuf', text=self.sndbufParam)
Line 1305:
Line 1306: if hasattr(self, 'specParams'):
how about taking this code into a helper function, which would be called both from here
and from setLinkAndNetwork ?
Line 1307: if 'inbound' in self.specParams or 'outbound' in
self.specParams:
Line 1308: bandwidth = self.createXmlElem('bandwidth', None)
Line 1309: # Inbound and Outbound traffic can be indipendently shaped.
Line 1310: inbound = self.specParams.get('inbound')
Line 3113:
Line 3114: @contextmanager
Line 3115: def setLinkAndNetwork(self, dev, conf, linkValue, networkValue, custom,
Line 3116: specParams):
Line 3117: def updateXMLWithQoS(vnicXML, qos):
"qos" is a misleading argument name, as this is actually the specParam
dictionary, containing much more than that.
Line 3118: bandwidth = vnicXML.getElementsByTagName('bandwidth')[0]
Line 3119: inbound = bandwidth.getElementsByTagName('inbound')[0]
Line 3120: outbound = bandwidth.getElementsByTagName('outbound')[0]
Line 3121: if 'inbound' in qos:
Line 3114: @contextmanager
Line 3115: def setLinkAndNetwork(self, dev, conf, linkValue, networkValue, custom,
Line 3116: specParams):
Line 3117: def updateXMLWithQoS(vnicXML, qos):
Line 3118: bandwidth = vnicXML.getElementsByTagName('bandwidth')[0]
the QoS elements may be missing (e.g. if VM was migrated from an old vdsm). You should
handle that case, too. As well of having only one of the inbound/outbound elements below
it.
Line 3119: inbound = bandwidth.getElementsByTagName('inbound')[0]
Line 3120: outbound = bandwidth.getElementsByTagName('outbound')[0]
Line 3121: if 'inbound' in qos:
Line 3122: bandwidth.remove(inbound)
--
To view, visit
http://gerrit.ovirt.org/19545
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I43a383b2a9cf96366927beebf63f1344027169fb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Ĺ ebek <psebek(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes