Antoni Segura Puimedon has uploaded a new change for review.
Change subject: Simplify netinfo get. ......................................................................
Simplify netinfo get.
This patch refactors netinfo to have the network data fetching on a separate function. This reduces the get() complexity and will allow the possibility of reusing the new method for more fine- grained information retrieval, e.g., update just a network info of a Netinfo instance.
Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/netinfo.py 1 file changed, 26 insertions(+), 45 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/80/11080/1
diff --git a/vdsm/netinfo.py b/vdsm/netinfo.py index f6250c2..e1fd798 100644 --- a/vdsm/netinfo.py +++ b/vdsm/netinfo.py @@ -274,57 +274,38 @@ return paddr
+def getNetData(net, bridged, routes): + try: + if bridged: + return {'ports': ports(net), 'stp': bridge_stp_state(net), + 'cfg': getIfaceCfg(net)} + else: + # ovirt-engine-3.1 expects to see the "interface" attribute iff the + # network is bridgeless. Please remove the attribute and this + # comment when the version is no longer supported. + return {'interface': net, 'iface': net, 'bridged': False, + 'addr': getaddr(net), 'netmask': getnetmask(net), + 'gateway': routes.get(net, '0.0.0.0'), 'mtu': getMtu(net)} + except OSError as e: + if e.errno == errno.ENOENT: + logging.info('Obtaining info for net %s.', net, exc_info=True) + raise KeyError('Network %s was not found') + else: + raise + + def get(): d = {} routes = getRoutes() d['networks'] = {} - nets = networks()
- for netname in nets.iterkeys(): - if nets[netname]['bridged']: - devname = netname - try: - d['networks'][netname] = {'ports': ports(devname), - 'stp': bridge_stp_state(devname), - 'cfg': getIfaceCfg(devname), } - except OSError as e: - # If the bridge reported by libvirt does not exist anymore, do - # not report it, as this already assures that the bridge is not - # added to d['networks'] - if e.errno == errno.ENOENT: - logging.info('Obtaining info for net %s.', devname, - exc_info=True) - continue - else: - raise - else: - devname = nets[netname]['iface'] - d['networks'][netname] = {} - - # ovirt-engine-3.1 expects to see "interface" iff the network is - # bridgeless. Please remove this line when that Engine version is - # no longer supported - d['networks'][netname]['interface'] = devname - + for net, netAttr in networks().iteritems(): try: - d['networks'][netname].update( - {'iface': devname, - 'bridged': nets[netname]['bridged'], - 'addr': getaddr(devname), - 'netmask': getnetmask(devname), - 'gateway': routes.get(devname, '0.0.0.0'), - 'mtu': getMtu(devname), }) - except OSError as e: - if e.errno == errno.ENOENT: - logging.info('Obtaining info for net %s.', devname, - exc_info=True) - # When a net is not bridged it will have left an entry in - # d['networks'] - if netname in d['networks']: - del d['networks'][netname] - continue - else: - raise + d['networks'][net] = getNetData(net, netAttr['bridged'], routes) + except KeyError: + continue + except OSError: + raise
d['bridges'] = dict([(bridge, {'ports': ports(bridge), 'stp': bridge_stp_state(bridge),
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/777/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 1:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/742/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/778/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 2:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/743/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/743/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/778/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/755/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/790/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/755/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/790/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm/netinfo.py Line 283: else: Line 284: # ovirt-engine-3.1 expects to see the "interface" attribute iff the Line 285: # network is bridgeless. Please remove the attribute and this Line 286: # comment when the version is no longer supported. Line 287: data['interface'] = net the 'interface' attribute represents the actual network interface name, not the libvirt network name. For bridged network, they're the same if ignoring the prefix 'vdsm-'. But for bridgeless network, it's different. So I think it's wrong. The same problem exists in line 288, 'iface' for bridgeless network. Line 288: data.update({'iface': net, 'bridged': False, Line 289: 'addr': getaddr(net), 'netmask': getnetmask(net), Line 290: 'gateway': routes.get(net, '0.0.0.0'), Line 291: 'mtu': getMtu(net)})
Line 284: # ovirt-engine-3.1 expects to see the "interface" attribute iff the Line 285: # network is bridgeless. Please remove the attribute and this Line 286: # comment when the version is no longer supported. Line 287: data['interface'] = net Line 288: data.update({'iface': net, 'bridged': False, why is 'bridged' also False for bridged network? Line 289: 'addr': getaddr(net), 'netmask': getnetmask(net), Line 290: 'gateway': routes.get(net, '0.0.0.0'), Line 291: 'mtu': getMtu(net)}) Line 292: return data
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3: (1 inline comment)
.................................................... File vdsm/netinfo.py Line 292: return data Line 293: except OSError as e: Line 294: if e.errno == errno.ENOENT: Line 295: logging.info('Obtaining info for net %s.', net, exc_info=True) Line 296: raise KeyError('Network %s was not found') It seems that we don't we need handle the exception here. Instead, we can just leave it to the caller, get(). Then you needn't consider to re-raise the exception. Line 297: else: Line 298: raise Line 299: Line 300:
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3: No score
(1 inline comment)
.................................................... File vdsm/netinfo.py Line 292: return data Line 293: except OSError as e: Line 294: if e.errno == errno.ENOENT: Line 295: logging.info('Obtaining info for net %s.', net, exc_info=True) Line 296: raise KeyError('Network %s was not found') Now I see why you handle the exception here. I am fine with it. Line 297: else: Line 298: raise Line 299: Line 300:
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/784/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 4:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/819/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Antoni Segura Puimedon has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 3: (2 inline comments)
.................................................... File vdsm/netinfo.py Line 283: else: Line 284: # ovirt-engine-3.1 expects to see the "interface" attribute iff the Line 285: # network is bridgeless. Please remove the attribute and this Line 286: # comment when the version is no longer supported. Line 287: data['interface'] = net Good catch. I'll fix both. Line 288: data.update({'iface': net, 'bridged': False, Line 289: 'addr': getaddr(net), 'netmask': getnetmask(net), Line 290: 'gateway': routes.get(net, '0.0.0.0'), Line 291: 'mtu': getMtu(net)})
Line 284: # ovirt-engine-3.1 expects to see the "interface" attribute iff the Line 285: # network is bridgeless. Please remove the attribute and this Line 286: # comment when the version is no longer supported. Line 287: data['interface'] = net Line 288: data.update({'iface': net, 'bridged': False, My stupid way of moving around the code from previous version of the patch. I'll fix it too. Line 289: 'addr': getaddr(net), 'netmask': getnetmask(net), Line 290: 'gateway': routes.get(net, '0.0.0.0'), Line 291: 'mtu': getMtu(net)}) Line 292: return data
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/784/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/819/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/820/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 5:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/785/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/785/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/820/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/821/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/786/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/786/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/821/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/822/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/787/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 7:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/787/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/822/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/824/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/789/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 8:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/789/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/824/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 8: I would prefer that you didn't submit this
(2 inline comments)
very minor comments, the split makes sense.
.................................................... File vdsm/netinfo.py Line 273: paddr[slave] = addr.upper() Line 274: return paddr Line 275: Line 276: Line 277: def getNetData(iface, bridged, routes): is this declared public intentionally? Line 278: data = {} Line 279: try: Line 280: if bridged: Line 281: data.update({'ports': ports(iface), 'stp': bridge_stp_state(iface),
Line 288: data.update({'iface': iface, 'bridged': bridged, Line 289: 'addr': getaddr(iface), 'netmask': getnetmask(iface), Line 290: 'gateway': routes.get(iface, '0.0.0.0'), Line 291: 'mtu': getMtu(iface)}) Line 292: return data nit: "return" does not raise, so it should be out of the try-block. Line 293: except OSError as e: Line 294: if e.errno == errno.ENOENT: Line 295: logging.info('Obtaining info for net %s.', iface, exc_info=True) Line 296: raise KeyError('Network %s was not found')
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Antoni Segura Puimedon has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 8: (2 inline comments)
.................................................... File vdsm/netinfo.py Line 273: paddr[slave] = addr.upper() Line 274: return paddr Line 275: Line 276: Line 277: def getNetData(iface, bridged, routes): More or less. I think it could be useful generally. But I don't have anything against prefixing it with an underscore. Line 278: data = {} Line 279: try: Line 280: if bridged: Line 281: data.update({'ports': ports(iface), 'stp': bridge_stp_state(iface),
Line 288: data.update({'iface': iface, 'bridged': bridged, Line 289: 'addr': getaddr(iface), 'netmask': getnetmask(iface), Line 290: 'gateway': routes.get(iface, '0.0.0.0'), Line 291: 'mtu': getMtu(iface)}) Line 292: return data Done Line 293: except OSError as e: Line 294: if e.errno == errno.ENOENT: Line 295: logging.info('Obtaining info for net %s.', iface, exc_info=True) Line 296: raise KeyError('Network %s was not found')
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/831/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/796/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Antoni Segura Puimedon has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9: (1 inline comment)
.................................................... File vdsm/netinfo.py Line 273: paddr[slave] = addr.upper() Line 274: return paddr Line 275: Line 276: Line 277: def _getNetInfo(iface, bridged, routes): I finally decided that I don't want to marry with the name as a public thing just yet. As there is too much refactoring going on. Thanks for the tip Dan. Line 278: '''Returns a dictionary of properties about the network's interface status. Line 279: Raises a KeyError if the iface does not exist.''' Line 280: data = {} Line 281: try:
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/796/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/831/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/netinfo.py Line 293: 'mtu': getMtu(iface)}) Line 294: except OSError as e: Line 295: if e.errno == errno.ENOENT: Line 296: logging.info('Obtaining info for net %s.', iface, exc_info=True) Line 297: raise KeyError('Network %s was not found') what about second part of %s ? Line 298: else: Line 299: raise Line 300: return data Line 301:
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9: (1 inline comment)
.................................................... Commit Message Line 9: This patch refactors netinfo to have the network data fetching on Line 10: a separate function. This reduces the get() complexity and will Line 11: allow the possibility of reusing the new method for more fine- Line 12: grained information retrieval, e.g., update just a network info Line 13: of a Netinfo instance. Hmmm, if your new function is not public anymore, you not really can to use it outside of get(), but if will you will wrap it ... Line 14: Line 15: Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Antoni Segura Puimedon has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 9: (2 inline comments)
.................................................... Commit Message Line 9: This patch refactors netinfo to have the network data fetching on Line 10: a separate function. This reduces the get() complexity and will Line 11: allow the possibility of reusing the new method for more fine- Line 12: grained information retrieval, e.g., update just a network info Line 13: of a Netinfo instance. I'm working on a wrapper in a future patch that exposes the functionality in a nicer way? Line 14: Line 15: Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c
.................................................... File vdsm/netinfo.py Line 293: 'mtu': getMtu(iface)}) Line 294: except OSError as e: Line 295: if e.errno == errno.ENOENT: Line 296: logging.info('Obtaining info for net %s.', iface, exc_info=True) Line 297: raise KeyError('Network %s was not found') Ooops. Right. I'll fix that. Thanks! Line 298: else: Line 299: raise Line 300: return data Line 301:
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/825/ (1/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/860/ (2/2)
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/825/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/860/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10: Looks good to me, but someone else must approve
(2 inline comments)
Just minor logging messages issue inline.
.................................................... File vdsm/netinfo.py Line 292: 'gateway': routes.get(iface, '0.0.0.0'), Line 293: 'mtu': getMtu(iface)}) Line 294: except OSError as e: Line 295: if e.errno == errno.ENOENT: Line 296: logging.info('Obtaining info for net %s.', iface, exc_info=True) 'interface' could be more accurate than 'net' here. Line 297: raise KeyError('Network %s was not found' % iface) Line 298: else: Line 299: raise Line 300: return data
Line 293: 'mtu': getMtu(iface)}) Line 294: except OSError as e: Line 295: if e.errno == errno.ENOENT: Line 296: logging.info('Obtaining info for net %s.', iface, exc_info=True) Line 297: raise KeyError('Network %s was not found' % iface) The same problem for 'Network'. Line 298: else: Line 299: raise Line 300: return data Line 301:
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10: Looks good to me, but someone else must approve
(1 inline comment)
Toni, please verify it carefully
.................................................... File vdsm/netinfo.py Line 282: if bridged: Line 283: data.update({'ports': ports(iface), 'stp': bridge_stp_state(iface), Line 284: 'cfg': getIfaceCfg(iface)}) Line 285: else: Line 286: # ovirt-engine-3.1 expects to see the "interface" attribute iff the s/iff/if Line 287: # network is bridgeless. Please remove the attribute and this Line 288: # comment when the version is no longer supported. Line 289: data['interface'] = iface Line 290: data.update({'iface': iface, 'bridged': bridged,
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Antoni Segura Puimedon has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10: (2 inline comments)
.................................................... File vdsm/netinfo.py Line 282: if bridged: Line 283: data.update({'ports': ports(iface), 'stp': bridge_stp_state(iface), Line 284: 'cfg': getIfaceCfg(iface)}) Line 285: else: Line 286: # ovirt-engine-3.1 expects to see the "interface" attribute iff the I think this comment is consistent with the code. http://en.wikipedia.org/wiki/If_and_only_if#Origin_of_iff If we would change it to just "if" it would mean that interface could also be put to the bridged case and the else could be dropped. Line 287: # network is bridgeless. Please remove the attribute and this Line 288: # comment when the version is no longer supported. Line 289: data['interface'] = iface Line 290: data.update({'iface': iface, 'bridged': bridged,
Line 292: 'gateway': routes.get(iface, '0.0.0.0'), Line 293: 'mtu': getMtu(iface)}) Line 294: except OSError as e: Line 295: if e.errno == errno.ENOENT: Line 296: logging.info('Obtaining info for net %s.', iface, exc_info=True) It would be more accurate. My only concert is: Should I also rename the method to "_getIfaceInfo"? Line 297: raise KeyError('Network %s was not found' % iface) Line 298: else: Line 299: raise Line 300: return data
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Simplify netinfo get. ......................................................................
Patch Set 10: Verified
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Simplify netinfo get. ......................................................................
Simplify netinfo get.
This patch refactors netinfo to have the network data fetching on a separate function. This reduces the get() complexity and will allow the possibility of reusing the new method (via a future patch wrapping of it) for more fine-grained information retrieval, e.g., update just a network info of a Netinfo instance.
Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/netinfo.py 1 file changed, 31 insertions(+), 45 deletions(-)
Approvals: Mark Wu: Looks good to me, but someone else must approve Dan Kenigsberg: Verified; Looks good to me, approved Igor Lvovsky: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/11080 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I5c3e617d10308d0bfa4deaed329b2e10ef54077c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org