[ovs-dev] [PATCH v2 1/2] ovn-nbctl: Help catch lport-set-addresses mistakes.

[Ben Pfaff]

On Thu, Jan 21, 2016 at 09:11:19AM -0500, Russell Bryant wrote:
> On 01/20/2016 05:43 PM, Ben Pfaff wrote:
> > On Wed, Jan 20, 2016 at 05:33:16PM -0500, Russell Bryant wrote:
> >> While debugging a broken OVN environment yesterday, the problem turned
> >> out to be invalid entries in the logical port addresses column.  In
> >> particular, the following command had been used:
> >>
> >>   $ ovn-nbctl lport-set-addresses lp0 MAC IP
> >>
> >> instead of:
> >>
> >>   $ ovn-nbctl lport-set-addresses lp0 "MAC IP"
> >>
> >> This is really easy to mess up, so add some simple validation to the
> >> lport-set-addresses command.  If the beginning of an argument is ever
> >> an IP address, it's wrong.
> >>
> >> In passing, also add a note to the ovn-nb db documentation to note that
> >> the order of "MAC IP" is required, as "IP MAC" is not valid.
> >>
> >> Signed-off-by: Russell Bryant <russell at ovn.org>
> > 
> > Indentation in the "if" condition looks a little weird, please check it.
> 
> Yeah, for some reason I wasn't sure the best way to indent it.  Here's
> what I changed it to, but if you'd prefer something else let me know and
> I'll change it.
> 
> > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> > index 507df47..8c3b3c8 100644
> > --- a/ovn/utilities/ovn-nbctl.c
> > +++ b/ovn/utilities/ovn-nbctl.c
> > @@ -672,9 +672,9 @@ nbctl_lport_set_addresses(struct ctl_context *ctx)
> >      for (i = 2; i < ctx->argc; i++) {
> >          struct eth_addr ea;
> >  
> > -        if (strcmp(ctx->argv[i], "unknown") &&
> > -                !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
> > -                          ETH_ADDR_SCAN_ARGS(ea))) {
> > +        if (strcmp(ctx->argv[i], "unknown")
> > +            && !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
> > +                         ETH_ADDR_SCAN_ARGS(ea))) {

That's the format I usually expect.

Thanks!