[ovs-dev] Bug#691508: [PATCH] ovs-vsctl: Allow command-specific options to mingle with global options.

Adam Heath doogie at brainfood.com
Fri Nov 2 20:40:20 UTC 2012


On 11/02/2012 12:49 PM, Ben Pfaff wrote:
> On Mon, Oct 29, 2012 at 03:03:13PM -0500, Adam Heath wrote:
>> On 10/29/2012 11:34 AM, Ben Pfaff wrote:
>>> Until now, a command like "ovs-vsctl --may-exist add-br br0" yielded a
>>> confusing error message.  Users had to realize that the correct form was
>>> "ovs-vsctl -- --may-exist add-br br0", but instead they often reported a
>>> bug or gave up in frustration.  Even though the behavior was documented, it
>>> was counterintuitive.
>>>
>>> This commit allows command-specific options to be mixed with global
>>> options, making both forms of the command listed above equally acceptable.
>>
>> Tbh, I would actually prefer to have command-specific options that
>> appear in the global area issue an error.
>>
>> ==
>> one-vsctl: Found command-specific --may-exist in global area, please
>> use -- instead.
>> ==
>>
>> See (1) for a reason why I'd prefer it the way I suggested above.
>>
>> 1: http://lwn.net/Articles/520198/
>>    you might need to wait a few days for it to become publically
>>    available.
> 
> OK, it's available now, and I skimmed over it, but the analogy to
> ovs-vsctl isn't really obvious to me.  Can you elaborate?

Providing variants for valid parameter processing makes it harder to
understand.  Instead, a warning/error should be issued saying the arg
is not understood(and if it appears to be a per-cmd option in global
space, then a message saying such should be displayed), but allowing
for per-option to work for global is actually *breaking* existing
scripts that error out.

Any existing script that places --may-exist in global space, without
--, will currently have an error.  Your patch changes that.  So, that
is bad.  Instead, if you just change the error message displayed in
those cases, then there isn't any breakage.

The article applies to this senario, because your new patch will break
existing scripts.  The article also applies because ovs-vsctl should
have had better handling of border conditions in arg processing, by
providing more concise error messages.

==
ovs-vsctl: Found an unrecognized option(--may-exist) in the global
option area; perhaps you need to use -- to start a per-command option
area?
==



More information about the dev mailing list