On 20 September 2016 at 13:27, Andy Zhou <az...@ovn.org> wrote:
> The newly added replication logic makes it possible for a monitor to
> receive delete and insertion of the same row back to back, which
> was not possible before. Add logic (and comment) to handle this
> case to avoid follow crash reported by Valgrind:
>
> #0 0x0000000000453edd in ovsdb_datum_compare_3way
> (a=0x5efbe60, b=0x0, type=0x5e6a848) at lib/ovsdb-data.c:1626
> #1 0x0000000000453ea4 in ovsdb_datum_equals
> (a=0x5efbe60, b=0x0, type=0x5e6a848) at lib/ovsdb-data.c:1616
> #2 0x000000000041b651 in update_monitor_row_data
> (mt=0x5eda4a0, row=0x5efbe00, data=0x0) at ovsdb/monitor.c:310
> #3 0x000000000041ed14 in ovsdb_monitor_changes_update
> (old=0x0, new=0x5efbe00, mt=0x5eda4a0, changes=0x5ef7180)
> at ovsdb/monitor.c:1255
> #4 0x000000000041f12e in ovsdb_monitor_change_cb
> (old=0x0, new=0x5efbe00, changed=0x5efc218, aux_=0xffefff040)
> at ovsdb/monitor.c:1339
> #5 0x000000000042ded9 in ovsdb_txn_for_each_change
> (txn=0x5efbd90, cb=0x41ef50 <ovsdb_monitor_change_cb>,
> aux=0xffefff040) at ovsdb/transaction.c:906
> #6 0x0000000000420155 in ovsdb_monitor_commit
> (replica=0x5eda2c0, txn=0x5efbd90, durable=false)
> at ovsdb/monitor.c:1553
> #7 0x000000000042dc04 in ovsdb_txn_commit_
> (txn=0x5efbd90, durable=false) at ovsdb/transaction.c:868
> #8 0x000000000042ddd4 in ovsdb_txn_commit (txn=0x5efbd90, durable=false)
> at ovsdb/transaction.c:893
> #9 0x0000000000422e0c in process_notification
> (table_updates=0x5efad10, db=0x5e6bd40) at ovsdb/replication.c:575
> #10 0x0000000000420ff3 in replication_run () at ovsdb/replication.c:184
> #11 0x0000000000405cc8 in main_loop
> (jsonrpc=0x5e67770, all_dbs=0xffefff3a0, unixctl=0x5ebd980,
> remotes=0xffefff360, run_process=0x0, exiting=0xffefff3c0,
> is_backup=0xffefff2de) at ovsdb/ovsdb-server.c:198
> #12 0x0000000000406edb in main (argc=1, argv=0xffefff550)
> at ovsdb/ovsdb-server.c:429
>
> Reported-by: Joe Stringer <j...@ovn.org>
> Reported-at: http://openvswitch.org/pipermail/dev/2016-September/079315.html
> Reported-by: Alin Serdean <aserd...@cloudbasesolutions.com>
> Reported-at: http://openvswitch.org/pipermail/dev/2016-September/079586.html
> Co-authored-by: Joe Stringer <j...@ovn.org>
> Signed-off-by: Andy Zhou <az...@ovn.org>
Signed-off-by: Joe Stringer <j...@ovn.org>
> ---
> ovsdb/monitor.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index a590943..7e6ddcb 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -1252,7 +1252,46 @@ ovsdb_monitor_changes_update(const struct ovsdb_row
> *old,
> change->new = clone_monitor_row_data(mt, new);
> } else {
> if (new) {
> - update_monitor_row_data(mt, new, change->new);
> + if (!change->new) {
Usually we do "if (positive condition) { ... } else { ... }", so
readers don't have to double-negate in their mind to figure out what
the "else" condition is.
I'll leave the review of the reasoning below to others.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
