Ben Pfaff <b...@ovn.org> wrote on 21/03/2016 10:14:47 PM:
> On Fri, Mar 04, 2016 at 08:08:58AM +0000, Liran Schour wrote:
> > This commit allows to add unmonitored columns to a monitored table
> > due to condition update.
> > It will be used to evaluate conditions on unmonitored columns.
> > Update notification includes only monitored columns.
> > Due to the limited number of columns We do not remove unused
unmonitored
> > columns due to condition update to keep the code simple.
> >
> > Signed-off-by: Liran Schour <lir...@il.ibm.com>
>
> Thanks for the revised series!
>
> Can the following check in ovsdb_monitor_add_column() be reduced to a
> check for mt->columns_index_map[column->index] != -1? Why check only
> for unmonitored columns? Also, I don't understand the comment about not
> removing unmonitored columns.
>
> > + /* Check duplication only for unmonitored columns.
> > + * We do not remove unused unmonitored columns due to condition
> > + * update */
> > + if (!monitored) {
> > + int i;
> > + for (i = 0; i < mt->n_columns; i++) {
> > + if (mt->columns[i].column == column) {
> > + /* column exists */
> > + return;
> > + }
> > + }
> > + }
>
Changed the check to mt->columns_index_map[column->index] != -1.
We check duplication only for unmonitored columns because for duplicated
monitored columns we cancel the whole monitor request on
ovsdb_monitor_table_check_duplicates() call. Duplicated unmonitored
columns should only cause a return without adding this column.
The comment is not clear, will change it to the following:
/* Check duplication only for unmonitored columns.
* Duplicated monitored columns will cause cancelation of monitor request
* on ovsdb_monitor_table_check_duplicates() call */
> ovsdb_monitor_compose_row_update() and
> ovsdb_monitor_compose_row_update2() iterate through all columns and then
> skip the ones that are not monitored. Can they just iterate through the
> monitored ones, e.g. change
> for (i = 0; i < mt->n_columns; i++) {
> to
> for (i = 0; i < mt->n_monitored_columns; i++) {
>
Right, applied this change to the next patch series.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
