[ovs-dev] [PATCH] ovs-vsctl: Improve error reporting

[Andy Zhou]

On Thu, Mar 27, 2014 at 9:08 AM, Thomas Graf <tgraf at redhat.com> wrote:
> On 03/27/2014 04:48 PM, Andy Zhou wrote:
>>
>> On Thu, Mar 27, 2014 at 3:36 AM, Thomas Graf <tgraf at redhat.com> wrote:
>>>
>>> On 03/27/2014 10:04 AM, Andy Zhou wrote:
>>>>
>>>>
>>>> * Thomas: Some of the code and ideas are imported from your
>>>>     12/11/2012 patch titled "ovs-vsctl: Check "ofport" column after
>>>>     adding a new port", and subsequent mailing list discussions.
>>>>     Please feel free to review, fix and enhance the patch as
>>>>     you see fit. More testing will is definitly needed. If make sense,
>>>>     we can add more unit test cases. The ovs-vsctl manpage needs to be
>>>>     updated. Please re-post the patch as v2 when you are done with
>>>>     your changes.
>>>
>>>
>>>
>>> This looks excellent. I sent a V2 with a slightly improved error
>>> message.
>>
>> Thanks.  Did the unit tests, i.e. "make check", pass? It seems
>> ovs-vsctl.at needs to be adjusted accordingly.
>>
>> Do you mind adjust ovs-vsctl man page about the new output?
>
>
> Will fix this. Thanks for catching.
>
>
>>> We might considering renaming 'iface' to object as we add additional
>>> status columns for other mutable objects that can fail.
>>>
>> My intention is to scope this patch within the outlines of  the
>> hackathon's project description.  Having said that, I won't object if
>> you want
>> to make the change.
>
>
> Sounds good.
>
>
>>> Also, another idea that we can consider is coming up with additional
>>> status in 'ovs-vsctl show' and check for ofport -1 and indicate that
>>> the port has not been set up correctly. What do you think?
>>
>>
>> This is a very good idea.  How about break this into its own patch?
>
>
> Definitely.
>
>
>> I am slightly worried the new output may throw off existing scripts
>> out there. How about we only enable this check under a command line
>> argument so we don't change the default behavior?
>
>
> True. OTOH, I believe it must be easily visible in order for it to
> be effective. Wouldn't scripts rather parse ovs-vsctl list [...]?
I agree.  We are changing "add-br/add-port" output in the same way.