| Message ID | 20210612020008.3944088-5-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OVSDB Relay Service Model. (Was: OVSDB 2-Tier deployment) | expand |
On 12/06/2021 03:00, Ilya Maximets wrote: > This will be used to apply update3 type updates to ovsdb tables > while processing updates for future ovsdb 'relay' service model. > > 'ovsdb_datum_apply_diff' is allowed to fail, so adding support > to return this error. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > ovsdb/execution.c | 5 +++-- > ovsdb/replication.c | 2 +- > ovsdb/row.c | 30 +++++++++++++++++++++++++----- > ovsdb/row.h | 6 ++++-- > ovsdb/table.c | 9 +++++---- > ovsdb/table.h | 2 +- > 6 files changed, 39 insertions(+), 15 deletions(-) > > diff --git a/ovsdb/execution.c b/ovsdb/execution.c > index 3a0dad5d0..f6150e944 100644 > --- a/ovsdb/execution.c > +++ b/ovsdb/execution.c > @@ -483,8 +483,9 @@ update_row_cb(const struct ovsdb_row *row, void *ur_) > > ur->n_matches++; > if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) { > - ovsdb_row_update_columns(ovsdb_txn_row_modify(ur->txn, row), > - ur->row, ur->columns); > + ovsdb_error_assert(ovsdb_row_update_columns( > + ovsdb_txn_row_modify(ur->txn, row), > + ur->row, ur->columns, false)); > } > > return true; > diff --git a/ovsdb/replication.c b/ovsdb/replication.c > index b755976b0..d8b56d813 100644 > --- a/ovsdb/replication.c > +++ b/ovsdb/replication.c > @@ -677,7 +677,7 @@ process_table_update(struct json *table_update, const char *table_name, > struct ovsdb_error *error; > error = (!new ? ovsdb_table_execute_delete(txn, &uuid, table) > : !old ? ovsdb_table_execute_insert(txn, &uuid, table, new) > - : ovsdb_table_execute_update(txn, &uuid, table, new)); > + : ovsdb_table_execute_update(txn, &uuid, table, new, false)); > if (error) { > if (!strcmp(ovsdb_error_get_tag(error), "consistency violation")) { > ovsdb_error_assert(error); > diff --git a/ovsdb/row.c b/ovsdb/row.c > index 755ab91a8..65a054621 100644 > --- a/ovsdb/row.c > +++ b/ovsdb/row.c > @@ -163,20 +163,40 @@ ovsdb_row_equal_columns(const struct ovsdb_row *a, > return true; > } > > -void > +struct ovsdb_error * > ovsdb_row_update_columns(struct ovsdb_row *dst, > const struct ovsdb_row *src, > - const struct ovsdb_column_set *columns) > + const struct ovsdb_column_set *columns, > + bool xor) > { > size_t i; > > for (i = 0; i < columns->n_columns; i++) { > const struct ovsdb_column *column = columns->columns[i]; > + struct ovsdb_datum xor_datum; > + struct ovsdb_error *error; > + > + if (xor) { > + error = ovsdb_datum_apply_diff(&xor_datum, > + &dst->fields[column->index], > + &src->fields[column->index], > + &column->type); > + if (error) { > + return error; > + } > + } > + > ovsdb_datum_destroy(&dst->fields[column->index], &column->type); > - ovsdb_datum_clone(&dst->fields[column->index], > - &src->fields[column->index], > - &column->type); Could you move ovsdb_datum_destroy(&dst->fields[column->index], &column->type) into the "else" clause below and then merge the "if" clause below into the "if" clause above? > + > + if (xor) { > + ovsdb_datum_swap(&dst->fields[column->index], &xor_datum); > + } else { > + ovsdb_datum_clone(&dst->fields[column->index], > + &src->fields[column->index], > + &column->type); > + } > } > + return NULL; > } > > /* Appends the string form of the value in 'row' of each of the columns in > diff --git a/ovsdb/row.h b/ovsdb/row.h > index 2c441b5a4..394ac8eb4 100644 > --- a/ovsdb/row.h > +++ b/ovsdb/row.h > @@ -82,8 +82,10 @@ bool ovsdb_row_equal_columns(const struct ovsdb_row *, > int ovsdb_row_compare_columns_3way(const struct ovsdb_row *, > const struct ovsdb_row *, > const struct ovsdb_column_set *); > -void ovsdb_row_update_columns(struct ovsdb_row *, const struct ovsdb_row *, > - const struct ovsdb_column_set *); > +struct ovsdb_error *ovsdb_row_update_columns(struct ovsdb_row *, > + const struct ovsdb_row *, > + const struct ovsdb_column_set *, > + bool xor); > void ovsdb_row_columns_to_string(const struct ovsdb_row *, > const struct ovsdb_column_set *, struct ds *); > struct ovsdb_error *ovsdb_row_from_json(struct ovsdb_row *, > diff --git a/ovsdb/table.c b/ovsdb/table.c > index 2935bd897..455a3663f 100644 > --- a/ovsdb/table.c > +++ b/ovsdb/table.c > @@ -384,7 +384,8 @@ ovsdb_table_execute_delete(struct ovsdb_txn *txn, const struct uuid *row_uuid, > > struct ovsdb_error * > ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid, > - struct ovsdb_table *table, struct json *json_row) > + struct ovsdb_table *table, struct json *json_row, > + bool xor) > { > const struct ovsdb_row *row = ovsdb_table_get_row(table, row_uuid); > if (!row) { > @@ -399,9 +400,9 @@ ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid, > struct ovsdb_error *error = ovsdb_row_from_json(update, json_row, > NULL, &columns); > > - if (!error && !ovsdb_row_equal_columns(row, update, &columns)) { > - ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row), > - update, &columns); > + if (!error && (xor || !ovsdb_row_equal_columns(row, update, &columns))) { > + error = ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row), > + update, &columns, xor); > } > > ovsdb_column_set_destroy(&columns); > diff --git a/ovsdb/table.h b/ovsdb/table.h > index e21ec7b31..ce69a5d13 100644 > --- a/ovsdb/table.h > +++ b/ovsdb/table.h > @@ -82,6 +82,6 @@ struct ovsdb_error *ovsdb_table_execute_delete(struct ovsdb_txn *txn, > struct ovsdb_error *ovsdb_table_execute_update(struct ovsdb_txn *txn, > const struct uuid *row_uuid, > struct ovsdb_table *table, > - struct json *new); > + struct json *new, bool xor); > > #endif /* ovsdb/table.h */ >
On 6/12/21 4:00 AM, Ilya Maximets wrote: > This will be used to apply update3 type updates to ovsdb tables > while processing updates for future ovsdb 'relay' service model. > > 'ovsdb_datum_apply_diff' is allowed to fail, so adding support > to return this error. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks!
On 6/19/21 8:01 PM, Mark Gray wrote: > On 12/06/2021 03:00, Ilya Maximets wrote: >> This will be used to apply update3 type updates to ovsdb tables >> while processing updates for future ovsdb 'relay' service model. >> >> 'ovsdb_datum_apply_diff' is allowed to fail, so adding support >> to return this error. >> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- >> ovsdb/execution.c | 5 +++-- >> ovsdb/replication.c | 2 +- >> ovsdb/row.c | 30 +++++++++++++++++++++++++----- >> ovsdb/row.h | 6 ++++-- >> ovsdb/table.c | 9 +++++---- >> ovsdb/table.h | 2 +- >> 6 files changed, 39 insertions(+), 15 deletions(-) >> >> diff --git a/ovsdb/execution.c b/ovsdb/execution.c >> index 3a0dad5d0..f6150e944 100644 >> --- a/ovsdb/execution.c >> +++ b/ovsdb/execution.c >> @@ -483,8 +483,9 @@ update_row_cb(const struct ovsdb_row *row, void *ur_) >> >> ur->n_matches++; >> if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) { >> - ovsdb_row_update_columns(ovsdb_txn_row_modify(ur->txn, row), >> - ur->row, ur->columns); >> + ovsdb_error_assert(ovsdb_row_update_columns( >> + ovsdb_txn_row_modify(ur->txn, row), >> + ur->row, ur->columns, false)); >> } >> >> return true; >> diff --git a/ovsdb/replication.c b/ovsdb/replication.c >> index b755976b0..d8b56d813 100644 >> --- a/ovsdb/replication.c >> +++ b/ovsdb/replication.c >> @@ -677,7 +677,7 @@ process_table_update(struct json *table_update, const char *table_name, >> struct ovsdb_error *error; >> error = (!new ? ovsdb_table_execute_delete(txn, &uuid, table) >> : !old ? ovsdb_table_execute_insert(txn, &uuid, table, new) >> - : ovsdb_table_execute_update(txn, &uuid, table, new)); >> + : ovsdb_table_execute_update(txn, &uuid, table, new, false)); >> if (error) { >> if (!strcmp(ovsdb_error_get_tag(error), "consistency violation")) { >> ovsdb_error_assert(error); >> diff --git a/ovsdb/row.c b/ovsdb/row.c >> index 755ab91a8..65a054621 100644 >> --- a/ovsdb/row.c >> +++ b/ovsdb/row.c >> @@ -163,20 +163,40 @@ ovsdb_row_equal_columns(const struct ovsdb_row *a, >> return true; >> } >> >> -void >> +struct ovsdb_error * >> ovsdb_row_update_columns(struct ovsdb_row *dst, >> const struct ovsdb_row *src, >> - const struct ovsdb_column_set *columns) >> + const struct ovsdb_column_set *columns, >> + bool xor) >> { >> size_t i; >> >> for (i = 0; i < columns->n_columns; i++) { >> const struct ovsdb_column *column = columns->columns[i]; >> + struct ovsdb_datum xor_datum; >> + struct ovsdb_error *error; >> + >> + if (xor) { >> + error = ovsdb_datum_apply_diff(&xor_datum, >> + &dst->fields[column->index], >> + &src->fields[column->index], >> + &column->type); >> + if (error) { >> + return error; >> + } >> + } >> + >> ovsdb_datum_destroy(&dst->fields[column->index], &column->type); >> - ovsdb_datum_clone(&dst->fields[column->index], >> - &src->fields[column->index], >> - &column->type); > > Could you move ovsdb_datum_destroy(&dst->fields[column->index], > &column->type) into the "else" clause below and then merge the "if" > clause below into the "if" clause above? We still need to destroy for both branches, so what I can do is something like this: @@ -184,13 +185,11 @@ ovsdb_row_update_columns(struct ovsdb_row *dst, if (error) { return error; } - } - ovsdb_datum_destroy(&dst->fields[column->index], &column->type); - - if (xor) { + ovsdb_datum_destroy(&dst->fields[column->index], &column->type); ovsdb_datum_swap(&dst->fields[column->index], &xor_datum); } else { + ovsdb_datum_destroy(&dst->fields[column->index], &column->type); ovsdb_datum_clone(&dst->fields[column->index], &src->fields[column->index], &column->type); --- i.e. copy the ovsdb_datum_destroy() to both branches and merge the "if"s, but I found this harder to read. I'll keep as is for now, but if you think that above version will be better, I can use it. Best regards, Ilya Maximets.
diff --git a/ovsdb/execution.c b/ovsdb/execution.c index 3a0dad5d0..f6150e944 100644 --- a/ovsdb/execution.c +++ b/ovsdb/execution.c @@ -483,8 +483,9 @@ update_row_cb(const struct ovsdb_row *row, void *ur_) ur->n_matches++; if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) { - ovsdb_row_update_columns(ovsdb_txn_row_modify(ur->txn, row), - ur->row, ur->columns); + ovsdb_error_assert(ovsdb_row_update_columns( + ovsdb_txn_row_modify(ur->txn, row), + ur->row, ur->columns, false)); } return true; diff --git a/ovsdb/replication.c b/ovsdb/replication.c index b755976b0..d8b56d813 100644 --- a/ovsdb/replication.c +++ b/ovsdb/replication.c @@ -677,7 +677,7 @@ process_table_update(struct json *table_update, const char *table_name, struct ovsdb_error *error; error = (!new ? ovsdb_table_execute_delete(txn, &uuid, table) : !old ? ovsdb_table_execute_insert(txn, &uuid, table, new) - : ovsdb_table_execute_update(txn, &uuid, table, new)); + : ovsdb_table_execute_update(txn, &uuid, table, new, false)); if (error) { if (!strcmp(ovsdb_error_get_tag(error), "consistency violation")) { ovsdb_error_assert(error); diff --git a/ovsdb/row.c b/ovsdb/row.c index 755ab91a8..65a054621 100644 --- a/ovsdb/row.c +++ b/ovsdb/row.c @@ -163,20 +163,40 @@ ovsdb_row_equal_columns(const struct ovsdb_row *a, return true; } -void +struct ovsdb_error * ovsdb_row_update_columns(struct ovsdb_row *dst, const struct ovsdb_row *src, - const struct ovsdb_column_set *columns) + const struct ovsdb_column_set *columns, + bool xor) { size_t i; for (i = 0; i < columns->n_columns; i++) { const struct ovsdb_column *column = columns->columns[i]; + struct ovsdb_datum xor_datum; + struct ovsdb_error *error; + + if (xor) { + error = ovsdb_datum_apply_diff(&xor_datum, + &dst->fields[column->index], + &src->fields[column->index], + &column->type); + if (error) { + return error; + } + } + ovsdb_datum_destroy(&dst->fields[column->index], &column->type); - ovsdb_datum_clone(&dst->fields[column->index], - &src->fields[column->index], - &column->type); + + if (xor) { + ovsdb_datum_swap(&dst->fields[column->index], &xor_datum); + } else { + ovsdb_datum_clone(&dst->fields[column->index], + &src->fields[column->index], + &column->type); + } } + return NULL; } /* Appends the string form of the value in 'row' of each of the columns in diff --git a/ovsdb/row.h b/ovsdb/row.h index 2c441b5a4..394ac8eb4 100644 --- a/ovsdb/row.h +++ b/ovsdb/row.h @@ -82,8 +82,10 @@ bool ovsdb_row_equal_columns(const struct ovsdb_row *, int ovsdb_row_compare_columns_3way(const struct ovsdb_row *, const struct ovsdb_row *, const struct ovsdb_column_set *); -void ovsdb_row_update_columns(struct ovsdb_row *, const struct ovsdb_row *, - const struct ovsdb_column_set *); +struct ovsdb_error *ovsdb_row_update_columns(struct ovsdb_row *, + const struct ovsdb_row *, + const struct ovsdb_column_set *, + bool xor); void ovsdb_row_columns_to_string(const struct ovsdb_row *, const struct ovsdb_column_set *, struct ds *); struct ovsdb_error *ovsdb_row_from_json(struct ovsdb_row *, diff --git a/ovsdb/table.c b/ovsdb/table.c index 2935bd897..455a3663f 100644 --- a/ovsdb/table.c +++ b/ovsdb/table.c @@ -384,7 +384,8 @@ ovsdb_table_execute_delete(struct ovsdb_txn *txn, const struct uuid *row_uuid, struct ovsdb_error * ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid, - struct ovsdb_table *table, struct json *json_row) + struct ovsdb_table *table, struct json *json_row, + bool xor) { const struct ovsdb_row *row = ovsdb_table_get_row(table, row_uuid); if (!row) { @@ -399,9 +400,9 @@ ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid, struct ovsdb_error *error = ovsdb_row_from_json(update, json_row, NULL, &columns); - if (!error && !ovsdb_row_equal_columns(row, update, &columns)) { - ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row), - update, &columns); + if (!error && (xor || !ovsdb_row_equal_columns(row, update, &columns))) { + error = ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row), + update, &columns, xor); } ovsdb_column_set_destroy(&columns); diff --git a/ovsdb/table.h b/ovsdb/table.h index e21ec7b31..ce69a5d13 100644 --- a/ovsdb/table.h +++ b/ovsdb/table.h @@ -82,6 +82,6 @@ struct ovsdb_error *ovsdb_table_execute_delete(struct ovsdb_txn *txn, struct ovsdb_error *ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid, struct ovsdb_table *table, - struct json *new); + struct json *new, bool xor); #endif /* ovsdb/table.h */
This will be used to apply update3 type updates to ovsdb tables while processing updates for future ovsdb 'relay' service model. 'ovsdb_datum_apply_diff' is allowed to fail, so adding support to return this error. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- ovsdb/execution.c | 5 +++-- ovsdb/replication.c | 2 +- ovsdb/row.c | 30 +++++++++++++++++++++++++----- ovsdb/row.h | 6 ++++-- ovsdb/table.c | 9 +++++---- ovsdb/table.h | 2 +- 6 files changed, 39 insertions(+), 15 deletions(-)