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

[Arun Sharma]

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.


>
>> -            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.
>