Mark Wu has posted comments on this change. Change subject: Added required ipwrapper functions for iproute2 configurator ......................................................................
Patch Set 1: (2 comments) .................................................... File lib/vdsm/ipwrapper.py Line 265: command = [_IP_BINARY.cmd, 'addr', 'flush', 'dev', dev] Line 266: _execCmd(command) Line 267: Line 268: Line 269: def linkAdd(link=None, name=None, type=None, linkArgs=None): the word 'name' is optional in command line as what described in the manual of 'ip-link' Line 270: command = [_IP_BINARY.cmd, 'link', 'add'] Line 271: if link: Line 272: command += ['link', link] Line 273: if name: Line 265: command = [_IP_BINARY.cmd, 'addr', 'flush', 'dev', dev] Line 266: _execCmd(command) Line 267: Line 268: Line 269: def linkAdd(link=None, name=None, type=None, linkArgs=None): I would like suggest: linkAdd(dev, type, link=None, linkArgs=None): I don't think we need match the order of ip command. In our API, we just the use first argument to represent the entity we want to operate, and others the arguments for the operation. That makes sense to me. Line 270: command = [_IP_BINARY.cmd, 'link', 'add'] Line 271: if link: Line 272: command += ['link', link] Line 273: if name: -- To view, visit http://gerrit.ovirt.org/18585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9283ae90f84fa930e401b3ecd2e18935ba9ca2f5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller <amul...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Assaf Muller <amul...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
