Attachment 15401 Details for Bug 14059 – Patch for 4.11

[patch] Patch for 4.11

patch-for-4.11.patch (text/plain), 25.83 KB, created by Garming Sam on 2019-08-20 05:15:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Garming Sam

Created: 2019-08-20 05:15:47 UTC

Size: 25.83 KB

patch

obsolete

>From 3b84258bb534af5b2216794b50f598a8a0d53eed Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date: Mon, 22 Jul 2019 15:29:03 +1200
>Subject: [PATCH 01/11] downgradedatabase: comply with samba.tests.source
>
>In next commit we'll install the script, samba.tests.source picked up the
>lack of a copyright message and some whitespace errors, so this patch
>fixes that stuff first.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit c4aebb15001c830a46d5a6ad8ea11a6f9ea4fd04)
>---
> source4/scripting/bin/sambadowngradedatabase | 26 +++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
>diff --git a/source4/scripting/bin/sambadowngradedatabase b/source4/scripting/bin/sambadowngradedatabase
>index 9d1e2b8cc76..87a989bfd6a 100755
>--- a/source4/scripting/bin/sambadowngradedatabase
>+++ b/source4/scripting/bin/sambadowngradedatabase
>@@ -1,4 +1,24 @@
>-#!/usr/bin/env python3
>+#!/usr/bin/python3
>+#
>+# Unix SMB/CIFS implementation.
>+# Copyright (C) Andrew Bartlett <abartlet@samba.org> 2019
>+#
>+# Downgrade a database from 4.11 format to 4.7 format. 4.7 Format will
>+# run on any version of Samba AD, and Samba will repack/reconfigure the
>+# database if necessary.
>+#
>+# 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/>.
> from __future__ import print_function
> import optparse
> import sys
>@@ -38,9 +58,9 @@ samdb = ldb.Ldb(url=url,
>                 options=["modules:"])
> 
> partitions = samdb.search(base="@PARTITION",
>-			  scope=ldb.SCOPE_BASE,
>+                          scope=ldb.SCOPE_BASE,
>                           attrs=["backendStore", "partition"])
>-    
>+
> backend = str(partitions[0].get('backendStore', 'tdb'))
> 
> if backend == "mdb":
>-- 
>2.17.1
>
>
>From 881f4d146eb4fe8429fb26b58f60b24f5af746eb Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 29 Jul 2019 13:35:08 +1200
>Subject: [PATCH 02/11] tests: Avoid hardcoding relative filepath
>
>If we move the test file, the test will break.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit a8cdbe0b824f57f73eee09143148f009a9c58582)
>---
> python/samba/tests/blackbox/downgradedatabase.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/python/samba/tests/blackbox/downgradedatabase.py b/python/samba/tests/blackbox/downgradedatabase.py
>index a5e540c1354..4a8411b1f1a 100644
>--- a/python/samba/tests/blackbox/downgradedatabase.py
>+++ b/python/samba/tests/blackbox/downgradedatabase.py
>@@ -23,8 +23,8 @@ import shutil
> from subprocess import check_output
> from samba.samdb import SamDB
> 
>-COMMAND = os.path.join(os.path.dirname(__file__),
>-               "../../../../../source4/scripting/bin/sambadowngradedatabase")
>+COMMAND = os.path.join(os.environ.get("SRCDIR_ABS"),
>+               "source4/scripting/bin/sambadowngradedatabase")
> 
> 
> class DowngradeTestBase(BlackboxTestCase):
>-- 
>2.17.1
>
>
>From d4194ec36efd803f3ae9c66e346c8fd0880b4e22 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Mon, 29 Jul 2019 13:39:04 +1200
>Subject: [PATCH 03/11] downgradedatabase: rename to samba_downgrade_db
>
>Just so that it's slightly less of a mouthful for users.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit fdaaee8d3aac77d91642a7d75d4bcd15d4df8657)
>---
> python/samba/tests/blackbox/downgradedatabase.py                | 2 +-
> selftest/knownfail.d/usage                                      | 2 +-
> .../bin/{sambadowngradedatabase => samba_downgrade_db}          | 0
> 3 files changed, 2 insertions(+), 2 deletions(-)
> rename source4/scripting/bin/{sambadowngradedatabase => samba_downgrade_db} (100%)
>
>diff --git a/python/samba/tests/blackbox/downgradedatabase.py b/python/samba/tests/blackbox/downgradedatabase.py
>index 4a8411b1f1a..3d230609efc 100644
>--- a/python/samba/tests/blackbox/downgradedatabase.py
>+++ b/python/samba/tests/blackbox/downgradedatabase.py
>@@ -24,7 +24,7 @@ from subprocess import check_output
> from samba.samdb import SamDB
> 
> COMMAND = os.path.join(os.environ.get("SRCDIR_ABS"),
>-               "source4/scripting/bin/sambadowngradedatabase")
>+               "source4/scripting/bin/samba_downgrade_db")
> 
> 
> class DowngradeTestBase(BlackboxTestCase):
>diff --git a/selftest/knownfail.d/usage b/selftest/knownfail.d/usage
>index 23d52c0b727..0a495858e8c 100644
>--- a/selftest/knownfail.d/usage
>+++ b/selftest/knownfail.d/usage
>@@ -25,7 +25,7 @@ samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_rebuildextendedd
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_renamedc.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_repl_cleartext_pwd_py.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_rodcdns.none.
>-samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_sambadowngradedatabase.none.
>+samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_downgrade_db.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_gpupdate.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_gpupdate_.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_kcc.none.
>diff --git a/source4/scripting/bin/sambadowngradedatabase b/source4/scripting/bin/samba_downgrade_db
>similarity index 100%
>rename from source4/scripting/bin/sambadowngradedatabase
>rename to source4/scripting/bin/samba_downgrade_db
>-- 
>2.17.1
>
>
>From 7782e20e605e5597fe5d5db555c27b424688f29f Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date: Mon, 22 Jul 2019 13:35:21 +1200
>Subject: [PATCH 04/11] downgradedatabase: Add man-page documentation
>
>A man-page is needed so that we can install this tool as part of the
>Samba package.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit c89df3862b17fad9c4648b5d9c6805120d732df8)
>---
> docs-xml/manpages/samba_downgrade_db.8.xml | 95 ++++++++++++++++++++++
> docs-xml/wscript_build                     |  1 +
> 2 files changed, 96 insertions(+)
> create mode 100644 docs-xml/manpages/samba_downgrade_db.8.xml
>
>diff --git a/docs-xml/manpages/samba_downgrade_db.8.xml b/docs-xml/manpages/samba_downgrade_db.8.xml
>new file mode 100644
>index 00000000000..7b0c822cf21
>--- /dev/null
>+++ b/docs-xml/manpages/samba_downgrade_db.8.xml
>@@ -0,0 +1,95 @@
>+<?xml version="1.0" encoding="iso-8859-1"?>
>+<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
>+<refentry id="samba_downgrade_db.8">
>+
>+<refmeta>
>+	<refentrytitle>samba_downgrade_db</refentrytitle>
>+	<manvolnum>8</manvolnum>
>+	<refmiscinfo class="source">Samba</refmiscinfo>
>+	<refmiscinfo class="manual">User Commands</refmiscinfo>
>+	<refmiscinfo class="version">&doc.version;</refmiscinfo>
>+</refmeta>
>+
>+
>+<refnamediv>
>+	<refname>samba_downgrade_db</refname>
>+	<refpurpose>Samba tool for downgrading AD databases
>+	</refpurpose>
>+</refnamediv>
>+
>+<refsynopsisdiv>
>+	<cmdsynopsis>
>+		<command>samba_downgrade_db</command>
>+		<arg choice="opt">-H</arg>
>+		<arg choice="opt">-s</arg>
>+	</cmdsynopsis>
>+</refsynopsisdiv>
>+
>+<refsect1>
>+	<title>DESCRIPTION</title>
>+	<para>This tool is part of the <citerefentry><refentrytitle>samba</refentrytitle>
>+	<manvolnum>8</manvolnum></citerefentry> suite.</para>
>+
>+	<para>The format of the Samba Active Directory (AD) database changed in
>+	version 4.8 and 4.11. When downgrading a Samba AD Domain Controller (DC)
>+	to a release that is older than either of these versions (e.g. 4.11 to
>+	4.10), the AD database must be manually downgraded
>+	<emphasis>before</emphasis> the Samba packages can be safely downgraded.
>+	</para>
>+
>+	<para>This tool downgrades a Samba sam.ldb database from the format
>+	used in version &doc.version; to that of version 4.7. The v4.7 database
>+	format can safely be read by any version of Samba. If necessary, later
>+	versions of Samba will repack and reconfigure a v4.7-format database when
>+	the samba executable is first started.</para>
>+
>+	<para>Note that all Samba services must be stopped on the DC before running
>+	this tool. Once the tool has run, do not restart samba or modify the
>+	database before the Samba software package has been downgraded.
>+	</para>
>+</refsect1>
>+
>+<refsect1>
>+	<title>OPTIONS</title>
>+
>+	<variablelist>
>+
>+	<varlistentry>
>+	<term>-H [sam.ldb file]</term>
>+	<listitem><para>
>+	Link directly to a sam.ldb file instead of using path in system
>+	smb.conf
>+	</para></listitem>
>+	</varlistentry>
>+
>+	<varlistentry>
>+	<term>-s [smb.conf file]</term>
>+	<listitem><para>
>+	Link directly to smb.conf file instead of system default (usually
>+	in /usr/local/samba/etc/smb.conf)
>+	</para></listitem>
>+	</varlistentry>
>+
>+	</variablelist>
>+</refsect1>
>+
>+<refsect1>
>+	<title>VERSION</title>
>+
>+	<para>This man page is complete for version &doc.version; of the Samba
>+	suite.</para>
>+</refsect1>
>+
>+<refsect1>
>+	<title>AUTHOR</title>
>+
>+	<para>The original Samba software and related utilities
>+	were created by Andrew Tridgell. Samba is now developed
>+	by the Samba Team as an Open Source project similar
>+	to the way the Linux kernel is developed.</para>
>+
>+	<para>The samba_downgrade_db tool was developed by the Samba team
>+	at Catalyst IT Ltd.</para>
>+</refsect1>
>+
>+</refentry>
>diff --git a/docs-xml/wscript_build b/docs-xml/wscript_build
>index 575fb702b46..3dad0a21313 100644
>--- a/docs-xml/wscript_build
>+++ b/docs-xml/wscript_build
>@@ -31,6 +31,7 @@ manpages='''
>          manpages/samba-tool.8
>          manpages/samba.7
>          manpages/samba.8
>+         manpages/samba_downgrade_db.8
>          manpages/sharesec.1
>          manpages/smbcacls.1
>          manpages/smbclient.1
>-- 
>2.17.1
>
>
>From 29188aefefefbac2567c2bbb2c0c02be1b8b8ace Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date: Mon, 22 Jul 2019 13:35:21 +1200
>Subject: [PATCH 05/11] downgradedatabase: installing script
>
>Installing downgrade script so people don't need the source tree for it.
>
>Exception added in usage test because running the script without arguments
>is valid. (This avoids the need to knownfail it).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit 6dcf00ba0a470ba25aabae06b409ec95404c246f)
>---
> python/samba/tests/usage.py         | 2 ++
> selftest/knownfail.d/usage          | 1 -
> source4/scripting/bin/wscript_build | 3 ++-
> source4/scripting/wscript_build     | 2 +-
> 4 files changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/python/samba/tests/usage.py b/python/samba/tests/usage.py
>index ba18a3e0729..4b7bccde758 100644
>--- a/python/samba/tests/usage.py
>+++ b/python/samba/tests/usage.py
>@@ -78,6 +78,8 @@ EXCLUDE_USAGE = {
>     'selftest/tap2subunit',
>     'script/show_test_time',
>     'source4/scripting/bin/subunitrun',
>+    'bin/samba_downgrade_db',
>+    'source4/scripting/bin/samba_downgrade_db',
>     'source3/selftest/tests.py',
>     'selftest/tests.py',
>     'python/samba/subunit/run.py',
>diff --git a/selftest/knownfail.d/usage b/selftest/knownfail.d/usage
>index 0a495858e8c..3e54f80a2de 100644
>--- a/selftest/knownfail.d/usage
>+++ b/selftest/knownfail.d/usage
>@@ -25,7 +25,6 @@ samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_rebuildextendedd
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_renamedc.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_repl_cleartext_pwd_py.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_rodcdns.none.
>-samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_downgrade_db.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_gpupdate.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_gpupdate_.none.
> samba.tests.usage.samba.tests.usage.PythonScriptUsageTests.test_samba_kcc.none.
>diff --git a/source4/scripting/bin/wscript_build b/source4/scripting/bin/wscript_build
>index 42b37faa32a..87d23545487 100644
>--- a/source4/scripting/bin/wscript_build
>+++ b/source4/scripting/bin/wscript_build
>@@ -7,6 +7,7 @@ if bld.CONFIG_SET('AD_DC_BUILD_IS_ENABLED'):
>                    'samba_kcc',
>                    'samba_upgradeprovision',
>                    'samba_upgradedns',
>-                   'gen_output.py']:
>+                   'gen_output.py',
>+                   'samba_downgrade_db']:
>         bld.SAMBA_SCRIPT(script, pattern=script, installdir='.')
> bld.SAMBA_SCRIPT('samba-gpupdate', pattern='samba-gpupdate', installdir='.')
>diff --git a/source4/scripting/wscript_build b/source4/scripting/wscript_build
>index df24e921cd9..31c395d3e4b 100644
>--- a/source4/scripting/wscript_build
>+++ b/source4/scripting/wscript_build
>@@ -4,7 +4,7 @@ from samba_utils import MODE_755
> 
> sbin_files = ''
> if bld.CONFIG_SET('AD_DC_BUILD_IS_ENABLED'):
>-    sbin_files = 'bin/samba_dnsupdate bin/samba_spnupdate bin/samba_upgradedns bin/samba_kcc '
>+    sbin_files = 'bin/samba_downgrade_db bin/samba_dnsupdate bin/samba_spnupdate bin/samba_upgradedns bin/samba_kcc '
> sbin_files += 'bin/samba-gpupdate'
> man_files = 'man/samba-gpupdate.8'
> 
>-- 
>2.17.1
>
>
>From 3ac338ba411833a1009bca86267af9f2de83e119 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 30 Jul 2019 16:40:55 +1200
>Subject: [PATCH 06/11] ldb: Always log when the database pack format changes
>
>LDB_DEBUG_WARNING gets logged by Samba as level 2, whereas the default
>log level for Samba is 0. It's not really fair to the user to change the
>format of their database on disk and potentially not tell them.
>
>This patch adds a log with level zero (using a alias define, as this
>technically isn't a fatal problem).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit a2b0fc7c00360f37ed6819f21380294b70d4a195)
>---
> lib/ldb/include/ldb.h                | 3 +++
> lib/ldb/ldb_key_value/ldb_kv_index.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ldb/include/ldb.h b/lib/ldb/include/ldb.h
>index f06d5e95528..3cba0f4d543 100644
>--- a/lib/ldb/include/ldb.h
>+++ b/lib/ldb/include/ldb.h
>@@ -220,6 +220,9 @@ struct tevent_context;
> enum ldb_debug_level {LDB_DEBUG_FATAL, LDB_DEBUG_ERROR,
> 		      LDB_DEBUG_WARNING, LDB_DEBUG_TRACE};
> 
>+/* alias for something that's not a fatal error but we really want to log */
>+#define LDB_DEBUG_ALWAYS_LOG  LDB_DEBUG_FATAL
>+
> /**
>   the user can optionally supply a debug function. The function
>   is based on the vfprintf() style of interface, but with the addition
>diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
>index ef275b28013..5de316579d7 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
>+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
>@@ -3571,7 +3571,7 @@ static int re_pack(struct ldb_kv_private *ldb_kv,
> 	 * want to spam the log.
> 	 */
> 	if ((!ctx->normal_record_seen) && (!ldb_dn_is_special(msg->dn))) {
>-		ldb_debug(ldb, LDB_DEBUG_WARNING,
>+		ldb_debug(ldb, LDB_DEBUG_ALWAYS_LOG,
> 			  "Repacking database with format %#010x",
> 			  ldb_kv->pack_format_version);
> 		ctx->normal_record_seen = true;
>-- 
>2.17.1
>
>
>From 26eab76b20c1b6a7de93bd75b97a7530d018bacc Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 30 Jul 2019 15:02:25 +1200
>Subject: [PATCH 07/11] ldb: Move where we update the pack format version
>
>Store it on the repack context so that we can log a more informative
>message "Repacking from format x to format y".
>
>While this is not really a big deal currently, it could be worth
>recording for potential future scenarios (i.e. supporting three or more
>pack versions), where upgrades could potentially skip an intermediary
>pack format version.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit d427bd6c775d8117504e76eed42cd2c383512e34)
>---
> lib/ldb/ldb_key_value/ldb_kv.c       | 2 --
> lib/ldb/ldb_key_value/ldb_kv.h       | 1 +
> lib/ldb/ldb_key_value/ldb_kv_index.c | 3 +++
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/lib/ldb/ldb_key_value/ldb_kv.c b/lib/ldb/ldb_key_value/ldb_kv.c
>index f768fb5e1e4..4e7b8a116b3 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv.c
>+++ b/lib/ldb/ldb_key_value/ldb_kv.c
>@@ -315,8 +315,6 @@ static int ldb_kv_maybe_repack(struct ldb_kv_private *ldb_kv) {
> 	    ldb_kv->target_pack_format_version) {
> 		int r;
> 		struct ldb_context *ldb = ldb_module_get_ctx(ldb_kv->module);
>-		ldb_kv->pack_format_version =
>-			ldb_kv->target_pack_format_version;
> 		r = ldb_kv_repack(ldb_kv->module);
> 		if (r != LDB_SUCCESS) {
> 			ldb_debug(ldb, LDB_DEBUG_ERROR,
>diff --git a/lib/ldb/ldb_key_value/ldb_kv.h b/lib/ldb/ldb_key_value/ldb_kv.h
>index e627644ba34..f9dffae2dcf 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv.h
>+++ b/lib/ldb/ldb_key_value/ldb_kv.h
>@@ -175,6 +175,7 @@ struct ldb_kv_repack_context {
> 	int error;
> 	uint32_t count;
> 	bool normal_record_seen;
>+	uint32_t old_version;
> };
> 
> 
>diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
>index 5de316579d7..eb84a790e00 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
>+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
>@@ -3595,10 +3595,13 @@ int ldb_kv_repack(struct ldb_module *module)
> 	struct ldb_kv_repack_context ctx;
> 	int ret;
> 
>+	ctx.old_version = ldb_kv->pack_format_version;
> 	ctx.count = 0;
> 	ctx.error = LDB_SUCCESS;
> 	ctx.normal_record_seen = false;
> 
>+	ldb_kv->pack_format_version = ldb_kv->target_pack_format_version;
>+
> 	/* Iterate all database records and repack them in the new format */
> 	ret = ldb_kv->kv_ops->iterate(ldb_kv, re_pack, &ctx);
> 	if (ret < 0) {
>-- 
>2.17.1
>
>
>From e291a13bf874691c08f58b9e2a7f4ab22a1252b6 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 30 Jul 2019 15:15:40 +1200
>Subject: [PATCH 08/11] ldb: Change pack format defines to enum
>
>The main reason is so that any future pack formats will continue
>incrementing this number in a sequential fashion.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit 38e3e7cd328edac302e95ac8839e858c4a225485)
>---
> lib/ldb/include/ldb_module.h | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
>diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
>index ab3d25c5c6e..8c1e5ee7936 100644
>--- a/lib/ldb/include/ldb_module.h
>+++ b/lib/ldb/include/ldb_module.h
>@@ -559,12 +559,15 @@ int ldb_unpack_get_format(const struct ldb_val *data,
> #define LDB_UNPACK_DATA_FLAG_NO_ATTRS        0x0008
> #define LDB_UNPACK_DATA_FLAG_READ_LOCKED     0x0010
> 
>-/* In-use packing formats */
>-#define LDB_PACKING_FORMAT 0x26011967
>-#define LDB_PACKING_FORMAT_V2 0x26011968
>+enum ldb_pack_format {
> 
>-/* Old packing formats */
>-#define LDB_PACKING_FORMAT_NODN 0x26011966
>+	/* Old packing format (based on a somewhat arbitrary date) */
>+	LDB_PACKING_FORMAT_NODN = 0x26011966,
>+
>+	/* In-use packing formats */
>+	LDB_PACKING_FORMAT,
>+	LDB_PACKING_FORMAT_V2
>+};
> 
> /**
>  Forces a specific ldb handle to use the global event context.
>-- 
>2.17.1
>
>
>From 4b765dace0f26e024202c7a6f5da6d9b2e20357a Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Tue, 30 Jul 2019 16:40:55 +1200
>Subject: [PATCH 09/11] ldb: Log pack format in user-friendly way
>
>The "format 0x26011968" log confused me (and I'm a developer).
>We can subtract the base offset from the pack format to get a more
>user-friendly number, e.g. v0 (not actually used), v1, v2, etc.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit 5fee9388422e259c2a56e4dccbf44d22ba426ca3)
>---
> lib/ldb/ldb_key_value/ldb_kv_index.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
>diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
>index eb84a790e00..27454d3f734 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
>+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
>@@ -3526,6 +3526,18 @@ static int re_index(struct ldb_kv_private *ldb_kv,
> 	return 0;
> }
> 
>+/*
>+ * Convert the 4-byte pack format version to a number that's slightly
>+ * more intelligible to a user e.g. version 0, 1, 2, etc.
>+ */
>+static uint32_t displayable_pack_version(uint32_t version) {
>+	if (version < LDB_PACKING_FORMAT_NODN) {
>+		return version; /* unknown - can't convert */
>+	}
>+
>+	return (version - LDB_PACKING_FORMAT_NODN);
>+}
>+
> static int re_pack(struct ldb_kv_private *ldb_kv,
> 		   _UNUSED_ struct ldb_val key,
> 		   struct ldb_val val,
>@@ -3572,8 +3584,9 @@ static int re_pack(struct ldb_kv_private *ldb_kv,
> 	 */
> 	if ((!ctx->normal_record_seen) && (!ldb_dn_is_special(msg->dn))) {
> 		ldb_debug(ldb, LDB_DEBUG_ALWAYS_LOG,
>-			  "Repacking database with format %#010x",
>-			  ldb_kv->pack_format_version);
>+			  "Repacking database from v%u to v%u format",
>+			  displayable_pack_version(ctx->old_version),
>+			  displayable_pack_version(ldb_kv->pack_format_version));
> 		ctx->normal_record_seen = true;
> 	}
> 
>-- 
>2.17.1
>
>
>From bf62e772a7794176d8ea24e7ba79fd2b4663c9a8 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 31 Jul 2019 10:33:49 +1200
>Subject: [PATCH 10/11] ldb: Log the partition we're repacking
>
>Firstly, with Samba AD this looks a little weird because we log the same
>message 5 times (once for every partition). If we log that we're doing
>this to records in different partitions, hopefully someone with a little
>Samba knowledge can figure out what's going on.
>
>Secondly, the info about what partitions are actually changing might be
>useful. E.g. if we hit a fatal error repacking the 3rd partition, and
>the transaction doesn't abort properly, then it would be useful to know
>what partitions were repacked and which ones weren't.
>
>There doesn't appear to be a useful name for the partition
>(ldb_kv->kv_ops->name() doesn't seem any more intelligible to a user),
>so just log the first record that we update. We can use that to infer
>the partition database).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>(cherry picked from commit ee6537c29e747206ee607493ce15d4532fb670c8)
>---
> lib/ldb/ldb_key_value/ldb_kv_index.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
>index 27454d3f734..d955517ea10 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
>+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
>@@ -3584,9 +3584,11 @@ static int re_pack(struct ldb_kv_private *ldb_kv,
> 	 */
> 	if ((!ctx->normal_record_seen) && (!ldb_dn_is_special(msg->dn))) {
> 		ldb_debug(ldb, LDB_DEBUG_ALWAYS_LOG,
>-			  "Repacking database from v%u to v%u format",
>+			  "Repacking database from v%u to v%u format "
>+			  "(first record %s)",
> 			  displayable_pack_version(ctx->old_version),
>-			  displayable_pack_version(ldb_kv->pack_format_version));
>+			  displayable_pack_version(ldb_kv->pack_format_version),
>+			  ldb_dn_get_linearized(msg->dn));
> 		ctx->normal_record_seen = true;
> 	}
> 
>-- 
>2.17.1
>
>
>From f048bbb25a1a8d40475382b3d0d3c4c794865174 Mon Sep 17 00:00:00 2001
>From: Tim Beale <timbeale@catalyst.net.nz>
>Date: Wed, 31 Jul 2019 10:54:29 +1200
>Subject: [PATCH 11/11] ldb: Free memory when repacking database
>
>The msg for each database record is allocated on the module context, but
>never freed. The module seems like it could be a long-running context (as
>the database would normally get repacked by the samba executable).
>
>Even if it's not a proper leak, it shouldn't hurt to cleanup the memory.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14059
>
>Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Garming Sam <garming@catalyst.net.nz>
>
>Autobuild-User(master): Garming Sam <garming@samba.org>
>Autobuild-Date(master): Tue Aug 20 04:57:10 UTC 2019 on sn-devel-184
>
>(cherry picked from commit b6516dbd24df8c78ed909c7ef9058b0844abb917)
>---
> lib/ldb/ldb_key_value/ldb_kv_index.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
>index d955517ea10..0853b28fe40 100644
>--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
>+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
>@@ -3599,6 +3599,7 @@ static int re_pack(struct ldb_kv_private *ldb_kv,
> 			  ctx->count);
> 	}
> 
>+	talloc_free(msg);
> 	return 0;
> }
> 
>-- 
>2.17.1
>

Flags:

timbeale: review+
garming: review-

Actions: View

Attachments on bug 14059: 15401 | 15418