Attachment 11255 Details for Bug 57297 – revert-series-sambabug14676.diff

[patch] revert-series-sambabug14676.diff

revert-series-sambabug14676.diff (text/plain), 14.58 KB, created by Arvid Requate on 2024-10-23 19:33:23 CEST

(hide)

Description:

Filename:

MIME Type:

Creator: Arvid Requate

Created: 2024-10-23 19:33:23 CEST

Size: 14.58 KB

patch

obsolete

>From 62ce0db00b289f38b12204d776f29241ac88c6ff Mon Sep 17 00:00:00 2001
>From: Arvid Requate <requate@univention.de>
>Date: Wed, 23 Oct 2024 18:52:02 +0200
>Subject: [PATCH 1/6] Revert "samba-tool domain backup: Use new
> ldb.disconnect() method to force-close files during backup"
>
>This reverts commit 0bf80c10ca50515fc64561db68d3c3283945252a.
>---
> python/samba/netcmd/domain/backup.py | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/python/samba/netcmd/domain/backup.py b/python/samba/netcmd/domain/backup.py
>index b27105116dc..871d5b72404 100644
>--- a/python/samba/netcmd/domain/backup.py
>+++ b/python/samba/netcmd/domain/backup.py
>@@ -1181,7 +1181,6 @@ def run(self, sambaopts=None, targetdir=None):
>         # Close the original samdb, to avoid any confusion, we will
>         # not use this any more as the data has all been copied under
>         # the transaction
>-        samdb.disconnect()
>         samdb = None
> 
>         # Open the new backed up samdb, flag it as backed up, and write
>@@ -1199,7 +1198,6 @@ def run(self, sambaopts=None, targetdir=None):
>         add_backup_marker(samdb, "backupType", "offline")
> 
>         # Close the backed up samdb
>-        samdb.disconnect()
>         samdb = None
> 
>         # Now handle all the LDB and TDB files that are not linked to
>-- 
>2.34.1
>
>
>From ca5ab8fa4823b96532f6a9704601605574c0402b Mon Sep 17 00:00:00 2001
>From: Arvid Requate <requate@univention.de>
>Date: Wed, 23 Oct 2024 18:52:15 +0200
>Subject: [PATCH 2/6] Revert "samba-tool: Rework transations/locks to hold a
> lock during mdb backup"
>
>This reverts commit 958931ad379af26dcbc55cfbc49e7886ef8e0550.
>---
> python/samba/netcmd/domain/backup.py | 38 +++++-----------------------
> 1 file changed, 6 insertions(+), 32 deletions(-)
>
>diff --git a/python/samba/netcmd/domain/backup.py b/python/samba/netcmd/domain/backup.py
>index 871d5b72404..741a9a58f0b 100644
>--- a/python/samba/netcmd/domain/backup.py
>+++ b/python/samba/netcmd/domain/backup.py
>@@ -1019,12 +1019,7 @@ def backup_secrets(self, private_dir, lp, logger):
> 
>     # sam.ldb must have a transaction started on it before backing up
>     # everything in sam.ldb.d with the appropriate backup function.
>-    #
>-    # Obtains the sidForRestore (SID for the new DC) and returns it
>-    # from under the transaction
>     def backup_smb_dbs(self, private_dir, samdb, lp, logger):
>-        sam_ldb_path = os.path.join(private_dir, 'sam.ldb')
>-
>         # First, determine if DB backend is MDB.  Assume not unless there is a
>         # 'backendStore' attribute on @PARTITION containing the text 'mdb'
>         store_label = "backendStore"
>@@ -1032,24 +1027,11 @@ def backup_smb_dbs(self, private_dir, samdb, lp, logger):
>                            attrs=[store_label])
>         mdb_backend = store_label in res[0] and str(res[0][store_label][0]) == 'mdb'
> 
>-        # This is needed to keep this variable in scope until the end
>-        # of the transaction.
>-        res_iterator = None
>-
>+        sam_ldb_path = os.path.join(private_dir, 'sam.ldb')
>         copy_function = None
>         if mdb_backend:
>             logger.info('MDB backend detected.  Using mdb backup function.')
>             copy_function = self.offline_mdb_copy
>-
>-            # We can't backup with a write transaction open, so get a
>-            # read lock with a search_iterator().
>-            #
>-            # We have tests in lib/ldb/tests/python/api.py that the
>-            # search iterator takes a read lock effective against a
>-            # transaction.  This in turn will ensure there are no
>-            # transactions on either the main or sub-database, even if
>-            # the read locks were not enforced globally (they are).
>-            res_iterator = samdb.search_iterator()
>         else:
>             logger.info('Starting transaction on ' + sam_ldb_path)
>             copy_function = self.offline_tdb_copy
>@@ -1070,16 +1052,9 @@ def backup_smb_dbs(self, private_dir, samdb, lp, logger):
>                 logger.info('   copying locked/related file ' + sam_file)
>                 shutil.copyfile(sam_file, sam_file + self.backup_ext)
> 
>-        sid = get_sid_for_restore(samdb, logger)
>-
>-        if mdb_backend:
>-            # Delete the iterator, release the read lock
>-            del(res_iterator)
>-        else:
>+        if not mdb_backend:
>             samdb.transaction_cancel()
> 
>-        return sid
>-
>     # Find where a path should go in the fixed backup archive structure.
>     def get_arc_path(self, path, conf_paths):
>         backup_dirs = {"private": conf_paths.private_dir,
>@@ -1170,17 +1145,16 @@ def run(self, sambaopts=None, targetdir=None):
>         samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>                       flags=ldb.FLG_DONT_CREATE_DB)
> 
>-        # Backup secrets, sam.ldb and their downstream files
>         self.backup_secrets(paths.private_dir, lp, logger)
>-        sid = self.backup_smb_dbs(paths.private_dir, samdb, lp, logger)
>+        self.backup_smb_dbs(paths.private_dir, samdb, lp, logger)
> 
>         # Get the domain SID so we can later place it in the backup
>         dom_sid_str = samdb.get_domain_sid()
>         dom_sid = security.dom_sid(dom_sid_str)
> 
>-        # Close the original samdb, to avoid any confusion, we will
>-        # not use this any more as the data has all been copied under
>-        # the transaction
>+        sid = get_sid_for_restore(samdb, logger)
>+
>+        # Close the original samdb
>         samdb = None
> 
>         # Open the new backed up samdb, flag it as backed up, and write
>-- 
>2.34.1
>
>
>From 9a505f4363354584a29b40e2b661f893523fa81b Mon Sep 17 00:00:00 2001
>From: Arvid Requate <requate@univention.de>
>Date: Wed, 23 Oct 2024 18:52:29 +0200
>Subject: [PATCH 3/6] Revert "samba-tool domain backup offline: Use passed in
> samdb when backing up sam.ldb"
>
>This reverts commit 423f808ff48e297745f576a52b2118c4b920a3e4.
>---
> python/samba/netcmd/domain/backup.py | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
>diff --git a/python/samba/netcmd/domain/backup.py b/python/samba/netcmd/domain/backup.py
>index 741a9a58f0b..4874c19e447 100644
>--- a/python/samba/netcmd/domain/backup.py
>+++ b/python/samba/netcmd/domain/backup.py
>@@ -1035,7 +1035,8 @@ def backup_smb_dbs(self, private_dir, samdb, lp, logger):
>         else:
>             logger.info('Starting transaction on ' + sam_ldb_path)
>             copy_function = self.offline_tdb_copy
>-            samdb.transaction_start()
>+            sam_obj = Ldb(sam_ldb_path, lp=lp, flags=ldb.FLG_DONT_CREATE_DB)
>+            sam_obj.transaction_start()
> 
>         logger.info('   backing up ' + sam_ldb_path)
>         self.offline_tdb_copy(sam_ldb_path)
>@@ -1053,7 +1054,7 @@ def backup_smb_dbs(self, private_dir, samdb, lp, logger):
>                 shutil.copyfile(sam_file, sam_file + self.backup_ext)
> 
>         if not mdb_backend:
>-            samdb.transaction_cancel()
>+            sam_obj.transaction_cancel()
> 
>     # Find where a path should go in the fixed backup archive structure.
>     def get_arc_path(self, path, conf_paths):
>@@ -1089,6 +1090,9 @@ def run(self, sambaopts=None, targetdir=None):
> 
>         check_targetdir(logger, targetdir)
> 
>+        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>+                      flags=ldb.FLG_RDONLY)
>+
>         # Iterating over the directories in this specific order ensures that
>         # when the private directory contains hardlinks that are also contained
>         # in other directories to be backed up (such as in paths.binddns_dir),
>@@ -1139,12 +1143,7 @@ def run(self, sambaopts=None, targetdir=None):
>                     all_files.append(full_path)
>                     all_stats.add((s.st_ino, s.st_dev))
> 
>-        # We would prefer to open with FLG_RDONLY but then we can't
>-        # start a transaction which is the strong isolation we want
>-        # for the backup.
>-        samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>-                      flags=ldb.FLG_DONT_CREATE_DB)
>-
>+        # Backup secrets, sam.ldb and their downstream files
>         self.backup_secrets(paths.private_dir, lp, logger)
>         self.backup_smb_dbs(paths.private_dir, samdb, lp, logger)
> 
>-- 
>2.34.1
>
>
>From 85d4988fa80d633d879eda70d6d95fea7ca2f934 Mon Sep 17 00:00:00 2001
>From: Arvid Requate <requate@univention.de>
>Date: Wed, 23 Oct 2024 19:16:46 +0200
>Subject: [PATCH 4/6] Revert "netcmd: Avoid conflicting SIDs when creating an
> offline backup"
>
>This reverts commit 739d7e54e78046dc77385b882fbba38ab5e7bd60.
>---
> python/samba/netcmd/domain/backup.py | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/python/samba/netcmd/domain/backup.py b/python/samba/netcmd/domain/backup.py
>index 4874c19e447..315f8772840 100644
>--- a/python/samba/netcmd/domain/backup.py
>+++ b/python/samba/netcmd/domain/backup.py
>@@ -1092,6 +1092,7 @@ def run(self, sambaopts=None, targetdir=None):
> 
>         samdb = SamDB(url=paths.samdb, session_info=system_session(), lp=lp,
>                       flags=ldb.FLG_RDONLY)
>+        sid = get_sid_for_restore(samdb, logger)
> 
>         # Iterating over the directories in this specific order ensures that
>         # when the private directory contains hardlinks that are also contained
>@@ -1151,8 +1152,6 @@ def run(self, sambaopts=None, targetdir=None):
>         dom_sid_str = samdb.get_domain_sid()
>         dom_sid = security.dom_sid(dom_sid_str)
> 
>-        sid = get_sid_for_restore(samdb, logger)
>-
>         # Close the original samdb
>         samdb = None
> 
>-- 
>2.34.1
>
>
>From 52e9bef1388713ba0050cd0ef7519a264a836234 Mon Sep 17 00:00:00 2001
>From: Arvid Requate <requate@univention.de>
>Date: Wed, 23 Oct 2024 19:17:55 +0200
>Subject: [PATCH 5/6] Revert "netcmd: Fix opening SamDB database for offline
> backup"
>
>This reverts commit d7c111514ad53787af5a7084355126df9999a34f.
>---
> python/samba/netcmd/domain/backup.py | 18 ++++--------------
> python/samba/ntacls.py               |  5 ++++-
> python/samba/tests/ntacls_backup.py  |  2 +-
> 3 files changed, 9 insertions(+), 16 deletions(-)
>
>diff --git a/python/samba/netcmd/domain/backup.py b/python/samba/netcmd/domain/backup.py
>index 315f8772840..56171cc3adf 100644
>--- a/python/samba/netcmd/domain/backup.py
>+++ b/python/samba/netcmd/domain/backup.py
>@@ -1147,31 +1147,21 @@ def run(self, sambaopts=None, targetdir=None):
>         # Backup secrets, sam.ldb and their downstream files
>         self.backup_secrets(paths.private_dir, lp, logger)
>         self.backup_smb_dbs(paths.private_dir, samdb, lp, logger)
>-
>-        # Get the domain SID so we can later place it in the backup
>-        dom_sid_str = samdb.get_domain_sid()
>-        dom_sid = security.dom_sid(dom_sid_str)
>-
>-        # Close the original samdb
>         samdb = None
> 
>         # Open the new backed up samdb, flag it as backed up, and write
>-        # the next SID so the restore tool can add objects. We use
>-        # options=["modules:"] here to prevent any modules from loading.
>+        # the next SID so the restore tool can add objects.
>         # WARNING: Don't change this code unless you know what you're doing.
>         #          Writing to a .bak file only works because the DN being
>         #          written to happens to be top level.
>-        samdb = Ldb(url=paths.samdb + self.backup_ext,
>+        samdb = SamDB(url=paths.samdb + self.backup_ext,
>                       session_info=system_session(), lp=lp,
>-                      options=["modules:"], flags=ldb.FLG_DONT_CREATE_DB)
>+                      flags=ldb.FLG_DONT_CREATE_DB)
>         time_str = get_timestamp()
>         add_backup_marker(samdb, "backupDate", time_str)
>         add_backup_marker(samdb, "sidForRestore", sid)
>         add_backup_marker(samdb, "backupType", "offline")
> 
>-        # Close the backed up samdb
>-        samdb = None
>-
>         # Now handle all the LDB and TDB files that are not linked to
>         # anything else.  Use transactions for LDBs.
>         for path in all_files:
>@@ -1197,7 +1187,7 @@ def run(self, sambaopts=None, targetdir=None):
>         logger.info('running offline ntacl backup of sysvol')
>         sysvol_tar_fn = 'sysvol.tar.gz'
>         sysvol_tar = os.path.join(temp_tar_dir, sysvol_tar_fn)
>-        backup_offline(paths.sysvol, sysvol_tar, paths.smbconf, dom_sid)
>+        backup_offline(paths.sysvol, sysvol_tar, samdb, paths.smbconf)
>         tar.add(sysvol_tar, sysvol_tar_fn)
>         os.remove(sysvol_tar)
> 
>diff --git a/python/samba/ntacls.py b/python/samba/ntacls.py
>index 24af056d2a4..04365646046 100644
>--- a/python/samba/ntacls.py
>+++ b/python/samba/ntacls.py
>@@ -551,7 +551,7 @@ def backup_online(smb_conn, dest_tarfile_path, dom_sid):
>     shutil.rmtree(localdir)
> 
> 
>-def backup_offline(src_service_path, dest_tarfile_path, smb_conf_path, dom_sid):
>+def backup_offline(src_service_path, dest_tarfile_path, samdb_conn, smb_conf_path):
>     """
>     Backup files and ntacls to a tarfile for a service
>     """
>@@ -559,6 +559,9 @@ def backup_offline(src_service_path, dest_tarfile_path, smb_conf_path, dom_sid):
>     tempdir = tempfile.mkdtemp()
>     session_info = system_session_unix()
> 
>+    dom_sid_str = samdb_conn.get_domain_sid()
>+    dom_sid = security.dom_sid(dom_sid_str)
>+
>     ntacls_helper = NtaclsHelper(service, smb_conf_path, dom_sid)
> 
>     for dirpath, dirnames, filenames in os.walk(src_service_path):
>diff --git a/python/samba/tests/ntacls_backup.py b/python/samba/tests/ntacls_backup.py
>index d4e42940d4f..c8e8336fd3c 100644
>--- a/python/samba/tests/ntacls_backup.py
>+++ b/python/samba/tests/ntacls_backup.py
>@@ -184,7 +184,7 @@ def test_backup_offline(self):
>         """
>         ntacls.backup_offline(
>             self.service_root, self.tarfile_path,
>-            self.smb_conf_path, self.dom_sid)
>+            self.samdb_conn, self.smb_conf_path)
>         self._check_tarfile()
> 
>         self.smb_helper.delete_tree()
>-- 
>2.34.1
>
>
>From 8b324537e3fb5ac53d1d7fb4e80c57455cec5336 Mon Sep 17 00:00:00 2001
>From: Arvid Requate <requate@univention.de>
>Date: Wed, 23 Oct 2024 19:18:10 +0200
>Subject: [PATCH 6/6] Revert "netcmd: Workaround issue backing up offline
> domain with lmdb >= 0.9.26"
>
>This reverts commit bb3dcd403ced922574a89011dd3814c4fe87dd76.
>---
> python/samba/netcmd/domain/backup.py | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/python/samba/netcmd/domain/backup.py b/python/samba/netcmd/domain/backup.py
>index 56171cc3adf..5cee5021538 100644
>--- a/python/samba/netcmd/domain/backup.py
>+++ b/python/samba/netcmd/domain/backup.py
>@@ -1147,7 +1147,6 @@ def run(self, sambaopts=None, targetdir=None):
>         # Backup secrets, sam.ldb and their downstream files
>         self.backup_secrets(paths.private_dir, lp, logger)
>         self.backup_smb_dbs(paths.private_dir, samdb, lp, logger)
>-        samdb = None
> 
>         # Open the new backed up samdb, flag it as backed up, and write
>         # the next SID so the restore tool can add objects.
>-- 
>2.34.1
>

Actions: View | Diff

Attachments on bug 57297: 11255