Antoni Segura Puimedon has posted comments on this change. Change subject: bugfix: UpdateVmDevice QoS ......................................................................
Patch Set 3: Code-Review-1 (4 comments) Some small changes. .................................................... File vdsm/vm.py Line 1309: return iface Line 1310: Line 1311: def getXMLBandwidth(self, specParams, old_bandwidth=None): Line 1312: bandwidth = self.createXmlElem('bandwidth', None) Line 1313: # Inbound and Outbound traffic can be indipendently shaped. s/indipendently/independently/ Line 1314: for attr in ('inbound', 'outbound'): Line 1315: new_settings = self.specParams.get(attr) Line 1316: if old_bandwidth is not None: Line 1317: attrXMLs = old_bandwidth.getElementsByTagName Line 1311: def getXMLBandwidth(self, specParams, old_bandwidth=None): Line 1312: bandwidth = self.createXmlElem('bandwidth', None) Line 1313: # Inbound and Outbound traffic can be indipendently shaped. Line 1314: for attr in ('inbound', 'outbound'): Line 1315: new_settings = self.specParams.get(attr) vdsm convention is for variables to be camel cased. Since this is inside a loop (only holds a setting at a time) I'd call it newSetting. Line 1316: if old_bandwidth is not None: Line 1317: attrXMLs = old_bandwidth.getElementsByTagName Line 1318: attrXML = attrXMLs[0] if len(attrXMLs) else None Line 1319: else: Line 1312: bandwidth = self.createXmlElem('bandwidth', None) Line 1313: # Inbound and Outbound traffic can be indipendently shaped. Line 1314: for attr in ('inbound', 'outbound'): Line 1315: new_settings = self.specParams.get(attr) Line 1316: if old_bandwidth is not None: I think I would first check if there are newSettings and in that case don't fetch attrXML (which I would leave for the else of "if newSettings". Line 1317: attrXMLs = old_bandwidth.getElementsByTagName Line 1318: attrXML = attrXMLs[0] if len(attrXMLs) else None Line 1319: else: Line 1320: attrXML = None Line 3133: except IndexError: Line 3134: link = xml.dom.minidom.Element('link') Line 3135: vnicXML.appendChildWithArgs(link) Line 3136: link.setAttribute('state', linkValue) Line 3137: if specParams and \ Instead of '\' it is preferred for the whole condition to be encased in parenthesis. Line 3138: ('inbound' in specParams or 'outbound' in specParams): Line 3139: old_bandwidths = vnicXML.getElementsByTagName('bandwidth') Line 3140: old_bandwidth = old_bandwidths[0] if len(old_bandwidths) else None Line 3141: new_bandwidth = dev.getXMLBandwidth(specParams, old_bandwidth) -- 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: 3 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: Petr Šebek <pse...@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