GNOME Bugzilla – Bug 571170
Initial btrfs support
Last modified: 2010-10-29 16:22:46 UTC
The patch add initial support for btrfs. Only .create, .check and .read_label are done for now, via external btrfs-tools. Other methods are still only stubs. btrfs isn't actually recognized by libparted 1.8, so it will be shown as unknown for some time.
Created attachment 128373 [details] [review] btrfs diff, with proper autotool integration Patch cleanly apply against latest svn (r1057). Tested against a two-partitions device, both btrfs, also with proper libparted discovery (being separately worked); result can be seen at http://people.debian.org/~lucab/images/gparted_btrfs.png Please commit this initial part, as I probably won't have enough time soon to add full support (or let me know if I can directly commit to your svn).
Hi Luca, Detection of btrfs file systems was on my radar for the next release. :-) As you have discovered, the structure of the application (as designed by Bart Hakvoort) makes it fairly easy to add support for new file systems. At least when the new file systems provide command line tools to perform the various operations. I was glad to see that you have are working on and have submitted a patch for btrfs to the parted project. From reading the official btrfs web page, I noted the following warning: <--- begin quote from http://btrfs.wiki.kernel.org/index.php/Main_Page ---> Btrfs is under heavy development, and is not suitable for any uses other than benchmarking and review. The Btrfs disk format is not yet finalized, but it will only be changed if a critical bug is found and no workarounds are possible. <--- end quote ---> Based on this warning, I think that full support for btrfs should not be added to GParted at this time. However, I see no problem with adding detection of btrfs. Since it has been a long time since the last official parted release, it is very useful to add detection of btrfs to GParted as well. Do you have time to work detection of btrfs directly in GParted to your patch?
Created attachment 128768 [details] [review] Discovery added directly in gparted, configure option added to conditionally enable it all So I basically ported here the snippet from my parted patch, to add discovery in gparted; it'll have to be disabled once accepted in a stable libparted release. I also added a --enable-btrfs to enable btrfs support in gparted (disabled by default), so you can safely merge it and leave disabled until you think the format it's stable enough (per what I understood from your reply). Almost all my code is now #ifdef wrapped, fell free to directly insert what you can consider safe now, and leave the rest as is :)
Thank you Bruno for adding detection of btrfs file systems to your patch. Hopefully the btrfs file format will be finalized soon.
Ok, let me know if there's anything more I can do :) I'd also like to point out that all major backends are already adding btrfs support right now (eg. blkid and udev, see their latest release).
Thank you Luca for providing this additional information. I guess my concern is that people will start using btrfs for production data, but not realize that the developers are warning against such use. After discussing this with another team member, he suggested that perhaps a generic dialog could be created to indicate that btrfs is currently experimental. This dialog (or other warning mechanism) could also be used for future new file systems. My preference is to limit the number of conditional code inclusions. If a caution could be built in to warn a user when using btrfs, then I would be ready to add full btrfs support now. Would you be able to work that into the patch?
No, that's ok... If you're not comfortable at this point, I'm with you :) Adding big warning labels wouldn't be of any use; better waiting for a proper stabilization, as this was mainly a early-experimental-testing pushing effort. wrt the experimental dialog, it could be interesting if done in a generic way for other future fs. I don't think I'll find timeslots to work on it for now, so if anybody interested... I was actually pleased with gparted's sourcecode, hopely I/somebody will eventually work on this too.
Thanks for your understanding. For now I will add your code for detecting btrfs, but hold off on adding full support such as create, move, resize, etc. When the on-disk-format has stabilized and the developers no longer warn about only using btrfs for benchmarking and review, then full support can be added :-)
Detection of btrfs file systems has been committed to the Gnome SVN repository for inclusion in the next release of GParted (0.4.4). I modified sections of the patch, but have given the credit to you, Luca Bruno, for your work. This can bee seen in the ChangeLog, and in the AUTHORS file. Full support of btrfs should wait until the usage warning is lifted by the developers, and the on-disk-format is finalized.
It looks like btrfs is getting closer to production ready but not quite there yet. :-) According the btrfs wiki: http://btrfs.wiki.kernel.org/index.php/Main_Page it appears that there was a format change in the v0.19 release. Quote: "v0.19 Released (June 2009) For 2.6.31-rc (FORWARD ROLLING FORMAT CHANGE)" The ChangeLog contains more details: http://btrfs.wiki.kernel.org/index.php/Changelog
Magic signature was left untouched, so detection should still be working as is. Anyway I'll try and come back with patch, if something due...
Thank you for confirming that the magic signature was left untouched. Until btrfs is production ready, I plan to hold off on applying the patch.
It would actually be nice to have it as an experimental feature. --enable-brtfs should do the trick for optional features. Please reconsider. :)
Luca, Would you like to review your work for BTRFS support? In particular would you check to see if the code will support sector sizes > 512 bytes? I am working on adding support for sector sizes > 512 bytes (bug #607165) and I am about half way through implementing this. This progress is viewable in the git repository. The warning about suitable uses has been removed from the BTRFS WIKI page. Hence I believe we have exercised due caution and can now add this support into GParted. <--- begin quote from http://btrfs.wiki.kernel.org/index.php/Main_Page ---> Btrfs is under heavy development, but every effort is being made to keep the filesystem stable and fast. As of 2.6.31, we only plan to make forward compatible disk format changes, and many users have been experimenting with Btrfs on their systems with good results. Please email the Btrfs mailing list if you have any problems or questions while using Btrfs. <--- end quote ---> I am okay with the support being directly added without the need for a --enable-btrfs flag.
There are stable and unstable repositories. The warning has been moved as a note at: https://btrfs.wiki.kernel.org/index.php/Btrfs_source_repositories 'Note: "Stable" here doesn't mean that Btrfs is considered ready for production use, it means "the stable branch of the Btrfs development"' But as far as I am concerned, it's a great opportunity for testing. :)
I'll take another look soon. I'm also sorry that my patch is bind to 512B sector. I've just seen your mail on parted ML on how to setup a debug environment for 4096B, will try on that (after having found the lib interface to query sector-size, as I didn't found at a first glance).
The GParted code uses libparted to determine the device sector size (e.g., lp_device->sector_size). This value is later placed in the device method under the sector_size variable (e.g., device.sector_size). If it makes sense to do so, please feel free to use either of these methods to determine sector size. The relevant git commit can be viewed at the following link: http://git.gnome.org/browse/gparted/commit/?id=19d8f0dfc830e09f2861ecffd756cc3c2b7e9ccc
Several enhancements have now been committed to the git repository to support devices with sector sizes larger than 512 bytes, and also to add a partition alignment option to "align to MiB".
Luca, Today GParted 0.6.0 was released. I think it would be great to include your patch for btrfs support in the next release. Do you think you will get a chance to look at the patch and how it works with devices with sector sizes > 512 bytes?
Ok, tried with your scsi_debug tricks and it seems to work, both recognizing and creation, you fixed it in 892e56f9. I just pushed a new branch rebased on current master and without conflicts, please re-clone it.
Thank you Luca for reviewing the btrfs branch. I will re-clone with the intention to include btrfs support in the next GParted release.
Luca, I am hoping that you can help me. I am experiencing difficulty testing btrfs support. I cloned the origin/lucab/btrfs branch, using the following command: git checkout -b btrfs origin/lucab/btrfs Then I created a tarball using "make dist" and transferred this tarball to my Ubuntu Lucid Lynx 10.04 virtual machine. I used the following command to configure the code: ./configure --enable-btrfs=yes Then I ran "make" and "sudo make install". I have installed the btrfs-tools package which includes: btrfsck btrfsctl btrfs-convert btrfs-image btrfs-vol btrfs-debug-tree btrfs-show mkfs.btrfs While running GParted, I can see the "btrfs" file system option is greyed out when I try to create a new partition. I am trying to create the partition in an MSDOS partition table in an unallocated space which is greater than 256 MiB in size. Also "btrfs" does not show up after choosing the "View --> File System Support" menu entry. Do you know what I have missed, or perhaps that I am doing wrong?
It looks like you really didn't enable it. You have to re-run the complete autotool suite to have it in configure, did you do? Does your config.h "#define BTRFS_SUPPORT 1"? It should also be mentioned at the end of a ./configure run.
Thank you for the reminder. I re-ran the autotools suite, and also noticed that my "make dist" command was creating a different file name version than I had expected. My bad. Now that I have that sorted out I have been able to perform some testing. :-) After creating a btrfs file system, I noticed that GParted displays an exclamation mark beside the partition. The reason for this is often either that the file systems tools are not installed, or that the number of used sectors could not be determined. I took a quick look at the src/btrfs.cc file and noticed that the set_used_sectors method had not been implemented. This is likely the cause of the behaviour I saw. Do you have time to develop this method? Another thought I had is that I think it might be best to have the "ext2" file system selected by default when a user creates a new partition. Also, I was thinking of including btrfs support in GParted by default since the grave warning about using this file system has been removed. Hence users would not need to use the --enable-btrfs=yes configure option to enable btrfs support. What are your thoughts on this (removing the configure option and making btrfs support standard)?
> I took a quick look at the src/btrfs.cc file and noticed that the > set_used_sectors method had not been implemented. This is likely the cause of > the behaviour I saw. > Do you have time to develop this method? > Another thought I had is that I think it might be best to have the "ext2" file > system selected by default when a user creates a new partition. Done today, branch rebased for you to check/clone. It may need a bit of review. > What are your thoughts on this (removing the configure option and making btrfs > support standard)? Also given that ubuntu is already using it, it may be now doable. I'd only add that: * btrfs warning has only been moved somewhere else in the wiki, BUT lots of distro are considering its (early) adoption * userspace tools have been completely rewritten, so my branch will see a some changes once a stable version of them is out (v0.20 presumably, no ETA for now)
Thank you Luca for implementing the set_used_sectors method for btrfs. :-) I am in the process of preparing to accept the lucab/btrfs branch for the upcoming GParted 0.7.0 release. Unfortunately my testing with the code enhancement to default to "ext2" fails on my older Ubuntu 8.04 LTS GNU/Linux distribution. The error I encounter is as follows: <snip> g++ -DHAVE_CONFIG_H -I. -I.. -I/usr/include/gtkmm-2.4 -I/usr/lib/gtkmm-2.4/include -I/usr/include/glibmm-2.4 -I/usr/lib/glibmm-2.4/include -I/usr/include/gdkmm-2.4 -I/usr/lib/gdkmm-2.4/include -I/usr/include/pangomm-1.4 -I/usr/include/atkmm-1.6 -I/usr/include/gtk-2.0 -I/usr/include/sigc++-2.0 -I/usr/lib/sigc++-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/lib/gtk-2.0/include -I/usr/include/cairomm-1.0 -I/usr/include/pango-1.0 -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/pixman-1 -I/usr/include/atk-1.0 -DGPARTED_DATADIR=\""/usr/local/share"\" -DGNOMELOCALEDIR=\""/usr/local/share/locale"\" -Wall -g -O2 -MT Dialog_Partition_New.o -MD -MP -MF .deps/Dialog_Partition_New.Tpo -c -o Dialog_Partition_New.o Dialog_Partition_New.cc Dialog_Partition_New.cc: In member function ‘void GParted::Dialog_Partition_New::Build_Filesystems_Menu(bool)’: Dialog_Partition_New.cc:343: error: ‘class Gtk::MenuItem’ has no member named ‘get_label’ make[2]: *** [Dialog_Partition_New.o] Error 1 make[2]: Leaving directory `/home/gedakc/workspace/gparted/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/gedakc/workspace/gparted' make: *** [all] Error 2 gedakc@quad:~/workspace/gparted$ It appears that the get_label method might be a relatively new method since gtkmm-2.16: http://library.gnome.org/devel/gtkmm/unstable/classGtk_1_1MenuItem.html#a6418a2af3cb9b174d1fef5bbccafd1bb I believe that my Ubuntu 8.04 is running 2.10. My preference would be to have the code also support Ubuntu 8.04 LTS since Ubuntu still supports this version. I think we can probably implement this feature a different way so that it is compatible with more GNU/Linux distributions. Would you like to try reworking the "default to ext2" code? If not then I will try to come up with a solution.
Woops, I didn't notice it. I committed a revert and a more hacky workaround for it, just pushed. While merging, you may want to squash together those three commits (or I may do a rebase for you, but you'll need to re-checkout the branch).
Thank you for the updated patch for defaulting to ext2. I have cherry picked the appropriate patches and committed these to the Gnome git repository master branch. I have also committed the patch from Seth Heeren to stop using Glib::ustring when performing file system signature recognition. These enhancements will be included in the next release of GParted (0.7.0).
I have made btrfs support a normal part of gparted by removing the need to configure with the --enable-btrfs option. The relevant git commit can be viewed at the following link: http://git.gnome.org/browse/gparted/commit/?id=b0e36132b09064f97c99736560bcfe26c93cc669
Thank you Luca for all of your work to add initial support for the btrfs file system. The enhancements to address this bug report have been included in GParted 0.7.0 which was released on October 29, 2010. Closing this bug report.