On Thu, Jan 04, 2024 at 01:39:34PM +0100, Dumitru Ceara wrote:
> On 1/4/24 12:53, Ilya Maximets wrote:
> > On 1/4/24 12:34, Simon Horman wrote:
> >> On Tue, Dec 19, 2023 at 02:26:36PM +0100, Dumitru Ceara wrote:
> >>> On 12/12/23 18:43, Mike Pattrick wrote:
> >>>> In the case of a weak reference, clearing all change_seqno's can delete
> >>>> useful information. Instead of clearing all seqno's when removing
> >>>> track_node, only clear those values in cases including row insertion,
> >>>> and row deleting if no dst_arcs remain.
> >>>>
> >>>> Fixes: 95689f166818 ("ovsdb-idl: Preserve references for deleted rows.")
> >>>> Reported-at: https://issues.redhat.com/browse/FDP-193
> >>>> Signed-off-by: Mike Pattrick <m...@redhat.com>
> >>>>
> >>>> ---
> >>>
> >>> Thanks a lot for fixing this, Mike!
> >>>
> >>> Acked-by: Dumitru Ceara <dce...@redhat.com>
> >>>
> >>> I think I'd squash patch 2/2 into this one though. In which case, if
> >>> maintainers agree, I'd ask them to remove my signed-off/co-authored-by
> >>> from 2/2 and keep my ack. The test case was just the easiest way of
> >>> creating a sane bug report. :)
> >>
> >> Can do.
> >> I will work on applying the patch-set in the way you describe.
> >
> > Hi, Simon, others.
> >
> > + [[000: simple6: conditions unchanged
> > +000: simple: conditions unchanged
> > +001: table simple6: inserted row: name=row0_s6 weak_ref=[<0>] uuid=<1>
> > +001: table simple6: updated columns: name weak_ref
> > +001: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[]
> > ra=[] ba=[] sa=[] ua=[] uuid=<0>
> > +001: table simple: updated columns: s
> > +002: {"error":null,"result":[{"uuid":["uuid","<3>"]},{"count":1}]}
> > +003: {"error":null,"result":[{"count":1}]}
> > +004: table simple6: name=row0_s6 weak_ref=[<0>] uuid=<1>
> >
> > Yesterday we discussed the test with Mike off-list and I thought that
> > the row in simple6 is still referencing the deleted row. With it
> > being potentially a result of the other IDL issue we have:
> >
> > https://patchwork.ozlabs.org/project/openvswitch/patch/20230702095922.3365471-1-taoliu...@163.com/
> > (This one needs a a better fix, I'll ask if the author is still working
> > on a new version.)
> >
> > But it actually references a previously added row with uuid=<0>, which
> > is fine.
> >
> > Mike, Dumitru, can you confirm?
> >
>
> I confirm, thanks for double checking.
>
> > If that's so, then the test should be fine as-is.
> > FWIW, the fix will need to be backported all the way down to 2.17.
> > And I agree that it's better to squash the patches into one.
> >
>
> Cool.
Thanks everyone.
I have gone ahead and applied this, squashed as requested.
- ovsdb-idl: Preserve change_seqno when deleting rows.
https://github.com/openvswitch/ovs/commit/4102674b3eca
Backport to branch-3.2
- ovsdb-idl: Preserve change_seqno when deleting rows.
https://github.com/openvswitch/ovs/commit/ec1d730163d9
Backport to branch-3.1
- ovsdb-idl: Preserve change_seqno when deleting rows.
https://github.com/openvswitch/ovs/commit/8fd5f77cd84e
Backport to branch-3.0
- ovsdb-idl: Preserve change_seqno when deleting rows.
https://github.com/openvswitch/ovs/commit/94191b7a4926
Backport to branch-2.17
- ovsdb-idl: Preserve change_seqno when deleting rows.
https://github.com/openvswitch/ovs/commit/b3f8c32edad6
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
