Inline.
On Wed, Jun 22, 2011 at 3:42 PM, Ethan Jackson <et...@nicira.com> wrote:
> Thanks for the review Reid, comments inline.
>
>
> > This might be slightly more obvious as
> >
> > col = 'external-ids:"%s"="%s"' % (key, value)
>
> I agree, that would be clearer. I'm just doing an indentation change
> on this line though, so I'd rather leave it the same and address it
> later as a wider python cleanup.
>
> > Minor note, could do ifaces.values(), and avoid the [i], since you don't
> use
> > it
>
> Good idea, ill change it.
>
> > Is it possible (and would it matter) to have a condition where
> >
> > vxid = {'foo': None}
> > txid = {}
>
> Nah, vxid can only hold strings so I don't think it's possible. Even
> if it was possible it wouldn't matter because None is not a valid
> value for any of these keys. Is there a canonically correct way to do
> this in python that I should switch to? Otherwise, I'll leave it.
>
Hmm, just went back and looked again, I'm not sure of the precise
behavior you are going for - I am assuming it is to copy everything
from the vif to the tap. One other issue you could run into here if
some value is set on the tap but not the vif:
Before:
vxid: {}
txid: {'attached-mac': 'aa:bb:cc:dd:ee:ff'}
set_external_id('Interface', 'tap_name', 'attached-mac', None)
After:
vxid: {}
txid: {'attached-mac': None}
Which would cause an error with set_external_id, based upon
what you said earlier.
You could fix this by doing something like
if k in vxid and vxid[k] != txid.get(k):
set_external_id("Interface", tap_name, k, vxid[k])
-Reid
> Ethan
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
