Created attachment 308611 [details] [review]
ZFS detection (v1)

Hi Curtis,

Here's patchset v1 for this.  (Not that adding half a dozen lines to
detect a new file system type takes multiple patches.  But there's a
minor colour adjustment and removal of that skip setting lvm flag
message tidyup patches too).

Because I was vacillating about colours I also generated a GParted file
system colour table and a before and after rainbow screenshot including
every detected file system type.

Thanks,
Mike

Created attachment 308612 [details]
GParted file system colours table

Created attachment 308613 [details]
GParted rainbow screenshot 1 [before]

Created attachment 308614 [details]
GParted rainbow screenshot 2 [after]

Forgot to mention that I used the ZFS test file system image from the
util-linux package.  Downloadable directly out of GIT from here:
http://git.kernel.org/cgit/utils/util-linux/util-linux.git/plain/tests/ts/blkid/images-fs/zfs.img.bz2

Thanks Mike for working to add ZFS recognition.  I plan to review and test the patch in comment #1 after the upcoming GParted release.

Curtis

Thank you Mike for the patch set and especially for the detailed screen shots and link to a zfs image for testing.  These made review significantly easier.

The patch set v1 in comment #1 looks good to me and passed all my tests on kubuntu 12.04.  As such I have committed patch set v1 to the git repository for inclusion in the next release of GParted.

The relevant git commits can be viewed at the following links:

Add detection of ZFS (#752862)
https://git.gnome.org/browse/gparted/commit/?id=71715a1c2924498f432bdc9921d17274a307d58b

Minor update of colours for LVM2 PVs and ReFS
https://git.gnome.org/browse/gparted/commit/?id=3276951ccd0aab15b09125ad607acf31ba53998d

Remove "Skip setting unsupported partition flag" message
https://git.gnome.org/browse/gparted/commit/?id=9f0b5ee633ea2cb3311da3a3883f1ea0c7a95a53

Hi Mike,

For the release notes I was thinking of the following text in an effort to avoid confusion for persons packaging GParted:

Dependencies (new/updated)
--------------------------
  * [Optional] ZFS recognition requires blkid from util-linux >= 2.15


Your thoughts on this wording, or a suggestion of improved wording would be appreciated.

Curtis

Hi Curtis,

For ZFS detection I don't think that util-linux counts as an updated
dependency.  GParted already needs util-linux for detection of a number
of file systems.  (Also the needed version of util-linux package is
older or the same as that needed for detection of many other file
systems).

I also looked at previous release notes and util-linux wasn't mentioned
as an updated dependency for detection of any of the newly added file
system.

GParted release notes
---------------------
0.6.3  Added swap labeling          - no mention of util-linux >= 2.18
0.10.0 Added exfat detection        - no mention of util-linux >= 2.18
0.16.0 Added f2fs support           - Mentions f2fs-tools,
                                      not util-linux >= 2.22.2
0.17.0 Added SWRaid detection       - no mention of util-linux >= 2.15
0.17.0 Added swap suspend detection - no mention of util-linux >= 2.15
0.21.0 Added ReFS detection         - no mention of util-linux >= 2.24

I recommend not adding the above from comment #8 to the dependencies
section of the release notes at all.

Mike

This has made me think about GParted's dependency on blkid.  Blkid is
needed for detection of a number of file systems and in a number of
situations where libparted doesn't work.

blkid is needed to detect these:
  exfat
  f2fs
  ReFS
  SWRaid
  ZFS

blkid detects these (but GParted also has internal detection):
  btrfs    (also detected by libparted >= 3.2)
  LVM2 PV
  LUKS
  nilfs2   (also detected by libparted >= 2.4)
  reiser4

Also blkid is needed for:

Avoid whole disk FAT being detected as MSDOS partition table (#743181)
https://git.gnome.org/browse/gparted/commit/?id=f8faee637787329c07771e495c9b26abc9ac1603
(With libparted 1.9.0 to 2.3)

Enhance file system detection to use FS_Info method - blkid
https://git.gnome.org/browse/gparted/commit/?id=9e5e9f5627e0fb1b547f07cdaaac773c7b6035b9
(Detect file systems with sector sizes other than 512 bytes with
libparted 2.2)


So I think we should declare GParted as depending on the blkid command
and strongly recommend to packagers that they make a hard dependency on
the blkid command or the util-linux package as appropriate for their
distribution and packaging tool.  We should update the README file and
future release notes for this.


Mike

Hi Mike,

Thank you for your thoughts.

I will remove the line from the dependencies section of the future release notes.

Originally GParted used only libparted to detect file systems.  Next recognition was built directly into GParted for some file systems.  More recently we've relied on blkid to detect the file system so now blkid is indeed more important.

Please feel free to make changes to the README and other files as you see fit and post the patch in this report.  I can review and then commit the patch.

Curtis

Hi Curtis,

I've raised a separate bug report for updating the documentation of the
software dependencies because there are a couple of other small changes
I will be bundling with it.

Bug 753436 - Update documentation of GParted software dependencies

Mike

This enhancement was included in the GParted 0.24.0 release on October 27, 2015.