[PATCH] vfs_shadow_copy2: add snapsharepath parameter

Uri Simchoni uri at samba.org
Thu Jan 14 05:23:21 UTC 2016


Hi,

This is something that's been in my queue for some time waiting for the 
vfs_shadow_copy2 fixes to get pushed. It adds some needed flexibility in 
locating the share's files in a snapshot.

Michael - I submit it now hoping that all the path arithmetic is still 
in your L2 or L3 cache (although it doesn't drastically change anything).

Review appreciated.
Thanks,
Uri.

-------------- next part --------------
From 5b844bfc7902ec0bdf70c33fd5e7a30b9f115ef4 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 3 Nov 2015 09:15:12 +0200
Subject: [PATCH 1/5] vfs_shadow_copy2: remove basedir state variable

Remove the basedir state variable from the module-specific data
of vfs_shadow_copy2 - this variable is not being used.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_shadow_copy2.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 018ec88..7ea7d35 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -45,9 +45,8 @@ struct shadow_copy2_config {
 	bool fixinodes;
 	char *sort_order;
 	bool snapdir_absolute;
-	char *basedir;
 	char *mount_point;
-	char *rel_connectpath; /* share root, relative to the basedir */
+	char *rel_connectpath; /* share root, relative to a snapshot root */
 	char *snapshot_basepath; /* the absolute version of snapdir */
 };
 
@@ -1873,7 +1872,7 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 	const char *snapdir;
 	const char *gmt_format;
 	const char *sort_order;
-	const char *basedir;
+	const char *basedir = NULL;
 	const char *mount_point;
 
 	DEBUG(10, (__location__ ": cnum[%u], connectpath[%s]\n",
@@ -1991,6 +1990,7 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 				  "relative ('%s'), but it has to be an "
 				  "absolute path. Disabling basedir.\n",
 				  basedir));
+			basedir = NULL;
 		} else {
 			char *p;
 			p = strstr(basedir, config->mount_point);
@@ -2000,37 +2000,30 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 					  "mount point (%s). "
 					  "Disabling basedir\n",
 					  basedir, config->mount_point));
-			} else {
-				config->basedir = talloc_strdup(config,
-								basedir);
-				if (config->basedir == NULL) {
-					DEBUG(0, ("talloc_strdup() failed\n"));
-					errno = ENOMEM;
-					return -1;
-				}
+				basedir = NULL;
 			}
 		}
 	}
 
-	if (config->snapdirseverywhere && config->basedir != NULL) {
+	if (config->snapdirseverywhere && basedir != NULL) {
 		DEBUG(1, (__location__ " Warning: 'basedir' is incompatible "
 			  "with 'snapdirseverywhere'. Disabling basedir.\n"));
-		TALLOC_FREE(config->basedir);
+		basedir = NULL;
 	}
 
-	if (config->crossmountpoints && config->basedir != NULL) {
+	if (config->crossmountpoints && basedir != NULL) {
 		DEBUG(1, (__location__ " Warning: 'basedir' is incompatible "
 			  "with 'crossmountpoints'. Disabling basedir.\n"));
-		TALLOC_FREE(config->basedir);
+		basedir = NULL;
 	}
 
-	if (config->basedir == NULL) {
-		config->basedir = config->mount_point;
+	if (basedir == NULL) {
+		basedir = config->mount_point;
 	}
 
-	if (strlen(config->basedir) != strlen(handle->conn->connectpath)) {
+	if (strlen(basedir) != strlen(handle->conn->connectpath)) {
 		config->rel_connectpath = talloc_strdup(config,
-			handle->conn->connectpath + strlen(config->basedir));
+			handle->conn->connectpath + strlen(basedir));
 		if (config->rel_connectpath == NULL) {
 			DEBUG(0, ("talloc_strdup() failed\n"));
 			errno = ENOMEM;
@@ -2068,7 +2061,6 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 
 	DEBUG(10, ("shadow_copy2_connect: configuration:\n"
 		   "  share root: '%s'\n"
-		   "  basedir: '%s'\n"
 		   "  mountpoint: '%s'\n"
 		   "  rel share root: '%s'\n"
 		   "  snapdir: '%s'\n"
@@ -2081,7 +2073,6 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 		   "  sort order: %s\n"
 		   "",
 		   handle->conn->connectpath,
-		   config->basedir,
 		   config->mount_point,
 		   config->rel_connectpath,
 		   config->snapdir,
-- 
2.4.3


From f2ba7515c2dd1c2e8c55b16a1f0d2aa4f223e7cf Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 3 Nov 2015 10:42:00 +0200
Subject: [PATCH 2/5] vfs_shadow_copy2: check crossmountpoints against
 snapdirseverywhere

If crossmountpoints is enabled, verify that snapdirseverywhere is
enabled too, since crossmountpoints has no meaning otherwise.

This obviates the check of crossmountpoints against other config
variables.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_shadow_copy2.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 7ea7d35..27dbe51 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -1927,6 +1927,11 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 						"shadow", "crossmountpoints",
 						false);
 
+	if (config->crossmountpoints && !config->snapdirseverywhere) {
+		DBG_WARNING("Warning: 'crossmountpoints' depends on "
+			    "'snapdirseverywhere'. Disabling crossmountpoints.\n");
+	}
+
 	config->fixinodes = lp_parm_bool(SNUM(handle->conn),
 					 "shadow", "fixinodes",
 					 false);
@@ -2011,12 +2016,6 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 		basedir = NULL;
 	}
 
-	if (config->crossmountpoints && basedir != NULL) {
-		DEBUG(1, (__location__ " Warning: 'basedir' is incompatible "
-			  "with 'crossmountpoints'. Disabling basedir.\n"));
-		basedir = NULL;
-	}
-
 	if (basedir == NULL) {
 		basedir = config->mount_point;
 	}
-- 
2.4.3


From b7af4b9ec8f0b4b6162c759ff746b7134aec679c Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 3 Nov 2015 10:57:13 +0200
Subject: [PATCH 3/5] vfs_shadow_copy2: add snapsharepath parameter

This new parameter defines how to get from the snapshot's
root directory to the share's root directory. It is an
alternative to the "basedir" parameter, but functionally
is a superset of basedir.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_shadow_copy2.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 27dbe51..6e6b087 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -1873,6 +1873,7 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 	const char *gmt_format;
 	const char *sort_order;
 	const char *basedir = NULL;
+	const char *snapsharepath = NULL;
 	const char *mount_point;
 
 	DEBUG(10, (__location__ ": cnum[%u], connectpath[%s]\n",
@@ -2016,11 +2017,45 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle,
 		basedir = NULL;
 	}
 
+	snapsharepath = lp_parm_const_string(SNUM(handle->conn), "shadow",
+					     "snapsharepath", NULL);
+	if (snapsharepath != NULL) {
+		if (snapsharepath[0] == '/') {
+			DBG_WARNING("Warning: 'snapsharepath' is "
+				    "absolute ('%s'), but it has to be a "
+				    "relative path. Disabling snapsharepath.\n",
+				    snapsharepath);
+			snapsharepath = NULL;
+		}
+		if (config->snapdirseverywhere && snapsharepath != NULL) {
+			DBG_WARNING("Warning: 'snapsharepath' is incompatible "
+				    "with 'snapdirseverywhere'. Disabling "
+				    "snapsharepath.\n");
+			snapsharepath = NULL;
+		}
+	}
+
+	if (basedir != NULL && snapsharepath != NULL) {
+		DBG_WARNING("Warning: 'snapsharepath' is incompatible with "
+			    "'basedir'. Disabling snapsharepath\n");
+		snapsharepath = NULL;
+	}
+
+	if (snapsharepath != NULL) {
+		config->rel_connectpath = talloc_strdup(config, snapsharepath);
+		if (config->rel_connectpath == NULL) {
+			DBG_ERR("talloc_strdup() failed\n");
+			errno = ENOMEM;
+			return -1;
+		}
+	}
+
 	if (basedir == NULL) {
 		basedir = config->mount_point;
 	}
 
-	if (strlen(basedir) != strlen(handle->conn->connectpath)) {
+	if (config->rel_connectpath == NULL &&
+	    strlen(basedir) != strlen(handle->conn->connectpath)) {
 		config->rel_connectpath = talloc_strdup(config,
 			handle->conn->connectpath + strlen(basedir));
 		if (config->rel_connectpath == NULL) {
-- 
2.4.3


From d8b25f46b460b465b6548681a98f326b70f5fe40 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 3 Nov 2015 11:15:52 +0200
Subject: [PATCH 4/5] vfs_shadow_copy2: add tests for snapsharepath

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 selftest/target/Samba3.pm                | 8 ++++++++
 source3/script/tests/test_shadow_copy.sh | 1 +
 2 files changed, 9 insertions(+)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index cd293a6..44dadfb 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1616,6 +1616,14 @@ sub provision($$$$$$$$)
 	shadow:mountpoint = $shadow_mntdir
 	shadow:snapdirseverywhere = yes
 
+[shadow8]
+	path = $shadow_shrdir
+	comment = previous versions using snapsharepath
+	vfs objects = shadow_copy2
+	shadow:mountpoint = $shadow_mntdir
+	shadow:snapdir = $shadow_tstdir/.snapshots
+	shadow:snapsharepath = share
+
 [shadow_wl]
 	path = $shadow_shrdir
 	comment = previous versions with wide links allowed
diff --git a/source3/script/tests/test_shadow_copy.sh b/source3/script/tests/test_shadow_copy.sh
index eecd5b8..171008d 100755
--- a/source3/script/tests/test_shadow_copy.sh
+++ b/source3/script/tests/test_shadow_copy.sh
@@ -278,6 +278,7 @@ test_shadow_copy_fixed shadow3 mount/base share "sub volume snapshots mounted un
 test_shadow_copy_fixed shadow4 . share "sub volume snapshots mounted outside"
 test_shadow_copy_fixed shadow5 mount/base/share "" "full volume snapshots and share mounted under volume"
 test_shadow_copy_fixed shadow6 . "" "full volume snapshots and share mounted outside"
+test_shadow_copy_fixed shadow8 . share "logical snapshot layout"
 
 # tests for snapshot everywhere - one snapshot location
 test_shadow_copy_fixed shadow7 mount base/share "'everywhere' full volume snapshots"
-- 
2.4.3


From 55d8f260e3255dd2d3203a9986e9c35da902cfbe Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 3 Nov 2015 13:21:58 +0200
Subject: [PATCH 5/5] vfs_shadow_copy2: documentation for snapsharepath

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 docs-xml/manpages/vfs_shadow_copy2.8.xml | 40 ++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/docs-xml/manpages/vfs_shadow_copy2.8.xml b/docs-xml/manpages/vfs_shadow_copy2.8.xml
index ad2e9c7..7fd1600 100644
--- a/docs-xml/manpages/vfs_shadow_copy2.8.xml
+++ b/docs-xml/manpages/vfs_shadow_copy2.8.xml
@@ -226,6 +226,46 @@
                 </varlistentry>
 
 		<varlistentry>
+		<term>shadow:snapsharepath = SNAPSHAREPATH
+		</term>
+		<listitem>
+		<para>
+		With this parameter, one can specify the path of the share's
+		root directory in snapshots, relative to the snapshot's
+		root directory. It is an alternative method to
+		<command>shadow:basedir</command>, allowing greater control.
+		</para>
+		<para>
+		For example, if within each
+		snapshot the files of the share have a
+		<command>path/to/share/</command> prefix, then
+		<command>shadow:snapsharepath</command> can be
+		set to <command>path/to/share</command>.
+		</para>
+		<para>
+		With this parameter, it is no longer assumed that a
+		snapshot represents an image of the original file system or
+		a portion of it. For example, a system could perform
+		backups of only files contained in shares, and then
+		expose the backup files in a logical structure:
+		</para>
+		<itemizedlist>
+		<listitem><para>share1/</para></listitem>
+		<listitem><para>share2/</para></listitem>
+		<listitem><para>.../</para></listitem>
+		</itemizedlist>
+		<para>
+		Note that the <command>shadow:snapdirseverywhere</command>
+		and the <command>shadow:basedir</command> options
+		are incompatible with <command>shadow:snapsharepath</command>
+		and disable <command>shadow:snapsharepath</command> setting.
+		</para>
+		<para>Example: shadow:snapsharepath = path/to/share</para>
+		<para>Default: shadow:snapsharepath = NOT SPECIFIED</para>
+                </listitem>
+		</varlistentry>
+
+		<varlistentry>
                 <term>shadow:sort = asc/desc
                 </term>
                 <listitem>
-- 
2.4.3



More information about the samba-technical mailing list