[PATCH] for bug 13091 (Re: [PATCH] shadow_copy2: mount_point is valid only for locally mountable file-system)

Michael Adam obnox at samba.org
Fri Oct 20 17:13:57 UTC 2017


On 2017-04-26 at 18:18 +0200, Michael Adam via samba-technical wrote:
> On 2017-04-11 at 12:56 +0200, Michael Adam via samba-technical wrote:
> > On 2016-10-21 at 09:53 +0200, Michael Adam wrote:
> > > On 2016-10-10 at 17:11 -0700, Jeremy Allison wrote:
> > > > On Fri, Sep 30, 2016 at 01:31:58PM +0530, Rajesh Joseph wrote:
> > > > > shadow_copy2 currently assumes that all the underlying file-system are
> > > > > mountable and
> > > > > are mounted on samba server. But this may not be true for all cases, e.g.
> > > > > GlusterFS is
> > > > > accessed via libgfapi and is not mounted.
> > > > > 
> > > > > This can lead to connection failure if such file-systems are used along
> > > > > with shadow_copy2.
> > > > > The attached patch assume "/" as the default mount-point in such cases
> > > > > instead of failing
> > > > > the connect.
> > > > > 
> > > > > This may not be a complete fix because shadow_copy2_find_mount_point can
> > > > > return wrong
> > > > > mount_point if by coincidence the "connectpath" is actually present in the
> > > > > local file-system.
> > > > > 
> > > > > I had a brief discussion on this with Michael, and we think that it might
> > > > > be good if VFS layer
> > > > > can tell if the underlying file-system is mountable or not.
> > > > > 
> > > > > It would be great to know what others think on this problem.
> > > > 
> > > > Yeah, setting it to "/" can also cause problems here:
> > > > 
> > > > 2613         } else {
> > > > 2614                 config->snapshot_basepath = talloc_asprintf(config, "%s/%s",
> > > > 2615                                 config->mount_point, config->snapdir);
> > > > 2616                 if (config->snapshot_basepath == NULL) {
> > > > 2617                         DEBUG(0, ("talloc_asprintf() failed\n"));
> > > > 2618                         errno = ENOMEM;
> > > > 2619                         return -1;
> > > > 2620                 }
> > > > 2621         }
> > > > 
> > > > Let me think some more about what "connectpath" should be in
> > > > this case.
> > > > 
> > > > We have an existing VFS function SMB_VFS_CONNECTPATH(), maybe
> > > > this should be expanded to have a different meaning when
> > > > we're dealing with a completely virtual underlying FS.
> > > 
> > > Hm,
> > > 
> > > the connectpath is already set to the path relative to the
> > > volume by vfs_glusterfs/config. (Starting with "/" for the
> > > volume root.)
> > > 
> > > The problem Rajesh was pointing out was that the path
> > > of the share might be set to something like
> > > "path = /usr/local" (signifying a subdir of the gluster volume)
> > > which conincides to be a local path too, and might be
> > > a mount point. So the find-mountpoint function would
> > > return this as the mountpoint, which is wrong.
> > > In gluster's case we would always have to end up with "/".
> > > If there was a flag inside the vfs that indicates whether
> > > the filsystem is a local / mounted or mountable filesystem
> > > or not, then shadow_copy2_find_mount_point and possibly
> > > other places could behave accordingly.
> > 
> > Hi Jeremy,
> > 
> > Any results from thinking more about it? :-)
> > 
> > Even though the glusterfs vfs module has a configurative
> > workaround, I would still like to conceptually adress
> > this...
> 
> Ping. :-)

It's been a while...

We have now created a BZ:

https://bugzilla.samba.org/show_bug.cgi?id=13091

and come up with a different patch, that just
enforces our configurative workaround for
vfs_shadow_copy2 on top of vfs_glusterfs
inside the vfs_glusterfs connect function.

See the attached patch.

Review / push appreciated!

Eventually we may want to add the concept
to Samba that a connectpath may not reflect
a real local path...

Thanks - Michael
-------------- next part --------------
From 0529219de214dcb3a51b2bd9c70dc02b1bb4eefd Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Fri, 20 Oct 2017 14:55:10 +0200
Subject: [PATCH] vfs_glusterfs: Fix exporting subdirs with shadow_copy2

Since the glusterfs vfs module does not operate on a
locally mounted path, but on a "virtual" path starting
at the volume root, some assumptions of the code about
the vfs connect path fail. One example is the shadow_copy2
module which tries to detect the mount point from the
connectpath. In order to circumvent this problem, this
patch forces the "shadow:mountpoint" option to "/", which
skips the mount-point-detection code.

This patch will only have an effect if both the glusterfs
and the shadow_copy2 module are listed in vfs objects
in the right order, i.e. first shadow_copy2, and then
glusterfs.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13091

Pair-Programmed-With: Anoop C S <anoopcs at redhat.com>

Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Anoop C S <anoopcs at redhat.com>
---
 source3/modules/vfs_glusterfs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
index 91cf87faa11..8d12ac65f02 100644
--- a/source3/modules/vfs_glusterfs.c
+++ b/source3/modules/vfs_glusterfs.c
@@ -352,6 +352,16 @@ static int vfs_gluster_connect(struct vfs_handle_struct *handle,
 			  volume, strerror(errno)));
 		goto done;
 	}
+
+	/*
+	 * The shadow_copy2 module will fail to export subdirectories
+	 * of a gluster volume unless we specify the mount point,
+	 * because the detection fails if the file system is not
+	 * locally mounted:
+	 * https://bugzilla.samba.org/show_bug.cgi?id=13091
+	 */
+	lp_do_parameter(SNUM(handle->conn), "shadow:mountpoint", "/");
+
 done:
 	if (ret < 0) {
 		if (fs)
-- 
2.13.6

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20171020/7dffec51/signature.sig>


More information about the samba-technical mailing list