Details

Reviewers

Commits

R16:d24191ebd8ee: Add support for filesystem-specific features

Summary

In some cases, it may be necessary to create a filesystem with specific features enabled/disabled.

This PR is a proposal of an API extension, making it possible to create filesystems this way. It does so by introducing a new m_Features member variable and the createWithFeatures() function to the FileSystem base class. The latter function is implemented for the btrfs and ext2/3/4 filesystems.

Additionnally, the CreateFileSystemJob has been modified to enable creating/formatting filesystems with specific features enabled.

Please note this PR should be considered *work in progress*: most filesystems lack a proper implementation, and it has not been thoroughly tested. However, it is open for discussion, and comments are more than welcome.

Diff Detail

Repository

R16 KPMCore

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

aferraris requested review of this revision.Jun 19 2019, 3:43 PM

aferraris created this revision.

Harbormaster completed remote builds in B12975: Diff 60067.Jun 19 2019, 3:43 PM

aferraris added inline comments.Jun 19 2019, 3:45 PM

src/jobs/createfilesystemjob.cpp
60	We could use a `createWithFeaturesAndLabel()` function, I just prefered going for the simplest approach for now.

aferraris added a reviewer: stikonas.Jul 8 2019, 8:17 AM

-i bytes-per-inodeI am a bit worried that this API would be hard to use from GUI clients. Although in general, I think direction is good.

In this patch features are just passed as QStringList. So the logic of what features are support would have to reside in the client. I think it would be better if you can query kpmcore FileSystem for extra features that it supports. Then it would be easier to implement them in GUI.

E.g. btrfs would return skinny-metadata and a few others, similarly for ext4.

By the way some filesystems have other features passed through other command line flags. Do you think we can add support for those too? For mke2fs you implemented -O feature but some other fs specific features can be specified using other flags, like -C cluster-size. (Some people are actually interested in block size in FAT fs https://bugs.kde.org/show_bug.cgi?id=342178). Although, these "features" need an extra input... So they would need some richer API then just passing it as string.

fs: Change type for filesystem-specific features
fs: Implement a way for the application to query fs features

These patches allow the handling of "complex" filesystem features requiring an argument, and can also provide the application with a list of features supported by a specific filesystem.

Implementation is provided only for "binary" (enabled or not) features of btrfs and ext2/3/4, but upcoming changes will address several complex features, such as cluster-size in ext2/3/4, or FAT block size.

Harbormaster completed remote builds in B14282: Diff 62321.Jul 22 2019, 4:39 PM

stikonas added inline comments.Jul 22 2019, 7:42 PM

src/fs/filesystem.h
64	Can we turn it into a proper class? Filesystem features sound quite general, so I imagine it might be required to modify/extend structure of this class. I would put class feature into `fs/feature.h` Please d-pointerize the class (see FileSystemPrivate for example). That would help to modify the class without breaking C++ ABI.

This new version adds a few improvements:

the FileSystem::Feature class has been moved to a new FSFeature top-level class, as requested
the API has been made simpler by letting the existing create and createWithLabel functions handle the filesystem-specific features, removing the need for a createWithFeatures function
a bug has been fixed in the parsing of available features for btrfs

Finally, this patchset has been thoroughly tested for ext4 and btrfs filesystems.
If everything is OK, I can add support for fat32 blocksize before merging.

Harbormaster completed remote builds in B14876: Diff 63352.Aug 8 2019, 12:15 PM

In D21903#508769, @aferraris wrote:

Finally, this patchset has been thoroughly tested for ext4 and btrfs filesystems.
If everything is OK, I can add support for fat32 blocksize before merging.

Thanks for updating the patch. I think everything is fine. Feel free to merge
(if you can add fat blocksize before merging).

This revision is now accepted and ready to land.Aug 8 2019, 8:27 PM

This revision adds support for setting a custom block size on FAT12/16/32 filesystems.

Harbormaster completed remote builds in B14908: Diff 63419.Aug 9 2019, 2:42 PM

btw, I'm not a kde developer, so you'll probably have to merge my changes yourself.

Hello here! Is there anything blocking this RFC? Reading the backlog, it seems that everything was good, but that was 6 months ago. Is there anything I can do to help this RFC move forward? Thanks!

In D21903#591199, @arnaudr wrote:

Hello here! Is there anything blocking this RFC? Reading the backlog, it seems that everything was good, but that was 6 months ago. Is there anything I can do to help this RFC move forward? Thanks!

Oh, sorry for not merging it yet. I need @aferraris email to make a commit.

Closed by commit R16:d24191ebd8ee: Add support for filesystem-specific features (authored by aferraris, committed by stikonas). · Explain WhyJan 12 2020, 9:06 PM

This revision was automatically updated to reflect the committed changes.

Thanks for merging!

Hmm, it looks like it breaks ABI, so we should have bumped kpmcore SOVERSION in this commit. I'll do that now.

		Path
M		src/fs/CMakeLists.txt (2 lines)
M		src/fs/apfs.h (2 lines)
M		src/fs/apfs.cpp (4 lines)
M		src/fs/bitlocker.h (2 lines)
M		src/fs/bitlocker.cpp (4 lines)
M		src/fs/btrfs.h (5 lines)
M		src/fs/btrfs.cpp (38 lines)
M		src/fs/exfat.h (2 lines)
M		src/fs/exfat.cpp (4 lines)
M		src/fs/ext2.h (5 lines)
M		src/fs/ext2.cpp (53 lines)
M		src/fs/ext3.h (2 lines)
M		src/fs/ext3.cpp (22 lines)
M		src/fs/ext4.h (2 lines)
M		src/fs/ext4.cpp (22 lines)
M		src/fs/extended.h (2 lines)
M		src/fs/extended.cpp (4 lines)
M		src/fs/f2fs.h (2 lines)
M		src/fs/f2fs.cpp (4 lines)
M		src/fs/fat12.h (5 lines)
M		src/fs/fat12.cpp (45 lines)
M		src/fs/fat16.h (3 lines)
M		src/fs/fat16.cpp (17 lines)
M		src/fs/fat32.h (2 lines)
M		src/fs/fat32.cpp (7 lines)
A	M	src/fs/feature.h (77 lines)
A	M	src/fs/feature.cpp (126 lines)
M		src/fs/filesystem.h (21 lines)
M		src/fs/filesystem.cpp (52 lines)
M		src/fs/filesystemfactory.h (2 lines)
M		src/fs/filesystemfactory.cpp (74 lines)
M		src/fs/hfs.h (2 lines)
M		src/fs/hfs.cpp (4 lines)
M		src/fs/hfsplus.h (2 lines)
M		src/fs/hfsplus.cpp (4 lines)
M		src/fs/hpfs.h (2 lines)
M		src/fs/hpfs.cpp (4 lines)
M		src/fs/iso9660.h (2 lines)
M		src/fs/iso9660.cpp (4 lines)
M		src/fs/jfs.h (2 lines)
M		src/fs/jfs.cpp (4 lines)
M		src/fs/linuxraidmember.h (2 lines)
M		src/fs/linuxraidmember.cpp (4 lines)
M		src/fs/linuxswap.h (2 lines)
M		src/fs/linuxswap.cpp (4 lines)
M		src/fs/luks.h (2 lines)
M		src/fs/luks.cpp (3 lines)
M		src/fs/luks2.h (2 lines)
M		src/fs/luks2.cpp (4 lines)
M		src/fs/lvm2_pv.h (2 lines)
M		src/fs/lvm2_pv.cpp (4 lines)
M		src/fs/minix.h (4 lines)
M		src/fs/minix.cpp (4 lines)
M		src/fs/nilfs2.h (2 lines)
M		src/fs/nilfs2.cpp (4 lines)
M		src/fs/ntfs.h (2 lines)
M		src/fs/ntfs.cpp (4 lines)
M		src/fs/ocfs2.h (2 lines)
M		src/fs/ocfs2.cpp (4 lines)
M		src/fs/reiser4.h (2 lines)
M		src/fs/reiser4.cpp (4 lines)
M		src/fs/reiserfs.h (2 lines)
M		src/fs/reiserfs.cpp (4 lines)
M		src/fs/udf.h (2 lines)
M		src/fs/udf.cpp (4 lines)
M		src/fs/ufs.h (2 lines)
M		src/fs/ufs.cpp (4 lines)
M		src/fs/unformatted.h (2 lines)
M		src/fs/unformatted.cpp (4 lines)
M		src/fs/unknown.h (2 lines)
M		src/fs/unknown.cpp (4 lines)
M		src/fs/xfs.h (2 lines)
M		src/fs/xfs.cpp (4 lines)
M		src/fs/zfs.h (2 lines)
M		src/fs/zfs.cpp (4 lines)
M		src/jobs/createfilesystemjob.cpp (5 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	60067	6ea88cc		Jun 19 2019, 3:43 PM	★	★
Diff 2	62321	6ea88cc	- fs: Change type for filesystem-specific features	Jul 22 2019, 4:39 PM	★	★
Diff 3	63352	6ea88cc	This new version adds a few improvements:	Aug 8 2019, 12:15 PM	★	★
Diff 4	63419	6ea88cc	This revision adds support for setting a custom block size on FAT12/16/32…	Aug 9 2019, 2:42 PM	★	★
Diff 5	73356	50697be	R16:d24191ebd8ee4a1792003e5bd280cb7679f5e834	Jan 12 2020, 11:58 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
a4c1a3ebbeea	6113419c7e39	49393d407314	Arnaud Ferraris	fat12/16/32: Add support for filesystem block size (Show More…)	Aug 9 2019, 2:30 PM
49393d407314	ff77c5bf174f	89fcc622b0b3	Arnaud Ferraris	feature: Add comments and cleanup code (Show More…)	Aug 8 2019, 11:59 AM
89fcc622b0b3	c031f432ad8e	027ae1755b82	Arnaud Ferraris	btrfs: Fix parsing of filesystem's available features (Show More…)	Aug 7 2019, 4:55 PM
027ae1755b82	5d236a9edb68	ab042656d619	Arnaud Ferraris	fs: Simplify the creation of filesystem with specific features (Show More…)	Aug 7 2019, 4:52 PM
ab042656d619	3903ccf59c43	debab0683777	Arnaud Ferraris	filesystem: Add new addAvailableFeature method (Show More…)	Aug 7 2019, 4:35 PM
debab0683777	3f915f75bbde	a8d7cc1d1dc6	Arnaud Ferraris	fs: make FSFeature a class of its own (Show More…)	Aug 7 2019, 4:33 PM
a8d7cc1d1dc6	9ac95c145179	df1cd991ce49	Arnaud Ferraris	fs: Implement a way for the application to query fs features (Show More…)	Jul 22 2019, 4:29 PM
df1cd991ce49	24e8a5d4efa2	63f1ee0bc5aa	Arnaud Ferraris	fs: Change type for filesystem-specific features (Show More…)	Jul 22 2019, 4:24 PM
63f1ee0bc5aa	245f7522e374	2f0233dfe46d	Arnaud Ferraris	RFC: Add support for filesystem-specific features (Show More…)	Jun 19 2019, 3:17 PM
2f0233dfe46d	c0e3151627a4	8dfe4793ff51	Arnaud Ferraris	ext2/3/4: Implement creation with filesystem-specific features (Show More…)	Jun 19 2019, 3:11 PM
8dfe4793ff51	d2b68ca63f35	287e58a092b0	Arnaud Ferraris	btrfs: Implement creation with filesystem-specific features (Show More…)	Jun 19 2019, 3:09 PM
287e58a092b0	091d17c64205	d78a54174c74	Arnaud Ferraris	filesystemfactory: Add support for filesystem-specific features (Show More…)	Jun 19 2019, 3:06 PM
d78a54174c74	026a36e8a069	f410a63f3c95	Arnaud Ferraris	fat16: Remove useless constructor (Show More…)	Jun 19 2019, 3:04 PM
f410a63f3c95	13a278484798	6ea88cc737e0	Arnaud Ferraris	fs: Add new member to FileSystem class for filesystem-specific features (Show More…)	Jun 19 2019, 3:00 PM

RFC: Add support for filesystem-specific featuresClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 73356

src/fs/CMakeLists.txt

src/fs/apfs.h

src/fs/apfs.cpp

src/fs/bitlocker.h

src/fs/bitlocker.cpp

src/fs/btrfs.h

src/fs/btrfs.cpp

src/fs/exfat.h

src/fs/exfat.cpp

src/fs/ext2.h

src/fs/ext2.cpp

src/fs/ext3.h

src/fs/ext3.cpp

src/fs/ext4.h

src/fs/ext4.cpp

src/fs/extended.h

src/fs/extended.cpp

src/fs/f2fs.h

src/fs/f2fs.cpp

src/fs/fat12.h

src/fs/fat12.cpp

src/fs/fat16.h

src/fs/fat16.cpp

src/fs/fat32.h

src/fs/fat32.cpp

src/fs/feature.h

src/fs/feature.cpp

src/fs/filesystem.h

src/fs/filesystem.cpp

src/fs/filesystemfactory.h

src/fs/filesystemfactory.cpp

src/fs/hfs.h

src/fs/hfs.cpp

src/fs/hfsplus.h

src/fs/hfsplus.cpp

src/fs/hpfs.h

src/fs/hpfs.cpp

src/fs/iso9660.h

src/fs/iso9660.cpp

src/fs/jfs.h

src/fs/jfs.cpp

src/fs/linuxraidmember.h

src/fs/linuxraidmember.cpp

src/fs/linuxswap.h

src/fs/linuxswap.cpp

src/fs/luks.h

src/fs/luks.cpp

src/fs/luks2.h

src/fs/luks2.cpp

src/fs/lvm2_pv.h

src/fs/lvm2_pv.cpp

src/fs/minix.h

src/fs/minix.cpp

src/fs/nilfs2.h

src/fs/nilfs2.cpp

src/fs/ntfs.h

src/fs/ntfs.cpp

src/fs/ocfs2.h

src/fs/ocfs2.cpp

src/fs/reiser4.h

src/fs/reiser4.cpp

src/fs/reiserfs.h

src/fs/reiserfs.cpp

src/fs/udf.h

src/fs/udf.cpp

src/fs/ufs.h

src/fs/ufs.cpp

src/fs/unformatted.h

src/fs/unformatted.cpp

src/fs/unknown.h

src/fs/unknown.cpp

src/fs/xfs.h

src/fs/xfs.cpp

src/fs/zfs.h

src/fs/zfs.cpp

RFC: Add support for filesystem-specific features
ClosedPublic
Actions

Revision Contents
Changeset List