[ovs-dev] [PATCH 2/3] ovs-vsctl: Allow "get" commands to create @names also.

Ben Pfaff blp at nicira.com
Tue Sep 21 22:31:53 UTC 2010


On Tue, Sep 21, 2010 at 03:27:43PM -0700, Jesse Gross wrote:
> On Tue, Sep 21, 2010 at 2:32 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Tue, Sep 21, 2010 at 02:22:12PM -0700, Jesse Gross wrote:
> >> On Mon, Sep 20, 2010 at 11:57 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > +        *create_symbol(ctx->symtab, id, &new) = row->uuid;
> >> > +        if (!new) {
> >> > +            vsctl_fatal("row id \"%s\" specified on \"get\" command was used "
> >> > +                        "before it was defined", id);
> >> > +        }
> >>
> >> I'm not really sure what this error string means.  insert does a get
> >> and then we do an explicit get here as well.  What is the condition
> >> where these would be different?
> >
> > This is valid:
> >        ovs-vsctl -- --id=@m get mirror mymirror -- set bridge br0 mirrors=@m
> >
> > This isn't:
> >        ovs-vsctl -- set bridge br0 mirrors=@m -- --id=@m get mirror mymirror
> >
> > You can't use the row id "@m" specified on the "get" command before you
> > define it.
> 
> OK, the message makes sense in this context.  This behavior doesn't
> seem all that weird to me.

OK, glad to hear that, at least.

> > The situation is different for "create"s: you can use them before you
> > create them.
> 
> The fact that the behavior is different seems pretty surprising to me.
>  I wonder if it is actually better to artificially limit the
> flexibility of create in the interest of consistency.

I've had the need to create circularly linked structures before and so
that's the reason it's supported.

> > The reason is artificial (I didn't want to make the commit
> > bigger or the IDL architecture even uglier), but it is documented.
> 
> OK, I'll have to take your word that it isn't worth doing.

I won't go that far: it isn't worth doing *yet* :-)

Anyway, thanks for the sanity check, and I'll push this soon.




More information about the dev mailing list