On Tue, Jul 28, 2015 at 04:53:24PM -0700, Justin Pettit wrote:
>
>
> > On Jul 28, 2015, at 4:18 PM, Justin Pettit <[email protected]> wrote:
> >
> > Acked-by: Justin Pettit <[email protected]>
>
> I should amend my Ack. I think this introduces a problem.
>
> >
> >> On Jul 28, 2015, at 8:44 AM, Ben Pfaff <[email protected]> wrote:
> >>
> >>
> >> /* Returns true if the database is all cleaned up, false if more work is
> >> * required. */
> >> bool
> >> -chassis_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge
> >> *br_int)
> >> +chassis_cleanup(struct controller_ctx *ctx)
> >> {
> >> - if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
> >> - return false;
> >> - }
>
> I think you still want to check if "ctx->ovnsb_idl_txn" is null.
If you look a little farther, the code here still checks ovnsb_idl_txn,
but only if it needs to actually change anything:
/* Delete Chassis row. */
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (!chassis_rec) {
return true;
}
if (ctx->ovnsb_idl_txn) {
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: unregistering chassis '%s'",
ctx->chassis_id);
sbrec_chassis_delete(chassis_rec);
}
return false;
> >> /* Returns true if the database is all cleaned up, false if more work is
> >> * required. */
> >> bool
> >> -chassis_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge
> >> *br_int)
> >> +encaps_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge
> >> *br_int)
> >> {
> >> - if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
> >> - return false;
> >> - }
>
> And, here, I think you want to check if "ctx->ovs_idl_txn" is null.
Same deal here.
> If you agree, there's no reason to repost these patches.
I think you should take another look, just to be sure.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
