On 6/19/21 7:28 PM, Mark Gray wrote:
> 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 <[email protected]>
>> ---
>> 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?
I can do that, but that measn adding a CONST_CASTs in other places,
otherwise compiler would complain about discarded qualifier, like
in case of free():
ovsdb/storage.c:144:14: error: passing 'const char *' to parameter
of type 'void *' discards qualifiers
free(storage->unbacked_name);
^~~~~~~~~~~~~~~~~~~~~~
So, I'm not sure it it worth complications.
>
>> /* 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?
This is a NULL in most cases, because allocated with xzalloc and
non-NULL for relays. But there is no problem with that.
free(NULL) is a no-op according to the C standard and not checking
for NULL is encouraged by the coding style.
>
>> 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_storage_close() is called as part of ovsdb_destroy, so it's
tested.
>>
>> 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);
>>
>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
