Change in vdsm[master]: ipwrapper: Make Link.flags official as they are used for RUN...

Ondřej Svoboda has uploaded a new change for review.

Change subject: ipwrapper: Make Link.flags official as they are used for RUNNING and
PROMISC
......................................................................

ipwrapper: Make Link.flags official as they are used for RUNNING and PROMISC

Since 'flags' come from libnl as an integer it makes no sense to populate
them with None though. The two said flags are used in Link's own methods and
cannot be derived from 'state' so 'flags' deserve their own attribute.

Please note that 'flags' can also be a list like ['BROADCAST',
'MULTICAST']
which should be avoided in the future by only using libnl and not parsing
iproute2's output.

pylint2 is now silent for ipwrapper.

Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
---
M lib/vdsm/ipwrapper.py
1 file changed, 3 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/36621/1

diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py
index 31affaa..0a1d78d 100644
--- a/lib/vdsm/ipwrapper.py
+++ b/lib/vdsm/ipwrapper.py
@@ -121,7 +121,7 @@
 
     def __init__(self, address, index, linkType, mtu, name, qdisc, state,
                  vlanid=None, vlanprotocol=None, master=None, device=None,
-                 **kwargs):
+                 flags=None, **kwargs):
         self.address = address
         self.index = index
         self.type = linkType
@@ -136,6 +136,8 @@
             self.vlanprotocol = vlanprotocol
         if device is not None:
             self.device = device
+        if flags is not None:
+            self.flags = flags
         for key, value in kwargs.items():
             setattr(self, key, value)
 


-- 
To view, visit http://gerrit.ovirt.org/36621
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: ipwrapper: Make Link.flags official as they are used for RUNNING and
PROMISC
......................................................................


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit http://gerrit.ovirt.org/36621
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Ondřej Svoboda has posted comments on this change.

Change subject: ipwrapper: Make Link.flags official as they are used for RUNNING and
PROMISC
......................................................................


Patch Set 1: Verified+1

(1 comment)

Static verification for now.

http://gerrit.ovirt.org/#/c/36621/1/lib/vdsm/ipwrapper.py
File lib/vdsm/ipwrapper.py:

Line 136:             self.vlanprotocol = vlanprotocol
Line 137:         if device is not None:
Line 138:             self.device = device
Line 139:         if flags is not None:
Line 140:             self.flags = flags
Because this is a functional change (previously flags could be None) a verification seems
necessary.

Here goes: the only users of .flags (there are only a few occurences of .flags in git
grep) reside in the same class (Link) and use it unconditionally as an integer, e.g.:

    @property
    def oper_up(self):
        return bool(self.flags & self.IFF_RUNNING)
Line 141:         for key, value in kwargs.items():
Line 142:             setattr(self, key, value)
Line 143: 
Line 144:     def __repr__(self):


-- 
To view, visit http://gerrit.ovirt.org/36621
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: ipwrapper: Make Link.flags official as they are used for RUNNING and
PROMISC
......................................................................


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14810/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14642/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13854/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/36621
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: ipwrapper: Make Link.flags official as they are used for RUNNING and
PROMISC
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/36621/1/lib/vdsm/ipwrapper.py
File lib/vdsm/ipwrapper.py:

Line 118:     _hiddenVlans = config.get('vars',
'hidden_vlans').split(',')
Line 119:     IFF_RUNNING = 1 << 6
Line 120:     IFF_PROMISC = 1 << 8
Line 121: 
Line 122:     def __init__(self, address, index, linkType, mtu, name, qdisc, state,
our code ALREADY assumes that "flags" is always passed here. So please make
"flags" mandatory.

Change the commit message accordingly - we want to fail hard on a missing flag.
Line 123:                  vlanid=None, vlanprotocol=None, master=None, device=None,
Line 124:                  flags=None, **kwargs):
Line 125:         self.address = address
Line 126:         self.index = index


-- 
To view, visit http://gerrit.ovirt.org/36621
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

automation(a)ovirt.org has posted comments on this change.

Change subject: ipwrapper: Make Link.flags official as they are used for RUNNING and
PROMISC
......................................................................


Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/36621
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc9e516e28d3413274af85dc15f72cabdb64074a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No