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