Bug 13499 - CTDB_BROADCAST_VNNMAP should not be used
Summary: CTDB_BROADCAST_VNNMAP should not be used
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: CTDB (show other bugs)
Version: 4.8.3
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on:
Reported: 2018-07-02 01:17 UTC by Martin Schwenke
Modified: 2020-12-11 08:22 UTC (History)
4 users (show)

See Also:

Patch for 4.7 (11.36 KB, patch)
2018-07-06 01:57 UTC, Martin Schwenke
amitay: review+
Patch for v4.8 (12.96 KB, patch)
2018-07-06 01:57 UTC, Martin Schwenke
amitay: review+
Proposed additional patch for 4.7 to fix the build (1.44 KB, patch)
2018-07-06 10:49 UTC, Karolin Seeger
martins: review+
amitay: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Schwenke 2018-07-02 01:17:32 UTC
In both places where this is used a broadcast to all active nodes should be used instead.  In particular, traverses of volatile databases will not find records that have been updated on non-LMASTER nodes.

CTDB_BROADCAST_VNNMAP should simply be changed to CTDB_BROADCAST_ACTIVE and its implementation should be changed accordingly.
Comment 1 Martin Schwenke 2018-07-06 01:57:08 UTC
Created attachment 14286 [details]
Patch for 4.7
Comment 2 Martin Schwenke 2018-07-06 01:57:54 UTC
Created attachment 14287 [details]
Patch for v4.8
Comment 3 Amitay Isaacs 2018-07-06 08:15:42 UTC
Hi Karolin,

This is ready for v4.7 and v4.8.
Comment 4 Karolin Seeger 2018-07-06 09:02:25 UTC
(In reply to Amitay Isaacs from comment #3)
Pushed to autobuild-v4-[8,7]-test.
Comment 5 Karolin Seeger 2018-07-06 10:46:33 UTC
Patch for 4.7 seems to be incomplete, breaks the build:

../source3/smbd/notifyd/notifyd.c: In function ‘notifyd_broadcast_reclog’:
../source3/smbd/notifyd/notifyd.c:1001:15: error: ‘CTDB_BROADCAST_VNNMAP’ undeclared (first use in this function)
../source3/smbd/notifyd/notifyd.c:1001:15: note: each undeclared identifier is reported only once for each function it appears in
Waf: Leaving directory `/memdisk/kseeger/a47/b661372/samba-ctdb/bin'
Build failed:  -> task failed (err #1): 
	{task: cc notifyd.c
Comment 6 Karolin Seeger 2018-07-06 10:49:45 UTC
Created attachment 14307 [details]
Proposed additional patch for 4.7 to fix the build
Comment 7 Martin Schwenke 2018-07-06 11:42:16 UTC
Hi Karolin,

Sorry I missed that.  I checked that there were no instances of CTDB_BROADCAST_VNNMAP in 4.8 but forgot to do that for 4.7  :-(

That looks sane.  The only complication is that in 4.8+ that particular instance has been changed to CTDB_BROADCAST_CONNECTED.

Your change is logically consistent and is more conservative, so I'm going to give it a +1.

That change from CTDB_BROADCAST_VNNMAP -> CTDB_BROADCAST_CONNECTED doesn't appear to have been considered a serious enough bug fix to need backporting, so we should be OK. Amitay is on CC: for this defect and I'll also add Volker to see if either of them disagree.
Comment 8 Martin Schwenke 2018-07-06 11:44:33 UTC
Karolin, reassigning to you so you can move this along if you want to.  I'm going to bed.  :-)
Comment 9 Karolin Seeger 2018-07-09 10:55:20 UTC
(In reply to Karolin Seeger from comment #6)
Pushed to autobuild-v4-7-test.
Comment 10 Karolin Seeger 2018-07-12 11:02:34 UTC
Pushed to both branches.
Closing out bug report.