[ovs-dev] **SPAM** Re: [PATCH v4] ovs-bugtool: Added --ovs option to get only ovs related information

[Arun Sharma]

Thanks Ben. I will send v5 patch.

On 7/19/12 12:00 AM, "Ben Pfaff" <blp at 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.sharma at calsoftinc.com> wrote:
>> 
>> >Please see comments inline.
>> >
>> >On 7/13/12 10:08 PM, "Ben Pfaff" <blp at 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.sharma at 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.
>