Change in vdsm[master]: iproute2 binary wrapper

amuller at redhat.com amuller at redhat.com
Thu Jun 6 10:49:38 UTC 2013


Assaf Muller has posted comments on this change.

Change subject: iproute2 binary wrapper
......................................................................


Patch Set 4: (11 inline comments)

Replied to comments. Patch coming up.

....................................................
File vdsm/netconf/iproute2.py
Line 23: 
Line 24: ipBinary = str(utils.CommandPath("ip", "/sbin/ip"))
Line 25: 
Line 26: 
Line 27: class Route(object):
Done
Line 28:     def __init__(self, device=None, network=None, ipaddr=None, table=None,
Line 29:                  gateway=None, text=None):
Line 30:         '''
Line 31:             * Two allowed possibilities:


Line 25: 
Line 26: 
Line 27: class Route(object):
Line 28:     def __init__(self, device=None, network=None, ipaddr=None, table=None,
Line 29:                  gateway=None, text=None):
Done
Line 30:         '''
Line 31:             * Two allowed possibilities:
Line 32:             1) default via <gateway> ...
Line 33:             2) <network> via <ip> ...


Line 26: 
Line 27: class Route(object):
Line 28:     def __init__(self, device=None, network=None, ipaddr=None, table=None,
Line 29:                  gateway=None, text=None):
Line 30:         '''
Done
Line 31:             * Two allowed possibilities:
Line 32:             1) default via <gateway> ...
Line 33:             2) <network> via <ip> ...
Line 34: 


Line 35:             * device is required
Line 36:             * table is optional
Line 37:         '''
Line 38:         if network and gateway:
Line 39:             raise ValueError("A text cannot be a default text and a "
The error message was a search and replace fumble.
I was unaware that the ip binary allows "0.0.0.0/0" instead of "default". This indeed simplifies things :)
Line 40:                              "non-default text simultaneously")
Line 41:         if not text and device is None:
Line 42:             raise ValueError("device is a required parameter")
Line 43: 


Line 45:                 (network is None or ipaddr is None):
Line 46:             raise ValueError("network and ip are required for non-default "
Line 47:                              "routing entry")
Line 48: 
Line 49:         if text and (device or network or ipaddr or table or gateway):
Done
Line 50:             raise ValueError("Must supply either a text in textual format "
Line 51:                              "or parameters, not both")
Line 52: 
Line 53:         if not text:


Line 57:             self.table = table
Line 58:             self.gateway = gateway
Line 59:         else:
Line 60:             (self.device, self.network, self.ipaddr, self.table,
Line 61:             self.gateway) = self._parse(text)
Done
Line 62: 
Line 63:     def _parse(self, route):
Line 64:         route = route.split()
Line 65: 


Line 59:         else:
Line 60:             (self.device, self.network, self.ipaddr, self.table,
Line 61:             self.gateway) = self._parse(text)
Line 62: 
Line 63:     def _parse(self, route):
Done
Line 64:         route = route.split()
Line 65: 
Line 66:         if len(route) not in (5, 7):
Line 67:                 raise ValueError("Route parse error")


Line 87:                                  "while parsing route")
Line 88: 
Line 89:         device = route[4]
Line 90:         table = route[6] if len(route) == 7 else None
Line 91:         return (device, network, ipaddr, table, gateway)
Done
Line 92: 
Line 93:     def __str__(self):
Line 94:         if self.network:
Line 95:             str = "%s via %s" % (self.network, self.ipaddr)


Line 92: 
Line 93:     def __str__(self):
Line 94:         if self.network:
Line 95:             str = "%s via %s" % (self.network, self.ipaddr)
Line 96:         else:  # default route
Done
Line 97:             str = "default via %s" % self.gateway
Line 98: 
Line 99:         str += " dev %s" % self.device
Line 100: 


Line 108:             yield word
Line 109: 
Line 110: 
Line 111: class Rule(object):
Line 112:     def __init__(self, source=None, destination=None, table=None, text=None):
Done
Line 113:         if not text and table is None:
Line 114:             raise ValueError("table is a required parameter")
Line 115: 
Line 116:         if text and (source or destination or table):


Line 184:     return output
Line 185: 
Line 186: 
Line 187: def ipRoute():
Line 188:     command = [ipBinary, "route"]
In my eyes the current way is more readable. I'll keep the next patch set the way it is, and if anyone else has any input on this matter we'll go with a tiebreaker?
Line 189:     return _execCmd(command)
Line 190: 
Line 191: 
Line 192: def ipRouteShowTable(table):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1d315c3294fd7f058cdc840dea329d91a658a304
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list