On Fri, Jan 12, 2018 at 1:52 PM, Eryu Guan <eguan@xxxxxxxxxx> wrote: > On Thu, Jan 11, 2018 at 01:52:02PM +0200, Amir Goldstein wrote: >> On Thu, Jan 11, 2018 at 1:43 PM, Eryu Guan <eguan@xxxxxxxxxx> wrote: >> > On Sun, Jan 07, 2018 at 08:07:18PM +0200, Amir Goldstein wrote: >> >> Eryu, >> >> >> >> Following patches were used to test overlayfs NFS export. >> >> >> >> Patches 1-5 improve generic/exportfs tests, by adding test coverage >> >> of some directory file handle use cases. >> > >> > open_by_handle test is covering more and more cases, thanks! OTOH, it's >> > getting more and more complicated :) >> >> Yeh, sorry about that ;-) >> >> > >> > But from a quick scan, I think it's time to stop adding new sub-tests to >> > existing tests (maybe we should have done this since last time we added >> > new sub-tests to generic/467), because generic/467 is getting more >> > complex too and makes review harder. >> >> Probably a good idea, but please note that patch 4/7 is fixing a test that was >> not implemented correctly and test 5/7 adds a very minor variant. > > I've gone through the first 5 patches now. All look fine except the > comments to patch 1/7. > > I think we can add a new test that contains the last part of patch 4/7 > ("Check non-stale file handles of renamed dirs") and patch 5/7. And > leave the "fix test" part in patch 4/7. > >> An option is to split the directory related tests (those that are fixed by 4/7) >> out of generic/467 and then fix them and add the new variant. > > This should be fine too, but I slightly tend to not move existing tests, > just add new cases to new tests. > So what I think I will do is not change generic/467, but add a variant of generic/467 that tests open_by_handle after mount cycle that will use -i -o. I will try re-work patch 1/7 and maybe add more changes so open_by_handle becomes more of a tool to write file handle tests instead of a file handle test program that does all tests and just invoked from a tests script. If that doesn't work out, I'll just add more commentary to the patch ;-) Thanks, Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html