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. > I have to ask: for simplicify can't we just support COPY compound if > and only if it's in a specific order and then only allow it? We could probably narrow the possibilities down to a few, but I'm a little afraid of overlooking some possible creative client behavior. I don't think it's that hard to follow the spec here, and it may be simpler than verifying an argument about which cases matter. --b.
