[ovs-dev] [PATCH v2] ovs-bugtool: Added --ovs option to get only ovs related information
Arun Sharma
arun.sharma at calsoftinc.com
Thu Jul 12 09:41:45 UTC 2012
Reworked and its present in v3. Also updated ovs-bugtool manual file.
Comments inline.
From: Reid Price <reid at nicira.com>
Date: Wed, 11 Jul 2012 12:00:02 -0700
To: Arun Sharma <arun.sharma at calsoftinc.com>
Cc: <dev at openvswitch.org>
Subject: Re: [ovs-dev] [PATCH v2] ovs-bugtool: Added --ovs option to get
only ovs related information
Thanks for the fixes! A couple nitpick comments inline. v2 and your
responses in v1 resolved all of my previous concerns.
On Tue, Jul 10, 2012 at 11:40 PM, Arun Sharma <arun.sharma at calsoftinc.com>
wrote:
> + # Filter out ovs relevant information if --ovs option passed
> + # else collect all information
> + if only_ovs_info:
> + ovs_info_caps = [CAP_NETWORK_STATUS, CAP_SYSTEM_LOGS,
> + CAP_NETWORK_CONFIG]
> + ovs_info_list = ['process-tree']
> + # We cannot use iteritems, since we modify 'data' as we pass through
> + for (k, v) in data.items():
> + cap = v['cap']
> + if 'filename' in v:
> + info = k[0]
> + else:
> + info = k
> + if info not in ovs_info_list and cap not in ovs_info_caps:
> + del data[k]
> +
> + try:
> + load_plugins(filter='ovs')
> + except:
> + pass
> + else:
> + try:
> + load_plugins()
> + except:
> + pass
> +
This probably will be easier to maintain by having the
try/except outside of the if/else.
filters = set()
if only_ovs_info:
filters.add('ovs')
...
filter = ",".join(filters)
try:
load_plugins(filter=filter)
except:
pass
[Arun] - Implemented it as below.
It requires it to have "filter = None" in else clause. Since in
load_plugins function, condition "if not(filter is None or filter in
filters): continue" will be always true when filter=''.
if len(filters):
filter = ",".join(filters)
else:
filter = None
try:
load_plugins(filter=filter)
except:
pass
> for el in xmldoc.documentElement.getElementsByTagName("*"):
> - if el.tagName == "files":
> - newest_first = getBoolAttr(el, 'newest_first')
> - file_output(dir, getText(el.childNodes).split(),
> - newest_first=newest_first)
> - elif el.tagName == "directory":
> - pattern = el.getAttribute("pattern")
> - if pattern == '': pattern = None
> - negate = getBoolAttr(el, 'negate')
> - newest_first = getBoolAttr(el, 'newest_first')
> - tree_output(dir, getText(el.childNodes), pattern and
> re.compile(pattern) or None,
> - negate=negate, newest_first=newest_first)
> - elif el.tagName == "command":
> - label = el.getAttribute("label")
> - if label == '': label = None
> - cmd_output(dir, getText(el.childNodes), label)
> + filters_tmp = el.getAttribute("filters")
> + if filters_tmp == '':
> + filters = []
> + else:
> + filters = filters_tmp.split(',')
> + if filter is None or filter in filters:
Nit, you can save this whole level of indentation by doing
if not(filter is None or filter in filters): continue
[Arun] - Done
> + if el.tagName == "files":
> + newest_first = getBoolAttr(el, 'newest_first')
> + file_output(dir, getText(el.childNodes).split(),
> + newest_first=newest_first)
> + elif el.tagName == "directory":
> + pattern = el.getAttribute("pattern")
> + if pattern == '': pattern = None
> + negate = getBoolAttr(el, 'negate')
> + newest_first = getBoolAttr(el, 'newest_first')
> + tree_output(dir, getText(el.childNodes),
> + pattern and re.compile(pattern) or None,
> + negate=negate, newest_first=newest_first)
> + elif el.tagName == "command":
> + label = el.getAttribute("label")
> + if label == '': label = None
> + cmd_output(dir, getText(el.childNodes), label)
Looks good to me otherwise.
-Reid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20120712/988f453d/attachment-0003.html>
More information about the dev
mailing list