Please let me know your comments. On 7/14/12 9:20 AM, "Arun Sharma" <arun.sha...@calsoftinc.com> wrote:
>Please see comments inline. > >On 7/13/12 10:08 PM, "Ben Pfaff" <b...@nicira.com> wrote: > >>On Wed, Jul 11, 2012 at 07:41:59PM -0700, Arun Sharma wrote: >>> Option --ovs is added for ovs-bugtool command to collect >>> only OpenvSwitch relevant information. To perform >>> filtering in plugins, a new xml attribute filters="ovs" (optional) >>> would be required in element 'command','files','directory' in >>> openvswitch.xml. Value of 'filters' attribute will be compared >>> with filtering option in load_plugins to get all relevant operation >>> to collect information. If no "--ovs" option is passed then it will >>> behave as earlier. >>> >>> Fixed an issue which occurs in scenario where option '--yestoall' >>> is not passed and user keeps entering "y" or "n" on prompt. >>> >>> Plus, trailing whitespaces are fixed. White space before '=' and >>> after in function def and call is also fixed. >>> >>> Signed-off-by: Arun Sharma <arun.sha...@calsoftinc.com> >> >>It looks to me like exactly one of only_ovs_info or collect_all_info >>is always set. Is that true? If it is, then why would we ever write >>"if collect_all_info or only_ovs_info:", as below, since it would >>always be true: > >[Arun] - Yes, exactly one will be true. Currently the command passed in >cmd_output is applicable for both collect_all_info and only_ovs_info. > But in future if any other filtering requirement comes then this >cmd_output call may not be applicable. > I think we can remove this condition for now. Pl. confirm. > > >> >>> - for b in bond_list(vspid): >>> - cmd_output(CAP_NETWORK_STATUS, >>> - [OVS_APPCTL, '-t', >>>'@RUNDIR@/ovs-vswitchd.%s.ctl' % vspid, '-e' 'bond/show %s' % b], >>> - 'ovs-appctl-bond-show-%s.out' % b) >>> + if collect_all_info or only_ovs_info: >>> + for b in bond_list(vspid): >>> + cmd_output(CAP_NETWORK_STATUS, >>> + [OVS_APPCTL, '-t', >>> + '@RUNDIR@/ovs-vswitchd.%s.ctl' % >>>vspid, '-e' 'bond/show %s' % b], >>> + 'ovs-appctl-bond-show-%s.out' % b) >>> except e: >>> pass >> >>This commit changes the plugins, adding a new "filter" attribute. The >>plugins are used not just by ovs-bugtool but also by xen-bugtool on >>XenServer. Will xen-bugtool accept the plugins that have been >>modified in this way, or reject them because they have unknown >>attributes? > >[Arun] - Test using xen-bugtool with modified plugins works after adding >"filters" attribute in xml files. > The reason to add 'filters' attribute is to filter specific >element relevant to 'ovs' from a capability in plugins. > As an other alternative(which would not require any change in xml >files), thought earlier was not to add 'filters' attribute in element and >just fetch based on the capabilities like 'system-configuration'. But by >adding it into list ovs_info_caps would fetch all elements in 'data' from >'system-configuration'. This yields non relevant information in archive. >So decided to add 'filters' attribute. > > >> >>Thanks, >> >>Ben. >> > > >_______________________________________________ >dev mailing list >dev@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
