[PATCH] Samba: CephFS Snapshots VFS module
David Disseldorp
ddiss at samba.org
Fri May 3 11:49:10 UTC 2019
On Thu, 2 May 2019 14:09:48 -0700, Jeremy Allison wrote:
> 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 ?
Thanks for the feedback Jeremy.
As these uses are just temporary buffers I decided to use the stack.
I don't really have a strong preference here, though I'd normally like
to avoid talloc_tos().
Cheers, David
More information about the samba-technical
mailing list