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
