[PATCHES] Add 'net tdb' command to allow debugging of contended records in locking.tdb

Christof Schmitt cs at samba.org
Fri Apr 28 22:04:59 UTC 2017


On Wed, Apr 26, 2017 at 11:54:12AM -0700, Christof Schmitt via samba-technical wrote:
> On Tue, Apr 25, 2017 at 01:09:14PM +0200, vl at samba.org wrote:
> > Hi, Christof!
> > 
> > On Thu, Apr 13, 2017 at 01:00:19PM -0700, Christof Schmitt via samba-technical wrote:
> > > One problem in cluster environments can be contended access to the same
> > > file, in which case ctdb has to transfer the locking.tdb record across
> > > nodes for every open and close of that file.
> > 
> > Attached find two patches that you might want to take into
> > consideration. I haven't run a full autobuild on the patches, but if
> > you find my two to-SQUASH patches acceptable, Reviewed-By: me.
> 
> Thank you. I added the two changes and will push the patches to
> autobuild.

I tried to push these to autobuild, but get a consistent failure at:

[1689(10768)/2100 at 1h51m38s] samba4.blackbox.trust_ntlm(fl2008r2dc:local)
[1690(10790)/2100 at 1h51m39s] samba4.blackbox.trust_ntlm(fl2003dc:local)
[1691(10812)/2100 at 1h51m40s] samba4.blackbox.trust_ntlm(ad_member:local)
UNEXPECTED(error): samba4.blackbox.trust_ntlm.Test01 rpcclient getusername with LOCALADMEMBERtime: 2017-04-28 01:12:50.225336Z(ad_member:local)
REASON: Exception: Exception: Test was never started
UNEXPECTED(error): samba4.blackbox.trust_ntlm.Test01 rpcclient getusername with LOCALADMEMBERtime: 2017-04-28 01:12:50.223650Z(ad_member:local) (samba.subunit.RemotedTestCase)
REASON: was started but never finished!

FAILED (0 failures, 2 errors and 0 unexpected successes in 0 testsuites)


I am still debugging, something seems to be tripping up the tracking in
selftest/subunithelper.py. Running a 'make test' for the affected testcase and
the new net_tdb testcase works.

FYI, the attached patches are what i am trying to push.

Christof
-------------- next part --------------
From d9d09a4cc94d0c15533d54c3716c830c269d5141 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 16 Feb 2017 16:23:39 -0700
Subject: [PATCH 1/6] ctdb: Print key as hex string instead of just the hash in
 hot record message

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 ctdb/server/ctdb_call.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index ed943f9..3b84e75 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -825,6 +825,7 @@ ctdb_update_db_stat_hot_keys(struct ctdb_db_context *ctdb_db, TDB_DATA key,
 			     int count)
 {
 	int i, id;
+	char *keystr;
 
 	/* smallest value is always at index 0 */
 	if (count <= ctdb_db->statistics.hot_keys[0].count) {
@@ -860,9 +861,13 @@ ctdb_update_db_stat_hot_keys(struct ctdb_db_context *ctdb_db, TDB_DATA key,
 	ctdb_db->statistics.hot_keys[id].key.dsize = key.dsize;
 	ctdb_db->statistics.hot_keys[id].key.dptr  = talloc_memdup(ctdb_db, key.dptr, key.dsize);
 	ctdb_db->statistics.hot_keys[id].count = count;
-	DEBUG(DEBUG_NOTICE,
-	      ("Updated hot key database=%s key=0x%08x id=%d count=%d\n",
-	       ctdb_db->db_name, ctdb_hash(&key), id, count));
+
+	keystr = hex_encode_talloc(ctdb_db,
+				   (unsigned char *)key.dptr, key.dsize);
+	DEBUG(DEBUG_NOTICE,("Updated hot key database=%s key=%s id=%d "
+			    "count=%d\n", ctdb_db->db_name,
+			    keystr ? keystr : "" , id, count));
+	talloc_free(keystr);
 
 sort_keys:
 	for (i = 1; i < MAX_HOT_KEYS; i++) {
-- 
1.8.3.1


From dd2a5b899270c09eb8eef3d02feffc046b80589f Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 16 Feb 2017 16:22:38 -0700
Subject: [PATCH 2/6] net: Add net tdb command to print information from tdb
 records

The main purpose is to debug "hot" records from ctdb. ctdb tracks
contended records and identifies them by key in the dbstatistics:

DB Statistics: locking.tdb
[...]
 Num Hot Keys:     1
     Count:3 Key:6a4128e3ced4681b02a00000000000000000000000000000

This command allows querying additional information for the associated
key to identify the affected file. For now this only adds a subcommand
for the locking.tdb, but could be extended to others:

net tdb locking 6a4128e3ced4681b02a00000000000000000000000000000
Share path:            /test/share
Name:                  testfile
Number of share modes: 2

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source3/utils/net.c         |   8 +++
 source3/utils/net_proto.h   |   3 ++
 source3/utils/net_tdb.c     | 120 ++++++++++++++++++++++++++++++++++++++++++++
 source3/utils/wscript_build |   1 +
 4 files changed, 132 insertions(+)
 create mode 100644 source3/utils/net_tdb.c

diff --git a/source3/utils/net.c b/source3/utils/net.c
index beb8760..34884f0 100644
--- a/source3/utils/net.c
+++ b/source3/utils/net.c
@@ -751,6 +751,14 @@ static struct functable net_func[] = {
 		   "'net notify' commands.")
 	},
 
+	{	"tdb",
+		net_tdb,
+		NET_TRANSPORT_LOCAL,
+		N_("Show information from tdb records"),
+		N_("  Use 'net help tdb' to get more information about "
+		   "'net tdb' commands.")
+	},
+
 #ifdef WITH_FAKE_KASERVER
 	{	"afs",
 		net_afs,
diff --git a/source3/utils/net_proto.h b/source3/utils/net_proto.h
index 093aa4b..f0ae538 100644
--- a/source3/utils/net_proto.h
+++ b/source3/utils/net_proto.h
@@ -462,4 +462,7 @@ int net_rpc_trust(struct net_context *c, int argc, const char **argv);
 int net_rpc_conf(struct net_context *c, int argc, const char **argv);
 
 int net_notify(struct net_context *c, int argc, const char **argv);
+
+int net_tdb(struct net_context *c, int argc, const char **argv);
+
 #endif /*  _NET_PROTO_H_  */
diff --git a/source3/utils/net_tdb.c b/source3/utils/net_tdb.c
new file mode 100644
index 0000000..a03cc0e
--- /dev/null
+++ b/source3/utils/net_tdb.c
@@ -0,0 +1,120 @@
+/*
+ * Samba Unix/Linux client library
+ * net tdb commands to query tdb record information
+ * Copyright (C) 2016, 2017 Christof Schmitt <cs at samba.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "includes.h"
+#include "utils/net.h"
+#include "locking/proto.h"
+#include "librpc/gen_ndr/open_files.h"
+#include "librpc/gen_ndr/ndr_open_files.h"
+
+static int net_tdb_locking_dump(TALLOC_CTX *mem_ctx,
+				struct share_mode_data *data)
+{
+	struct ndr_print *ndr_print;
+
+	ndr_print = talloc_zero(mem_ctx, struct ndr_print);
+	if (ndr_print == NULL) {
+		d_printf("Could not allocate memory.\n");
+		return -1;
+	}
+
+	ndr_print->print = ndr_print_printf_helper;
+	ndr_print->depth = 1;
+	ndr_print_share_mode_data(ndr_print, "SHARE_MODE_DATA", data);
+	TALLOC_FREE(ndr_print);
+
+	return 0;
+}
+
+static int net_tdb_locking_fetch(TALLOC_CTX *mem_ctx, const char *hexkey,
+				 struct share_mode_lock **lock)
+{
+	DATA_BLOB blob;
+	struct file_id id;
+	bool ok;
+
+	blob = strhex_to_data_blob(mem_ctx, hexkey);
+	if (blob.length != sizeof(struct file_id)) {
+		d_printf("Invalid length of key\n");
+		return -1;
+	}
+
+	id = *(struct file_id *)blob.data;
+
+	ok = locking_init_readonly();
+	if (!ok) {
+		d_printf("locking_init_readonly failed\n");
+		return -1;
+	}
+
+	*lock = fetch_share_mode_unlocked(mem_ctx, id);
+
+	if (*lock == NULL) {
+		d_printf("Record with key %s not found.\n", hexkey);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int net_tdb_locking(struct net_context *c, int argc, const char **argv)
+{
+	TALLOC_CTX *mem_ctx = talloc_stackframe();
+	struct share_mode_lock *lock;
+	int ret;
+
+	if (argc < 1) {
+		d_printf("Usage: net tdb locking <key> [ dump ]\n");
+		ret = -1;
+		goto out;
+	}
+
+	ret = net_tdb_locking_fetch(mem_ctx, argv[0], &lock);
+	if (ret != 0) {
+		goto out;
+	}
+
+	if (argc == 2 && strequal(argv[1], "dump")) {
+		ret = net_tdb_locking_dump(mem_ctx, lock->data);
+	} else {
+		d_printf("Share path:            %s\n", lock->data->servicepath);
+		d_printf("Name:                  %s\n", lock->data->base_name);
+		d_printf("Number of share modes: %" PRIu32 "\n",
+			 lock->data->num_share_modes);
+	}
+
+out:
+	TALLOC_FREE(mem_ctx);
+	return ret;
+}
+
+int net_tdb(struct net_context *c, int argc, const char **argv)
+{
+	struct functable func[] = {
+		{ "locking",
+		  net_tdb_locking,
+		  NET_TRANSPORT_LOCAL,
+		  N_("Show information for a record in locking.tdb"),
+		  N_("net tdb locking <key>")
+		},
+		{NULL, NULL, 0, NULL, NULL}
+	};
+
+	return net_run_function(c, argc, argv, "net tdb", func);
+}
diff --git a/source3/utils/wscript_build b/source3/utils/wscript_build
index 4295f85..3c1f9a4 100644
--- a/source3/utils/wscript_build
+++ b/source3/utils/wscript_build
@@ -205,6 +205,7 @@ bld.SAMBA3_BINARY('net',
                  net_rpc_conf.c
                  net_afs.c
                  net_notify.c
+                 net_tdb.c
                  ../registry/reg_parse.c
                  ../registry/reg_format.c
                  ../registry/reg_import.c
-- 
1.8.3.1


From 82744671b84ba9255bcd6f23f6fd112a2ce2b7d3 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 16 Feb 2017 16:23:34 -0700
Subject: [PATCH 3/6] docs-xml: Document net tdb command

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 docs-xml/manpages/net.8.xml | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/docs-xml/manpages/net.8.xml b/docs-xml/manpages/net.8.xml
index 4482ae8..a43a062 100644
--- a/docs-xml/manpages/net.8.xml
+++ b/docs-xml/manpages/net.8.xml
@@ -2707,6 +2707,28 @@ Dump the locking table of a certain global lock.
 </refsect2>
 
 <refsect2>
+	<title>TDB</title>
+
+	<para>Print information from tdb records.</para>
+
+	<refsect3>
+		<title>TDB LOCKING <replaceable>key</replaceable> [DUMP]</title>
+
+		<para>List sharename, filename and number of share modes
+		for a record from locking.tdb. With the optional DUMP options,
+		dump the complete record.</para>
+
+		<itemizedlist>
+			<listitem>
+				<para><replaceable>KEY</replaceable>
+				Key of the tdb record as hex string.</para>
+			</listitem>
+		</itemizedlist>
+
+	</refsect3>
+</refsect2>
+
+<refsect2>
 <title>HELP [COMMAND]</title>
 
 <para>Gives usage information for the specified command.</para>
-- 
1.8.3.1


From 57f088dc70b5317d7833a61487982b211df04cb9 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 16 Feb 2017 16:23:43 -0700
Subject: [PATCH 4/6] selftest: Make lockdir available in test environment

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 selftest/selftest.pl      | 1 +
 selftest/target/Samba3.pm | 1 +
 2 files changed, 2 insertions(+)

diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index 32fc845..66bda38 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -855,6 +855,7 @@ my @exported_envvars = (
 	"DNS_FORWARDER2",
 	"RESOLV_CONF",
 	"UNACCEPTABLE_PASSWORD",
+	"LOCK_DIR",
 
 	# nss_wrapper
 	"NSS_WRAPPER_PASSWD",
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index c241bd1..1485a23 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2104,6 +2104,7 @@ force_user:x:$gid_force_user:
 	$ret{SMBD_TEST_LOG_POS} = 0;
 	$ret{SERVERCONFFILE} = $conffile;
 	$ret{CONFIGURATION} ="-s $conffile";
+	$ret{LOCK_DIR} = $lockdir;
 	$ret{SERVER} = $server;
 	$ret{USERNAME} = $unix_name;
 	$ret{USERID} = $unix_uid;
-- 
1.8.3.1


From 037550481ccfe0bfc35ee44be53402ccced543f7 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 16 Feb 2017 16:23:47 -0700
Subject: [PATCH 5/6] selftest: Add test for 'net tdb' command

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source3/script/tests/test_net_tdb.sh | 99 ++++++++++++++++++++++++++++++++++++
 source3/selftest/tests.py            |  5 ++
 2 files changed, 104 insertions(+)
 create mode 100755 source3/script/tests/test_net_tdb.sh

diff --git a/source3/script/tests/test_net_tdb.sh b/source3/script/tests/test_net_tdb.sh
new file mode 100755
index 0000000..731cad3
--- /dev/null
+++ b/source3/script/tests/test_net_tdb.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+#
+# Test 'net tdb' command.
+#
+# Verify that the command returns the correct information in the
+# expected format. The 'dump' option is tested, but the output is not
+# checked, since the internal data structure could change in the
+# future.
+#
+# Copyright (C) 2017 Christof Schmitt
+
+if [ $# -lt 7 ]; then
+cat <<EOF
+Usage: $0 SMBCLIENT SERVER SHARE USER PASS CONFIGURATION LOCALPATH LOCKDIR
+EOF
+exit 1;
+fi
+
+SMBCLIENT=$1
+SERVER=$2
+SHARE=$3
+USER=$4
+PASS=$5
+CONFIGURATION=$6
+LOCALPATH=$7
+LOCKDIR=$8
+
+FILENAME=net_tdb_testfile
+
+failed=0
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+touch $LOCALPATH/$FILENAME
+
+printf "open %s\n"'!sleep 10'"\n" ${FILENAME} | \
+	$SMBCLIENT //$SERVER/$SHARE -U$USER%$PASS &
+SMBCLIENTPID=$!
+
+# Give smbclient a chance to open the file
+sleep 1
+
+testit "Looking for record key of open file" \
+       $BINDIR/tdbtool $LOCKDIR/locking.tdb hexkeys || \
+	failed=$(expr $failed + 1)
+
+# The assumption here is that only one file is open, so only one
+# record can exist in the database.
+
+# Output of 'tdbtool hexkeys' is in this format:
+#[000] 01 FD 00 00 00 00 00 00  56 02 5C 00 00 00 00 00  ....... V.\....
+#[010] 00 00 00 00 00 00 00 00                           .......
+# Select only the hex data, remove space and join every thing together
+key=0x$($BINDIR/tdbtool $LOCKDIR/locking.tdb hexkeys | \
+	grep '\[' | cut -c 7-56 | sed -e 's/ //g' | tr -d '\n')
+
+testit "Looking for open file in locking.tdb" \
+       $BINDIR/net $CONFIGURATION tdb locking $key || \
+   failed=$(expr $failed + 1)
+out=$($BINDIR/net $CONFIGURATION tdb locking $key)
+
+out=$($BINDIR/net $CONFIGURATION tdb locking $key | \
+	      grep 'Share path: ' | sed -e 's/Share path: \+//')
+testit "Verify pathname in output" \
+       test "$out" = "$LOCALPATH" || \
+	failed=$(expr $failed + 1)
+
+out=$($BINDIR/net $CONFIGURATION tdb locking $key | \
+	      grep 'Name:' | sed -e 's/Name: \+//')
+testit "Verify filename in output" \
+       test "$out" = "$FILENAME" || \
+	failed=$(expr $failed + 1)
+
+out=$($BINDIR/net $CONFIGURATION tdb locking $key | \
+	      grep 'Number of share modes:' | \
+	      sed -e 's/Number of share modes: \+//')
+testit "Verify number of share modes in output" \
+       test "$out" = "1" || \
+	failed=$(expr $failed + 1)
+
+testit "Complete record dump" \
+       $BINDIR/net $CONFIGURATION tdb locking $key dump || \
+	failed=$(expr $failed + 1)
+
+$BINDIR/net $CONFIGURATION tdb locking $key dump | grep -q $FILENAME
+RC=$?
+testit "Verify filename in dump output" \
+       test $RC = 0 || \
+	failed=$(expr $failed + 1)
+$BINDIR/net $CONFIGURATION tdb locking $key dump | grep -q $LOCALPATH
+RC=$?
+testit "Verify share path in dump output" \
+       test $RC = 0 || \
+	failed=$(expr $failed + 1)
+
+kill $SMBCLIENTPID
+
+testok $0 $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 9bb7903..56a86bc 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -502,6 +502,11 @@ plantestsuite("samba3.blackbox.sharesec", "simpleserver:local",
               [os.path.join(samba3srcdir, "script/tests/test_sharesec.sh"),
                configuration, os.path.join(bindir(), "sharesec"), "tmp"])
 
+plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local",
+              [ os.path.join(samba3srcdir, "script/tests/test_net_tdb.sh"),
+                smbclient3, '$SERVER', 'tmp', '$USERNAME', '$PASSWORD',
+                configuration, '$LOCAL_PATH', '$LOCK_DIR' ])
+
 plantestsuite("samba3.blackbox.net_dom_join_fail_dc", "nt4_dc",
               [os.path.join(samba3srcdir, "script/tests/test_net_dom_join_fail_dc.sh"),
                "$USERNAME", "$PASSWORD", "$SERVER", "$PREFIX/net_dom_join_fail_dc",
-- 
1.8.3.1


From 464934eec1c81ddf62ed6670d4ba13ea438e7803 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 20 Feb 2017 11:52:58 -0700
Subject: [PATCH 6/6] WHATSNEW: Add new 'net tdb locking' command

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 WHATSNEW.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index d9324e7..3f0c009 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -22,6 +22,11 @@ obey client requests to synchronize unwritten data in operating
 system buffers safely onto disk. This is a safer default setting
 for modern SMB1/2/3 clients.
 
+The record attached to an open file or directory in Samba can be
+queried through the 'net tdb locking' command. In clustered Samba this
+can be useful to determine the file or directory triggering
+corresponding "hot" record warnings in ctdb.
+
 Authentication and Authorization audit support
 ----------------------------------------------
 
-- 
1.8.3.1



More information about the samba-technical mailing list