On Tue, Sep 27, 2011 at 12:06:15PM -0700, Ethan Jackson wrote:
> > Forbidding creating more Vlog modules after initializing Vlog seems
> > like an odd limitation.
> 
> I basically do this so that I can have a handle on what configuration
> of "any" module means.  This is much easier if you know what the set
> of possible modules is because you can simply iterate over them.
> Otherwise you'd have to maintain some sort of default settings
> structure hand special case modifications to "any" to update this
> default settings structure. Definitely possible, but seemed like an
> inconvenience since I don't expect people to be creating vlog modules
> dynamically at runtime.  Given that explanation, do you still think
> it's worth implementing? If so, I'll change it.

I guess it won't actually make a difference yet, so there's no need.

> > Why are a Vlog module's own settings stored in a dict in a class
> > attribute? ?Wouldn't an instance attribute be more natural?
> 
> This was a possibly incorrect design decision.  Basically, I want all
> Vlog modules with the same module name to share the same settings.
> The current design enforces this invariant by only allowing one entry
> in the __mfl table per module. Alternatively, I would have to maintain
> an __all_vlogs list which would contain a reference to each vlog
> instance.  In this list there could be multiple vlog instances for a
> given module which would need to be updated in set_level.  I'm
> personally not sure which approach is cleaner, do you have a
> preference?

What about keeping __mfl but caching __mfl[module] in a Vlog instance
variable?

Otherwise, again, it's not that important, it just surprised me.

> > The C version of vlog accepts only "ANY" (uppercase). ?The Python
> > version only accepts any case. ?(It's probably best to just make the C
> > version accept any case.)
> 
> I agree, I don't really want to fix the c code right now though.

It's trivial.  I sent out a patch.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to