[ovs-dev] [PATCH v2 0/2] ovsdb-idl: Preserve references for tracked deleted rows.
blp at ovn.org
Wed Mar 10 20:23:54 UTC 2021
On Wed, Mar 10, 2021 at 08:43:40PM +0100, Dumitru Ceara wrote:
> On 3/10/21 7:46 PM, Ben Pfaff wrote:
> > On Wed, Mar 03, 2021 at 03:39:27PM +0100, Dumitru Ceara wrote:
> > > The second patch of the series fixes a problem with the IDL change
> > > tracking code for deleted records which was causing OVN to crash due to
> > > strong reference fields in IDL records not being preserved at row
> > > deletion.
> > There are many difficult cases here, both difficult to implement
> > properly and difficult to conceptualize. It might be worth spending
> > some time thinking about whether it is possible and valuable to write a
> > very general test, one that tries to transition from every possible
> > state to every other possible state (within a limited but interesting
> > state space) and verifies that the results are what is expected. When
> > I've been able to do this in other contexts, it found lots of bugs that
> > I didn't expect and it protected against regression.
> Thanks for the suggestion.
> We have a few change tracking related tests in ovsdb-idl.at but, as you
> said, they only deal with specific cases. As a matter of fact, I'm adding 4
> new similar tests for the bugs we identified so far.
> I can try to also add a single, more general, test to cover all relevant
> transitions. Do you have anything specific in mind or would a single,
> large, OVSDB_CHECK_IDL_TRACK-based test, be good enough?
I was thinking of something systematic and general-purpose like this:
or the code documented here:
I built something equivalent for OVS at one point but it didn't ever
make it into the tree; I could probably dig it up if anyone actually
wanted to use it though.
It's a lot of effort so it is only worth it if the code is hard enough
to get right. I do not know whether that is the case here. I cannot
devote the time to do it myself in this case, so it would only happen if
someone felt themselves motivated to do it.
More information about the dev