[ovs-dev] [PATCH 3/3 v2] ovsdb-server: Add and remove databases during run time.

[Ben Pfaff]

On Thu, Jun 27, 2013 at 12:52:47PM -0700, Gurucharan Shetty wrote:
> On Thu, Jun 27, 2013 at 10:27 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Wed, Jun 26, 2013 at 11:29:36AM -0700, Gurucharan Shetty wrote:
> > > On Wed, Jun 26, 2013 at 10:51 AM, Gurucharan Shetty <shettyg at nicira.com
> > >wrote:
> > >
> > > >
> > > >
> > > > On Tue, Jun 25, 2013 at 5:01 PM, Ben Pfaff <blp at nicira.com> wrote:
> > > >
> > > >> On Tue, Jun 25, 2013 at 01:18:36AM -0700, Gurucharan Shetty wrote:
> > > >> > The commit allows a user to add a database file to a
> > > >> > ovsdb-server during run time. One can also remove a
> > > >> > database file from ovsdb-server's control.
> > > >> >
> > > >> > Feature #14595.
> > > >> > Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
> > > >>
> > > >> I could see a number of ways to improve this.  What do you think of my
> > > >> version?
> > > >>
> > > >
> > > >> --8<--------------------------cut here-------------------------->8--
> > > >>
> > > >> From: Ben Pfaff <blp at nicira.com>
> > > >> Date: Tue, 25 Jun 2013 17:00:56 -0700
> > > >> Subject: [PATCH] ovsdb-server: Add and remove databases during run
> > time.
> > > >>
> > > >> The commit allows a user to add a database file to a
> > > >> ovsdb-server during run time. One can also remove a
> > > >> database file from ovsdb-server's control.
> > > >>
> > > >> Feature #14595.
> > > >> Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
> > > >> Signed-off-by: Ben Pfaff <blp at nicira.com>
> > > >>
> > > > I am fine with this. There is still a reference for
> > reconfigure_from_db()
> > > > in a comment.
> > > > I pushed the first 2 patches of this series.
> > > >
> > > > (This commit will make add-db and remove-db needing different
> > arguments.
> > > > add-db needs
> > > > file name and remove-db needs schema name. I don't have any strong
> > > > feelings about it).
> > > >
> > > You will also need the following incremental.
> >
> > Thanks.  I folded that in as well as the following:
> >
> > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> > index 016dd33..5f80502 100644
> > --- a/ovsdb/jsonrpc-server.c
> > +++ b/ovsdb/jsonrpc-server.c
> > @@ -136,8 +136,16 @@ ovsdb_jsonrpc_server_add_db(struct
> > ovsdb_jsonrpc_server *svr, struct ovsdb *db)
> >   * true if successful, false if there is no database associated with
> > 'db'. */
> >  bool
> >  ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *svr,
> > -                                struct ovsdb *db)
> > +                               struct ovsdb *db)
> >  {
> > +    /* There might be pointers to 'db' from 'svr', such as monitors or
> > +     * outstanding transactions.  Disconnect all JSON-RPC connections to
> > avoid
> > +     * accesses to freed memory.
> > +     *
> > +     * If this is too big of a hammer in practice, we could be more
> > selective,
> > +     * e.g. disconnect only connections that actually reference 'db'. */
> > +    ovsdb_jsonrpc_server_reconnect(svr);
> > +
> >      return ovsdb_server_remove_db(&svr->up, db);
> >  }
> >
> Thanks for doing this. I think we will also need to do the same after
> adding a DB. The following incremental does the job for me,
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 3520ffc..9847d80 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -1095,6 +1095,7 @@ ovsdb_server_add_database(struct unixctl_conn *conn,
> int argc OVS_UNU
>      error = open_db(config, filename);
>      if (!error) {
>          save_config(config);
> +        ovsdb_jsonrpc_server_reconnect(config->jsonrpc);
>          unixctl_command_reply(conn, NULL);
>      } else {
>          unixctl_command_reply_error(conn, error);

Thanks.  I added this to ovsdb_jsonrpc_server_add_db() instead and
pushed:

    /* The OVSDB protocol doesn't have a way to notify a client that a
     * database has been added.  If some client tried to use the database
     * that we're adding and failed, then forcing it to reconnect seems like
     * a reasonable way to make it try again.
     *
     * If this is too big of a hammer in practice, we could be more selective,
     * e.g. disconnect only connections that actually tried to use a database
     * with 'db''s name. */
    ovsdb_jsonrpc_server_reconnect(svr);