[ovs-dev] [PATCH monitor_cond V3 01/10] ovsdb: create column index mapping between ovsdb row to monitor row

[Liran Schour]

Andy Zhou <azhou at ovn.org> wrote on 09/02/2016 01:33:12 AM:


> On Mon, Feb 8, 2016 at 12:43 PM, Liran Schour <LIRANS at il.ibm.com> wrote:
> Andy Zhou <azhou at ovn.org> wrote on 08/02/2016 09:41:47 PM:
> 
> > On Sun, Feb 7, 2016 at 12:08 AM, Liran Schour <LIRANS at il.ibm.com> 
wrote:
> > Andy Zhou <azhou at ovn.org> wrote on 04/02/2016 11:54:06 PM:
> >
> > > On Sun, Jan 31, 2016 at 11:03 PM, Liran Schour <lirans at il.ibm.com>
> > wrote:
> > > Columns indexing is different in ovsdb_row then in 
ovsdb_monitor_row.
> > > We need mapping between the 2 for condition evaluation.
> > >
> > > signed-off-by: Liran Schour <lirans at il.ibm.com>
> > >
> > > It may be simpler to and more efficient to have  ovsdb_clause to
> > > have pointer to ovsdb_monitor_column* than
> > > to ovsdb_column directly.  We can add a field in
> > > ovsdb_monitor_column to store the index for accessing row data.
> > >
> > > struct ovsdb_caluse {
> > >          enum ovsdb_function function;
> > >          const struct ovsdb_monitor_column *column;
> > >          struct ovsdb_datum arg
> > > };
> > >
> > > If this approach works, then we don't need to maintain the index 
map.
> > >
> >
> > Ovsdb_clause and ovsdb_condition are being used originally to evaluate
> > conditions on ovsdb_row. Because of that ovsdb_clause do needs the 
field
> > ovsdb_column.
> > I agree that the index map array does not looks too good but the 
option
> to
> > overcome it as I see it is to duplicate the code in condition.c for
> > ovsdb_monitor_row that will have ovsdb_monitor_clause with
> > ovsdb_monitor_column field. What do you think?
> >
> > Yes, I am concerned with idea of using index map array to bridge API
> gaps.
> >
> > How about the following:
> >
> > struct ovsdb_clause {
> >         enum ovsdb_function function;
> >         const struct ovsdb_column * column;
> >         unsigned int index;    /* column Index into ovsdb_datum for
> > evaluation.*/
> >         struct ovsdb_datum arg;
> > }
> >
> > Without monitor_cond,  the new 'index' will be the same
> > column->index.  monitor_cond
> > can maintain the 'index' field with the same logic as index map. The
> > condition API can
> > mostly be reused, No?
> >

> Yes it can be reused. At default index will be initialize to 
column->index
> and if the caller calls to ovsdb_condition_set_index(condition*,
> index_map[]) after condition creation. Indexes will be set to the
> index_map values. I will go with this option to simplify the condition
> API. (but still needs to maintain index_map in monitor.c)

> I wonder how difficult it would be to hide the index assignment 
> within ovsdb_monitor_add_column? 
> Thanks
> 

It is not straightforward, the caller has no correlation between the 
condition's clause to the added column.
I will go with the option of adding the API: 
ovsdb_condition_set_index(condition*, index_map[]) that is optionally 
called after condition creation and set the index field in the 
ovsdb_clause.