[ovs-dev] [PATCH v3] ovsdb-idl: Fix *_is_new() IDL functions

Ilya Maximets i.maximets at ovn.org
Mon Nov 16 19:33:56 UTC 2020


On 10/20/20 9:46 PM, Han Zhou wrote:
> On Tue, Oct 20, 2020 at 8:07 AM Mark Gray <mark.d.gray at redhat.com> wrote:
>>
>> Currently all functions of the type *_is_new() always return
>> 'false'. This patch resolves this issue by using the
>> 'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' instead of the
>> 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno' to determine if a row
>> is new and by resetting the 'OVSDB_IDL_CHANGE_INSERT'
>> 'change_seqno' on clear.
>>
>> Further to this, the code is also updated to match the following
>> behaviour:
>>
>> When a row is inserted, the 'OVSDB_IDL_CHANGE_INSERT'
>> 'change_seqno' is updated to match the new database
>> change_seqno. The 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno'
>> is not set for inserted rows (only for updated rows).
>>
>> At the end of a run, ovsdb_idl_db_track_clear() should be
>> called to clear all tracking information, this includes
>> resetting all row 'change_seqno' to zero. This will ensure
>> that subsequent runs will not see a previously 'new' row.
>>
>> add_tracked_change_for_references() is updated to only
>> track rows that reference the current row.
>>
>> Also, update unit tests in order to test the *_is_new(),
>> *_is_delete() functions.
>>
>> Suggested-by: Dumitru Ceara <dceara at redhat.com>
>> Reported-at: https://bugzilla.redhat.com/1883562
>> Fixes: ca545a787ac0 ("ovsdb-idl.c: Increase seqno for change-tracking of
> table references.")
>> Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
>> ---
>> v3: Update comments for _is_new(), is_deleted() and is_updated() functions
>>     Removed change that modifies flags on local uncommitted changes
>>

<snip>

>>  static void
>>  add_tracked_change_for_references(struct ovsdb_idl_row *row)
>>  {
>> -    if (ovs_list_is_empty(&row->track_node) &&
>> -            ovsdb_idl_track_is_set(row->table)) {
>> -        ovs_list_push_back(&row->table->track_list,
>> -                           &row->track_node);
>> -        row->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
>> -            = row->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
>> -            = row->table->db->change_seqno + 1;
>> -
>>          const struct ovsdb_idl_arc *arc;
>>          LIST_FOR_EACH (arc, dst_node, &row->dst_arcs) {
>> -            add_tracked_change_for_references(arc->src);
>> +            struct ovsdb_idl_row *ref = arc->src;
>> +
>> +            if (ovs_list_is_empty(&ref->track_node) &&
>> +                ovsdb_idl_track_is_set(ref->table)) {
>> +                    ovs_list_push_back(&ref->table->track_list,
>> +                                       &ref->track_node);
>> +
>> +                ref->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
>> +                    = ref->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
>> +                    = ref->table->db->change_seqno + 1;
>> +
>> +                add_tracked_change_for_references(ref);
>> +            }
>>          }
>> -    }

I fixed indentation of this block before applying.

>>  }
>>

<snip>

> 
> Thanks Mark.
> Acked-by: Han Zhou <hzhou at ovn.org>

Thanks!

Applied to master and backported down to 2.11.



Best regards, Ilya Maximets.


More information about the dev mailing list