diff --git a/ovsdb/execution.c b/ovsdb/execution.c index 5587ef96f..8c20c3b54 100644 --- a/ovsdb/execution.c +++ b/ovsdb/execution.c @@ -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; @@ -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; } diff --git a/ovsdb/file.c b/ovsdb/file.c index 8bd1d4af3..77a89fd1a 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -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; @@ -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; @@ -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 { diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 4d29043f4..dbf85fe3b 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -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. */ @@ -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); } } diff --git a/ovsdb/table.c b/ovsdb/table.c index 0792e1580..3e89ddd44 100644 --- a/ovsdb/table.c +++ b/ovsdb/table.c @@ -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); diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index a482588a0..60c4e2acb 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -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 @@ -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 @@ -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) { @@ -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; @@ -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; @@ -433,6 +441,9 @@ delete_garbage_row(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) return NULL; } + ovsdb_row_destroy(txn_row->diff); + txn_row->diff = NULL; + row = txn_row->new; txn_row->new = NULL; hmap_remove(&txn_row->table->rows, &row->hmap_node); @@ -544,6 +555,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; @@ -1178,6 +1190,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); } @@ -1439,7 +1452,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_); @@ -1453,6 +1467,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; @@ -1465,6 +1480,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, @@ -1473,24 +1491,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); } } @@ -1502,7 +1534,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); } @@ -1518,9 +1550,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 { diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h index 0e054eef3..f659838dc 100644 --- a/ovsdb/transaction.h +++ b/ovsdb/transaction.h @@ -21,6 +21,7 @@ struct json; struct ovsdb; +struct ovsdb_row; struct ovsdb_schema; struct ovsdb_table; struct uuid; @@ -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 *); diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index d6a47de33..c4ab899d4 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -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]); }