[PATCH] shadow_copy2: Allow configurable prefix for snapshots

[Rajesh Joseph]

On Mon, May 9, 2016 at 12:16 PM, Ira Cooper <ira at wakeful.net> wrote:

> Uri Simchoni <uri at samba.org> writes:
>
> > I haven't gone through the patch line-by-line yet but if Jeremy's
> > commenting, I have comments too:
> >
> > 1. Most of the config->x --> priv->config->x changes can be avoided by
> > adding a "config" local variable set to priv->config. This both makes
> > the patch smaller (and easier to review!), and in most if not all of the
> > cases represents (IMHO) better coding style.
>
> I had him do that due to bugs I hit in similar code.  Jeremy warned me,
> he ok'd it.. and then we both missed a crash bug ;).  That's why he did
> it that way.  Blame me.  The original code was you suggest.
>
> > 2. The patch seems to require that the client first call
> > get_shadow_copy_data_fn, because that's where the snapentries get
> > populated. I think there's no such assumption in the code prior to the
> > patch. The unit tests go directly to the snapshots without listing them
> > first. Not sure what to make of this, perhaps we can start that way and
> > add lazy-population of the list if need arises.
>
> Maybe we should just lazy populate? :)  One code path beats two!
>

As per my assumption get_shadow_copy_data_fn should always be called.
Otherwise how will Windows client get the snapshot list to work with?
Yes, the previous code does not make any such assumption, but I guess
this is the function which will filter the valid snapshots from all the
available
snapshots using shadow:format option. If we can bypass this function that
means we are not doing this validation. May be I am missing something here.
Can you point me to the unit tests so that I can look into it?

Thanks & Regards,
Rajesh