[ovs-dev,4/4] ovn-ctl: Ability to upgrade databases.
diff mbox

Message ID 1443737396-2585-4-git-send-email-gshetty@nicira.com
State Superseded
Headers show

Commit Message

Gurucharan Shetty Oct. 1, 2015, 10:09 p.m. UTC
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
---
 ovn/utilities/ovn-ctl | 40 ++++++++++++++++++++--------------------
 utilities/ovs-ctl.in  | 41 +----------------------------------------
 utilities/ovs-lib.in  | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 60 deletions(-)

Comments

Ben Pfaff Oct. 2, 2015, 1:21 p.m. UTC | #1
On Thu, Oct 01, 2015 at 03:09:56PM -0700, Gurucharan Shetty wrote:
> Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>

In upgrade_ovn_dbs, I'd consider upgrading both databases even if they
don't appear in the list of databases.  It seems more robust to me,
especially given this comment in start_northd:

 # We expect ovn-northd to be co-located with ovsdb-server handling both the
 # OVN_Northbound and OVN_Southbound dbs.

I'm not sure about error handling.  If an upgrade to one database fails,
it might be better to try to upgrade the other database instead of
exiting immediately.

Acked-by: Ben Pfaff <blp@nicira.com>
Gurucharan Shetty Oct. 2, 2015, 2:07 p.m. UTC | #2
On Fri, Oct 2, 2015 at 6:21 AM, Ben Pfaff <blp@nicira.com> wrote:
> On Thu, Oct 01, 2015 at 03:09:56PM -0700, Gurucharan Shetty wrote:
>> Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
>
> In upgrade_ovn_dbs, I'd consider upgrading both databases even if they
> don't appear in the list of databases.  It seems more robust to me,
> especially given this comment in start_northd:

Agree. This patch did have a bug for first time start which gets fixed
with your suggestion.

>
>  # We expect ovn-northd to be co-located with ovsdb-server handling both the
>  # OVN_Northbound and OVN_Southbound dbs.
>
> I'm not sure about error handling.  If an upgrade to one database fails,
> it might be better to try to upgrade the other database instead of
> exiting immediately.

Okay. Will do.

>
> Acked-by: Ben Pfaff <blp@nicira.com>

Patch
diff mbox

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 9f16021..69d4db8 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -30,31 +30,31 @@  done
 ## start ##
 ## ----- ##
 
-check_ovn_dbs () {
-    if test ! -e "$DB_NB_FILE"; then
-        create_db "$DB_NB_FILE" "$DB_NB_SCHEMA"
-    fi
-
-    if test ! -e "$DB_SB_FILE"; then
-        create_db "$DB_SB_FILE" "$DB_SB_SCHEMA"
-    fi
-
-    running_ovn_dbs=$(ovs-appctl -t ovsdb-server ovsdb-server/list-dbs | grep OVN | wc -l)
-    if [ "$running_ovn_dbs" != "2" ] ; then
-        ovs-appctl -t ovsdb-server ovsdb-server/add-db $DB_NB_FILE
-        ovs-appctl -t ovsdb-server ovsdb-server/add-db $DB_SB_FILE
-        running_ovn_dbs=$(ovs-appctl -t ovsdb-server ovsdb-server/list-dbs | grep OVN | wc -l)
-        if [ "$running_ovn_dbs" != "2" ] ; then
-            echo >&2 "$0: Failed to add OVN dbs to ovsdb-server"
-            exit 1
-        fi
-    fi
+upgrade_ovn_dbs () {
+    ovn_dbs=$(ovs-appctl -t ovsdb-server ovsdb-server/list-dbs 2>/dev/null)
+    for db in $ovn_dbs; do
+        case $db in
+            OVN_Northbound)
+                action "Removing $db from ovsdb-server" \
+                    ovs-appctl -t ovsdb-server ovsdb-server/remove-db $db
+                upgrade_db "$DB_NB_FILE" "$DB_NB_SCHEMA" || exit 1
+                ;;
+            OVN_Southbound)
+                action "Removing $db from ovsdb-server" \
+                    ovs-appctl -t ovsdb-server ovsdb-server/remove-db $db
+                upgrade_db "$DB_SB_FILE" "$DB_SB_SCHEMA" || exit 1
+        esac
+    done
+    for db in $DB_NB_FILE $DB_SB_FILE; do
+        action "Adding $db to ovsdb-server" \
+            ovs-appctl -t ovsdb-server ovsdb-server/add-db $db || exit 1
+    done
 }
 
 start_northd () {
     # We expect ovn-northd to be co-located with ovsdb-server handling both the
     # OVN_Northbound and OVN_Southbound dbs.
-    check_ovn_dbs
+    upgrade_ovn_dbs
 
     set ovn-northd
     set "$@" -vconsole:emer -vsyslog:err -vfile:info
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 322e420..0082bed 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -76,45 +76,6 @@  ovs_vsctl () {
     ovs-vsctl --no-wait "$@"
 }
 
-upgrade_db () {
-    schemaver=`ovsdb_tool schema-version "$DB_SCHEMA"`
-    if test ! -e "$DB_FILE"; then
-        log_warning_msg "$DB_FILE does not exist"
-        install -d -m 755 -o root -g root `dirname $DB_FILE`
-        create_db "$DB_FILE" "$DB_SCHEMA"
-    elif test X"`ovsdb_tool needs-conversion "$DB_FILE" "$DB_SCHEMA"`" != Xno; then
-        # Back up the old version.
-        version=`ovsdb_tool db-version "$DB_FILE"`
-        cksum=`ovsdb_tool db-cksum "$DB_FILE" | awk '{print $1}'`
-        backup=$DB_FILE.backup$version-$cksum
-        action "Backing up database to $backup" cp "$DB_FILE" "$backup" || return 1
-
-        # Compact database.  This is important if the old schema did not enable
-        # garbage collection (i.e. if it did not have any tables with "isRoot":
-        # true) but the new schema does.  In that situation the old database
-        # may contain a transaction that creates a record followed by a
-        # transaction that creates the first use of the record.  Replaying that
-        # series of transactions against the new database schema (as "convert"
-        # does) would cause the record to be dropped by the first transaction,
-        # then the second transaction would cause a referential integrity
-        # failure (for a strong reference).
-        #
-        # Errors might occur on an Open vSwitch downgrade if ovsdb-tool doesn't
-        # understand some feature of the schema used in the OVSDB version that
-        # we're downgrading from, so we don't give up on error.
-        action "Compacting database" ovsdb_tool compact "$DB_FILE"
-
-        # Upgrade or downgrade schema.
-        if action "Converting database schema" ovsdb_tool convert "$DB_FILE" "$DB_SCHEMA"; then
-            :
-        else
-            log_warning_msg "Schema conversion failed, using empty database instead"
-            rm -f "$DB_FILE"
-            create_db "$DB_FILE" "$DB_SCHEMA"
-        fi
-    fi
-}
-
 set_system_ids () {
     set ovs_vsctl set Open_vSwitch .
 
@@ -182,7 +143,7 @@  start_ovsdb () {
         log_success_msg "ovsdb-server is already running"
     else
         # Create initial database or upgrade database schema.
-        upgrade_db || return 1
+        upgrade_db $DB_FILE $DB_SCHEMA || return 1
 
         # Start ovsdb-server.
         set ovsdb-server "$DB_FILE"
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index ef3150a..91e5d48 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -335,3 +335,45 @@  create_db () {
     DB_SCHEMA="$2"
     action "Creating empty database $DB_FILE" ovsdb_tool create "$DB_FILE" "$DB_SCHEMA"
 }
+
+upgrade_db () {
+    DB_FILE="$1"
+    DB_SCHEMA="$2"
+
+    schemaver=`ovsdb_tool schema-version "$DB_SCHEMA"`
+    if test ! -e "$DB_FILE"; then
+        log_warning_msg "$DB_FILE does not exist"
+        install -d -m 755 -o root -g root `dirname $DB_FILE`
+        create_db "$DB_FILE" "$DB_SCHEMA"
+    elif test X"`ovsdb_tool needs-conversion "$DB_FILE" "$DB_SCHEMA"`" != Xno; then
+        # Back up the old version.
+        version=`ovsdb_tool db-version "$DB_FILE"`
+        cksum=`ovsdb_tool db-cksum "$DB_FILE" | awk '{print $1}'`
+        backup=$DB_FILE.backup$version-$cksum
+        action "Backing up database to $backup" cp "$DB_FILE" "$backup" || return 1
+
+        # Compact database.  This is important if the old schema did not enable
+        # garbage collection (i.e. if it did not have any tables with "isRoot":
+        # true) but the new schema does.  In that situation the old database
+        # may contain a transaction that creates a record followed by a
+        # transaction that creates the first use of the record.  Replaying that
+        # series of transactions against the new database schema (as "convert"
+        # does) would cause the record to be dropped by the first transaction,
+        # then the second transaction would cause a referential integrity
+        # failure (for a strong reference).
+        #
+        # Errors might occur on an Open vSwitch downgrade if ovsdb-tool doesn't
+        # understand some feature of the schema used in the OVSDB version that
+        # we're downgrading from, so we don't give up on error.
+        action "Compacting database" ovsdb_tool compact "$DB_FILE"
+
+        # Upgrade or downgrade schema.
+        if action "Converting database schema" ovsdb_tool convert "$DB_FILE" "$DB_SCHEMA"; then
+            :
+        else
+            log_warning_msg "Schema conversion failed, using empty database instead"
+            rm -f "$DB_FILE"
+			create_db "$DB_FILE" "$DB_SCHEMA"
+        fi
+    fi
+}