On 04/02/2015 11:27 AM, Ben Pfaff wrote:
> On Thu, Apr 02, 2015 at 11:24:51AM -0400, Russell Bryant wrote:
>> On 04/02/2015 11:18 AM, Ben Pfaff wrote:
>>> On Wed, Apr 01, 2015 at 08:04:21PM -0400, Russell Bryant wrote:
>>>> When the state of the chassis column in the Bindings table changes for
>>>> any row, ovn-nbd will notice and trigger recalculating the 'up' state
>>>> for all logical ports.
>>>>
>>>> This can be tesed manually by starting up ovs-sandbox with ovn support
>>>> enabled, running ovn-nbd, and then running the following commands:
>>>>
>>>> ovn-nbctl lswitch-add sw0
>>>> ovn-nbctl lport-add sw0-port0 sw0
>>>> port_uuid=$(ovn-nbctl lport-list sw0 | awk '{print $1}')
>>>> ovsdb-client transact "[\"OVN\",\
>>>> {\"uuid-name\":\"rowd4eca046_9233_4094_bc55_e28dd49217f9\",\
>>>> \"row\":{\"logical_port\":\"$port_uuid\",\"chassis\":\"hostname\"},\
>>>> \"op\":\"insert\",\"table\":\"Bindings\"}]"
>>>>
>>>> ovn-nbd will then see that the 'chassis' column is set in the Bindings
>>>> row for the logical port and will mark the logical port as 'up' in the
>>>> northbound db.
>>>>
>>>> Signed-off-by: Russell Bryant <rbry...@redhat.com>
>>>
>>> "sparse" complained:
>>> ../ovn/ovn-nbd.c:106:51: warning: expression using sizeof bool
>>> ../ovn/ovn-nbd.c:109:51: warning: expression using sizeof bool
>>> about this code:
>>>> + if (*bindings->chassis && (!lport->up || !*lport->up)) {
>>>> + bool up = true;
>>>> + nbrec_logical_port_set_up(lport, &up, sizeof up);
>>>> + } else if (!*bindings->chassis && (!lport->up || *lport->up)) {
>>>> + bool up = false;
>>>> + nbrec_logical_port_set_up(lport, &up, sizeof up);
>>>
>>> While I don't think there's anything wrong with "sizeof bool", I didn't
>>> think that "sizeof" was a reasonable way to get the number of elements
>>> in 'up' (which is just 1 of course), so I changed these to literal 1s.
>>>
>>> With that change, I applied both patches to the ovn branch. Thank you!
>>
>> Thanks for the fix! I guess I misinterpreted the meaning of that 3rd
>> argument.
>>
>> The prototype for that function seems kind of odd, anyway. It's just a
>> single boolean, so something like (lport, true) seems sufficient. The
>> same applies to how "up" appears in the generated struct. It probably
>> has something to do with convenience in how the code is auto generated
>> though, so it's not a big deal.
>
> Yes, it's a little odd. It's because it's really a tri-valued column:
> it can be true or false or empty. But the interface could still be a
> little nicer.
>
Ah, good point. I also didn't realize it was tri-valued at first, which
made my code a little crashy when 'up' was NULL. :-)
--
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
