[PATCH] File Server Remote VSS Protocol server

[Jeremy Allison]

On Fri, Mar 20, 2015 at 12:08:59PM -0700, Jeremy Allison wrote:
> 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 '\\'

Yep, other client pathname processing uses _m
functions for uses of '\\' so I think this
needs to change.

I think '/' is safe, as it's low ASCII and
all MB encodings leave those alone (at least
all the ones we support :-).