On 08/07/2015 03:46 AM, Alex Wang wrote:
> idl-loop is needed in implementing other controller (i.e., vtep controller).
> So, this commit moves the logic into ovsdb-idl library module.
>
> Signed-off-by: Alex Wang <al...@nicira.com>
This looks fine to me. I have a comment but it's not a blocker. It's
up to you if you want to change it or not.
Acked-by: Russell Bryant <rbry...@redhat.com>
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index a49f84f..28aa787 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -222,5 +222,31 @@ const struct ovsdb_idl_row *ovsdb_idl_txn_insert(
> const struct uuid *);
>
> struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *);
> +
> +
> +/* ovsdb_idl_loop provides an easy way to manage the transactions related
> + * to 'idl' and to cope with different status during transaction. */
> +struct ovsdb_idl_loop {
> + struct ovsdb_idl *idl;
> + unsigned int skip_seqno;
> +
> + struct ovsdb_idl_txn *committing_txn;
> + unsigned int precommit_seqno;
> +
> + struct ovsdb_idl_txn *open_txn;
> +};
> +
> +#define OVSDB_IDL_LOOP_INITIALIZER(IDL) { .idl = (IDL) }
> +
> +static inline void
> +ovsdb_idl_loop_init(struct ovsdb_idl_loop *loop, struct ovsdb_idl *idl)
> +{
> + memset(loop, 0, sizeof *loop);
> + loop->idl = idl;
Most of the code looks like an unmodified code move, but this function
appears to be new. I guess you're using it later in the series?
It seems you could also write this as:
*loop = OVSDB_IDL_LOOP_INITIALIZER(idl);
or you could drop this function completely since this one line does the
same thing, right?
--
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
