netcmd: Improve domain backup targetdir checks

+ Added check that specified targetdir is actually a directory (if it
exists)
+ Deleted a redundant 'Creating targetdir' check that would never be hit
+ Move code into a separate function so we can reuse it for offline
backups (which take a different set of parameters, but still have a
targetdir)

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index cfd97960515ba57a7780bc36793e4216ad58423f..f7b8edb28408a3bc5f75442204079a07db8072c8 100644 (file)
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -139,6 +139,17 @@ def add_backup_marker(samdb, marker, value):
     samdb.modify(m)
 
 
+def check_targetdir(logger, targetdir):
+    if targetdir is None:
+        raise CommandError('Target directory required')
+
+    if not os.path.exists(targetdir):
+        logger.info('Creating targetdir %s...' % targetdir)
+        os.makedirs(targetdir)
+    elif not os.path.isdir(targetdir):
+        raise CommandError("%s is not a directory" % targetdir)
+
+
 def check_online_backup_args(logger, credopts, server, targetdir):
     # Make sure we have all the required args.
     u_p = {'user': credopts.creds.get_username(),
@@ -147,12 +158,8 @@ def check_online_backup_args(logger, credopts, server, targetdir):
         raise CommandError("Creds required.")
     if server is None:
         raise CommandError('Server required')
-    if targetdir is None:
-        raise CommandError('Target directory required')
 
-    if not os.path.exists(targetdir):
-        logger.info('Creating targetdir %s...' % targetdir)
-        os.makedirs(targetdir)
+    check_targetdir(logger, targetdir)
 
 
 # For '--no-secrets' backups, this sets the Administrator user's password to a
@@ -211,10 +218,6 @@ class cmd_domain_backup_online(samba.netcmd.Command):
         lp = sambaopts.get_loadparm()
         creds = credopts.get_credentials(lp)
 
-        if not os.path.exists(targetdir):
-            logger.info('Creating targetdir %s...' % targetdir)
-            os.makedirs(targetdir)
-
         tmpdir = tempfile.mkdtemp(dir=targetdir)
 
         # Run a clone join on the remote