[PATCH v2] New VFS module vfs_fruit

[Ralph Böhme]

Hi

thanks for taking time!

On Tue, Jul 08, 2014 at 03:27:03PM -0700, Jeremy Allison wrote:
> On Tue, Jul 08, 2014 at 11:29:55AM +0200, Ralph Böhme wrote:
> > Hi
> > 
> > thanks to Volker's feedback off-list, here's an updated patchset
> > adding {} to all one-line if's and else's.
> > 
> > Review appreciated.
> 
> Just a few comments.
> 
> -------------------------------------------------------
> +/**
> + * Prepend "._" to a path
> + **/
> +static int adouble_path(TALLOC_CTX *ctx, const char *path_in, char **path_out)
> 
> Could be rewritten to use the existing:
> 
> parent_dirname() function (avoid code duplication).

done.

> -------------------------------------------------------
> 
> +/**
> + * Allocate and initialize an AfpInfo struct
> + *
> + * All fields are initialised in network byte order
> + **/
> +static AfpInfo *afpinfo_new(TALLOC_CTX *ctx)
> +{
> +       AfpInfo *ai = talloc_zero(ctx, AfpInfo);
> +       if (ai == NULL) {
> +               return NULL;
> +       }
> +       ai->afpi_Signature = htonl(AFP_Signature);
> +       ai->afpi_Version = htonl(AFP_Version);
> +       ai->afpi_BackupTime = AD_DATE_START;
> +       return ai;
> +}
> 
> In Samba we usually keep in-memory values
> as native host order, then convert to
> network order at wire marshalling/demarshalling
> time. Check out these macros for network byte
> order:
> 
> RSVAL(buf,pos) - like SVAL() but for NMB byte ordering
> RSVALS(buf,pos) - like SVALS() but for NMB byte ordering
> RIVAL(buf,pos) - like IVAL() but for NMB byte ordering
> RIVALS(buf,pos) - like IVALS() but for NMB byte ordering
> RSSVAL(buf,pos,val) - like SSVAL() but for NMB ordering
> RSIVAL(buf,pos,val) - like SIVAL() but for NMB ordering
> RSIVALS(buf,pos,val) - like SIVALS() but for NMB ordering

done.

> -------------------------------------------------------
> 
> +#else
> +                       /* FIXME: direct Solaris xattr syscall */
> +                       fd = attropen(adpath, AFPRESOURCE_EA_NETATALK,
> +                                     mode, 0);
> +#endif
> +               } else {
> +                       /* FIXME: direct open(), don't have an fsp */
> +                       fd = open(adpath, mode);
> +               }
> 
> These direct calls mean the module isn't stackable.
> That's not fatal, but it needs to be called out in
> the docs (apologies if you already do this).

updated the manpage mentioning this.

> -------------------------------------------------------
> 
> This module is certainly coming along. I'm very
> interested in getting it merged,

thanks! Next patch series incorporating all your suggestions,
including the promised selftest, is ready for submission.

> and I'd also love to hear about the requirements you found to modify
> or extend the VFS to fit your needs better !

I guess you're referring to the few places where I had to make direct
syscalls?

Well, for one, it would be nice to have a VFS abstraction to work with
the Solaris xattr API.

Another thing that drove me into directly using syscalls (ftruncate(),
open(), pread(), ...), was the lack of an fsp so I couldn't use the
corresponding VFS functions. With some decent refactoring this may be
fixable inside the VFS module though. I'm happy to take on this one in
the next iteration. :)

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de