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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
