GNOME Bugzilla – Bug 642842
nilfs is not detected
Last modified: 2012-02-22 00:44:06 UTC
The nilfs file system is not detected by gparted. It was added to the kernel in release 2.6.30.
Thank you Michael for bringing this to my attention. Do you have some good links to information about nilfs that you would like to share? Do you know if there are free software tools that run on GNU/Linux to work with nilfs file systems?
There is of course the nilfs home page http://www.nilfs.org/en/ and http://www.nilfs.org/en/about_nilfs.html is particularly helpful if you wanted to create a partition with it. My interest in the file system is, unlike any other supported by gparted currently, it is more SSD friendly in the way it works. Another is logfs which was added in 2.6.34, but I will leave filing a bug about that to someone with more interest in it then me.
Thanks for the extra info Michael.
Created attachment 201775 [details] [review] Minimal nilfs2 detection
Just attached minimal nilfs2 detection patch (gparted-nilfs2-01.patch) for consideration. Wondering if the colour I have chosen is OK though. I intent to write 1 or 2 more patches to provide full nilfs2 support utilising nilfs-utils. Can then produced just 1 patch containing all changes if preferred. Mike
Created attachment 201903 [details] [review] Minimal nilfs2 detection v2 1) Softened the colour for NILFS2 slightly to better fit the colour palette. 2) Tidied up the commit message.
Created attachment 201958 [details] [review] Draft GParted NILFS2 support patch Here's a draft of the full nilfs2 support patch. Not sure of relevant copyright etiquette. I've just put my name at the top of the 2 new files, although nilfs2.h is only really s/xxx/nilfs2/ from any other FS header. Seems better to let blkid read the fs label because its faster and less code than running nilfs-tune -l /dev/DEVICE and parsing: Filesystem volume name: LABEL or Filesystem volume name: (none) (colon, tab, space, space then LABEL) unless it is needed because of Display ext2/3/4 unicode volume labels properly (#662537). Don't know how to query the file systems used/free space when unmounted so I haven't coded set_used_sectors(). Therefore the Unable to read the contents of the file system! ... warning is displayed. Could mount, statvfs(), umount but that doesn't seem desirable.
Created attachment 202300 [details] [review] Minimal nilfs2 detection v3 Updated minimal nilfs2 detection patch with improved message reporting libparted can also detect nilfs2.
Created attachment 202301 [details] [review] Draft GParted NILFS2 support v2 patch Updated nilfs2 support patch with updated commit message and README tweek. Previous 3 questions about copyright etiquette, use blkid to read label or parse nilfs-tune output and not implementing set_used_sectors() still apply.
Thank you Mike for your work to add support for the NILFS file system. (In reply to comment #7) > Not sure of relevant copyright etiquette. I've just put my name at the > top of the 2 new files, although nilfs2.h is only really s/xxx/nilfs2/ > from any other FS header. Since you created the file, I belive that placing your name in the copyright is the proper thing to do. At least that is what I have done when adding new file system support to GParted. :-) > Seems better to let blkid read the fs label because its faster and > less code than running nilfs-tune -l /dev/DEVICE and parsing: > Filesystem volume name: LABEL > or Filesystem volume name: (none) > (colon, tab, space, space then LABEL) unless it is needed because of > Display ext2/3/4 unicode volume labels properly (#662537). Inside the file system class, I prefer to use the file system specific tools to read the label. From my testing, the problem with blkid (older versions) reading unicode labels does occur for nilfs. Hence even though it is slower, I think that accuracy in reading the label is more important than speed in this case. > Don't know how to query the file systems used/free space when unmounted > so I haven't coded set_used_sectors(). Therefore the > Unable to read the contents of the file system! ... > warning is displayed. Could mount, statvfs(), umount but that doesn't > seem desirable. The "nilfs-tune -l /path-to-partition" command appears to display enough information to calculate the free sectors. Specifically I think that the "Block size:" and "Free blocks count:" lines contain the necessary information. I think something similar in nature to the xfs::set_used_sectors() code would work. The free space is mostly needed when resizing partitions. Do you know if there are resize commands available for nilfs? Also since we haven't committed the nilfs detection patch, would you be able to create a single patch for nilfs support?
(In reply to comment #10) > (In reply to comment #7) > > Seems better to let blkid read the fs label because its faster and > > less code than running nilfs-tune -l /dev/DEVICE and parsing: > > Filesystem volume name: LABEL > > or Filesystem volume name: (none) > > (colon, tab, space, space then LABEL) unless it is needed because of > > Display ext2/3/4 unicode volume labels properly (#662537). > > Inside the file system class, I prefer to use the file system specific tools to > read the label. > > From my testing, the problem with blkid (older versions) reading unicode labels > does occur for nilfs. Hence even though it is slower, I think that accuracy in > reading the label is more important than speed in this case. OK. Will use nilfs specific method. > > Don't know how to query the file systems used/free space when unmounted > > so I haven't coded set_used_sectors(). Therefore the > > Unable to read the contents of the file system! ... > > warning is displayed. Could mount, statvfs(), umount but that doesn't > > seem desirable. > > The "nilfs-tune -l /path-to-partition" command appears to display enough > information to calculate the free sectors. Specifically I think that the > "Block size:" and "Free blocks count:" lines contain the necessary information. > > I think something similar in nature to the xfs::set_used_sectors() code would > work. I remember manually testing it but for some reason deciding the figures between df and nilfs-tune didn't make sence. Will look at it again and code it up. > The free space is mostly needed when resizing partitions. > Do you know if there are resize commands available for nilfs? I didn't think that it was functional (hence the comment in the code), but I have just tested it and it works! Will code this too and try and work out the minimum requirements. > Also since we haven't committed the nilfs detection patch, would you be able to > create a single patch for nilfs support? It'll take me a few days, but I'll code and test the above changes and submit a single patch.
Created attachment 203124 [details] [review] Draft GParted NILFS2 support v4 patch Here's a new draft of a merged nilfs2 support patch. Does fs specific label reading and sets used sectors. Does NOT do resizing. Still working on that.
Thank you for the updated patch Mike. I plan to test and review it mid week.
Created attachment 203288 [details] nilfs2 with resize, shallow detail tree
Created attachment 203289 [details] nilfs2 resize operation details picture with 100,000s hours
Created attachment 203290 [details] nilfs2 with resize, deep detail tree
Support v4 patch is ready for committing, subject to your testing and review of cause, but just doesn't contain any resizing code. Resizing code ... I initially tried to create a shallow operation detail as btrfs does, but also marking each command as either successful or errored. [code#1] + Shrink /dev/sda10 from x.xx GiB to y.yy GiB + shrink file system + "mkdir ..." + "mount ..." + "nilfs-resize ..." + "umount ..." + "rmdir ..." However this resulted in each command reportedly taking 100,000s of hours. [pic#1]. So instead I switched to a deeper operation detail as xfs does. I followed the tree as close as I could, but not so closely with the code. [code#2] + Shrink /dev/sda10 from x.xx GiB to y.yy GiB + shrink file system + create temporary mount point ... + "mkdir ..." + mount /dev/sda10 on ... + "mount ..." + resize mounted file system + "nilfs-resize ..." + unmount ... + "umount ..." + remove temporary mount point ... + "rmdir ..." I have a couple of outstanding issues with this code: 1) When performing a grow the final rmdir operation is displayed as a fail, however it must have succeeded because: 1) the directory is removed, 2) it is not displayed as a warning, 3) no child details with strerror text is added. Probably means I don't understand OperationDetails() yet. 2) About 1 in 10 times of performing a shrink the unmount operation is displayed as a fail, but the following rmdir is performed successfully and the fs is umounted. Every time I add debugging code it never happens. Really strange. [code#1] nilfs2-shallow.cc [pic#1] nilfs2-resize-shallow.png [code#2] nilfs2-deep.cc Suggestions welcome. Mike
Thank you Mike for the new patch and testing information. My preliminary testing with the patch in comment #12 (w/o resizing) is going well. I want to do some more testing, and perform a source code review before this is committed to the source code repository. I have not yet looked closely into your notes from comment #14 onwards. Since you mentioned thousands of seconds of elapsed time is being reported, this might be a problem with initializing the timer. Again I haven't had time to look into this yet. My plan is to thoroughly review the patch (w/o resizing) before looking into the resizing problems you have encountered.
Created attachment 203692 [details] Diff file with minor format and colour changes Great work on the patch Mike! I hope you don't mind but I made some minor format and colour changes before committing your patch to the gnome git repository. The colour change is a personal preference because the light yellow-orange colour appeared to be very close to the accent yellow colour used for XFS file systems. The relevant git commit can be viewed at the following link: http://git.gnome.org/browse/gparted/commit/?id=df20b54d00b0560707961255d3521d0c602e4f53 On the topic of nilfs2 resizing, it might be a while before I can look at these closely. Currently there are some bug reports, such as bug #664050, that I should focus on first before investigating new functionality. Thanks again for your work to improve GParted. :)
Created attachment 203793 [details] [review] Fix uninitialised read in OperationDetail::set_status() patch Hi Curtis, when you have a moment I found this bug. From the commit message ... Fix uninitialised read in OperationDetail::set_status() Setting the status and controlling the timing of operation details initialised with OperationDetail(desc, status, font) uses a conditional branch depending on the uninitialised variable this->status. ... This uninitialised read is triggered in lots places in the operation details. Just the ones I saw were: format partition as ext2, shrink ext2, resize btrfs, format partition as xfs, grow xfs. By chance it hasn't caused any harm until now where the undefined behaviour bites my nilfs2 resize code. For me this patch fixes issue #1 I was having with the deeper operation tree resize code I mentioned in comment #17. > 1) When performing a grow the final rmdir operation is displayed as a > fail, however it must have succeeded because: 1) the directory is > removed, 2) it is not displayed as a warning, 3) no child details > with strerror text is added. Probably means I don't understand > OperationDetails() yet. I'm continuing to look into the other issues I mentioned in comment #17 with my nilfs2 resizing code, so don't let me keep you from other bugs.
Uninitialised read in set_status() patch in comment #20 also definitely resolves issue #2 I was having with the deeper operation tree resize code I mentioned in comment #17. > 2) About 1 in 10 times of performing a shrink the unmount operation is > displayed as a fail, but the following rmdir is performed > successfully and the fs is umounted. Every time I add debugging > code it never happens. Really strange. It's coming along much better now.
It is good to hear that you found and fixed a bug, and are making good progress on nilfs resizing. I hope to take a look at the patch in comment #20 soon.
Created attachment 204299 [details] [review] Add helper functions for mounted file system resizing operations
Created attachment 204300 [details] [review] Add resize support to nilfs2 (#642842)
Created attachment 204301 [details] [review] Update jfs resize to use new helper functions
Created attachment 204302 [details] [review] Update btrfs resize to use new helper functions
Created attachment 204303 [details] [review] Update xfs resize and copy to use new helper functions
Hi Curtis, Here's the nilfs2 resizing code. I added some helper functions to make resizing easier to implement and then went on to use them update btrfs, jfs and xfs resizing and xfs copying code. Comments welcome, Mike
Great catch Mike on the uninitialised read in OperationDetail::set_status()! This patch (comment #20) has been committed to the gnome git repository. Thank you too for your continued work on this enhancement. My to-do list had included making a helper mount/umount function. It was a nice holiday gift to see that you had gone ahead and done created such a helper function. :) I appreciate your patience too. I am working through a backlog of bug reports, forum requests, and patches, and will continue working through the patches you have supplied. If I run into any problems I will be sure to let you know.
Hi Mike, My review and testing of your set of 5 patches in comments 23 through 27 has gone very well. The code looks good and my testing on Ubuntu 11.10 resulted in no problems at all. Thank you again for all of your hard work to enhance GParted. There is one minor item that we should address in patch 1/5 in comment #23. Since English words have been used in two of the log messages, these should either be made available for translation to other languages, or else remove these words. The two text messages in question are: "mkdir: created directory: " and "rmdir: removed directory: " I would be fine going either way on this. For example if you wished to delete the words "created directory: " and "removed directory: " that would be okay with me. If we want to make these words available for translation, it is better practice to make the entire sentence available for translation. This is an example from GParted_Core::update_bootsector: operationdetail .add_child( OperationDetail( /*TO TRANSLATORS: update boot sector of ntfs file system on /dev/sdd1 */ String::ucompose( _("update boot sector of %1 file system on %2"), Utils::get_filesystem_string( partition .filesystem ), partition .get_path() ) ) ) ; You can read more about language translation in the following GNU 'gettext' Utilities link: http://www.gnu.org/software/gettext/manual/gettext.html The short and quick instruction is: 1) To make text available for translation place it between the indicators _( and ) 2) If the text string is not constant, then use String::ucompose and % parameters for the substitution because this enables translators to create a better translation. 3) When using String::ucompose for translation, it is helpful to place a comment immediately above with an example of the sentence with the % parameters filled in. For fixing this problem if it is easier to create a sixth patch that deals with the "create directory" and "remove directory" strings, then that would be fine with me. I would then commit all six patches at the same time.
Created attachment 204995 [details] [review] nilfs2 resize 5 patch set (v2) Hi Curtis, I just made the text translateable in the end. Learnt some more Git Kung Fu (editing midstream patches), so here's version 2 of the resize 5 patch set, concatenated together. Just in case, apply with: git am gparted-nilfs2-resize-v2.mbox
Thanks Mike for this updated patch set. My testing went very smoothly with these new patches. Great job! And thank you for the tip about editing midstream patches. After some Internet searches I learned about using "git rebase bbc643cd^ --interactive". Hopefully you don't mind, but I tried it out to add translation comments to the first of the five patches. It worked wonderfully. The patch set in comment #31 has been committed to the Gnome git repository for inclusion in the next release of GParted. The relevant git commits can be viewed at the following links: Add helper functions for mounted file system resizing operations http://git.gnome.org/browse/gparted/commit/?id=4ab3fe0ee75ed35d049ba349628fb8ea9f6c9d15 Add resize support to nilfs2 (#642842) http://git.gnome.org/browse/gparted/commit/?id=15bca17a46e990f4757b8b7a9a83c5c1496ba3f2 Update jfs resize to use new helper functions http://git.gnome.org/browse/gparted/commit/?id=b0d818b8f9d364d27e0d370fded158393eb771d1 Update btrfs resize to use new helper functions http://git.gnome.org/browse/gparted/commit/?id=a13e1a38636ba76468023d14f3dcf9f43d48c42c Update xfs resize and copy to use new helper functions http://git.gnome.org/browse/gparted/commit/?id=e414b71b73d42c97fd0abc03f859288a1eff19f6 Please note that I added the files src/FileSystem.cc and src/nilfs2.cc to POTFILES.in so that these files will be included in the translator .po files. Add files for translation to POTFILES.in http://git.gnome.org/browse/gparted/commit/?id=08357b6e81f05eb11da34aad3551366db9658949
This enhancement has been included in GParted 0.12.0 released on February, 21, 2012.