[ovs-dev,v2,02/15] ovsdb-idl: Allow monitoring columns that are already monitored.

Message ID 20180712134016.14336-3-jkbs@redhat.com
State New
Headers show
Series
  • Daemon mode for ovn-nbctl
Related show

Commit Message

Jakub Sitnicki July 12, 2018, 1:40 p.m.
If IDL was created with monitoring and alerts turned on by default for
all columns, then there is no harm in allowing the API users to ask
again for monitoring and alerts to be enabled for any given column.

This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
commands once the IDL has already ran once.

Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
---
 lib/ovsdb-idl.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Mark Michelson July 12, 2018, 9:09 p.m. | #1
On 07/12/2018 09:40 AM, Jakub Sitnicki wrote:
> If IDL was created with monitoring and alerts turned on by default for
> all columns, then there is no harm in allowing the API users to ask
> again for monitoring and alerts to be enabled for any given column.
> 
> This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
> commands once the IDL has already ran once.
> 
> Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
> ---
>   lib/ovsdb-idl.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 9ab5d6723..ae0a55c3a 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -1116,6 +1116,20 @@ ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
>       return &table->modes[column - table->class_->columns];
>   }
>   
> +static void
> +ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
> +                      const struct ovsdb_idl_column *column,
> +                      unsigned char mode)
> +{
> +    const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
> +                                                                      column);
> +    size_t column_idx = column - table->class_->columns;
> +
> +    if (table->modes[column_idx] != mode) {
> +        *ovsdb_idl_db_get_mode(db, column) = mode;

Calling ovsdb_idl_db_get_mode() here seems wasteful. You already have 
retrieved the table and the column index. So you may as well just do:

table->modes[column_indx] = mode;


> +    }
> +}
> +
>   static void
>   add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
>   {
> @@ -1136,7 +1150,7 @@ static void
>   ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
>                           const struct ovsdb_idl_column *column)
>   {
> -    *ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
> +    ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
>       add_ref_table(db, &column->type.key);
>       add_ref_table(db, &column->type.value);
>   }
>
Jakub Sitnicki July 13, 2018, 8:37 a.m. | #2
On Thu, 12 Jul 2018 17:09:45 -0400
Mark Michelson <mmichels@redhat.com> wrote:

> On 07/12/2018 09:40 AM, Jakub Sitnicki wrote:
> > If IDL was created with monitoring and alerts turned on by default for
> > all columns, then there is no harm in allowing the API users to ask
> > again for monitoring and alerts to be enabled for any given column.
> > 
> > This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
> > commands once the IDL has already ran once.
> > 
> > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
> > ---
> >   lib/ovsdb-idl.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> > index 9ab5d6723..ae0a55c3a 100644
> > --- a/lib/ovsdb-idl.c
> > +++ b/lib/ovsdb-idl.c
> > @@ -1116,6 +1116,20 @@ ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
> >       return &table->modes[column - table->class_->columns];
> >   }
> >   
> > +static void
> > +ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
> > +                      const struct ovsdb_idl_column *column,
> > +                      unsigned char mode)
> > +{
> > +    const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
> > +                                                                      column);
> > +    size_t column_idx = column - table->class_->columns;
> > +
> > +    if (table->modes[column_idx] != mode) {
> > +        *ovsdb_idl_db_get_mode(db, column) = mode;  
> 
> Calling ovsdb_idl_db_get_mode() here seems wasteful. You already have 
> retrieved the table and the column index. So you may as well just do:
> 
> table->modes[column_indx] = mode;

That's true but we also want to perform run-time check that IDL has not
ran yet that is in ovsdb_idl_db_get_mode(). That was the main reason
for going through ovsdb_idl_db_get_mode() to set the mode.

> 
> 
> > +    }
> > +}
> > +
> >   static void
> >   add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
> >   {
> > @@ -1136,7 +1150,7 @@ static void
> >   ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
> >                           const struct ovsdb_idl_column *column)
> >   {
> > -    *ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
> > +    ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
> >       add_ref_table(db, &column->type.key);
> >       add_ref_table(db, &column->type.value);
> >   }
> >   
>

Patch

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9ab5d6723..ae0a55c3a 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1116,6 +1116,20 @@  ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
     return &table->modes[column - table->class_->columns];
 }
 
+static void
+ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
+                      const struct ovsdb_idl_column *column,
+                      unsigned char mode)
+{
+    const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
+                                                                      column);
+    size_t column_idx = column - table->class_->columns;
+
+    if (table->modes[column_idx] != mode) {
+        *ovsdb_idl_db_get_mode(db, column) = mode;
+    }
+}
+
 static void
 add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
 {
@@ -1136,7 +1150,7 @@  static void
 ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
                         const struct ovsdb_idl_column *column)
 {
-    *ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
+    ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
     add_ref_table(db, &column->type.key);
     add_ref_table(db, &column->type.value);
 }