Re: [PATCH] ext4/048: skip test of filename wipe if journal checkpoint is not supported

["Theodore Ts'o" <tytso@mit.edu>]

On Sun, Jul 04, 2021 at 06:47:22PM +0800, Eryu Guan wrote:
> 
> I think that depends on if we treat it as a feature or a bugfix. We
> should let the test fail on old kernels if that's a bugfix, and skip the
> test if it's a feature. Apparently, we take it as a feature here.

Yes, it's clearly a feature.

> > feature, and landed just a little bit after filename wipe feature, so
> > use support for the journal checkpoint ioctl as a proxy for support
> > for the filename wipe feature.
> 
> This seems fragile, they're related features but not always bounded
> together, I think it's possible for distros developers decide to
> backport only one of the features (though quite unlikely).

Given the discussion for the patches, I suspect that it will be only
the distros that are meant for Cloud environments (e.g., Google's COS,
maybe Amazon Linux, etc.) that will be interested at all in the first
place --- and if they do, they'll want to take both of them.

That's because it's only in a Cluod environment we can make specific
guarantees about how discard works, and it's mostly in a Cloud
environment where you have customers which appear to be the most
concerned about being able to certify to their auditors that PII can
be reliably wiped after they "lift and shift" from their private data
centers to third-party operated hyperscale Cloud providers.

> Is it worth to introduce features sysfs entry for journal checkpoint and
> filename wipe in ext4? In fstests' point of view, it's great to have
> such entries that could be checked. If it's not worth it, I think the
> "feature proxy" way is acceptable, given that the two features are
> closely related and aimed to the same security issue.

Well, we didn't when the patch first landed, so even if we did add the
features sysfs file, there's no guarantee that distros doing the
backport will pick up the patch to add the features file.  The sysfs
files do take up a few hundred bytes each, and so there is some
resistance in adding a lot of sysfs features, saving them for major
features.

For better or for worse, one of the principles of sysfs is to _not_
require any parsing, such that something like /proc/stat would have to
be replaced by something like individual 1,000+ sysfs files (try
running wc -w /proc/stat), depending on how cpu cores and interrupt
channels the system has.

This is why we are using separate sysfs files for each feature,
instead of a /proc file containing a list of features which fstests
could test by checking the exit status of "grep ^feature_name$
/proc/fs/ext4/feature-list".  But in retrospect, maybe we should have
gone down that path, since adding features to a feature-list
pseudo-file is essentially free, and would make life much more
convenient for fstests --- right now we do a lot of "create a scratch
file system; try to mount it and see if it succeed or fails, which is
a lot of needless church and extra write cycles on the test device,
and takes more time than just running "grep" on a festure-list file.

Alas, sysfs was considered the new hotness, and the superior approach,
while /proc was considered the legacy, hacky approach.  Ah, well...

Cheers,

						- Ted