[PATCH] File Server Remote VSS Protocol server

[Jeremy Allison]

On Fri, Mar 20, 2015 at 01:06:30AM +0100, David Disseldorp wrote:
> On Thu, 19 Mar 2015 16:47:03 -0700, Jeremy Allison wrote:
> 
> > > >   git://git.samba.org/ddiss/samba.git fsrvp_srv  
> > > 
> > > Did anyone get a chance to start looking at this?  
> > 
> > First comment I've found (this *really* is good...).
> > 
> > +       become_root();
> > +       ret = ioctl(dest_fd, BTRFS_IOC_SNAP_CREATE_V2, &ioctl_arg);
> > +       unbecome_root();
> > +       if (ret < 0) {
> > +               DEBUG(0, ("%s -> %s(%s) BTRFS_IOC_SNAP_CREATE_V2 failed: %s\n",
> > +                         base_volume, dest_path, dest_subvolume,
> > +                         strerror(errno)));
> > +               status = map_nt_error_from_unix(errno);
> > 
> > You need to save off errno inside of the become_root()/unbecome_root();
> > pairs if you want to examine it later, as this can change errno in the
> > unbecome_root() code path.
> 
> Good catch. I'll fix these and resend after you're done with the
> first pass.

More stuff:

2206 +       dest_path = dirname(dest_path);
2207 +       subvolume = basename(subvolume);

We don't check for the C functions dirname() or basename()
in lib/replace as far as I can see - so
if you want to use them they need adding.

The snapper stuff I'll just have to trust you on
mostly (can't test that, but can't see any obvious
errors in the code :-).

2623 +       str_idx = strrchr(path_dup, '/');

shouldn't this be strtchr_m ? Are '/' characters
safe for MB processing ? Need to check...

3066 +       s = strstr(unc, "\\\\");

Same for uses of '\\'

Ok, I'm undergoing review fatigue right now...

Can you fix up the known issues and resend ?

I'll take another look once I get round 2.

Cheers,

	Jeremy.