Dan Kenigsberg has posted comments on this change.
Change subject: QoS host networks: caps now reports QoS for networks.
......................................................................
Patch Set 4: (3 inline comments)
....................................................
File lib/vdsm/netinfo.py
Line 53: DEFAULT_MTU = '1500'
Line 54:
Line 55: REQUIRED_BONDINGS = frozenset(('bond0', 'bond1', 'bond2', 'bond3',
'bond4'))
Line 56:
Line 57: Qos = namedtuple('Qos', 'inbound outbound')
I'm reluctant to expose something prematurely. If we see that there's somewhere
else in vdsm that needs that type, we can always expose it then.
At the moment, Qos is only the output type of a function that I wish to replace
by net.XMLDesc(0), so I am not at all sure it is needed even in its private use
case.
Line 58:
Line 59:
Line 60: def _match_name(name, patterns):
Line 61: return any(map(lambda p: fnmatch(name, p), patterns))
Line 159: """
Line 160: nets = {}
Line 161: conn = libvirtconnection.get()
Line 162: allNets = ((net, net.name()) for net in conn.listAllNetworks(0))
Line 163: for net, netname in allNets:
the double (or should I say O(len(networks))) calls to libvirtconnect.get() is
not pretty, but worse is the O(n) calls to networkLookupByName(). The latter is
known to be slow when there are multiple networks defined.
Is there any reason not to use my suggestion, of collecting the netxml from the
net objects as we iterate them?
Line 164: if netname.startswith(LIBVIRT_NET_PREFIX):
Line 165: netname = netname[len(LIBVIRT_NET_PREFIX):]
Line 166: nets[netname] = {}
Line 167: xml = minidom.parseString(net.XMLDesc(0))
Line 447: def _getNetInfo(network, iface, bridged, routes, ipv6routes):
Line 448: '''Returns a dictionary of properties about the network's
interface status.
Line 449: Raises a KeyError if the iface does not exist.'''
Line 450: data = {}
Line 451:
So I'm not sure why you're adding the newline in this patch, and not why it's
not directly following the docstring.
Line 452: try:
Line 453: if bridged:
Line 454: data.update({'ports': ports(iface), 'stp':
bridge_stp_state(iface),
Line 455: 'cfg': getIfaceCfg(iface)})
--
To view, visit http://gerrit.ovirt.org/17218
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b9dde8c580b6087ada29bc3b0d682ed638d92f9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli <gvall...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <giuse...@openquake.org>
Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Petr Šebek <pse...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
