| Message ID | 20231115175146.9848-3-Nikolai.Kondrashov@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Thanks for the comments, Darrick! On 11/15/23 20:58, Darrick J. Wong wrote: > On Wed, Nov 15, 2023 at 07:43:50PM +0200, Nikolai Kondrashov wrote: >> +xfstests >> +-------- >> + >> +:Summary: File system regression test suite >> +:Source: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git > > You might as well use the https link to the fstests git repo. > https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git Sure! Queued for the next version. >> +:Docs: https://github.com/tytso/xfstests-bld/blob/master/Documentation/what-is-xfstests.md > > Awkardly, this github link is nice for rendering the markdown as html, > but I think the canonical source of xfstests-bld is also kernel.org: > > https://git.kernel.org/pub/scm/fs/ext2/xfstests-bld.git Alright. Changed to https://git.kernel.org/pub/scm/fs/ext2/xfstests-bld.git/tree/Documentation/kvm-quickstart.md And changed the kvm-xfstests docs link to https://git.kernel.org/pub/scm/fs/ext2/xfstests-bld.git/tree/Documentation/kvm-quickstart.md >> +kvm-xfstests smoke >> +------------------ >> + >> +:Summary: File system smoke tests >> +:Superset: xfstests > > Source: https://git.kernel.org/pub/scm/fs/ext2/xfstests-bld.git > > ? Well, I wasn't sure what to put here either :D I would defer to you guys in this matter. I'm actually not really sure we need the "Source:" field. I think maybe having just the "Docs" (HOWTO) field would less confusing. I.e. just go read the docs, they should tell you what and how to get. I mean you got the sources, and then what? Look for the docs there yourself? What do you think? >> +:Docs: https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md >> + >> +The "kvm-xfstests smoke" is a minimal subset of xfstests for testing all major >> +file systems, running under KVM. >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 2565c04f0490e..f81a47d87ac26 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -7974,6 +7974,7 @@ L: linux-ext4@vger.kernel.org >> S: Maintained >> W: http://ext4.wiki.kernel.org >> Q: http://patchwork.ozlabs.org/project/linux-ext4/list/ >> +V: kvm-xfstests smoke > > I wouldn't mind one of these being added to the XFS entry, though I've > cc'd the current and past maintainer(s) of XFS for their input. Sure, just give me a shout when you're ready and I'll add it :D Thanks! Nick
On Fri, Nov 17, 2023 at 12:39:56PM +0530, Chandan Babu R wrote: > IMHO, For XFS, The value of "V" field should refer to xfstests rather than a > framework built around xfstests. This is because xfstests project contains the > actual tests and also we could have several frameworks (e.g. Kdevops) for > running xfstests. For ext4, what I plan to do is to start requiring, in a soon-to-be written: Documentation/process/maintainer-ext4.rst that *all* patches (exempting spelling/grammer nits which only touch comments and result in zero changes in the compiled .o file) will require running the xfstests smoke test. The prooblem is that for newbie patch submitters, and for "drive-by" patches from, say, mm developers, installing and configuring xfstests, and then figuring out how to set up all of the config files, and/or environments variables, before you get to running "./check -g smoke"P, is really f**king hard. As far as other test frameworks are concerned, I suggest that you ask a new college grad that your company has just hired, or a new intern, or a new GSOC or Outreachy intern, to apply a patch to the upstream linux tree --- given only the framework's documentation, and with ***no*** help from anyone who knows how to use the framework. Just sit on your hands, and try as best you can to keep your mouth shut.... and wait. I spent a lot of work to make the instructures here: https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md easy enough that it meets this critera. What should be in the V: field for the MAINTAINERS field is less clear to me, because it's not clear whether we want it to be Dead Stupid Easy for anyone capable of creating a kernel patch can figure out how to run the tests. My personal opinion is that for someone who running through the Kernel Newbies' My First Kernel patch, to say, doing something trivial such as changing "(a < b) ? a : b" to "min(a,b)" and then being able compile kernel, and then be able to say, "it builds, ship it", and then following the kernel documentation to send a patch upstream --- the documentation and procedure necessary to do something better than "it builds, ship it!" by that Kernel Newbie. It's also my considered opinion that neither bare, upstream xfstests, *nor* kdevops meets this criteria. For example, kdevops may claim that you only need a few commands: "make menuconfig" "make" "make bringup" "make linux" "make fstests" "make fstests-baseline" "make fstests-results" ... but to be honest, I never got past the 2nd step before *I* got massively confused and decided there was better things to do with my time. First, the "make menuconfig" requires that you answer a whole bunch of questions, and it's not clear how you are supposed to answer them all. Secondly "make" issues a huge number of cmomands, and then fails because I didn't run them as root. But I won't download random git repositories and "make" as root. It goes against the grain. And so after trying to figure out what it did, and whether or not it was safe, I ultimetly gave up on it. For upstream xfstests, ask a New College Grad fresh hire, or intern, to read xfstests's README file, and ask them to set up xfstests, without help. Go ahead, I'll wait.... No doubt for someone who is willing to make a huge investment in time to become a file system developer specializing in that subsystem, you will eventually be able to figure it out. And in the case of kdevops, eventually I'd could set up my own VM, and install a kernel compile toolchian, and all of kdevops's prequisits, and then run "make" and "make linux" in that VM. But that's a lot more than just "four commands". So as for *me*, I'm going to point people at: https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md because I've simplified a lot of things, up to and including have kvm-xfstests automatically download the test appliance VM image from kernel.org if necessary. When it lists the handful of commands that need to be run, it includes downloading all of the prequisit packages, and there are no complex menu configuration steps, and doesn't require running random make processes of Makefile and Makefile fragments as root. (And note that I keep the xfstests-bld repo's on kernel.org and github.com both uptodate, and I prefer using the using the github.com URL because it's easier for the new developer to read and understand it.) Ultimately, at least for my planned Documentation/process/maintainer-ext4.rst, before I could make running a smoke test mandatory, I needed to make sure that the quickstart documentation for kvm-xfstests be made as simple and as fool-proof as possible. That was extremely important to me from a personal point of view. As far as what should go into Documentation/process/tests.rst, for "kvm-xfstests smoke" this may depend on whether other file system maintainers want to adopt something which is similarly simple for first-time developers to run. Also, I would assert that the proposed V: line in the Maintainer's file does not mean that this is the only way to test the patch. It is just the minimal amount of testing that should be done, and using the simplest test procedure that we would expect a non-subsystem developer to be able to use. It certainly wouldn't be the only way to run a satisfactory amount of pre-submit testing. For example, *I* wouldn't be using "kvm-xfstests smoke". I'd be using something like "gce-xfstests smoke", although that requires a bit more setup. Or I might do a much more substantive amount of testing, such as "gce-xfstests ltm -c ext4/all -g auto". Or I might establish a watch on a branch, via: "gce-xfstests ltm -c ext4/all -g auto --repo https://github.com/tytso/ext4.git --watch test", and then just push commits to the test branch. And similarly, just because the V: line might say, "kvm-xfstests smoke", someone could certainly use kdevops if they wanted to. So perhaps we need to be a bit clearer about what we expect the V: line to mean? Along these lines, we should perhaps be a bit more thoughtful about the intended audience for Documentation/process/tests.rst. I personally wouldn't try ask first-time kernel developers to look at the xfstests group files, because that's just way too complicated for them. And I had *assumed* that Documentation/process/tests.rst was not primarily intended for sophistiocated file system developers who wouldn't be afraid to start looking at the many ways that xfstests could be configured. But we should perhaps be a bit more explicit about who the intended audience would be for a certain set up Documentation files, since that will make it easier for us to come to consensus about how that particular piece of documentation would be worded. As E.B. White (author of the book "The Elements of Style" was reputed to have once said, "Always write with deep sympathy for the reader". Which means we need to understand who the reader is, and to try to walk in their shoes, first. Cheers, - Ted
On 11/20/23 00:54, Theodore Ts'o wrote: > So as for *me*, I'm going to point people at: > > https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md ... > (And note that I keep the xfstests-bld repo's on kernel.org and > github.com both uptodate, and I prefer using the using the github.com > URL because it's easier for the new developer to read and understand > it.) I already queued a switch to the kernel.org URL, which Darrick has suggested. I'll drop it now, but you guys would have to figure it out between yourselves, which one you want :D Personally, I agree that the one on GitHub is more reader-friendly, FWIW. > And similarly, just because the V: line might say, "kvm-xfstests > smoke", someone could certainly use kdevops if they wanted to. So > perhaps we need to be a bit clearer about what we expect the V: line > to mean? I tried to handle some of that with the "subsets", so that you can run a wider test suite and still pass the Tested-with: check. I think this has to be balanced between allowing all the possible ways to run the tests and a reasonable way to certify the commit was tested automatically. E.g. name the test "xfstests", and list all the ways it can be executed, thus communicating that it should still say "Tested-with: xfstests" regardless of the way. And if there is a smaller required subset, name it just "xfstests smoke" and list all the ways it can be run, including the simplest "kvm-xfstests smoke", but accept just "Tested-with: xfstests-smoke". I'm likely getting things wrong, but I hope you get what I'm trying to say. > Along these lines, we should perhaps be a bit more thoughtful about > the intended audience for Documentation/process/tests.rst. I > personally wouldn't try ask first-time kernel developers to look at > the xfstests group files, because that's just way too complicated for > them. > > And I had *assumed* that Documentation/process/tests.rst was not > primarily intended for sophistiocated file system developers who > wouldn't be afraid to start looking at the many ways that xfstests > could be configured. But we should perhaps be a bit more explicit > about who the intended audience would be for a certain set up > Documentation files, since that will make it easier for us to come to > consensus about how that particular piece of documentation would be > worded. > > As E.B. White (author of the book "The Elements of Style" was reputed > to have once said, "Always write with deep sympathy for the reader". > Which means we need to understand who the reader is, and to try to > walk in their shoes, first. Amen to that! Apart from the newbies and just people working on other subsystems, we should also remember to be kinder to ourselves and keep our own tools easier to use. So perhaps just say "newbies should be able to follow tests.rst", and enjoy it :D Ultimately, I think the (admittedly elusive) target should be the ability to just plop a command line into every V: entry, running something from the tree itself. Meanwhile, we would need the stepping stone of tests.rst, or something like that, to walk people through whatever setup is required. I'll see how we can accommodate the commands in the V: directly, though. Nick
On Wed, Nov 22, 2023 at 04:44:58PM +0200, Nikolai Kondrashov wrote: > On 11/20/23 00:54, Theodore Ts'o wrote: > > So as for *me*, I'm going to point people at: > > > > https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md > > ... > > > (And note that I keep the xfstests-bld repo's on kernel.org and > > github.com both uptodate, and I prefer using the using the github.com > > URL because it's easier for the new developer to read and understand > > it.) > > I already queued a switch to the kernel.org URL, which Darrick has suggested. > I'll drop it now, but you guys would have to figure it out between yourselves, > which one you want :D > > Personally, I agree that the one on GitHub is more reader-friendly, FWIW. For xfstests-bld links, I'm ok with whichever domain Ted wants. > > And similarly, just because the V: line might say, "kvm-xfstests > > smoke", someone could certainly use kdevops if they wanted to. So > > perhaps we need to be a bit clearer about what we expect the V: line > > to mean? > > I tried to handle some of that with the "subsets", so that you can run a wider > test suite and still pass the Tested-with: check. I think this has to be > balanced between allowing all the possible ways to run the tests and a > reasonable way to certify the commit was tested automatically. > > E.g. name the test "xfstests", and list all the ways it can be executed, thus > communicating that it should still say "Tested-with: xfstests" regardless of > the way. And if there is a smaller required subset, name it just "xfstests > smoke" and list all the ways it can be run, including the simplest > "kvm-xfstests smoke", but accept just "Tested-with: xfstests-smoke". > > I'm likely getting things wrong, but I hope you get what I'm trying to say. Not entirely -- for drive-by contributions and obvious bugfixes, a quick "V: xfstests-bld: kvm-xfstests smoke" / "V: fstests: ./check -g smoke" run is probably sufficient. (Insofar as n00bs running ./check isn't sufficient, but that's something that fstests needs to solve...) For nontrivial code tidying, the author really ought to run the whole test suite. It's still an open question as to whether xfs tidying should run the full fuzz suite too, since that increases the runtime from overnightish to a week. For /new features/, the developer(s) ought to come up with a testing plan and run that by the community. Eventually those will merge into fstests or ktest or wherever. --D > > Along these lines, we should perhaps be a bit more thoughtful about > > the intended audience for Documentation/process/tests.rst. I > > personally wouldn't try ask first-time kernel developers to look at > > the xfstests group files, because that's just way too complicated for > > them. > > > > And I had *assumed* that Documentation/process/tests.rst was not > > primarily intended for sophistiocated file system developers who > > wouldn't be afraid to start looking at the many ways that xfstests > > could be configured. But we should perhaps be a bit more explicit > > about who the intended audience would be for a certain set up > > Documentation files, since that will make it easier for us to come to > > consensus about how that particular piece of documentation would be > > worded. > > > > As E.B. White (author of the book "The Elements of Style" was reputed > > to have once said, "Always write with deep sympathy for the reader". > > Which means we need to understand who the reader is, and to try to > > walk in their shoes, first. > > Amen to that! Apart from the newbies and just people working on other > subsystems, we should also remember to be kinder to ourselves and keep our own > tools easier to use. So perhaps just say "newbies should be able to follow > tests.rst", and enjoy it :D > > Ultimately, I think the (admittedly elusive) target should be the ability to > just plop a command line into every V: entry, running something from the tree > itself. Meanwhile, we would need the stepping stone of tests.rst, or something > like that, to walk people through whatever setup is required. > > I'll see how we can accommodate the commands in the V: directly, though. > > Nick >
On 11/22/23 18:17, Darrick J. Wong wrote: > On Wed, Nov 22, 2023 at 04:44:58PM +0200, Nikolai Kondrashov wrote: >> On 11/20/23 00:54, Theodore Ts'o wrote: >> I already queued a switch to the kernel.org URL, which Darrick has suggested. >> I'll drop it now, but you guys would have to figure it out between yourselves, >> which one you want :D >> >> Personally, I agree that the one on GitHub is more reader-friendly, FWIW. > > For xfstests-bld links, I'm ok with whichever domain Ted wants. Great! I just hope I can keep track of all the requests :D >>> And similarly, just because the V: line might say, "kvm-xfstests >>> smoke", someone could certainly use kdevops if they wanted to. So >>> perhaps we need to be a bit clearer about what we expect the V: line >>> to mean? >> >> I tried to handle some of that with the "subsets", so that you can run a wider >> test suite and still pass the Tested-with: check. I think this has to be >> balanced between allowing all the possible ways to run the tests and a >> reasonable way to certify the commit was tested automatically. >> >> E.g. name the test "xfstests", and list all the ways it can be executed, thus >> communicating that it should still say "Tested-with: xfstests" regardless of >> the way. And if there is a smaller required subset, name it just "xfstests >> smoke" and list all the ways it can be run, including the simplest >> "kvm-xfstests smoke", but accept just "Tested-with: xfstests-smoke". >> >> I'm likely getting things wrong, but I hope you get what I'm trying to say. > > Not entirely -- for drive-by contributions and obvious bugfixes, a quick > "V: xfstests-bld: kvm-xfstests smoke" / "V: fstests: ./check -g smoke" > run is probably sufficient. > > (Insofar as n00bs running ./check isn't sufficient, but that's something > that fstests needs to solve...) > > For nontrivial code tidying, the author really ought to run the whole > test suite. It's still an open question as to whether xfs tidying > should run the full fuzz suite too, since that increases the runtime > from overnightish to a week. > > For /new features/, the developer(s) ought to come up with a testing > plan and run that by the community. Eventually those will merge into > fstests or ktest or wherever. Of course, makes sense. Thank you! Nick
diff --git a/Documentation/process/tests.rst b/Documentation/process/tests.rst index 907311e91ec45..9a9ea3fe65c37 100644 --- a/Documentation/process/tests.rst +++ b/Documentation/process/tests.rst @@ -33,3 +33,35 @@ particularly useful: :Source: A URL pointing to the source code of the test suite :Docs: A URL pointing to further test suite documentation + +xfstests +-------- + +:Summary: File system regression test suite +:Source: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git +:Docs: https://github.com/tytso/xfstests-bld/blob/master/Documentation/what-is-xfstests.md + +As the name might imply, xfstests is a file system regression test suite which +was originally developed by Silicon Graphics (SGI) for the XFS file system. +Originally, xfstests, like XFS was only supported on the SGI's Irix operating +system. When XFS was ported to Linux, so was xfstests, and now xfstests is +only supported on Linux. + +Today, xfstests is used as a file system regression test suite for all of +Linux's major file systems: xfs, ext2, ext4, cifs, btrfs, f2fs, reiserfs, gfs, +jfs, udf, nfs, and tmpfs. Many file system maintainers will run a full set of +xfstests before sending patches to Linus, and will require that any major +changes be tested using xfstests before they are submitted for integration. + +The easiest way to start running xfstests is under KVM with xfstests-bld: +https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md + +kvm-xfstests smoke +------------------ + +:Summary: File system smoke tests +:Superset: xfstests +:Docs: https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md + +The "kvm-xfstests smoke" is a minimal subset of xfstests for testing all major +file systems, running under KVM. diff --git a/MAINTAINERS b/MAINTAINERS index 2565c04f0490e..f81a47d87ac26 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7974,6 +7974,7 @@ L: linux-ext4@vger.kernel.org S: Maintained W: http://ext4.wiki.kernel.org Q: http://patchwork.ozlabs.org/project/linux-ext4/list/ +V: kvm-xfstests smoke T: git git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git F: Documentation/filesystems/ext4/ F: fs/ext4/
Signed-off-by: Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com> --- Documentation/process/tests.rst | 32 ++++++++++++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 33 insertions(+)