[ovs-dev] [add-remove v2 2/8] vswitchd: Break set_up_iface() into two different functions.

Ben Pfaff blp at nicira.com
Fri Oct 15 16:35:55 UTC 2010


On Thu, Oct 14, 2010 at 07:34:20PM -0700, Jesse Gross wrote:
> On Mon, Oct 11, 2010 at 10:01 AM, Ben Pfaff <blp at nicira.com> wrote:
> > On Fri, Oct 08, 2010 at 05:20:19PM -0700, Jesse Gross wrote:
> >> On Fri, Oct 1, 2010 at 4:48 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> > +    /* Skip reconfiguration if the device has the wrong type. This shouldn't
> >> > +     * happen, but... */
> >> > +    iface_type = (!iface->cfg->type[0] ? NULL
> >> > +                  : !strcmp(iface->cfg->type, "internal") ? "system"
> >> > +                  : iface->cfg->type);
> >>
> >> Shouldn't that NULL be "system"?
> >
> > I think my goal here was to allow a pre-existing device to have any type
> > and avoid warning about it if no type was specified.
> >
> > I'm going to leave this as-is for now.  We can easily change it later if
> > you want.
> 
> I think this is a pretty bad situation to get into, since it means
> that you could have a database with the same current configuration
> that results in two different outcomes depending on what was there
> beforehand.  Furthermore, not only will it not warn if the types don't
> match but it also won't stop the reconfigure.  So theoretically you
> could have a non-system device that exists and you try to reconfigure
> it with arguments meant for a system device (because that's why NULL
> usually implies) with undefined outcome.

You're right.

But it looks like one of my later patches changed (fixed) this behavior
anyhow, so I don't think there's anything to do now.




More information about the dev mailing list