[ovs-dev] ovn-ctl: Initialize the databases.
diff mbox

Message ID 1474266412-31329-1-git-send-email-guru@ovn.org
State Superseded
Headers show

Commit Message

Gurucharan Shetty Sept. 19, 2016, 6:26 a.m. UTC
Without initializing the databases, commands of the form
'ovn-nbctl --wait=sb ls-add ls0' will simply hang.

Signed-off-by: Gurucharan Shetty <guru@ovn.org>
---
 ovn/utilities/ovn-ctl | 2 ++
 1 file changed, 2 insertions(+)

Comments

Ben Pfaff Sept. 19, 2016, 6:08 p.m. UTC | #1
On Sun, Sep 18, 2016 at 11:26:52PM -0700, Gurucharan Shetty wrote:
> Without initializing the databases, commands of the form
> 'ovn-nbctl --wait=sb ls-add ls0' will simply hang.
> 
> Signed-off-by: Gurucharan Shetty <guru@ovn.org>

It's better if the separate initialization step is optional.  I think
that ovn-nbctl and ovn-sbctl should be initializing the databases
automatically when any command runs, if they are not already
initialized; that's what ovs-vsctl does, at least.  Is that missing?
Gurucharan Shetty Sept. 19, 2016, 6:18 p.m. UTC | #2
On 19 September 2016 at 11:08, Ben Pfaff <blp@ovn.org> wrote:

> On Sun, Sep 18, 2016 at 11:26:52PM -0700, Gurucharan Shetty wrote:
> > Without initializing the databases, commands of the form
> > 'ovn-nbctl --wait=sb ls-add ls0' will simply hang.
> >
> > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
>
> It's better if the separate initialization step is optional.

We do this everytime for OVS in do_start_ovsdb of utilities/ovs-ctl.in with:
ovs_vsctl -- init -- set Open_vSwitch . db-version="$schemaver"

Is there a subtlety here?



>   I think
> that ovn-nbctl and ovn-sbctl should be initializing the databases
> automatically when any command runs, if they are not already
> initialized; that's what ovs-vsctl does, at least.  Is that missing?
>

It does initialize when a command is run. But no-one runs ovn-sbctl
commands right (unless for debugging)?. So any ovn-nbctl command that is
run with --wait=sb will hang.
Ben Pfaff Sept. 19, 2016, 6:33 p.m. UTC | #3
On Mon, Sep 19, 2016 at 11:18:20AM -0700, Guru Shetty wrote:
> On 19 September 2016 at 11:08, Ben Pfaff <blp@ovn.org> wrote:
> 
> > On Sun, Sep 18, 2016 at 11:26:52PM -0700, Gurucharan Shetty wrote:
> > > Without initializing the databases, commands of the form
> > > 'ovn-nbctl --wait=sb ls-add ls0' will simply hang.
> > >
> > > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> >
> > It's better if the separate initialization step is optional.
> 
> We do this everytime for OVS in do_start_ovsdb of utilities/ovs-ctl.in with:
> ovs_vsctl -- init -- set Open_vSwitch . db-version="$schemaver"
> 
> Is there a subtlety here?

Setting the db-version isn't something that happens automatically, but
the ovs-vsctl "init" command is a no-op, literally an empty function,
because do_vsctl() always does what it would do:

    ovs = ovsrec_open_vswitch_first(idl);
    if (!ovs) {
        /* XXX add verification that table is empty */
        ovs = ovsrec_open_vswitch_insert(txn);
    }

So the first call to ovs-vsctl, regardless of what command it is, should
initialize the database enough for everything else to work.

It looks to me like ovn-sbctl and ovn-nbctl are the same way.

(Don't get me wrong, I think it's better if the databases are
initialized explicitly.  But I also think it's nice if stuff doesn't
hang if, for example, a database gets blanked out through data loss or
admin mistake.)

> >   I think
> > that ovn-nbctl and ovn-sbctl should be initializing the databases
> > automatically when any command runs, if they are not already
> > initialized; that's what ovs-vsctl does, at least.  Is that missing?
> >
> 
> It does initialize when a command is run. But no-one runs ovn-sbctl
> commands right (unless for debugging)?. So any ovn-nbctl command that is
> run with --wait=sb will hang.

I see.

I think that we can make ovn-northd do the initialization too.  It's
pretty trivial: just create an {NB,SB}_Global record if there's none at
the moment.  Then if the databases get cleared, the system recovers.

So: I think that we should initialize the databases, but I also think
that ovn-northd should be capable of it too.
Gurucharan Shetty Sept. 19, 2016, 8:57 p.m. UTC | #4
On 19 September 2016 at 11:33, Ben Pfaff <blp@ovn.org> wrote:

> On Mon, Sep 19, 2016 at 11:18:20AM -0700, Guru Shetty wrote:
> > On 19 September 2016 at 11:08, Ben Pfaff <blp@ovn.org> wrote:
> >
> > > On Sun, Sep 18, 2016 at 11:26:52PM -0700, Gurucharan Shetty wrote:
> > > > Without initializing the databases, commands of the form
> > > > 'ovn-nbctl --wait=sb ls-add ls0' will simply hang.
> > > >
> > > > Signed-off-by: Gurucharan Shetty <guru@ovn.org>
> > >
> > > It's better if the separate initialization step is optional.
> >
> > We do this everytime for OVS in do_start_ovsdb of utilities/ovs-ctl.in
> with:
> > ovs_vsctl -- init -- set Open_vSwitch . db-version="$schemaver"
> >
> > Is there a subtlety here?
>
> Setting the db-version isn't something that happens automatically, but
> the ovs-vsctl "init" command is a no-op, literally an empty function,
> because do_vsctl() always does what it would do:
>
>     ovs = ovsrec_open_vswitch_first(idl);
>     if (!ovs) {
>         /* XXX add verification that table is empty */
>         ovs = ovsrec_open_vswitch_insert(txn);
>     }
>
> So the first call to ovs-vsctl, regardless of what command it is, should
> initialize the database enough for everything else to work.
>
> It looks to me like ovn-sbctl and ovn-nbctl are the same way.
>
> (Don't get me wrong, I think it's better if the databases are
> initialized explicitly.  But I also think it's nice if stuff doesn't
> hang if, for example, a database gets blanked out through data loss or
> admin mistake.)
>
> > >   I think
> > > that ovn-nbctl and ovn-sbctl should be initializing the databases
> > > automatically when any command runs, if they are not already
> > > initialized; that's what ovs-vsctl does, at least.  Is that missing?
> > >
> >
> > It does initialize when a command is run. But no-one runs ovn-sbctl
> > commands right (unless for debugging)?. So any ovn-nbctl command that is
> > run with --wait=sb will hang.
>
> I see.
>
> I think that we can make ovn-northd do the initialization too.  It's
> pretty trivial: just create an {NB,SB}_Global record if there's none at
> the moment.  Then if the databases get cleared, the system recovers.
>
> So: I think that we should initialize the databases, but I also think
> that ovn-northd should be capable of it too.
>

I think I understand what you are saying. I sent a v2 based on that.


> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>

Patch
diff mbox

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 07bff8a..8544d14 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -55,6 +55,7 @@  start_ovsdb () {
         set "$@" --detach --monitor $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE --remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR --pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
 
         $@ $DB_NB_FILE
+        ovn-nbctl init
     fi
 
     # Check and eventually start ovsdb-server for Southbound DB
@@ -65,6 +66,7 @@  start_ovsdb () {
 
         set "$@" --detach --monitor $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE --remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR --pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
         $@ $DB_SB_FILE
+        ovn-sbctl init
     fi
 }