On Thu, Nov 8, 2018 at 2:28 PM J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > On Thu, Nov 08, 2018 at 02:25:02PM -0500, J. Bruce Fields wrote: > > On Thu, Nov 08, 2018 at 01:51:58PM -0500, Olga Kornievskaia wrote: > > > On Wed, Nov 7, 2018 at 1:57 PM J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > > > > > > > On Fri, Oct 19, 2018 at 11:29:04AM -0400, Olga Kornievskaia wrote: > > > > > From: Olga Kornievskaia <kolga@xxxxxxxxxx> > > > > > > > > > > The inter server to server COPY source server filehandle > > > > > is a foreign filehandle as the COPY is sent to the destination > > > > > server. > > > > > > > > Compounds can do a lot of different strange things, and I'm not > > > > convinced this code handles every case correctly. Examples: I think > > > > that > > > > > > > > PUTFH > > > > TEST_STATEID > > > > SAVEFH > > > > COPY > > > > > > > > will incorrectly return nfserr_stale if the PUTHF gets a foreign > > > > filehandle, even though that filehandle is only used as the source of > > > > the COPY. And: > > > > > > > > PUTFH > > > > SAVEFH > > > > RENAME > > > > COPY > > > > > > > > will pass an unverified source filehandle to rename. > > > > > > > > I can think of a couple ways to get this right for certain: > > > > > > > > - delay all filehandle verification till the time the filehandle > > > > isused. That would make checking this simple, but it would > > > > change our behavior so, for example PUTFH+READ with a bad > > > > filehandle will return the error on the READ where it used to > > > > return it on the PUTFH. I don't know if that's a problem. > > > > > > > > - somewhere at the start of nfsd4_proc_compound, do one pass > > > > through the compound checking where the filehandles will be > > > > used and marking those ops that can skip checking. E.g.: > > > > > > > > nfsd4_op *current, *saved > > > > > > > > foreach op in compound: > > > > - if op is putfh: > > > > current := op > > > > - if op is savefh: > > > > saved := current > > > > - if op is restorefh: > > > > current := saved > > > > - etc. > > > > - if op is copy: > > > > mark_no_verify(saved) > > > > > > > > Or something like that. > > > > > > Do you have a preference over the 2 proposed methods? I'm not sure if > > > there is anything wrong with returning ERR_STALE on READ instead of > > > the PUTFH but for historical reasons it seems wrong to change it. Thus > > > I'd say doing it the 2nd way is better. But then 2nd approach adds an > > > overhead of going thru operations twice for any compound. Is that > > > acceptable? > > > > I think so. Most compounds are pretty short and I don't think it'll be > > a big deal. > > So, yes, could you try the second approach? > > I've been working on the 1st approach--I have a patch here and I may > experiment with it some more. But I'm starting to think that the > separate scan will work better. > > --b.
