Dan Kenigsberg has posted comments on this change.

Change subject: iproute2: Add static and autoconf ipv6 support
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/27189/2/lib/vdsm/ipwrapper.py
File lib/vdsm/ipwrapper.py:

Line 578:         command += ['show', 'dev', dev]
Line 579:     return _execCmd(command)
Line 580: 
Line 581: 
Line 582: def addrAdd(dev, ipaddr, netmask, version=4):
iproute calls version "family". I do not expect supporting DECnet anytime soon, 
but conforming with the tool we wrap is a merit of its own.
Line 583:     command = [_IP_BINARY.cmd, '-%s' % version, 'addr', 'add', 'dev', 
dev,
Line 584:                '%s/%s' % (ipaddr, netmask)]
Line 585:     _execCmd(command)
Line 586: 


Line 584:                '%s/%s' % (ipaddr, netmask)]
Line 585:     _execCmd(command)
Line 586: 
Line 587: 
Line 588: def addrFlush(dev, version=None):
I do not see the modified lines ever used in this patch. did you intend to use 
vesion=6 somewhere? Or was it added for completeness? If it's the latter, a 
separate patch would have alleviated doubts.
Line 589:     command = [_IP_BINARY.cmd]
Line 590:     if version is not None:
Line 591:         command.append('-%s' % version)
Line 592:     command += ['addr', 'flush', 'dev', dev]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I372212dd9abc1dbb2ce4d966ef25726a439ac166
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to