Sorry, last note on expected behavior. One other seemingly reasonable
option
for the example below would be to delete that external id from tap, as
opposed
to just ignoring that they are different.
On Wed, Jun 22, 2011 at 9:44 PM, Reid Price <r...@nicira.com> wrote:
> 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
