GNOME Bugzilla – Bug 723842
GParted resizes the wrong filesystem (does not pass the devid to btrfs filesystem resize)
Last modified: 2014-10-20 17:07:26 UTC
1. Create a btrfs filesystem 2. Add a partition with `btrfs device add` 3. Launch GParted, resize the partition Expected result: GParted resizes the filesystem which is on that partition: it runs `btrfs filesystem resize N:max /tmp/gparted-H1naL8` where N is the devid of the partition. Actual result: GParted resizes the correct partition, but it resizes the filesystem which is on the first device of the multi-device btrfs filesystem. Quoting the btrfs man page: "filesystem resize [devid:][+/-]<size>[gkm]|[devid:]max <path> The devid can be found with btrfs filesystem show and defaults to 1 if not specified."
Hi Pkoroau, Thank you for your interest in GParted and raising this issue. I've reproduced this issue. Full steps are: 1) Use GParted to create 2 GiB btrfs file system (/dev/sda7). 2) Use GParted to create empty 2 GiB partition (/dev/sda8). 3) Add device to file system mount /dev/sda7 /mnt/1 btrfs device add /dev/sda8 /mnt/1 umount /mnt/1 5) Refresh in GParted 6) Use GParted to shrink sda8 to 1 GiB Gparted runs: mkdir -v /tmp/gparted-XXXXXX mount -v -t btrfs /dev/sda8 /tmp/gparted-XXXXXX btrfs filesystem resize 1048576K /tmp/gparted-XXXXXX umount -v /tmp/gparted-XXXXXX rmdir -v /tmp/gparted-XXXXXX Shrinks partition sda8 to 1 GiB # btrfs filesystem show /dev/sda7 Label: none uuid: 43d3e523-63da-4b2c-aee4-58a7fe9c2a73 Total devices 2 FS bytes used 28.00KB devid 1 size 1.00GB used 240.75MB path /dev/sda7 devid 2 size 2.00GB used 0.00 path /dev/sda8 # fdisk -l /dev/sda ... Units = sectors of 1 * 512 = 512 bytes ... Device Boot Start End Blocks Id System ... /dev/sda7 284172288 288366591 2097152 83 Linux /dev/sda8 288368640 290465791 1048576 83 Linux So btrfs changed the file system device sizes to 1G, 2G but GParted changed the underlying partitions to 2G, 1G respectively for sda7, sda8. Not good. Thanks, Mike
More testing of btrfs on Fedora 14. Deliberately an older distribution with old kernel and btrfs-progs where the manual page and help text don't mention devid at all. Version information for Fedora 14: # cat /proc/version Linux version 2.6.35.14-106.fc14.i686.PAE ... # rpm -q btrfs-progs btrfs-progs-0.19-12.fc14.i686 # btrfs help | tail -1 Btrfs Btrfs v0.19 Btrfs help text fragment: # btrfs help ... btrfs filesystem resize [+/-]<newsize>[gkm]|max <filesystem> Resize the file system. If 'max' is passed, the filesystem will occupe all available space on the device. Again with two 2 GiB partitions /dev/sda7 and sda8. # fdisk -l /dev/sda ... Device Boot Start End Blocks Id System ... /dev/sda7 284172288 288366591 2097152 83 Linux /dev/sda8 288368640 292562943 2097152 83 Linux Test resizing using "btrfs filesystem resize" specifying devid: # mkfs.btrfs /dev/sda7 # mount /dev/sda7 /mnt/1 # btrfs device add /dev/sda8 /mnt/1 # btrfs filesystem show /dev/sda7 2> /dev/null Label: none uuid: 8e8c512f-111a-4395-a7fc-4e8663cbd015 Total devices 2 FS bytes used 28.00KB devid 1 size 2.00GB used 240.75MB path /dev/sda7 devid 2 size 2.00GB used 0.00 path /dev/sda8 # btrfs filesystem resize 2:1048576K /mnt/1 Resize '/mnt/1' of '2:1048576K' # btrfs filesystem show /dev/sda7 2> /dev/nullLabel: none uuid: 8e8c512f-111a-4395-a7fc-4e8663cbd015 Total devices 2 FS bytes used 28.00KB devid 1 size 2.00GB used 240.75MB path /dev/sda7 devid 2 size 1.00GB used 0.00 path /dev/sda8 # umount /mnt/1 Test resizing using "btrfsctl -r" specifying devid: # wipefs -a /dev/sda7 # wipefs -a /dev/sda8 # mkfs.btrfs /dev/sda7 # mount /dev/sda7 /mnt/1 # btrfs device add /dev/sda8 /mnt/1 # btrfs-show /dev/sda7 2> /dev/null Label: none uuid: db66ee51-b2d8-4b9f-a0e5-f7f38634ad16 Total devices 2 FS bytes used 28.00KB devid 1 size 2.00GB used 240.75MB path /dev/sda7 devid 2 size 2.00GB used 0.00 path /dev/sda8 # btrfsctl -r 2:1048576K /mnt/1 operation complete (Short wait or sync) # btrfs-show /dev/sda7 2> /dev/null Label: none uuid: db66ee51-b2d8-4b9f-a0e5-f7f38634ad16 Total devices 2 FS bytes used 28.00KB devid 1 size 2.00GB used 240.75MB path /dev/sda7 devid 2 size 1.00GB used 0.00 path /dev/sda8 So using "devid:size" works on old tools and kernel. Looking at the kernel source for v2.6.32 (when btrfs was first added) it can be seen that it's the kernel that parses [devid:][[+-]size|"max"] so using "devid:size" will work everywhere. linux v2.6.32 fs/btrfs/ioctl.c btrfs_ioctl_resize() https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/ioctl.c?id=v2.6.32#n578
Damn... I had sent patches to btrfs a few years ago to fix this but I guess they never got applied. Btrfs should accept the device name you want to resize, rather than forcing gparted to look up the internal id.
Created attachment 269316 [details] [review] Multi-device btrfs support (Draft 1) Hi Curtis, When you've got some time I would appreciate some C++ help please. Quoting the 3rd message in the attached patch set: I have made the mount_info and fstab_info maps static private member variables and the functions that reference them also static member functions. Can compile GParted_Core.o but gpartedbin can't be linked. Why? In case it matters this in on CentOS 6.5 x86-64. I get lots of errors like this: GParted_Core.o: In function `std::_Rb_tree<Glib::ustring, std::pair<Glib::ustring const, std::vector<Glib::ustring, std::allocator<Glib::ustring> > >, std::_Select1st<std::pair<Glib::ustring const, std::vector<Glib::ustring, std::allocator<Glib::ustring> > > >, std::less<Glib::ustring>, std::allocator<std::pair<Glib::ustring const, std::vector<Glib::ustring, std::allocator<Glib::ustring> > > > >::_M_end()': /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_tree.h:493: undefined reference to `GParted::GParted_Core::mount_info' GParted_Core.o: In function `std::_Rb_tree<Glib::ustring, std::pair<Glib::ustring const, std::vector<Glib::ustring, std::allocator<Glib::ustring> > >, std::_Select1st<std::pair<Glib::ustring const, std::vector<Glib::ustring, std::allocator<Glib::ustring> > > >, std::less<Glib::ustring>, std::allocator<std::pair<Glib::ustring const, std::vector<Glib::ustring, std::allocator<Glib::ustring> > > > >::_M_begin()': /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/bits/stl_tree.h:482: undefined reference to `GParted::GParted_Core::mount_info' etc ... Thanks, Mike
Hi Mike, Thank you for the question. I encountered this problem a while ago. At the time my work-around was to create separate classes to hold the information. When I needed the information to be persistent between instantiations, I made the data static. Examples can be seen in DM_Raid(), FS_Info() and the method you created LVM2_PV_Info(). I don't recall the exact reason why this linking problem occurs, but I think it has to do with the fact that GParted_Core() mixes C code exceptions and libraries with C++. One key area is that GParted_Core() contains the ped_exception_handler for the C libparted library. If multiple instances of GParted_Core were permitted, then my head begins to hurt trying to figure out how errors would be propagated between the C++ code and the C libparted library. In other words I believe that with the current code base, there can be only _one_ instance of GParted_Core(), and that instance is invoked from Win_GParted. To summarize, I think the problem can be addressed by moving the data such as mount_info into a separate class, making it static so that it is persistent, and providing methods to operate on the data. Does that help? Curtis
Hi Mike, I know the above does not explain why the problem occurs. It is entirely possible that a simple solution is available. And now that I think more about it, I'm not even sure if I did encounter the linking problem before. If/when I get a chance I can look further into this. Curtis
A static member is like a global variable. The header only declares it so that all modules using that header can reference it; you must define it in a .cpp module or the linker can't find the thing being referenced.
Thank you both. Sorted now. With the error being reported in an STL header (rather than GParted_Core.cc), and because I wasn't thinking about what a static member variables actually are I couldn't see what was wrong. Mike
Yea, error messages involving templates are a bear.
Hi Larry and Curtis, I wonder if you could do some investigative testing on additional distributions I don't have for me please. OS linux[1] btrfs-progs[2][3] mounts[4] mtab[5] CentOS 5.10 2.6.18 (btrfs not supported) CentOS 6.5 2.6.32 v0.20-rc1 Old Old (file) 0.20-0.2.git91d9eec.el6.x86_64 Debian 6.0 2.6.32 0.19 Old Old (file) 0.19+20100601 Kubuntu 12.04 LTS 3.2.0 0.19 Old Old (file) 0.19+20100601-3ubuntu3 Ubuntu 13.10 3.11.0 3.12-rc1 New Old (file) 0.19+20130705-1 Fedora 19 3.12.11 3.12 New New (link) 3.12-1.fc19.i686 Fedora 20 3.13.5 3.12 New New (link) 3.12-1.fc20.x86_64 [1] Linux version # /cat /proc/version [2] btrfs-progs / btrfs-tools version # btrfs version [3] btrfs-progs / btrfs-tools full package version if possible (rpm and dpkg examples) # rpm -qa | grep btrfs # dpkg -l btrfs* [4] /proc/mounts shows the old or new device after removing the mounting device from a btrfs file system? Create 2, 2 GiB partitions Then create btrfs on the first, add the second and remove the first # mkfs.btrfs /dev/sdb1 # mkdir -p /mnt/1 # mount /dev/sdb1 /mnt/1 # btrfs device add /dev/sdb2 /mnt/1 # btrfs device delete /dev/sdb1 /mnt/1 # grep btrfs /proc/mounts /dev/sdb1 /mnt/1 btrfs rw,seclabel,relatime,ssd,space_cache 0 0 ^^^^^^^^^ Does the kernel show the old original partition used to mount the file system, or does it show the new device? Old/New [5] /etc/mtab show the old or new device after removing the mounting device from a btrfs file system? Old/New # grep btrfs /etc/mtab /dev/sdb1 /mnt/1 btrfs rw 0 0 ^^^^^^^^^ # ls -l /etc/mtab -rw-r--r--. 1 root root 322 Mar 13 21:12 /etc/mtab ^ Is /etc/mtab a plain file or symbolic link? "-" file, "l" symlink Why this matters: GParted reads /proc/mounts and /etc/mtab to determine busy status of a file system. If /proc/mounts displays the old device it is no longer possible to determine which partition / file system is busy. This looks to have been corrected between linux 3.2 and 3.11. It would be useful to determine when this was corrected, so that any hacks can be limited to older kernel versions. Such as display a warning in such a case. Also Ubuntu 13.10 is especially unusual because /proc/mounts shows the correct new device name, but /etc/mtab shows the old wrong name. That's going to need additional working around too. Thanks, Mike
Hi Mike, Here I have (in addition that you have) * Debian 7,2 & 7,4 * Fedora 18 * Opensuze 13.1 * Mint 16 Larry
Hi Mike, If you need more than currently supported distros then just let me know because I do have several older VMs still on disk. OS linux[1] btrfs-progs[2][3] mounts[4] mtab[5] Debian 7.0 Wheezy 3.2.0-4 v0.19 Old Old (link) 0.19+2012032 Debian 8 Jessie* 3.10-3 v0.20-rc1 New New (link) 0.19+2013070 openSUSE 12.3 3.7.10-1.1 v0.19+ New New (link) 0.19-49.1.1** Ubuntu 12.10 3.5.0-17 v0.19 New Old (file) 0.19+2012032 Ubuntu 13.04 3.8.0-19 v0.20-rc1 New Old (file) 0.19+2013011 Ubuntu 14.04-beta1 3.13.0-12 v3.12 New Old (file) Gnome edition 3.12-1 * Debian Jessie is a work-in-progress and not officially released ** On openSUSE I used "zypper info btrfsprogs" to learn the btrfs package version If you require any more checks, then just let us know. :-) Curtis
Thanks Curtis, With your extra results I've managed to track down the kernel version and patch which changed btrfs to update the device name shown in /proc/self/mounts. It's this one included in linux-3.5-rc3. (/proc/mounts has been a symlink to /proc/self/mounts since before linux 2.6.0). Btrfs: implement ->show_devname https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9c5085c147989d48dfe74194b48affc23f376650 Mike All distro results together, with a few extras using some rescue CDs. OS linux[1] btrfs-progs[2][3] mounts[4] mtab[5] CentOS 5.10 2.6.18 (btrfs not supported) CentOS 6.5 2.6.32 v0.20-rc1 Old Old (file) 0.20-0.2.git91d9eec.el6.x86_64 Debian 6.0 2.6.32 0.19 Old Old (file) 0.19+20100601 Kubuntu 12.04 LTS 3.2.0 0.19 Old Old (file) 0.19+20100601-3ubuntu3 Debian 7.0 Wheezy 3.2.0-4 v0.19 Old Old (link) 0.19+2012032 System Rescue CD 3.2.34 v0.20-rc1 Old Old (file) 3.1.2 0.20_rc1 System Rescue CD 3.4.66 v0.20-rc1-358-g194aa4a Old Old (file) 3.8.1 0.20_rc1_p358 Ubuntu 12.10 3.5.0-17 v0.19 New Old (file) 0.19+2012032 System Rescue CD 3.6.9 v0.20-rc1 New Old (file) 3.1.2 0.20_rc1 openSUSE 12.3 3.7.10-1.1 v0.19+ New New (link) 0.19-49.1.1** Ubuntu 13.04 3.8.0-19 v0.20-rc1 New Old (file) 0.19+2013011 Debian 8 Jessie* 3.10-3 v0.20-rc1 New New (link) 0.19+2013070 Ubuntu 13.10 3.11.0 3.12-rc1 New Old (file) 0.19+20130705-1 GParted Live CD 3.11-2 v0.20-rc1 New New (link) 0.16.2-11 0.19+20130705-3 Fedora 19 3.12.11 3.12 New New (link) 3.12-1.fc19.i686 Fedora 20 3.13.5 3.12 New New (link) 3.12-1.fc20.x86_64 Ubuntu 14.04-beta1 3.13.0-12 v3.12 New Old (file) Gnome edition 3.12-1
Hi Mike & Curtis, Sorry to be late. I should perhaps mention that I could not do tests until today. Is it still useful? Larry
Hi Larry, Sorted now. No need for any testing at the moment. Thanks, Mike
Created attachment 274798 [details] [review] Tidyups (v1) Hi Curtis, While working on the patchset to better handle multi-device btrfs file systems I've generated these tidyups patches. I would like you to review and apply these tidyups separately and before the main patchset. This is just to avoid adding them to the end of the main patchset which is already complicated enough at 10 patches and likely to grow to 12 to 15 patches. Thanks, Mike
Created attachment 274799 [details] [review] Multi-device btrfs support (Draft 2) Here's the current main patchset to handle multi-device btrfs file systems. It's just here to show where I've got to. It's not ready for applying, but if you do see something which you think needs changing you can always let me know :-). Having said it's complicated, it basically creates a cache of btrfs member devices parsed from btrfs filesystem show output and uses it where necessary. The code already does: 1) Busy detection for all devices in a multi-device btrfs 2) Reporting of mount point for all members in a multi-device btrfs 3) Slightly better usage calculation for devices in a multi-device btrfs 4) Uses devid when resizing a btrfs member device (What this bug report is about) Likely additions: 5) Report some membership detection failure cases as partition warnings 6) Display members in the partition information dialog (after bug #690542 is finalised)
Thank you Mike for the tidyups and draft 2 patch set. I have not yet looked at the draft 2 patch set, but plan to in the next while. I have reviewed and committed the "Tidyups (v1)" patch set from comment 16. Prior to committing I made one minor change in the last two patches. Specifically I made one really minor grammatical change in the git comment: from: use to to: used to Reference: Used to vs. Use to -- Common Mistakes in English http://www.grammar.cl/rules/used-to-use-to.htm This is a nit-picky thing but since I noticed it I thought it best to correct. The relevant git commits for the tidyups can be viewed at the following links: Remove outdated comment about ped_partition_is_busy() for DMRaid devices https://git.gnome.org/browse/gparted/commit/?id=438685577b3924cab19ded6bcee4e1dbcdfa9af8 Remove unused member variable GParted_Core::buf https://git.gnome.org/browse/gparted/commit/?id=67115eeff209db6966f7c0935e84a2504cb2005c Remove set_proper_filesystem() method https://git.gnome.org/browse/gparted/commit/?id=131098a7972f921b85c5ee518221ca594c7007d5 Initialise file system objects only once https://git.gnome.org/browse/gparted/commit/?id=5f6656f267151b452cc95e036f8ad138a153d397
Hi Mike, I just finished a code review of the draft 2 patch set from comment #17. The draft 2 patch set will need rebasing due to the code changes committed in the tidyups patch set. I didn't perform the rebasing because you are currently working on the patch set. Following are my code review comments: P1: Make partition busy detection method selectable per file system P2: Restore busy detection of unknown mounted file systems I really like the new is_busy() method because it enables placement of LVM2_PV custom handling in the LVM2_PV file system specific files. This is much cleaner and more maintainable for current and future file systems. :-) P3: Add is_dev_mounted() to expose core partition is mounted test In this new method, it appears that a partition path is checked to see if it is busy. If this is the case I think the method name and variable name should more closely reflect what is being tested (path, not device). Perhaps is_path_mounted( Glib::ustring & path ) is an improvement (i.e., replace "dev" with "path")? P4: Detect busy status of multi-device btrfs file systems Spelling mistake "increamentally" should be "incrementally". P5: Display mount points for multi-device btrfs file systems No change P6: Display usage for multi-device btrfs file system Spelling mistake "achive" should be "achieve". Plural mistake "two different level" should be "two different levels". P7: Add devid to the cache of btrfs device infomation Spelling mistake "infomation" should be "information". P8: Pass devid when resizing btrfs file systems P9: Add fallback busy detection for btrfs file systems P10: WORK-IN-PROGRESS: Stop reading file systems from /etc/mtab no changes To summarize, I reviewed the draft 2 patch set code, but have not tested the code. From reading through the patch set it sure looks like you've been very busy researching Linux, btrfs, and GParted. I like the detailed git comments. Curtis
Created attachment 277115 [details] [review] Multi-device btrfs support (Draft 3) Hi Curtis, Here's draft 3. Addresses all comments from comment #19. Also adds this patch, implementing item (6) from comment #17: P11) Display btrfs members in the Partition Information dialog So the code does these items: 1) Busy detection for all devices in a multi-device btrfs 2) Reporting of mount point for all members in a multi-device btrfs 3) Slightly better usage calculation for devices in a multi-device btrfs 4) Uses devid when resizing a btrfs member device (What this bug report is about) 6) Display members in the partition information dialog The code doesn't do these items: 5) Report some membership detection failure cases as partition warnings 7) Try to warn when mounting device has been removed from a btrfs file system on linux kernel < 3.5. Related (or maybe the same) as (5). (Action must have been performed outside GParted) 8) Provide a way to remove a device from a multi-device btrfs file system. (Could be implemented for LVM2 PV too) 9) Warning when overwritting when one device of a multi-device btrfs. (LVM2 PV does this) 10) Account for btrfs relocating extents when shrinking a device. (Usage is so much a guestimate it doesn't seem worth it). 11) Doesn't prevent copying of btrfs file systems. (Prevented for LVM2 PV for similar resaons, UUIDs can't be changed and UUID is part of device membership so duplicate UUIDs will probably cause trouble) Q1) Not reading /etc/mtab and testing udev I have one stumbling block, which is confirming that P10: "Stop reading file systems from /etc/mtab" isn't a regression against bug 333370 "udev Causes mountpointdetection to fail" and the associated patch: extended scan for mountpoints (see #333370) https://git.gnome.org/browse/gparted/commit/?id=170eb4b4a40ef7e8043343a073a235f09bd0dadd Using temporary udev rule file /dev/.udev/rules.d/98-local.rules I used this rule (matches bug 333370 comment 8): KERNEL=="sd*", SUBSYSTEMS=="scsi", SYSFS{model}=="U3 Cruzer Micro", SYMLINK+="usb_key%n", NAME="%k" And when I insert my USB Key, model "U3 Cruzer Micro" it creates these devices: # ls -l sde* usb_key* brw-rw----. 1 root disk 8, 64 May 24 14:29 sde brw-rw----. 1 root disk 8, 65 May 24 14:29 sde1 lrwxrwxrwx. 1 root root 3 May 24 14:29 usb_key -> sde lrwxrwxrwx. 1 root root 4 May 24 14:29 usb_key1 -> sde1 Mounting using /dev/usb_key1 records /dev/sde1 in /proc/mounts /etc/mtab. GParted works ignoring usb_key symlinks altogether. Using this rule (matches bug 333370 comment 5): KERNEL=="sd*", SUBSYSTEMS=="scsi", SYSFS{model}=="U3 Cruzer Micro", SYMLINK+="%k", NAME="usb_key%n" And inserting my USB Key creates these devices: # ls -l sde* usb_key* ls: cannot access sde*: No such file or directory brw-rw----. 1 root disk 8, 64 May 24 14:48 usb_key brw-rw----. 1 root disk 8, 65 May 24 14:48 usb_key1 but doesn't create the symlinks for the kernel generated names sde*. Like this GParted reports dialog: Libparted Bug Found! (-) Count not stat device /dev/sde - No such file or directory. # cat /proc/partitions major minor #blocks name ... 8 64 1999688 sde 8 65 1997793 sde1 The internal kernel name is sde (& sde1). Libparted gets the kernel name for the device and the message is correct as udev didn't create the device sde. It appears that this isn't a setup which is supported by udev any more. Further I suspect that making GParted read from /etc/mtab couldn't have fixed it before. I suspect bug 333370 went away because other stuff changed (udev, udev rules and/or libparted to use kernel supplied name as the "true" device name). If you have any thoughts or testing results let me know, otherwise I'm going to assume not reading from /etc/mtab is safe. Q2) Leaving other items unimplemented I'd like to leave all the unimplemented items as unimplemented and just apply what the code does now. Perhaps particularly items (7) & (8). If you want to test this see comment #10 above for steps to create this situation. Thoughts please. Thanks, Mike
Thank you Mike for your continued work on improving btrfs support. Since I have not yet begun the new release process, I will review this patch first with an eye towards including it in the upcoming release. It might be a day or two before I get a chance for the review and testing. Curtis
Hi Curtis, I've been looking more closely at the change labelled with bug 333370. extended scan for mountpoints (see #333370) https://git.gnome.org/browse/gparted/commit/?id=170eb4b4a40ef7e8043343a073a235f09bd0dadd Reading the code change I see this removed comment: //above list lacks the root mountpoint, try to get it from /etc/mtab This comes from this change: improved scanning for root mountpoint (/) ... https://git.gnome.org/browse/gparted/commit/?id=409096f739118af95e3bff4484fdedb7885c97a2 My CentOS 6.5 desktop has this: $ fgrep ' / ' /proc/mounts rootfs / rootfs rw 0 0 /dev/sda3 / ext4 rw,seclabel,relatime,barrier=1,data=ordered 0 0 $ fgrep ' / ' /etc/mtab /dev/sda3 / ext4 rw 0 0 $ ls -l /dev/root lrwxrwxrwx. 1 root root 4 May 24 16:08 /dev/root -> sda3 Therefore it can identify the true device of the "/" file system (/dev/sda3) in both /proc/mounts and /etc/mtab and doesn't need change 409096f "improved scanning for root mountpoint (/) ...". However CentOS 5.10 is like this: $ fgrep ' / ' /proc/mounts rootfs / rootfs rw 0 0 /dev/root / ext3 rw,data=ordered 0 0 $ fgrep ' / ' /etc/mtab /dev/mapper/VolGroup00-LogVol00 / ext3 rw 0 0 $ ls -l /dev/root brw------- 1 root root 253, 0 May 26 20:53 /dev/root $ ls -l /dev/mapper/VolGroup00-LogVol00 brw-rw---- 1 root disk 253, 0 May 26 20:53 /dev/mapper/VolGroup00-LogVol00 But here GParted needs to scan /etc/mtab to discover that the true device for "/" file system is /dev/mapper/VolGroup00-LogVol00. Observations: o1) So fix tagged with bug 333370 only changed how /etc/mtab was read, probably making it more robust against unusual udev configurations. (As discussed in comment #20 above I only managed to test 1 of the 2 scenarios with udev, the one with doesn't change anything). o2) Scanning /etc/mtab is required on CentOS 5.10, and possible other very old distributions so that it can identify the true device name containing the "/" file system and therefore allow GParted to determine whether it is busy or not. But on the other hand reading from /etc/mtab on Ubuntu >= 13.04 will result in incorrectly identifying a partition as busy when it was used to mount a multi-device btrfs but has since been removed without remounting. (This is true for all distros before Linux 3.5 where /proc/mounts shows the old mounting dev name). Feel free to test and review, but don't commit until we decide what we should do about (o2). Thanks, Mike
Hi Mike, My apologies for the delay in review. If I have missed answering any questions, please feel free to re-ask. This patch set is looking good. From testing I can confirm that this patch set does indeed fix the problem originally noted in this bug report. :) I also agree that we shouldn't commit this patch set just yet. One reason is that one of the patches (I think 10/11) still has "WORK-IN-PROGRESS" in the commit header. ;-) CODE REVIEW In reviewing the code changes, I was unsure if you had seen the P3 question in comment #19. My question was regarding whether the method should be named is_dev_mounted(), is_path_mounted(), or some other name. From reading the code, the method is passed a path to a partition. In my mind this makes the name "dev" seem misleading since the argument is not necessarily a device like a disk. What are your thoughts on the is_path_mounted() method name? This might be me getting a little too picky again. I'm not overly hung up on the method name. ANSWERS TO QUESTIONS A1) Regarding "not reading /etc/mtab and testing udev" I agree that the previous bug 333370 report was closed without extensive testing and that it is possible that the behaviour back then is indeed the same as now. It might also be said that one form of the udev rules is perhaps less valid than the other. Personally I do not have much experience in writing udev rules. Recent distros like ubuntu automatically mount usb drives using the volume name (e.g. /media/myvolumnename), and hence the need for custom udev rules is perhaps less not. If I understand observation 2 (o2) correctly, it appears that reading /etc/mtab can cause problems with multi-btrfs mount detection on newer distros, but is needed to determine the "/" mount point for older distros. This makes me think of a possible compromise that might handle both situations. Q3) What do you think of reading /etc/mtab, but only for the "/" file system? A2) Regarding "leaving other items unimplemented", I would be okay with that. The patch set is an improvement over the existing code. The other issues can be left to a later date. Curtis
Hi Curtis, CODE REVIEW I misunderstood what you meant about method is_dev_mounted(). I changed the parameter name (dev -> path) but didn't change the function name. The reason I chose is_dev_mounted() in preference to is_path_mounted() was that the later might be about testing whether mount mounts like "/", "/boot" or "/home" were mounted rather than names of block devices as found in /dev like "/dev/sda1". I have the impression the source code over path to mean a variety of different types of things. Anyway I'm OK with changing it to is_path_mounted(). QUESTIONS AND ANSWERS (In reply to comment #23) > A1) Not reading /etc/mtab ... > If I understand observation 2 (o2) correctly, it appears that reading > /etc/mtab can cause problems with multi-btrfs mount detection on newer > distros, but is needed to determine the "/" mount point for older > distros. This makes me think of a possible compromise that might > handle both situations. (reiterating just to be clear of all the conditions ...) The user also has to have deleted the device used to mount the multi- device btrfs (outside of GParted) and not unmounted the file system yet. (It's an online only operation). In this case the old and wrong device name will always be reported on Linux < 3.5 and there is nothing we can do, breaking device membership for a multi-device btrfs. After that, on Linux >= 3.5, /proc/mounts is updated with another device still remaining in the btrfs. But /etc/mtab, if a plain file, will be out of date and will cause GParted to incorrectly report the now free device as busy. > Q3) What do you think of reading /etc/mtab, but only for the "/" file > system? I had several other ideas which I didn't like and my front runner was to run /bin/mount (as it resolves /dev/root into a useful device name) and fall back to reading /proc/mounts if the mount executable wasn't found. It leverages existing command which appears to do the right thing but was going to take testing on a lot of distros for me to be happy with it. I think I like your idea as it's simple and doesn't involve running another command just to handle the old CentOS 5 distro (and possible others). Also newer versions of the kernel have been improving the contents of /proc/mounts until most distros are making /etc/mtab a symlink to it. I'll implement this method. I think the commit message for P10: "Stop reading file systems from /etc/mtab" is going to get even longer though :-). TESTING For this bit from P1: "Make partition busy detection method selectable per file system" can you also specifically test it being broken in current GParted and that this patch fixes it. I don't have any fake hardware arrays so the comment is just based on my understanding of what the code did and does now. This also fixes detection of busy LVM2 PVs and Linux Software RAID arrays on DMRaid partitions by calling the specific busy detection methods, rather than just looking at mount_info map of mounted partitions. Thanks, Mike
Hi Mike, CODE REVIEW Your explanation for the is_dev_mounted() function name makes sense too. I wonder if the blended name is_dev_path_mounted() is an improvement? If you prefer is_dev_mounted() then I am okay with that too. I think the most important part was that the argument being passed was a path. TESTING From my testing it looks like detection of active LVM on DMRaid fails before and after this patch set. ;-( To test I used the following steps 1) Use GParted (configured with --enable-libparted-dmraid) to create a 40 GiB LVM2 PV partition on my DMRaid device /dev/mapper/isw_efjbbijhh_Vol0. 2) Set up, format as ext2, and mount the logical volume. # vgcreate myvg /dev/mapper/isw_efjbbijhh_Vol0p1 Volume group "myvg" successfully created # lvcreate --name myarea --size 1024M myvg Logical volume "myarea" created # mke2fs /dev/myvg/myarea mke2fs 1.42 (29-Nov-2011) Filesystem label= OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) Stride=0 blocks, Stripe width=0 blocks 65536 inodes, 262144 blocks 13107 blocks (5.00%) reserved for the super user First data block=0 Maximum filesystem blocks=268435456 8 block groups 32768 blocks per group, 32768 fragments per group 8192 inodes per group Superblock backups stored on blocks: 32768, 98304, 163840, 229376 Allocating group tables: done Writing inode tables: done Writing superblocks and filesystem accounting information: done # mount /dev/myvg/myarea /mnt/mymnt 3) Run GParted and note that the partition is not marked as active. Note that the partition says it cannot read the info for the partition. Hence we might need to resolve the following bug first: Bug 715191 - LVM2 PV space usage details not working on fake BIOS RAID 4) Following is some information that I gathered after the fact. # df Filesystem 1K-blocks Used Available Use% Mounted on /dev/sda3 82569904 70431140 7944460 90% / udev 3991672 12 3991660 1% /dev tmpfs 800900 1056 799844 1% /run none 5120 0 5120 0% /run/lock none 4004496 84 4004412 1% /run/shm /dev/sda8 103212320 8302784 89666656 9% /data /dev/sda11 41284928 180108 39007668 1% /cdburn /dev/sda9 103212320 9794824 88174616 10% /sound /dev/sda10 123854820 56845592 60717772 49% /picture /dev/sda12 206424760 172384552 23554448 88% /vmimage /dev/mapper/myvg-myarea 1032088 1284 978376 1% /mnt/mymnt # ls -l /dev/myvg total 0 lrwxrwxrwx 1 root root 7 May 29 19:59 myarea -> ../dm-2 # ls -l /dev/mapper total 0 crw------- 1 root root 10, 236 May 29 19:47 control lrwxrwxrwx 1 root root 7 May 29 19:55 isw_efjbbijhh_Vol0 -> ../dm-0 lrwxrwxrwx 1 root root 7 May 29 19:57 isw_efjbbijhh_Vol0p1 -> ../dm-1 lrwxrwxrwx 1 root root 7 May 29 19:59 myvg-myarea -> ../dm-2 # ls -l /dev/dm-* brw-rw---- 1 root disk 252, 0 May 29 19:55 /dev/dm-0 brw-rw---- 1 root disk 252, 1 May 29 19:57 /dev/dm-1 brw-rw---- 1 root disk 252, 2 May 29 19:59 /dev/dm-2 # cat /proc/mounts rootfs / rootfs rw 0 0 sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0 proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0 udev /dev devtmpfs rw,relatime,size=3991672k,nr_inodes=997918,mode=755 0 0 devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0 tmpfs /run tmpfs rw,nosuid,noexec,relatime,size=800900k,mode=755 0 0 /dev/disk/by-uuid/8e2ad2c9-1ddb-4122-8c20-caa6933d5488 / ext4 rw,relatime,errors=remount-ro,user_xattr,acl,barrier=1,data=ordered 0 0 none /sys/fs/fuse/connections fusectl rw,relatime 0 0 none /sys/kernel/debug debugfs rw,relatime 0 0 none /sys/kernel/security securityfs rw,relatime 0 0 none /run/lock tmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0 none /run/shm tmpfs rw,nosuid,nodev,relatime 0 0 /dev/sda8 /data ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda11 /cdburn ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda9 /sound ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda10 /picture ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda12 /vmimage ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 binfmt_misc /proc/sys/fs/binfmt_misc binfmt_misc rw,nosuid,nodev,noexec,relatime 0 0 vmware-vmblock /run/vmblock-fuse fuse.vmware-vmblock rw,nosuid,nodev,relatime,user_id=0,group_id=0,default_permissions,allow_other 0 0 /dev/mapper/myvg-myarea /mnt/mymnt ext2 rw,relatime,errors=continue,user_xattr,acl 0 0 # cat /etc/mtab /dev/sda3 / ext4 rw,errors=remount-ro 0 0 proc /proc proc rw,noexec,nosuid,nodev 0 0 sysfs /sys sysfs rw,noexec,nosuid,nodev 0 0 none /sys/fs/fuse/connections fusectl rw 0 0 none /sys/kernel/debug debugfs rw 0 0 none /sys/kernel/security securityfs rw 0 0 udev /dev devtmpfs rw,mode=0755 0 0 devpts /dev/pts devpts rw,noexec,nosuid,gid=5,mode=0620 0 0 tmpfs /run tmpfs rw,noexec,nosuid,size=10%,mode=0755 0 0 none /run/lock tmpfs rw,noexec,nosuid,nodev,size=5242880 0 0 none /run/shm tmpfs rw,nosuid,nodev 0 0 /dev/sda8 /data ext4 rw 0 0 /dev/sda11 /cdburn ext4 rw 0 0 /dev/sda9 /sound ext4 rw 0 0 /dev/sda10 /picture ext4 rw 0 0 /dev/sda12 /vmimage ext4 rw 0 0 binfmt_misc /proc/sys/fs/binfmt_misc binfmt_misc rw,noexec,nosuid,nodev 0 0 vmware-vmblock /run/vmblock-fuse fuse.vmware-vmblock rw,nosuid,nodev,default_permissions,allow_other 0 0 /dev/mapper/myvg-myarea /mnt/mymnt ext2 rw 0 0 # I think the cause of the problem is similar to Bug 715191 and related to the fact there are three names for the logical volume: a) /dev/myvg/myarea -> /dev/dm-2 b) /dev/mapper/myvg-myarea -> /dev/dm-2 c) /dev/dm-2 Note that the above testing is with kubuntu 12.04. The links and names might be different with other distros. Curtis
Hi Curtis, Unfortunately I have realised that reading / (root) file system entry only from /etc/mtab will get the wrong device name if the root file system is btrfs and /etc/mtab is a separate file when the mounting device is removed. I have been reading and trying to understand the mount code from util-linux. The current code specifically looks for file system named "/dev/root" and looks it up from the root=VALUE from the kernel command line read from /proc/cmdline. It then resolves UUID and LABEL via blkid cache and canonicalised paths by resolving symlinks and changing device mapper private /dev/dm-N names to public /dev/mapper/NAME ones. From util-linux 2.24 (2014-04-24) ... kernel_fs_postparse() http://git.kernel.org/cgit/utils/util-linux/util-linux.git/tree/libmount/src/tab_parse.c?h=stable/v2.24#n560 mnt_resolve_spec() http://git.kernel.org/cgit/utils/util-linux/util-linux.git/tree/libmount/src/cache.c?h=stable/v2.24#n619 mnt_resolve_path() http://git.kernel.org/cgit/utils/util-linux/util-linux.git/tree/libmount/src/cache.c?h=stable/v2.24#n619 So we can: 1) Duplicate some or all of what mount does, 2) Just run mount command, 3) Just read /proc/mounts and not be able to detect busy / (root) on CentOS 5, 4) Just read / (root) from /etc/fstab and get an old device name when the root fs is btrfs and the mounting device has been removed, 4) Something else? Using the mount command seems promising as it canonicalises a lot of stuff. I'll give coding this a go. Mike
Hi Curtis, If any point you need to release GParted go ahead. Would you be able to repeat the test from comment #25, but using Linux software RAID instead of LVM2 as the non-file system busy content. I wonder if using Multiple Devices, instead of Device-Mapper, with different naming practices makes a difference. (Although I doubt a user will use Linux software RAID over the top of fake RAID). mdadm usage reminder bug #709640 comment #9 In the end I kept the code to read /proc/mounts and only changed from reading /etc/mtab to reading the output of the mount command, but only when required. I still have a stale cache / busy detection issue to look at before I send another patchset. Thanks, Mike
Hi Mike, Thanks for the go ahead to release GParted if/when needed. I'll think on this as there are some other patches outstanding, but I'm not sure all these other patches are ready due to lack of testing. Per your request I have performed some tests using Software RAID on top of DMRaid. TESTING To test SWRaid on DMRaid I used the following steps on kubuntu 12.04: 1) Use GParted (configured with --enable-libparted-dmraid) to create two 10 GiB unformatted partitions on my DMRaid device /dev/mapper/isw_efjbbijhh_Vol0 (p1 and p2). Use GParted to mark these partitions with RAID flag. 2) Set up SWRaid device (/dev/md0). # mdadm --create /dev/md0 --level raid1 --auto=p --raid-devices=2 \ /dev/mapper/isw_efjbbijhh_Vol0p1 /dev/mapper/isw_efjbbijhh_Vol0p2 mdadm: Note: this array has metadata at the start and may not be suitable as a boot device. If you plan to store '/boot' on this device please ensure that your boot-loader understands md/v1.x metadata, or use --metadata=0.90 Continue creating array? y mdadm: Defaulting to version 1.2 metadata mdadm: array /dev/md0 started. # 3) Use GParted to create an MSDOS partition table on SWRaid (/dev/md0). Use GParted to create a new 4 GiB ext2 partition (/dev/md0p1). 4) Mount the SWRaid device. # mount /dev/md0p1 /mnt/mymnt # 5) Run GParted and note the status of the associated partitions: /dev/md0p1 (busy ext2) /dev/mapper/isw_efjbbijhh_Vol0p1 (not-busy linux-raid) /dev/mapper/isw_efjbbijhh_Vol0p2 (not-busy linux-raid) 6) Following is some information that I gathered after the fact. # df Filesystem 1K-blocks Used Available Use% Mounted on /dev/sda3 82569904 70212740 8162860 90% / udev 3991672 12 3991660 1% /dev tmpfs 800900 1076 799824 1% /run none 5120 0 5120 0% /run/lock none 4004496 168 4004328 1% /run/shm /dev/sda11 41284928 180108 39007668 1% /cdburn /dev/sda9 103212320 9794824 88174616 10% /sound /dev/sda10 123854820 56845592 60717772 49% /picture /dev/sda12 206424760 174670816 21268184 90% /vmimage /dev/sda8 103212320 8302808 89666632 9% /data /dev/md0p1 4128448 8184 3910552 1% /mnt/mymnt # ls -l /dev/mapper total 0 crw------- 1 root root 10, 236 Jun 2 09:48 control lrwxrwxrwx 1 root root 7 Jun 2 10:57 isw_efjbbijhh_Vol0 -> ../dm-0 lrwxrwxrwx 1 root root 7 Jun 2 10:57 isw_efjbbijhh_Vol0p1 -> ../dm-1 lrwxrwxrwx 1 root root 7 Jun 2 10:57 isw_efjbbijhh_Vol0p2 -> ../dm-2 # ls -l /dev/dm-* brw-rw---- 1 root disk 252, 0 Jun 2 10:57 /dev/dm-0 brw-rw---- 1 root disk 252, 1 Jun 2 10:57 /dev/dm-1 brw-rw---- 1 root disk 252, 2 Jun 2 10:57 /dev/dm-2 # ls -l /dev/md* brw-rw---- 1 root disk 9, 0 Jun 2 10:57 /dev/md0 brw-rw---- 1 root disk 259, 0 Jun 2 10:56 /dev/md0p1 # cat /proc/mounts rootfs / rootfs rw 0 0 sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0 proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0 udev /dev devtmpfs rw,relatime,size=3991672k,nr_inodes=997918,mode=755 0 0 devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0 tmpfs /run tmpfs rw,nosuid,noexec,relatime,size=800900k,mode=755 0 0 /dev/disk/by-uuid/8e2ad2c9-1ddb-4122-8c20-caa6933d5488 / ext4 rw,relatime,errors=remount-ro,user_xattr,acl,barrier=1,data=ordered 0 0 none /sys/fs/fuse/connections fusectl rw,relatime 0 0 none /sys/kernel/debug debugfs rw,relatime 0 0 none /sys/kernel/security securityfs rw,relatime 0 0 none /run/lock tmpfs rw,nosuid,nodev,noexec,relatime,size=5120k 0 0 none /run/shm tmpfs rw,nosuid,nodev,relatime 0 0 /dev/sda11 /cdburn ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda9 /sound ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda10 /picture ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda12 /vmimage ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 /dev/sda8 /data ext4 rw,relatime,user_xattr,acl,barrier=1,data=ordered 0 0 binfmt_misc /proc/sys/fs/binfmt_misc binfmt_misc rw,nosuid,nodev,noexec,relatime 0 0 vmware-vmblock /run/vmblock-fuse fuse.vmware-vmblock rw,nosuid,nodev,relatime,user_id=0,group_id=0,default_permissions,allow_other 0 0 /dev/md0p1 /mnt/mymnt ext2 rw,relatime,errors=continue,user_xattr,acl 0 0 # ls -l /etc/mtab -rw-r--r-- 1 root root 877 Jun 2 10:57 /etc/mtab # cat /etc/mtab /dev/sda3 / ext4 rw,errors=remount-ro 0 0 proc /proc proc rw,noexec,nosuid,nodev 0 0 sysfs /sys sysfs rw,noexec,nosuid,nodev 0 0 none /sys/fs/fuse/connections fusectl rw 0 0 none /sys/kernel/debug debugfs rw 0 0 none /sys/kernel/security securityfs rw 0 0 udev /dev devtmpfs rw,mode=0755 0 0 devpts /dev/pts devpts rw,noexec,nosuid,gid=5,mode=0620 0 0 tmpfs /run tmpfs rw,noexec,nosuid,size=10%,mode=0755 0 0 none /run/lock tmpfs rw,noexec,nosuid,nodev,size=5242880 0 0 none /run/shm tmpfs rw,nosuid,nodev 0 0 /dev/sda11 /cdburn ext4 rw 0 0 /dev/sda9 /sound ext4 rw 0 0 /dev/sda10 /picture ext4 rw 0 0 /dev/sda12 /vmimage ext4 rw 0 0 /dev/sda8 /data ext4 rw 0 0 binfmt_misc /proc/sys/fs/binfmt_misc binfmt_misc rw,noexec,nosuid,nodev 0 0 vmware-vmblock /run/vmblock-fuse fuse.vmware-vmblock rw,nosuid,nodev,default_permissions,allow_other 0 0 /dev/md0p1 /mnt/mymnt ext2 rw 0 0 # The above testing is with kubuntu 12.04. If you need any further testing then please let me know. Curtis
Can you do: cat /proc/mdstat
Certainly. :) # cat /proc/mdstat Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10] md0 : active raid1 dm-2[1] dm-1[0] 10477440 blocks super 1.2 [2/2] [UU] unused devices: <none> #
So GParted UI is showing the following names in the UI: Device: isw_efjbbijhh_Vol0 Partitions: isw_efjbbijhh_Vol0p1 isw_efjbbijhh_Vol0p2 but Linux Software RAID is using names: dm-1 dm-2 Therefore GParted doesn't recognise isw_* partition devices as busy. Going to need to resolve symlinks when detecting busy status of SW RAID, and probably LVM2 PVs, in the same way read_mountpoints_from_file() does for plain file systems. I was wondering if busy detection of SW RAID and LVM2 PVs should be added into init_maps() and the mount_info array. This is suggesting yes. Anyway a change for Bug 715191, not this one. Thanks, Mike
Agreed. The problem with resolving the real path name for SW RAID on DMRaid belongs with bug 715191. I know from originally implementing DMRaid support, finding the real names was a challenge as different distros seemed to have differing paths. And as you pointed out earlier, setting up SW RAID on DMRaid is not likely a high usage scenario (if at all).
Rather than worry about mapping from one kind of name to another for the sake of uniform comparison, how about using dev_t instead? Then the two input names don't matter; you stat() each and if the dev_t matches, they refer to the same device. Also parsing /sys may be easier than /proc/mdstat. /sys/block/md0/slaves has symlinks to the nodes for the underlying devices, and the underlying devices have symlinks to the md stacked on top in their holders/ directory.
Created attachment 277905 [details] [review] Multi-device btrfs support (v1) Hi Curtis, Here's patchset v1. I've made the following changes compared to draft 3 from comment #20: P1: Make partition busy detection method selectable per file system - Dropped commit message paragraph claiming fixed busy detection of LVM2 PVs and SW RAID on DMRAID partitions. P10: Fallback to reading mount command output instead of /etc/mtab - Rewrote. Previously just stopped reading from /etc/mtab. Now detects only having 'rootfs' and '/dev/root' device names from /proc/mounts and then also reads from mount command output to find a real device name containing the / (root) file system. P12: Flush devices when scanning to prevent reading stale signatures - New patch. This is for after GParted 0.19.0 is released. Thanks, Mike
Hello Mike, thanks for your work! It is possible to know how space usage (in contrast to allocated space) in a device of a multi-device btrfs filesystem. Currently, one can obtain this information by inspecting btrfs data structures with the TREE_SEARCH ioctl. This is done in btrfs-gui: http://carfax.org.uk/btrfs-gui (see btrfs.py, hlp/size.py). Indeed, I have tested btrfs-gui and it displays space usage for each device, besides allocated space. I have asked on btrfs mailing list that this is implemented upstream, so GParted and other clients do not have to reimplement this behavior: http://comments.gmane.org/gmane.comp.file-systems.btrfs/35697 Thanks
Created attachment 278205 [details] [review] Multi-device btrfs support (v2) Hi Curtis, Here's patchset v2. The only difference is a 1 character change to P10: Fallback to reading mount command output instead of /etc/mtab to make it correctly parse mountpoint from mount command and therefore detect that / (root) file system is busy on old distros. Thanks, Mike
Hi Mike, Thanks for the updated v2 patch set. :-) For testing I looked at two main areas: 1) Check for correct root file system mount state (mounted/unmounted) 2) Test btrfs resize using the instructions in comment #1 Following are the results of my testing: Distro Test #1 Test #2 Notes ------------------ ------- ------- -------------------- debian 6 pass pass debian 7 pass pass fedora 20 pass pass LABELGONE opensuse 12.3 pass pass SEGFAULT opensuse 13.1 pass pass LABELGONE kubuntu 12.04 LTS pass pass ubuntu 14.04 LTS pass pass SEGFAULT, LABELGONE *Notes ----- SEGFAULT: Segmentation fault occurred when applying an operation (random) LABELGONE: If mounted, btrfs volume label disappears in gparted main window Problems: A) The problem with the segmentation fault is not consistent, but happens often enough for me to encounter it again. I will try adding debugging in an effort to trace the problem. It might not even be due to this patch set. B) The problem with the multi-device btrfs label disappearing when mounted is an odd one. More testing is required to trace the source of the problem. C) Now that this patch set v2 recognizes multi-device btrfs, I noticed that we do not warn a user about deleting a partition that is a member of a multi-device btrfs. Is a warning something that you would like to add to this patch set, or would you prefer to leave such an enhancement until later? Curtis
Hi Mike, After further testing in an ubuntu 14.04 virtual machine, I can confirm that the segmentation fault is related to the new patch set in comment #36. The crash sometimes occurs when refreshing the devices, and occasionally on start-up of GParted. The crash does not occur every time so it looks like we are dealing with some sort of race condition. To analyze the segmentation fault problem I installed some debug packages and built GParted as follows: ### install debug packages $ sudo apt-get install libglib2.0-0-dbg libglibmm-2.4-dbg \ libparted0debian1-dbg ### compile for debugging $ export CXXFLAGS=" -g -O0 " ### ^^ ### | ### that is a capital Oh followed by a zero $ ./configure --disable-scrollkeeper --enable-online-resize $ make I am not an expert on gdb, so if there is additional debug info that I can generate that would help, then please let me know. With the random nature of the crash and a hypothesis that this is from some kind of race condition, I'm wondering if we need to place the new btrfs cache code in a separate class that does not depend on GParted_Core. This is just a theory at the moment. Curtis Following is an emacs gud-gdb mode log from when a segmentation fault occurred on a manually invoked device refresh: --------------- Start Emacs gud-gdb Log --------------- Current directory is ~/gparted-0.19.0-git/ [sudo] password for user: GNU gdb (Ubuntu 7.7-0ubuntu3) 7.7 Copyright (C) 2014 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i686-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from src/gpartedbin...done. (gdb) set height 10000 (gdb) run Starting program: /home/user/gparted-0.19.0-git/src/gpartedbin [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1". ====================== libparted : 2.3 ====================== [New Thread 0xb628db40 (LWP 17843)] [New Thread 0xb1c4bb40 (LWP 17848)] [Thread 0xb1c4bb40 (LWP 17848) exited] [New Thread 0xb129fb40 (LWP 17884)] Program received signal SIGSEGV, Segmentation fault.
+ Trace 233696
Thread 2972318528 (LWP 17884)
37 textdomain( GETTEXT_PACKAGE ) ; 38 39 //check UID 40 if ( getuid() != 0 ) 41 { 42 Gtk::MessageDialog dialog( _("Root privileges are required for running GParted"), 43 false, 44 Gtk::MESSAGE_ERROR, 45 Gtk::BUTTONS_OK ) ; 46 dialog .set_secondary_text( (gdb) --------------- End Emacs gud-gdb Log ---------------
Interesting. It looks like bugzilla does some sort of processing on the comment text to recognize it as a backtrace. The end result is that some debug commands, comments, and code listings that I posted are not displayed. For example the text started with the following "info threads" output: (gdb) info threads Id Target Id Frame * 4 Thread 0xb129fb40 (LWP 17884) "gpartedbin" 0xb7f523f0 in getenv@plt () from /lib/i386-linux-gnu/libparted.so.0 2 Thread 0xb628db40 (LWP 17843) "gmain" 0xb7fdd424 in __kernel_vsyscall () 1 Thread 0xb66858c0 (LWP 17838) "gpartedbin" 0xb7fdd424 in __kernel_vsyscall () Anyways, the problem seems to trace back to the invocation of GParted_Core::get_device_and_disk(...) Curtis
It's going to be several days before I have time to look at these issues. Mike
Hi Curtis, Had an initial look at these issues. A) Segfault I have encountered a segfault too, but just when doing a refresh. (Not even tried any updates in GParted yet). Details: * Fedora 20 * GParted master (GPARTED_0_19_0-2-gbfaa53e) + patch set v2 from comment #36 * mounted btrfs on 2 devices: /dev/sdb1 & /dev/sdb2 * mounted btrfs on 1 device: /dev/sdb3 * Just do Refresh Devices [root@localhost ~]# gdb ~fedora/programming/c/gparted/src/gpartedbin-review-723842-multi-device-btrfs-v2-c36-O0 core.29733 ... Program terminated with signal SIGSEGV, Segmentation fault.
+ Trace 233697
This segfaults while trying to run 'btrfs filesystem show' in btrfs::read_label(). The call chain includes: GParted_Core::set_devices_thread() GParted_Core::set_device_partitions() GParted_Core::read_label() btrfs::read_label() Utils::execute_command() This segfault doesn't have GParted_Core::get_device_and_disk() in the chain anywhere. I have also seen this for GParted 0.19.0 (as per release tag) on Fedora 20. But only seen it once and haven't been able to generated it again and had core dumps disable at the time. [root@localhost sbin]# ~fedora/bin/gpartedbin-0.19.0 ====================== libparted : 3.1 ====================== *** stack smashing detected ***: /home/fedora/bin/gpartedbin-0.19.0 terminated ======= Backtrace: ========= /lib64/libc.so.6[0x3863a75cff] /lib64/libc.so.6(__fortify_fail+0x37)[0x3863b06b17] /lib64/libc.so.6(__fortify_fail+0x0)[0x3863b06ae0] /lib64/libglib-2.0.so.0[0x38662363d2] /lib64/libglib-2.0.so.0(g_find_program_in_path+0x12d)[0x3866279c2d] /lib64/libglibmm-2.4.so.1(_ZN4Glib20find_program_in_pathERKSs+0x21)[0x3a77a50c71] /home/fedora/bin/gpartedbin-0.19.0[0x420d89] /home/fedora/bin/gpartedbin-0.19.0[0x421051] /home/fedora/bin/gpartedbin-0.19.0[0x47820f] /lib64/libglibmm-2.4.so.1[0x3a77a4593d] /lib64/libglib-2.0.so.0[0x386626ea45] /lib64/libpthread.so.0[0x3864207f33] /lib64/libc.so.6(clone+0x6d)[0x3863af4ded] ======= Memory map: ======== ... With me never having seen any segfaults on my CentOS 6.5 desktop, but finding this one on Fedora 20, and with your findings I wonder if there is an issue which is manifest with recent versions of gtkmm (or related libs). More investigation required into segfaults. Perhaps valgrind will help. B) Labels disappearing I have a likely cause for this. "btrfs filesystem show /dev/DEV" reports failed exit status 1 when the file system is mounted. This effects btrfs-progs v3.14 and v3.14.1 only. v3.12 and v3.14.2 are not effected. So with this command reporting failure GParted will: 1) Fall back to reading the label via the blkid cache; 2) Not set any file system usage, thus raising a warning against the partition which will include the "btrfs filesystem show" command output in the message; 3) No load the btrfs_device_cache so membership breaks. Therefore multi-device busy detection and mount point reporting breaks for the non-mounting device. I'm going to have to patch around this, I guess by just ignoring the exit status. C) Warn when deleting a member of a multi-device btrfs At the moment I want don't want to add a patch warning when deleting such a partition, even though LVM2 has such a warning. I think what GParted really needs is a way to remove a partition from multi-device storage systems like LVM2 and btrfs. But is that just another Partition menu item or should the UI change more to accommodate such multi-device storage system. Users may then just need a way to destroy (is that just erase or partition delete) all occupied partitions. Anyway I would like to leave it for now until we have a better idea of what we should do. Thanks, Mike
Hi Mike, Thank you for the detailed response. RE: (A). I had tested gparted-0.19.0 as well, but during my testing it did not segfault. Since you did experience a segfault I will focus more testing on 0.19.0 to see if I can reproduce a segfault. Your suggestion of valgrind is a good idea too. RE: (C) "leave it for now" sounds good to me. This patch set includes many improvements already. I think addressing the other issues is more important. Curtis
Details of one issue found using valgrind in GParted master tip without this patchset applied has been logged as: bug 731752 - Write after free cross thread race in PipeCapture::_OnReadable()
Hi Mike, For my testing of patch set v2 in comment #36 I looked at the following two main areas: 1) Check for correct file system mount state (mounted/unmounted) 2) Test btrfs resize using the instructions in comment #1 Distro Test #1 Test #2 Notes ------------------ ------- ------- -------------------- debian 6 pass pass debian 7 pass pass fedora 12 pass n/a Lacks "btrfs" command fedora 19 pass pass fedora 20 pass pass opensuse 12.3 pass pass LABELGONE* opensuse 13.1 pass pass LABELGONE* ubuntu 10.04 LTS pass n/a Lacks "btrfs" command kubuntu 12.04 LTS pass pass ubuntu 14.04 LTS pass pass LABELGONE*, Online resize also works Notes ----- *LABELGONE: If mounted, btrfs volume label disappears in gparted main window. This minor problem also exists in GParted 0.19.0 and earlier and hence is not related to the patch set for this report (see also comment B "Labels disappearing" in comment #41). All this additional testing has built my confidence in the patch for bug 731752 - Write after free cross thread race in PipeCapture::_OnReadable(). GParted has been rock-solid in all subsequent testing. :-) Based on this round of testing, I am ready to commit this patch set to the git repository. QUESTION: Since patch set v2 fixes a bug, what are your thoughts about about including it in the GParted 0.19.1 bug-fix release? Curtis
Hi Curtis, I've also re-tested patchset v2 on Fedora 20 doing several resizes of multi-device btrfs successfully so from the point of view of crashes bug 731752 - "Write after free cross thread race in PipeCapture::OnReadable()" has done the job. Q: Include Multi-device btrfs support patch in GParted 0.19.1? I think not. We make 0.x.1 releases with minimal changes on top of 0.x.0 releases just to address crashes and other serious issues. While resizing the wrong device in a multi-device btrfs is bad I think it is a low volume use case and this patchset adds lots of other enhancements which for me makes it 0.y.0 release material. I think we should release 0.19.1 with what is already committed to master and no more. Thanks, Mike
Hi Mike, I agree that we should keep changes minimal with 0.x.1 releases. As you point out, even though the multi-device btrfs support patch does fix a bug, the patch includes many other changes that could potentially introduce new problems. As such I will hold off on committing this patch set until after the 0.19.1 release. Curtis
I plan to commit the patch in comment #36 to the git repository later in the week. My reasoning for the delay is to provide the 0.19.1 release at least a week to see if any major problems crop up (I am not expecting any).
Hi Curtis, I've been looking further into fault (B) label disappearing and found two separate issues. Issue (B.1): Btrfs partition shows a warning when mounted * Caused by btrfs filesystem show /dev/PTN returning failed exit status 1 when the file system is mounted. * Effects btrfs-progs 3.14 & 3.14.1. * Distros effected: ** Fedora 19 with btrfs-progs 3.14.1. * This is what I previously described in comment #41 above. Most significantly it breaks btrfs multi-device membership. # btrfs filesystem show /dev/sdb1 Label: 'test1-btrfs' uuid: 033e6b07-ee6a-4620-a585-8580a2b83275 Total devices 1 FS bytes used 192.00KiB devid 1 size 2.00GiB used 240.75MiB path /dev/sdb1 Btrfs v3.14.1 # echo $? 1 Probably caused by commit in btrfs-progs v3.14: btrfs-progs: make filesystem show by label work http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/commit/?id=a156b967ed9bd606afa8dc402451abcf07226c17 Fixed by commit in btrfs-progs v3.14.2: btrfs-progs: Fix the return value of btrfs_scan_kernel() http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/commit/?id=ccd14cbf62026fd84113bcf886732fcc3391cb5b Issue (B.2): btrfs filesystem label is not shown when mounted * Caused by btrfs filesystem show /dev/PTN changing it's output and removing the single quotes around the label, but only when mounted. * Effects btrfs-progs 3.12 only. * Distros effected: ** Xubuntu 14.04 LTS with btrfs-tools 3.12. ** CentOS 7.0 with btrfs-progs 3.12. * Causes the label to be read as blank, but only when mounted. This is what you reported against ubuntu 14.04 LTS in comment #37. # fgrep sdb1 /proc/mounts /dev/sdb1 /mnt/1 btrfs rw,relatime,space_cache 0 0 # btrfs filesystem show /dev/sdb1 Label: test1-btrfs uuid: 1f78fa38-2f85-41d3-9be6-ae0356ae9469 Total devices 1 FS bytes used 192.00KiB devid 1 size 2.00GiB used 240.75MiB path /dev/sdb1 Btrfs v3.12 # umount /dev/sdb1 # btrfs filesystem show /dev/sdb1 Label: 'test1-btrfs' uuid: 1f78fa38-2f85-41d3-9be6-ae0356ae9469 Total devices 1 FS bytes used 192.00KiB devid 1 size 2.00GiB used 240.75MiB path /dev/sdb1 Btrfs v3.12 Caused by commit in btrfs-progs v3.12: btrfs-progs: use kernel for mounted disk for show http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/commit/?id=6c2c30ce03c0beba716b5a054b64c6bf62068cb6 Fixed by commit in btrfs-progs v3.14: btrfs-progs: make filesystem show by label work http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/commit/?id=a156b967ed9bd606afa8dc402451abcf07226c17 My Fedora 20 has the latest updates applied so has btrfs-progs 3.14.2. However the yum log show that it previous did have btrfs-progs 3.14.1 (so would have experienced (B.1)) and btrfs-progs 3.12 before that (so would have experienced (B.2) too). These bugs appearing and disappearing in btrfs are a pain. I think I need to fix (B.1) as part of this patchset because it effects membership detection of multi-device btrfs file systems. May well also fix (B.2) as it probably won't be that hard. Please hold off committing this patchset. Thanks, Mike
Hi Mike, Thank you for the detailed description of these bugs appearing and disappearing in btrfs. I will hold off on committing and wait for another patch set. Curtis
For the record bug #733601 "Btrfs: Warnings and missing label with btrfs-progs 3.12 and 3.14" has been raised to address the issues described in comment #48 above.
Solving the problem of resizing the correct devid for btrfs has helped flush out several other bugs. Thanks to work by Mike Fleetwood, these bugs have been addressed so now we can commit the patches that fix the originally reported bug. The patch set contained in comment #36 has been committed to the git repository for inclusion in the next release of GParted. The relevant git commits can be viewed at the following links: Make partition busy detection method selectable per file system (#723842) https://git.gnome.org/browse/gparted/commit/?id=b1dc9e69e39a61a678534e3d83c4b94a89a695e6 Restore busy detection of unknown mounted file systems (#723842) https://git.gnome.org/browse/gparted/commit/?id=49a2e19462fdbe4b28a53cc21a2ddd29d256ff41 Add is_dev_mounted() to expose core partition is mounted test (#723842) https://git.gnome.org/browse/gparted/commit/?id=a0c0533e3ed24a931ea18c860749303882ae4b33 Detect busy status of multi-device btrfs file systems (#723842) https://git.gnome.org/browse/gparted/commit/?id=76e64f2905bdbe37c307bc9fab055f44914dc7b2 Display mount points for multi-device btrfs file systems (#723842) https://git.gnome.org/browse/gparted/commit/?id=a086e115e5ab2884c972036daed806d53da1f257 Display usage for multi-device btrfs file systems (#723842) https://git.gnome.org/browse/gparted/commit/?id=1712809e016af4f471dd06be64eea860ef78cddb https://git.gnome.org/browse/gparted/commit/?id=3bea067596e3e2d6513cda2a66df1b3e4fa432fb Add devid to the cache of btrfs device information (#723842) https://git.gnome.org/browse/gparted/commit/?id=287526681d37e88a83c96c06513eda2db852c163 Pass devid when resizing btrfs file systems (#723842) https://git.gnome.org/browse/gparted/commit/?id=0e980a47a24b2f030252ddc6e5106a7d0b1e303b Add fallback busy detection for btrfs file systems (#723842) https://git.gnome.org/browse/gparted/commit/?id=d47783eff8d01d4d887f4f3935021fd0f10e23b1 Fallback to reading mount command output instead of /etc/mtab (#723842) https://git.gnome.org/browse/gparted/commit/?id=4b63e46a4ed7655a876875fc7d9bea01b3e91871 Display btrfs members in the Partition Information dialog (#723842) https://git.gnome.org/browse/gparted/commit/?id=20f52e28664e29d93b1a9f13fc43d01638157522 Flush devices when scanning to prevent reading stale signatures (#723842) https://git.gnome.org/browse/gparted/commit/?id=3bea067596e3e2d6513cda2a66df1b3e4fa432fb
*** Bug 738211 has been marked as a duplicate of this bug. ***
This enhancement was included in the GParted 0.20.0 release on October 20, 2014.