[ovs-dev] [PATCH 08/23] db-ctl-base: Don't die in ctl_get_row() on error.

Jakub Sitnicki jkbs at redhat.com
Mon Jul 2 10:50:04 UTC 2018


Return the error message to the caller instead of reporting it and dying
so that the caller can handle the error without terminating the process
if needed.

Signed-off-by: Jakub Sitnicki <jkbs at redhat.com>
---
 lib/db-ctl-base.c         | 49 +++++++++++++++++++++++++++--------------------
 lib/db-ctl-base.h         |  7 +++----
 ovn/utilities/ovn-sbctl.c | 11 ++++++++---
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 34daaef68..431db91c3 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -366,14 +366,16 @@ get_row_by_id(struct ctl_context *ctx,
     return final;
 }
 
-const struct ovsdb_idl_row *
+char * OVS_WARN_UNUSED_RESULT
 ctl_get_row(struct ctl_context *ctx,
             const struct ovsdb_idl_table_class *table, const char *record_id,
-            bool must_exist)
+            bool must_exist, const struct ovsdb_idl_row **rowp)
 {
     const struct ovsdb_idl_row *row = NULL;
     struct uuid uuid;
 
+    ovs_assert(rowp);
+
     if (uuid_from_string(&uuid, record_id)) {
         row = ovsdb_idl_get_row_for_uuid(ctx->idl, table, &uuid);
     }
@@ -398,8 +400,8 @@ ctl_get_row(struct ctl_context *ctx,
                     ovsdb_idl_get_class(ctx->idl);
                 const struct ovsdb_idl_table_class *table_class =
                     ovsdb_idl_table_class_from_column(class, id->name_column);
-                ctl_fatal("multiple rows in %s match \"%s\"",
-                          table_class->name, record_id);
+                return xasprintf("multiple rows in %s match \"%s\"",
+                                 table_class->name, record_id);
             }
             if (row) {
                 break;
@@ -415,19 +417,21 @@ ctl_get_row(struct ctl_context *ctx,
                 if (!row) {
                     row = r;
                 } else {
-                    ctl_fatal("%s contains 2 or more rows whose UUIDs begin "
-                              "with %s: at least "UUID_FMT" and "UUID_FMT,
-                              table->name, record_id,
-                              UUID_ARGS(&row->uuid),
-                              UUID_ARGS(&r->uuid));
+                    return xasprintf("%s contains 2 or more rows whose UUIDs "
+                                     "begin with %s: at least "UUID_FMT" "
+                                     "and "UUID_FMT, table->name, record_id,
+                                     UUID_ARGS(&row->uuid),
+                                     UUID_ARGS(&r->uuid));
                 }
             }
         }
     }
     if (must_exist && !row) {
-        ctl_fatal("no row \"%s\" in table %s", record_id, table->name);
+        return xasprintf("no row \"%s\" in table %s", record_id, table->name);
     }
-    return row;
+
+    *rowp = row;
+    return NULL;
 }
 
 static char *
@@ -888,7 +892,7 @@ cmd_get(struct ctl_context *ctx)
     }
 
     die_if_error(get_table(table_name, &table));
-    row = ctl_get_row(ctx, table, record_id, must_exist);
+    die_if_error(ctl_get_row(ctx, table, record_id, must_exist, &row));
     if (!row) {
         return;
     }
@@ -1123,8 +1127,11 @@ cmd_list(struct ctl_context *ctx)
     out = ctx->table = list_make_table(columns, n_columns);
     if (ctx->argc > 2) {
         for (i = 2; i < ctx->argc; i++) {
-            list_record(ctl_get_row(ctx, table, ctx->argv[i], must_exist),
-                        columns, n_columns, out);
+            const struct ovsdb_idl_row *row;
+
+            die_if_error(ctl_get_row(ctx, table, ctx->argv[i], must_exist,
+                                     &row));
+            list_record(row, columns, n_columns, out);
         }
     } else {
         const struct ovsdb_idl_row *row;
@@ -1315,7 +1322,7 @@ cmd_set(struct ctl_context *ctx)
     int i;
 
     die_if_error(get_table(table_name, &table));
-    row = ctl_get_row(ctx, table, record_id, must_exist);
+    die_if_error(ctl_get_row(ctx, table, record_id, must_exist, &row));
     if (!row) {
         return;
     }
@@ -1355,7 +1362,7 @@ cmd_add(struct ctl_context *ctx)
 
     die_if_error(get_table(table_name, &table));
     die_if_error(get_column(table, column_name, &column));
-    row = ctl_get_row(ctx, table, record_id, must_exist);
+    die_if_error(ctl_get_row(ctx, table, record_id, must_exist, &row));
     if (!row) {
         return;
     }
@@ -1416,7 +1423,7 @@ cmd_remove(struct ctl_context *ctx)
 
     die_if_error(get_table(table_name, &table));
     die_if_error(get_column(table, column_name, &column));
-    row = ctl_get_row(ctx, table, record_id, must_exist);
+    die_if_error(ctl_get_row(ctx, table, record_id, must_exist, &row));
     if (!row) {
         return;
     }
@@ -1487,7 +1494,7 @@ cmd_clear(struct ctl_context *ctx)
     int i;
 
     die_if_error(get_table(table_name, &table));
-    row = ctl_get_row(ctx, table, record_id, must_exist);
+    die_if_error(ctl_get_row(ctx, table, record_id, must_exist, &row));
     if (!row) {
         return;
     }
@@ -1628,7 +1635,8 @@ cmd_destroy(struct ctl_context *ctx)
         for (i = 2; i < ctx->argc; i++) {
             const struct ovsdb_idl_row *row;
 
-            row = ctl_get_row(ctx, table, ctx->argv[i], must_exist);
+            die_if_error(ctl_get_row(ctx, table, ctx->argv[i], must_exist,
+                                     &row));
             if (row) {
                 ovsdb_idl_txn_delete(row);
             }
@@ -1661,8 +1669,7 @@ cmd_wait_until(struct ctl_context *ctx)
     int i;
 
     die_if_error(get_table(table_name, &table));
-
-    row = ctl_get_row(ctx, table, record_id, false);
+    die_if_error(ctl_get_row(ctx, table, record_id, false, &row));
     if (!row) {
         ctx->try_again = true;
         return;
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index 6aa3fe62d..5599666f2 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -272,10 +272,9 @@ struct ctl_table_class {
     struct ctl_row_id row_ids[4];
 };
 
-const struct ovsdb_idl_row *ctl_get_row(struct ctl_context *,
-                                        const struct ovsdb_idl_table_class *,
-                                        const char *record_id,
-                                        bool must_exist);
+char *ctl_get_row(struct ctl_context *, const struct ovsdb_idl_table_class *,
+                  const char *record_id, bool must_exist,
+                  const struct ovsdb_idl_row **);
 
 void ctl_set_column(const char *table_name,
                     const struct ovsdb_idl_row *, const char *arg,
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index 085f7c5c0..0e2c67c08 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -842,9 +842,14 @@ cmd_lflow_list(struct ctl_context *ctx)
 {
     const struct sbrec_datapath_binding *datapath = NULL;
     if (ctx->argc > 1) {
-        datapath = (const struct sbrec_datapath_binding *)
-            ctl_get_row(ctx, &sbrec_table_datapath_binding,
-                        ctx->argv[1], false);
+        const struct ovsdb_idl_row *row;
+        char *error = ctl_get_row(ctx, &sbrec_table_datapath_binding,
+                                  ctx->argv[1], false, &row);
+        if (error) {
+            ctl_fatal("%s", error);
+        }
+
+        datapath = (const struct sbrec_datapath_binding *)row;
         if (datapath) {
             ctx->argc--;
             ctx->argv++;
-- 
2.14.4



More information about the dev mailing list