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 <pse...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to