[ovs-dev] [ovn-controller-vtep V5 05/12] idl-loop: Move idl-loop into ovsdb-idl library.

Alex Wang alexw at nicira.com
Sat Aug 8 06:38:31 UTC 2015


On Fri, Aug 7, 2015 at 11:40 AM, Russell Bryant <rbryant at redhat.com> wrote:

> 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 <alexw at 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 <rbryant at 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?
>
>
Actually not, it is because I saw in other modules, there is always both
macro for designated initializer and initialization function.



> 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?
>

Right, I guess, we could add it later if needed.

Thanks,
Alex Wang,


>
> --
> Russell Bryant
>



More information about the dev mailing list