[ovs-dev] [PATCH v2 2/9] ovsdb: storage: Allow setting the name for the unbacked storage.
Mark Gray
mark.d.gray at redhat.com
Sat Jun 19 17:28:18 UTC 2021
On 12/06/2021 03:00, Ilya Maximets wrote:
> ovsdb_create() requires schema or storage to be nonnull, but in
> practice it requires to have schema name or a storage name to
> use it as a database name. Only clustered storage has a name.
> This means that only clustered database can be created without
> schema, Changing that by allowing unbacked storage to have a
> name. This way we can create database with unbacked storage
> without schema. Will be used in next commits to create database
> for ovsdb 'relay' service model.
>
> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> ---
> ovsdb/file.c | 2 +-
> ovsdb/ovsdb-server.c | 2 +-
> ovsdb/storage.c | 13 ++++++++++---
> ovsdb/storage.h | 2 +-
> tests/test-ovsdb.c | 6 +++---
> 5 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/ovsdb/file.c b/ovsdb/file.c
> index 0b8bdfe37..59220824f 100644
> --- a/ovsdb/file.c
> +++ b/ovsdb/file.c
> @@ -318,7 +318,7 @@ ovsdb_convert(const struct ovsdb *src, const struct ovsdb_schema *new_schema,
> struct ovsdb **dstp)
> {
> struct ovsdb *dst = ovsdb_create(ovsdb_schema_clone(new_schema),
> - ovsdb_storage_create_unbacked());
> + ovsdb_storage_create_unbacked(NULL));
> struct ovsdb_txn *txn = ovsdb_txn_create(dst);
> struct ovsdb_error *error = NULL;
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index b09232c65..23bd226a3 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -738,7 +738,7 @@ add_server_db(struct server_config *config)
> /* We don't need txn_history for server_db. */
>
> db->filename = xstrdup("<internal>");
> - db->db = ovsdb_create(schema, ovsdb_storage_create_unbacked());
> + db->db = ovsdb_create(schema, ovsdb_storage_create_unbacked(NULL));
> bool ok OVS_UNUSED = ovsdb_jsonrpc_server_add_db(config->jsonrpc, db->db);
> ovs_assert(ok);
> add_db(config, db);
> diff --git a/ovsdb/storage.c b/ovsdb/storage.c
> index 40415fcf6..d727b1eac 100644
> --- a/ovsdb/storage.c
> +++ b/ovsdb/storage.c
> @@ -45,6 +45,8 @@ struct ovsdb_storage {
> struct ovsdb_log *log;
> struct raft *raft;
>
> + char *unbacked_name; /* Name of the unbacked storage. */
> +
Bit of a nit but should this be a "const char *" type?
> /* All kinds of storage. */
> struct ovsdb_error *error; /* If nonnull, a permanent error. */
> long long next_snapshot_min; /* Earliest time to take next snapshot. */
> @@ -121,12 +123,14 @@ ovsdb_storage_open_standalone(const char *filename, bool rw)
> }
>
> /* Creates and returns new storage without any backing. Nothing will be read
> - * from the storage, and writes are discarded. */
> + * from the storage, and writes are discarded. If 'name' is nonnull, it will
> + * be used as a storage name. */
> struct ovsdb_storage *
> -ovsdb_storage_create_unbacked(void)
> +ovsdb_storage_create_unbacked(const char *name)
> {
> struct ovsdb_storage *storage = xzalloc(sizeof *storage);
> schedule_next_snapshot(storage, false);
> + storage->unbacked_name = nullable_xstrdup(name);
> return storage;
> }
>
> @@ -137,6 +141,7 @@ ovsdb_storage_close(struct ovsdb_storage *storage)
> ovsdb_log_close(storage->log);
> raft_close(storage->raft);
> ovsdb_error_destroy(storage->error);
> + free(storage->unbacked_name);
Can this ^ not be NULL?
> free(storage);
> }
> }
> @@ -230,7 +235,9 @@ ovsdb_storage_wait(struct ovsdb_storage *storage)
> const char *
> ovsdb_storage_get_name(const struct ovsdb_storage *storage)
> {
> - return storage->raft ? raft_get_name(storage->raft) : NULL;
> + return storage->unbacked_name ? storage->unbacked_name
> + : storage->raft ? raft_get_name(storage->raft)
> + : NULL;
> }
>
> /* Attempts to read a log record from 'storage'.
> diff --git a/ovsdb/storage.h b/ovsdb/storage.h
> index 02b6e7e6c..e120094d7 100644
> --- a/ovsdb/storage.h
> +++ b/ovsdb/storage.h
> @@ -29,7 +29,7 @@ struct uuid;
> struct ovsdb_error *ovsdb_storage_open(const char *filename, bool rw,
> struct ovsdb_storage **)
> OVS_WARN_UNUSED_RESULT;
> -struct ovsdb_storage *ovsdb_storage_create_unbacked(void);
> +struct ovsdb_storage *ovsdb_storage_create_unbacked(const char *name);
> void ovsdb_storage_close(struct ovsdb_storage *);
>
> const char *ovsdb_storage_get_model(const struct ovsdb_storage *);
> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> index a886f971e..fb6b3acca 100644
> --- a/tests/test-ovsdb.c
> +++ b/tests/test-ovsdb.c
> @@ -1485,7 +1485,7 @@ do_execute__(struct ovs_cmdl_context *ctx, bool ro)
> json = parse_json(ctx->argv[1]);
> check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
> json_destroy(json);
> - db = ovsdb_create(schema, ovsdb_storage_create_unbacked());
> + db = ovsdb_create(schema, ovsdb_storage_create_unbacked(NULL));
>
> for (i = 2; i < ctx->argc; i++) {
> struct json *params, *result;
> @@ -1551,7 +1551,7 @@ do_trigger(struct ovs_cmdl_context *ctx)
> json = parse_json(ctx->argv[1]);
> check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
> json_destroy(json);
> - db = ovsdb_create(schema, ovsdb_storage_create_unbacked());
> + db = ovsdb_create(schema, ovsdb_storage_create_unbacked(NULL));
I think destroying the ovsdb_storage object will trigger a segfault.
Maybe you should add a call to ovsdb_storage_close(struct ovsdb_storage
*storage) in order to test that path.
>
> ovsdb_server_init(&server);
> ovsdb_server_add_db(&server, db);
> @@ -1781,7 +1781,7 @@ do_transact(struct ovs_cmdl_context *ctx)
> " \"j\": {\"type\": \"integer\"}}}}}");
> check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
> json_destroy(json);
> - do_transact_db = ovsdb_create(schema, ovsdb_storage_create_unbacked());
> + do_transact_db = ovsdb_create(schema, ovsdb_storage_create_unbacked(NULL));
> do_transact_table = ovsdb_get_table(do_transact_db, "mytable");
> ovs_assert(do_transact_table != NULL);
>
>
More information about the dev
mailing list