[node-patches] Change in ovirt-node[master]: Don't show bonds without slaves

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: Don't show bonds without slaves
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/27434/2/src/ovirt/node/utils/network.py
File src/ovirt/node/utils/network.py:

Line 495:             slave.identify()
Line 496: 
Line 497:     @property
Line 498:     def slaves(self):
Line 499:         return open("/sys/class/net/%s/bonding/slaves" % self.ifname
Please use the File() class here.

File(…).read().split("\n") should work
Line 500:                     ).readline().split()
Line 501: 
Line 502:     def __str__(self):
Line 503:         return self.build_str(["ifname", "slave_nics"])


Line 580: 
Line 581:         if ifname == bond_name:
Line 582:             self.logger.debug(" Is bond master")
Line 583:             nic = BondedNIC(nic, bond_slaves)
Line 584:             nic = nic if nic.slaves else None
This branch is only active if a bond is configured. But IIU the bug correctly wewe should not display any bond device (no matter if it was configured or not) when it has no slaves.
Thus the logic must go somewhere else.

Eg

        if ifname in bond_slaves:
            nic = None
        elif nic.is_bond() and not nic.slaves():
            # Is a bond device, but has no slaves, ignore
            nic = None
Line 585: 
Line 586:         if ifname == bootif:
Line 587:             self.logger.debug(" Is bootif")
Line 588:             if vlanid:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2a718a9733a8e0981b68b706787a10768bd8d82
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes