[PATCH] Samba: CephFS Snapshots VFS module

[Jeremy Allison]

On Wed, May 01, 2019 at 11:20:47PM +0200, David Disseldorp via samba-technical wrote:
> On Fri, 29 Mar 2019 18:45:31 +0100, David Disseldorp via samba-technical wrote:
> 
> > The attached patchset adds a new ceph_snapshots Samba VFS module which
> > handles snapshot enumeration and timewarp/@GMT token mapping.
> > 
> > This patchset depends on a new CephFS virtual xattr to obtain the
> > snapshot creation time, which is pending merge:
> > https://tracker.ceph.com/issues/38838
> 
> The Ceph changes have been merged upstream, so I'd like to proceed with
> this patchset in it's current state.
> 
> Review/push appreciated.

Quick question during review.

There are a several uses of:

+       char snaps_path[PATH_MAX + 1];
..
+       ret = snprintf(snaps_path, sizeof(snaps_path), "%s/%s",
+                      parent_dir, snapdir);

You have a talloc context available (or talloc_tos()),
isn't it easier to just do a

char *snaps_path = NULL;
snaps_path = talloc_asprintf(mem_ctx,
			"%s/%s",
			parent_dir,
			snapdir);

and then TALLOC_FREE after use ?

I'm not really worried about optimization here
(unless you've got the numbers), just that
talloc_asprintf() seems to be the idiom we
commonly use inside source3/smbd/open.c to
construct pathnames.

Not really a criticism, just wondered if
this was a concious decision here ?

Jeremy.