On 3/27/23 21:42, Ilya Maximets wrote: > If the schema with no data was read from the clustered storage, it > should mean a database conversion request. In general, we can get: > > 1. Just data --> Transaction record. > 2. Schema + Data --> Database conversion or raft snapshot install. > 3. Just schema --> New. Database conversion request. > > We cannot distinguish between conversion and snapshot installation > request in the current implementation, so we will keep handling > conversion with data in the same way as before, i.e. if data is > provided, we should use it. > > ovsdb-tool is updated to handle this record type as well while > converting cluster to standalone. > > This change doesn't introduce a way for such records to appear in > the database. That will be added in the future commits targeting > conversion speed increase. > > Signed-off-by: Ilya Maximets <i.maxim...@ovn.org> > --- > ovsdb/ovsdb-server.c | 65 ++++++++++++++++++++++++++++++-------------- > ovsdb/ovsdb-tool.c | 35 ++++++++++++++++++------ > ovsdb/relay.c | 20 +++++++++++--- > ovsdb/relay.h | 7 +++-- > 4 files changed, 91 insertions(+), 36 deletions(-) > > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c > index 4fea2dbda..91c284e99 100644 > --- a/ovsdb/ovsdb-server.c > +++ b/ovsdb/ovsdb-server.c > @@ -573,8 +573,9 @@ close_db(struct server_config *config, struct db *db, > char *comment) > } > } > > -static void > -update_schema(struct ovsdb *db, const struct ovsdb_schema *schema, void *aux) > +static struct ovsdb_error * OVS_WARN_UNUSED_RESULT > +update_schema(struct ovsdb *db, const struct ovsdb_schema *schema, > + bool conversion_with_no_data, void *aux) > { > struct server_config *config = aux; > > @@ -586,13 +587,27 @@ update_schema(struct ovsdb *db, const struct > ovsdb_schema *schema, void *aux) > : xasprintf("database %s connected to storage", db->name))); > } > > - ovsdb_replace(db, ovsdb_create(ovsdb_schema_clone(schema), NULL)); > + if (db->schema && conversion_with_no_data) { > + struct ovsdb *new_db = NULL; > + struct ovsdb_error *error; > + > + error = ovsdb_convert(db, schema, &new_db); > + if (error) { > + /* Should never happen, because conversion should have been > + * checked before writing the schema to the storage. */ > + return error; > + } > + ovsdb_replace(db, new_db); > + } else { > + ovsdb_replace(db, ovsdb_create(ovsdb_schema_clone(schema), NULL)); > + } > > /* Force update to schema in _Server database. */ > struct db *dbp = shash_find_data(config->all_dbs, db->name); > if (dbp) { > dbp->row_uuid = UUID_ZERO; > } > + return NULL; > } > > static struct ovsdb_error * OVS_WARN_UNUSED_RESULT > @@ -600,23 +615,30 @@ parse_txn(struct server_config *config, struct db *db, > const struct ovsdb_schema *schema, const struct json *txn_json, > const struct uuid *txnid) > { > + struct ovsdb_error *error = NULL; > + struct ovsdb_txn *txn = NULL; > + > if (schema) { > - /* We're replacing the schema (and the data). Destroy the database > - * (first grabbing its storage), then replace it with the new schema. > - * The transaction must also include the replacement data. > + /* We're replacing the schema (and the data). If transaction > includes > + * replacement data, destroy the database (first grabbing its > storage), > + * then replace it with the new schema. If not, it's a conversion > + * without data specified. In this case, convert the current > database > + * to a new schema instead. > * > * Only clustered database schema changes and snapshot installs > * go through this path. > */ > - ovs_assert(txn_json); > ovs_assert(ovsdb_storage_is_clustered(db->db->storage)); > > - struct ovsdb_error *error = ovsdb_schema_check_for_ephemeral_columns( > - schema); > + error = ovsdb_schema_check_for_ephemeral_columns(schema); > + if (error) { > + return error; > + } > + > + error = update_schema(db->db, schema, txn_json == NULL, config); > if (error) { > return error; > } > - update_schema(db->db, schema, config); > } > > if (txn_json) { > @@ -624,24 +646,25 @@ parse_txn(struct server_config *config, struct db *db, > return ovsdb_error(NULL, "%s: data without schema", > db->filename); > } > > - struct ovsdb_txn *txn; > - struct ovsdb_error *error; > - > error = ovsdb_file_txn_from_json(db->db, txn_json, false, &txn); > - if (!error) { > - ovsdb_txn_set_txnid(txnid, txn); > - log_and_free_error(ovsdb_txn_replay_commit(txn)); > - } > - if (!error && !uuid_is_zero(txnid)) { > - db->db->prereq = *txnid; > - } > if (error) { > ovsdb_storage_unread(db->db->storage); > return error; > } > + } else if (schema) { > + /* We just performed conversion without data. Transaction history > + * was destroyed. Commit a dummy transaction to set the txnid. */ > + txn = ovsdb_txn_create(db->db); > } > > - return NULL; > + if (txn) { > + ovsdb_txn_set_txnid(txnid, txn); > + error = ovsdb_txn_replay_commit(txn); > + if (!error && !uuid_is_zero(txnid)) { > + db->db->prereq = *txnid; > + } > + } > + return error; > } > > static void > diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c > index ea2b75b46..e26536532 100644 > --- a/ovsdb/ovsdb-tool.c > +++ b/ovsdb/ovsdb-tool.c > @@ -1006,7 +1006,8 @@ raft_header_to_standalone_log(const struct raft_header > *h, > } > > static void > -raft_record_to_standalone_log(const struct raft_record *r, > +raft_record_to_standalone_log(const char *db_file_name, > + const struct raft_record *r, > struct ovsdb_log *db_log_data) > { > if (r->type == RAFT_REC_ENTRY) { > @@ -1024,15 +1025,30 @@ raft_record_to_standalone_log(const struct > raft_record *r, > > if (schema_json->type != JSON_NULL) { > /* This is a database conversion record. Reset the log and > - * write the new schema. Data JSON should also be part of > - * the conversion. */ > + * write the new schema. */ > struct ovsdb_schema *schema; > > + check_ovsdb_error(ovsdb_schema_from_json(schema_json, &schema)); > + > if (data_json->type == JSON_NULL) { > - ovs_fatal( > - 0, "Invalid database conversion in the log: no data"); > + /* We have a conversion request with no data. There is no > + * other way as to read back what we have and convert. */ > + struct ovsdb *old_db, *new_db; > + > + check_ovsdb_error(ovsdb_log_commit_block(db_log_data)); > + > + old_db = ovsdb_file_read(db_file_name, false); > + check_ovsdb_error(ovsdb_convert(old_db, schema, &new_db)); > + ovsdb_destroy(old_db); > + > + pa->elems[1] = ovsdb_to_txn_json( > + new_db, "converted by ovsdb-tool", true); > + ovsdb_destroy(new_db); > + > + json_destroy(data_json); > + data_json = pa->elems[1]; > } > - check_ovsdb_error(ovsdb_schema_from_json(schema_json, &schema)); > + > ovsdb_schema_destroy(schema); > check_ovsdb_error(ovsdb_log_reset(db_log_data)); > check_ovsdb_error(ovsdb_log_write(db_log_data, schema_json)); > @@ -1654,7 +1670,8 @@ do_compare_versions(struct ovs_cmdl_context *ctx) > } > > static void > -do_convert_to_standalone(struct ovsdb_log *log, struct ovsdb_log > *db_log_data) > +do_convert_to_standalone(const char *db_file_name, > + struct ovsdb_log *log, struct ovsdb_log > *db_log_data) > { > for (unsigned int i = 0; ; i++) { > struct json *json; > @@ -1671,7 +1688,7 @@ do_convert_to_standalone(struct ovsdb_log *log, struct > ovsdb_log *db_log_data) > } else { > struct raft_record r; > check_ovsdb_error(raft_record_from_json(&r, json)); > - raft_record_to_standalone_log(&r, db_log_data); > + raft_record_to_standalone_log(db_file_name, &r, db_log_data); > raft_record_uninit(&r); > } > json_destroy(json); > @@ -1694,7 +1711,7 @@ do_cluster_standalone(struct ovs_cmdl_context *ctx) > if (strcmp(ovsdb_log_get_magic(log), RAFT_MAGIC) != 0) { > ovs_fatal(0, "Database is not clustered db.\n"); > } > - do_convert_to_standalone(log, db_log_data); > + do_convert_to_standalone(db_file_name, log, db_log_data); > check_ovsdb_error(ovsdb_log_commit_block(db_log_data)); > ovsdb_log_close(db_log_data); > ovsdb_log_close(log); > diff --git a/ovsdb/relay.c b/ovsdb/relay.c > index 9ff6ed8f3..4a00f0a0a 100644 > --- a/ovsdb/relay.c > +++ b/ovsdb/relay.c > @@ -301,6 +301,8 @@ static void > ovsdb_relay_parse_update(struct relay_ctx *ctx, > const struct ovsdb_cs_update_event *update) > { > + struct ovsdb_error *error = NULL; > + > if (!ctx->db) { > return; > } > @@ -308,15 +310,25 @@ ovsdb_relay_parse_update(struct relay_ctx *ctx, > if (update->monitor_reply && ctx->new_schema) { > /* There was a schema change. Updating a database with a new schema > * before processing monitor reply with the new data. */ > - ctx->schema_change_cb(ctx->db, ctx->new_schema, > - ctx->schema_change_aux); > + error = ctx->schema_change_cb(ctx->db, ctx->new_schema, false, > + ctx->schema_change_aux); > + if (error) { > + /* Should never happen, but handle this case anyway. */ > + char *s = ovsdb_error_to_string_free(error); > + VLOG_ERR("%s", s);
Should this be VLOG_ERR_RL? Regardless: Acked-by: Dumitru Ceara <dce...@redhat.com> Thanks! _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev