Skip to content

Commit

Permalink
ovsdb: Preserve column diffs read from the storage.
Browse files Browse the repository at this point in the history 
Database file contains the column diff, but it is discarded once
the 'new' state of a row is constructed.  Keep it in the transaction
row, as it can be used later by other parts of the code.

Diffs do not live long, we keep them around only while transaction
is alive, so should not affect memory consumption.

Users for this data will be added in later commits.

Signed-off-by: Ilya Maximets <[email protected]>
Signed-off-by: 0-day Robot <[email protected]>
  • Loading branch information
@igsilya @ovsrobot
igsilya authored and ovsrobot committed Dec 18, 2023
1 parent 72d6e86 commit d625bd0
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 31 deletions.
14 changes: 10 additions & 4 deletions ovsdb/execution.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,11 @@ update_row_cb(const struct ovsdb_row *row, void *ur_)

ur->n_matches++;
if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) {
struct ovsdb_row *rw_row;

ovsdb_txn_row_modify(ur->txn, row, &rw_row, NULL);
ovsdb_error_assert(ovsdb_row_update_columns(
ovsdb_txn_row_modify(ur->txn, row),
ur->row, ur->columns, false));
rw_row, ur->row, ur->columns, false));
}

return true;
Expand Down Expand Up @@ -572,10 +574,14 @@ static bool
mutate_row_cb(const struct ovsdb_row *row, void *mr_)
{
struct mutate_row_cbdata *mr = mr_;
struct ovsdb_row *rw_row;

/* Not trying to track the row diff here, because user transactions
* may attempt to add duplicates or remove elements that do not exist. */
ovsdb_txn_row_modify(mr->txn, row, &rw_row, NULL);

mr->n_matches++;
*mr->error = ovsdb_mutation_set_execute(ovsdb_txn_row_modify(mr->txn, row),
mr->mutations);
*mr->error = ovsdb_mutation_set_execute(rw_row, mr->mutations);
return *mr->error == NULL;
}

Expand Down
22 changes: 16 additions & 6 deletions ovsdb/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ ovsdb_file_column_diff_disable(void)
}

static struct ovsdb_error *
ovsdb_file_update_row_from_json(struct ovsdb_row *row, bool converting,
bool row_contains_diff,
ovsdb_file_update_row_from_json(struct ovsdb_row *row, struct ovsdb_row *diff,
bool converting, bool row_contains_diff,
const struct json *json)
{
struct ovsdb_table_schema *schema = row->table->schema;
Expand Down Expand Up @@ -122,6 +122,12 @@ ovsdb_file_update_row_from_json(struct ovsdb_row *row, bool converting,
error = ovsdb_datum_apply_diff_in_place(
&row->fields[column->index],
&datum, &column->type);
if (!error && diff) {
ovs_assert(ovsdb_datum_is_default(&diff->fields[column->index],
&column->type));
ovsdb_datum_swap(&diff->fields[column->index], &datum);
}

ovsdb_datum_destroy(&datum, &column->type);
if (error) {
return error;
Expand Down Expand Up @@ -150,16 +156,20 @@ ovsdb_file_txn_row_from_json(struct ovsdb_txn *txn, struct ovsdb_table *table,
ovsdb_txn_row_delete(txn, row);
return NULL;
} else if (row) {
return ovsdb_file_update_row_from_json(ovsdb_txn_row_modify(txn, row),
converting, row_contains_diff,
json);
struct ovsdb_row *new, *diff = NULL;

ovsdb_txn_row_modify(txn, row, &new,
row_contains_diff ? &diff : NULL);
return ovsdb_file_update_row_from_json(new, diff, converting,
row_contains_diff, json);
} else {
struct ovsdb_error *error;
struct ovsdb_row *new;

new = ovsdb_row_create(table);
*ovsdb_row_get_uuid_rw(new) = *row_uuid;
error = ovsdb_file_update_row_from_json(new, converting, false, json);
error = ovsdb_file_update_row_from_json(new, NULL, converting,
false, json);
if (error) {
ovsdb_row_destroy(new);
} else {
Expand Down
7 changes: 5 additions & 2 deletions ovsdb/ovsdb-server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ update_remote_row(const struct ovsdb_row *row, struct ovsdb_txn *txn,
/* Bad remote spec or incorrect schema. */
return;
}
rw_row = ovsdb_txn_row_modify(txn, row);
ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
ovsdb_jsonrpc_server_get_remote_status(jsonrpc, target, &status);

/* Update status information columns. */
Expand Down Expand Up @@ -1301,7 +1301,10 @@ update_server_status(struct shash *all_dbs)
if (!db || !db->db) {
ovsdb_txn_row_delete(txn, row);
} else {
update_database_status(ovsdb_txn_row_modify(txn, row), db);
struct ovsdb_row *rw_row;

ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
update_database_status(rw_row, db);
}
}

Expand Down
6 changes: 4 additions & 2 deletions ovsdb/table.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,10 @@ ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid,
NULL, &columns, xor);

if (!error && (xor || !ovsdb_row_equal_columns(row, update, &columns))) {
error = ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row),
update, &columns, xor);
struct ovsdb_row *rw_row;

ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
error = ovsdb_row_update_columns(rw_row, update, &columns, xor);
}

ovsdb_column_set_destroy(&columns);
Expand Down
60 changes: 46 additions & 14 deletions ovsdb/transaction.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ struct ovsdb_txn_table {
* 'new'.
*
* - A row modified by a transaction will have non-null 'old' and 'new'.
* It may have non-null 'diff' as well in this case, but it is not
* necessary.
*
* - 'old' and 'new' both null indicates that a row was added then deleted
* within a single transaction. Most of the time we instead delete the
Expand All @@ -83,6 +85,7 @@ struct ovsdb_txn_row {
struct hmap_node hmap_node; /* In ovsdb_txn_table's txn_rows hmap. */
struct ovsdb_row *old; /* The old row. */
struct ovsdb_row *new; /* The new row. */
struct ovsdb_row *diff; /* The difference between old and new. */
size_t n_refs; /* Number of remaining references. */

/* These members are the same as the corresponding members of 'old' or
Expand Down Expand Up @@ -155,6 +158,7 @@ ovsdb_txn_row_abort(struct ovsdb_txn *txn OVS_UNUSED,
{
struct ovsdb_row *old = txn_row->old;
struct ovsdb_row *new = txn_row->new;
struct ovsdb_row *diff = txn_row->diff;

ovsdb_txn_row_prefree(txn_row);
if (!old) {
Expand Down Expand Up @@ -184,6 +188,7 @@ ovsdb_txn_row_abort(struct ovsdb_txn *txn OVS_UNUSED,
}

ovsdb_row_destroy(new);
ovsdb_row_destroy(diff);
free(txn_row);

return NULL;
Expand Down Expand Up @@ -250,7 +255,10 @@ find_or_make_txn_row(struct ovsdb_txn *txn, const struct ovsdb_table *table,
if (!txn_row) {
const struct ovsdb_row *row = ovsdb_table_get_row(table, uuid);
if (row) {
txn_row = ovsdb_txn_row_modify(txn, row)->txn_row;
struct ovsdb_row *rw_row;

ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
txn_row = rw_row->txn_row;
}
}
return txn_row;
Expand Down Expand Up @@ -433,6 +441,7 @@ delete_garbage_row(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row)
return NULL;
}

ovsdb_row_destroy(txn_row->diff);
row = txn_row->new;
txn_row->new = NULL;
hmap_remove(&txn_row->table->rows, &row->hmap_node);
Expand Down Expand Up @@ -544,6 +553,7 @@ ovsdb_txn_row_commit(struct ovsdb_txn *txn OVS_UNUSED,
txn_row->new->n_refs = txn_row->n_refs;
}
ovsdb_row_destroy(txn_row->old);
ovsdb_row_destroy(txn_row->diff);
free(txn_row);

return NULL;
Expand Down Expand Up @@ -1178,6 +1188,7 @@ ovsdb_txn_destroy_cloned(struct ovsdb_txn *txn)
if (r->new) {
ovsdb_row_destroy(r->new);
}
ovs_assert(!r->diff);
hmap_remove(&t->txn_rows, &r->hmap_node);
free(r);
}
Expand Down Expand Up @@ -1439,7 +1450,8 @@ ovsdb_txn_create_txn_table(struct ovsdb_txn *txn, struct ovsdb_table *table)

static struct ovsdb_txn_row *
ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
const struct ovsdb_row *old_, struct ovsdb_row *new)
const struct ovsdb_row *old_, struct ovsdb_row *new,
struct ovsdb_row *diff)
{
const struct ovsdb_row *row = old_ ? old_ : new;
struct ovsdb_row *old = CONST_CAST(struct ovsdb_row *, old_);
Expand All @@ -1453,6 +1465,7 @@ ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
txn_row->table = row->table;
txn_row->old = old;
txn_row->new = new;
txn_row->diff = diff;
txn_row->n_refs = old ? old->n_refs : 0;
txn_row->serial = serial - 1;

Expand All @@ -1465,6 +1478,9 @@ ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
if (new) {
new->txn_row = txn_row;
}
if (diff) {
diff->txn_row = txn_row;
}

txn_table = ovsdb_txn_create_txn_table(txn, table);
hmap_insert(&txn_table->txn_rows, &txn_row->hmap_node,
Expand All @@ -1473,24 +1489,38 @@ ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
return txn_row;
}

struct ovsdb_row *
ovsdb_txn_row_modify(struct ovsdb_txn *txn, const struct ovsdb_row *ro_row_)
void
ovsdb_txn_row_modify(struct ovsdb_txn *txn, const struct ovsdb_row *ro_row_,
struct ovsdb_row **rw_row, struct ovsdb_row **diff)
{
struct ovsdb_row *ro_row = CONST_CAST(struct ovsdb_row *, ro_row_);

ovs_assert(ro_row);
ovs_assert(rw_row);

if (ro_row->txn_row) {
ovs_assert(ro_row == ro_row->txn_row->new);
return ro_row;
*rw_row = ro_row;
if (diff) {
*diff = ro_row->txn_row->diff;
} else {
/* Caller will modify the row without updating the diff.
* Destroy the existing diff, if any, so it will not be
* used for this row anymore. Modification will always
* return NULL from this point on. */
ovsdb_row_destroy(ro_row->txn_row->diff);
ro_row->txn_row->diff = NULL;
}
} else {
struct ovsdb_table *table = ro_row->table;
struct ovsdb_row *rw_row;

rw_row = ovsdb_row_clone(ro_row);
rw_row->n_refs = ro_row->n_refs;
ovsdb_txn_row_create(txn, table, ro_row, rw_row);
hmap_replace(&table->rows, &ro_row->hmap_node, &rw_row->hmap_node);

return rw_row;
*rw_row = ovsdb_row_clone(ro_row);
(*rw_row)->n_refs = ro_row->n_refs;
if (diff) {
*diff = ovsdb_row_create(table);
}
ovsdb_txn_row_create(txn, table, ro_row, *rw_row, diff ? *diff : NULL);
hmap_replace(&table->rows, &ro_row->hmap_node, &(*rw_row)->hmap_node);
}
}

Expand All @@ -1502,7 +1532,7 @@ ovsdb_txn_row_insert(struct ovsdb_txn *txn, struct ovsdb_row *row)

uuid_generate(ovsdb_row_get_version_rw(row));

ovsdb_txn_row_create(txn, table, NULL, row);
ovsdb_txn_row_create(txn, table, NULL, row, NULL);
hmap_insert(&table->rows, &row->hmap_node, hash);
}

Expand All @@ -1518,9 +1548,11 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct ovsdb_row *row_)
hmap_remove(&table->rows, &row->hmap_node);

if (!txn_row) {
ovsdb_txn_row_create(txn, table, row, NULL);
ovsdb_txn_row_create(txn, table, row, NULL, NULL);
} else {
ovs_assert(txn_row->new == row);
ovsdb_row_destroy(txn_row->diff);
txn_row->diff = NULL;
if (txn_row->old) {
txn_row->new = NULL;
} else {
Expand Down
6 changes: 4 additions & 2 deletions ovsdb/transaction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

struct json;
struct ovsdb;
struct ovsdb_row;
struct ovsdb_schema;
struct ovsdb_table;
struct uuid;
Expand Down Expand Up @@ -50,8 +51,9 @@ const struct ovsdb_error *ovsdb_txn_progress_get_error(
const struct ovsdb_txn_progress *);
void ovsdb_txn_progress_destroy(struct ovsdb_txn_progress *);

struct ovsdb_row *ovsdb_txn_row_modify(struct ovsdb_txn *,
const struct ovsdb_row *);
void ovsdb_txn_row_modify(struct ovsdb_txn *, const struct ovsdb_row *,
struct ovsdb_row **row_new,
struct ovsdb_row **row_diff);
void ovsdb_txn_row_insert(struct ovsdb_txn *, struct ovsdb_row *);
void ovsdb_txn_row_delete(struct ovsdb_txn *, const struct ovsdb_row *);

Expand Down
2 changes: 1 addition & 1 deletion tests/test-ovsdb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,7 @@ do_transact_modify(struct ovs_cmdl_context *ctx)
struct ovsdb_row *row_rw;

row_ro = do_transact_find_row(ctx->argv[1]);
row_rw = ovsdb_txn_row_modify(do_transact_txn, row_ro);
ovsdb_txn_row_modify(do_transact_txn, row_ro, &row_rw, NULL);
do_transact_set_i_j(row_rw, ctx->argv[2], ctx->argv[3]);
}

Expand Down

0 comments on commit d625bd0

Please sign in to comment.