Thanks Ben. I will send v5 patch. On 7/19/12 12:00 AM, "Ben Pfaff" <b...@nicira.com> wrote:
>On Wed, Jul 18, 2012 at 11:59:27PM +0530, Arun Sharma wrote: >> Please let me know your comments. > >See inline. > >> 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. > >Please remove it. > >> >>> - 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. > >Since it works with xen-bugtool, I'm happy with the plugin >modifications. > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
