Running Device > Attempt Data Rescue... hangs the GParted UI for as long
as gpart takes to scan the whole disk device looking for file system
signatures.
Originally when gpart support was added by commit [1] a separate thread
was created to run gpart. Then most threading was removed by commit [2]
which left gpart running in the main thread blocking the UI.
[1] ef37bdb7de
Added support to lost data recovery using gpart
[2] 52a2a9b00a
Reduce threading (#685740)
guess_partition_table() hand codes using Glib to run the gpart command
asynchronously reading standard output, but it just doesn't run the Gtk
main loop to process events, hence the UI hangs. Instead just use
Utils::execute_command() which handles everything already. It runs the
commands asynchronously, reading output and if being run in the main
thread also calls the Gtk main loop to keep the UI responsive.
Bug 772123 - GParted is unresponsive while gpart is running
If the GParted main window is closed before the initial device load
completed gpartedbin never exits. The main window closes but the
process sits there idle forever. Subsequently running GParted reports
this error:
# gparted
The process gpartedbin is already running.
Only one gpartedbin process is permitted.
If the user is running GParted from a desktop menu they will never see
this error so they will never know why GParted won't start any more.
More technically, it is if the main window is closed before the
Win_GParted::on_show() callback completes.
I assume the Gtk main loop doesn't setup the normal quit handling until
the on_show() callback finishes drawing the main window for the first
time. Following this hint [1], move the initial device load from the
on_show() callback to immediately after it completes by using a run once
idle callback setup in on_show().
This looks exactly the same to the user except now gpartedbin exits when
the main window is closed during the initial device load. Note that
GParted finished the device load before exiting. This is exactly the
same as happens when exiting during subsequent device refreshes.
[1] How to know when a Gtk Window is fully shown?
http://stackoverflow.com/questions/14663212/how-to-know-when-a-gtk-window-is-fully-shown
"If you want to know when your main window appears the first time,
it is far easier (and saner) add a g_idle_add after your show_all
call."
Bug 771816 - GParted never exits if the main window is closed before the
initial device load completes
Again this is to handle a long list of mount points for a single
partition. To prevent the file system status value of "Mounted on [list
of 20 mount points]" causing the dialog to become wider than the screen,
line wrap the text. This instead makes the dialog taller, which already
automatically scrolls vertically as needed.
Bug 771693 - Mount Point column is wider than the screen on openSUSE
using btrfs root with lots of mounted subvolumes
openSUSE 42.1 with default btrfs root installation is heavily using
subvolumes. (Think of btrfs as a storage pool and subvolumes as
individual file systems within the pool for a rough approximation).
Thus the root partition is mounted on many mount points:
# df -k
Filesystem 1K-blocks Used Available Use% Mounted on
...
/dev/sda2 19445760 5820080 13157104 31% /
/dev/sda2 19445760 5820080 13157104 31% /var/lib/libvirt/images
/dev/sda2 19445760 5820080 13157104 31% /var/lib/mysql
/dev/sda2 19445760 5820080 13157104 31% /.snapshots
/dev/sda2 19445760 5820080 13157104 31% /home
/dev/sda2 19445760 5820080 13157104 31% /var/opt
/dev/sda2 19445760 5820080 13157104 31% /usr/local
/dev/sda2 19445760 5820080 13157104 31% /var/tmp
/dev/sda2 19445760 5820080 13157104 31% /var/lib/named
/dev/sda2 19445760 5820080 13157104 31% /var/lib/mariadb
/dev/sda2 19445760 5820080 13157104 31% /var/spool
/dev/sda2 19445760 5820080 13157104 31% /var/lib/pgsql
/dev/sda2 19445760 5820080 13157104 31% /var/lib/mailman
/dev/sda2 19445760 5820080 13157104 31% /srv
/dev/sda2 19445760 5820080 13157104 31% /opt
/dev/sda2 19445760 5820080 13157104 31% /var/crash
/dev/sda2 19445760 5820080 13157104 31% /tmp
/dev/sda2 19445760 5820080 13157104 31% /boot/grub2/i386-pc
/dev/sda2 19445760 5820080 13157104 31% /var/log
/dev/sda2 19445760 5820080 13157104 31% /boot/grub2/x86_64-efi
As the Mount Point column contains all 20 mount points it makes the
column wider than the screen, requiring lots of horizontal scrolling to
see the following columns. (Truncated to just 7 mounts in this
example here).
Partition | File System | Mount Point
/dev/sda1 * | # swap |
/dev/sda2 * | # btrfs | /, /.snapshots, /boot/grub2/i386-pc, /boot/grub2/x86_64-efi, /home, /opt, /srv
Fix by making the Mount Point column resizable and display truncated
text with ellipsis. The column now takes the initial and minimum width
from the width of the "Mount Point" column header text.
Partition | File System | Mount Point | Size
/dev/sda1 * | # swap | | 1.45 GiB
/dev/sda2 * | # btrfs | /, /.snapsho... | 18.54 GiB
Bug 771693 - Mount Point column is wider than the screen on openSUSE
using btrfs root with lots of mounted subvolumes
This commit stopped setting the text colours in the Partition, File
System and Mount Point columns to avoid hard coding text colours making
them impossible to read when using GNOME's High Contrast Inverse theme:
ff2a6c00dd
Changes post gparted-0.3.6 - code recreation from Source Forge
* src/TreeView_Detail.cc: Removed text_color hard coding
- Removed hard coding of Partition and Filesystem text_color
which was based on if partition was TYPE_UNALLOCATED.
- Removed hard coding of Mount text_color which was based
on if partition was busy. Lock symbol provides this indicator.
- Closes GParted bug #413810 - Don't hardcode text colour in
partition list
Now remove the remaining vestiges left behind. Remove the unused color
text and mount_text_color columns from the tree model. Also remove
setting of the column attributes which set the colour of the text in the
tree view from those unused columns in the tree model.
Unnecessary history. Added by:
b179990dc9
show greyed-out mountpoint of unmounted partitions in the treeview
as an improved way to identify partitions
Bug #333027 - Displaying unmounted partitions' default mount points
in grey
and by commit only in CVS history:
Bart Hakvoort <...> 2004-08-22 15:06:45
Made text in Partition column darkgrey for unallocated. this offers
more visual difference between partitions and unallocated space
Trying to create a new partition table on a device with active
partitions reports the number of active partitions in the error dialog.
However when there is a busy logical partition the number of reported
busy partitions will be one less than the number of partitions in the
main UI showing the busy symbol.
GParted considers extended partitions as busy when any of the logical
partitions it contains as busy. Display in the main UI reflects this.
Fix Win_GParted::active_partitions_on_device_count() to not exclude
extended partitions from the count.
Found that in some cases usage of active encrypted swap was not working,
but only for the first encrypted swap partition. This only failed on
the first Device Mapper device, dm-0:
# ls -l /dev/mapper/ /dev/dm-*
brw-rw---- 1 root disk 254, 0 Oct 4 20:58 /dev/dm-0
brw-rw---- 1 root disk 254, 1 Oct 4 20:58 /dev/dm-1
/dev/mapper/:
total 0
crw------- 1 root root 10,236 Oct 4 19:48 control
lrwxrwxrwx 1 root root 7 Oct 4 20:58 sdb1_crypt -> ../dm-0
lrwxrwxrwx 1 root root 7 Oct 4 20:58 sdb2_crypt -> ../dm-1
# cat /proc/swaps
Filename Type Size Used Priority
/dev/sda1 partition 1524732 92356 -1
/dev/dm-0 partition 1046524 0 -2
/dev/dm-1 partition 1046524 0 -3
Was failing because the minor number of dm-0 was 0, causing BlockSpecial
operator==() to fall back to name comparison rather than major, minor
number, and GParted name /dev/mapper/sdb1_crypt doesn't match /dev/dm-0.
Found on openSUSE and Ubuntu which don't use LVM by default and don't
already have dm-0 used as an LVM Logical Volume which GParted doesn't
support.
The LINUX ALLOCATED DEVICES document [1] says block special device 0, 0
(major, minor) is not used "reserved as null device number". (Not to
be confused with 1, 3 /dev/null the Null device). All other
non-negative pairs are valid block special device numbers. Therefore
update BlockSpecial operator==() accordingly; compare by major, minor
number when either is greater than 0 falling back to string compare
otherwise. This still fits in with the BlockSpecial() constructor using
major, minor numbers 0, 0 to represent plain files.
[1] LINUX ALLOCATED DEVICES
https://www.kernel.org/doc/Documentation/devices.txt
Bug 771670 - Usage of active encrypted swap is not shown
GParted does not show the usage of active encrypted swap partitions,
instead showing partition warning "Unable to read the contents of this
file system! ...". OS setup:
# ls -l /dev/mapper/sdb4_crypt /dev/dm-3
brw-rw----. 1 root disk 253, 3 Sep 14 07:26 /dev/dm-3
lrwxrwxrwx. 1 root root 7 Sep 14 07:26 /dev/mapper/sdb4_crypt -> ../dm-3
# mkswap -L encrypted_swap /dev/mapper/sdb4_crypt
# swapon /dev/mapper/sdb4_crypt
# cat /proc/swaps
Filename Type Size Used Priority
/dev/sda2 partition 2097148 237632 -1
/dev/dm-3 partition 1046524 0 -2
This is because the code was performing a string compare between the
canonical /dev/mapper/sdb4_crypt name GParted is using and the /dev/dm-3
name reported by the kernel via /proc/swaps. Fix by creating
BlockSpecial objects from the names and compare those so that comparison
is done correctly using major, minor numbers.
Bug 771670 - Usage of active encrypted swap is not shown
The comment became completely unnecessary with the transfer of
mount_info and fstab_info into separate Mount_Info module by commit:
63ec73dfda
Split mount_info and fstab_info maps into separate Mount_Info module
It was never necessary to clear one of the mappings at the end of the
device refresh because it was reloaded at the start of the next device
refresh anyway and it is only a small amount of memory.
Have an unmounted file system within an open encrypted mapping and an
entry in /etc/fstab for the file system like this:
# lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
...
sdb 8:16 0 8G 0 disk
+-sdb1 8:17 0 1G 0 part
+-sdb1_crypt 253:0 0 1022M 0 crypt
# blkid | grep sdb1
/dev/sdb1: TYPE="crypto_LUKS" ...
/dev/mapper/sdb1_crypt: TYPE="ext4" ...
# ls -l /dev/mapper/sdb1_crypt /dev/dm-0
brw-rw----. 1 root disk 253, 0 Sep 12 19:09 /dev/dm-0
lrwxrwxrwx. 1 root root 7 Sep 12 19:09 /dev/mapper/sdb1_crypt -> ../dm-0
# grep sdb1 /etc/fstab
/dev/mapper/sdb1_crypt /mnt/1 ext4 defaults 0 0
The mount point will be shown twice for the partition:
/mnt/1, /mnt/1
This is because add_node_and_mountpoint() adds two entries for both the
symbolic and real block special names:
map["/dev/mapper/sdb1_crypt"] = ["/mnt/1"]
map["/dev/dm-0"] = ["/mnt/1"]
This was needed for the old code which used string compare to match
block devices so that the mount point could be looked up by either name.
However since bug 767842 introduced major, minor number comparison it
became unnecessary. As both names refer to the same device the mount
point gets added twice to the same entry. Hence display of the double
mount.
map[BlockSpecial{"/dev/mapper/sdb1_crypt", 253, 0}] =
["/mnt/1", "/mnt/1"]
It is always going to be the case that the symbolic link and real block
special names have the same major, minor numbers. That was the
requirement of the BlockSpecial class and the reason for using stat() to
lookup the numbers. Therefore adding entries for both names will always
add duplicate entries. Fix by stop using realpath() to lookup the real
name and adding the duplicate entry.
Introduced by:
a800ca8b68
Add BlockSpecial into mount_info and fstab_info (#767842)
Bug 771323 - GParted is showing duplicate mount points for unmounted
encrypted file systems
The GParted_Core::mount_info and GParted_Core::fstab_info maps and the
methods that manipulate them are self-contained. Therefore move them to
a separate Mount_Info module and reduce the size of the monster
GParted_Core slightly.
The FS_Info module has a pseudo multi-object interface and used the
constructor to load the cache. However all the data in the class is
static. An FS_Info object doesn't contain any member variables, yet was
needed just to call the member functions.
Make all the member functions static removing the need to use any
FS_Info objects and provide an explicit load_cache() method.
Following earlier commit "Pre-populate BlockSpecial cache while reading
/proc/partitions (#767842)" load_proc_partitions_info_cache() has
extracted just the name field from each line of /proc/partitions.
Therefore simplify the regular expressions matching each type of whole
disk device to just matching in the name field rather than matching in
the whole line.
The Proc_Partitions_Info has a pseudo multi-object interface and uses
the constructor to load the cache. However all the data in the class is
static. A Proc_Partitions_Info object doesn't contain any member
variables, yet was needed just to call the member functions.
Make all the member functions static removing the need to use any
Proc_Partitions_Info objects and provide and explicit load_cache()
method.
Vol_id has been retired and removed from all supported distributions.
See earlier commit "Remove use of retired vol_id from FS_Info module
(#767842)" for more details. Therefore remove it's use from GParted
entirely.
GParted is already reading /proc/partitions to get whole disk device
names. The file also contains the major, minor device number of every
partition. Use this information to pre-populate the cache in the
BlockSpecial class.
# cat /proc/partitions
major minor #blocks name
8 0 20971520 sda
8 1 512000 sda1
8 2 20458496 sda2
...
9 3 1047552 md3
259 2 262144 md3p1
259 3 262144 md3p2
...
253 0 18317312 dm-0
253 1 2097152 dm-1
253 2 8383872 dm-2
253 3 1048576 dm-3
Note that for Device-Mapper names (dm-*) the kernel is not using the
canonical user space names (mapper/*). There is no harm in
pre-populating the cache with these names and will help if tools report
them too. It is just that for DMRaid, LVM and LUKS, GParted uses the
canonical /dev/mapper/* names so will still have to call stat() once for
each such name.
For plain disks (sd*) and Linux Software RAID arrays (md*) the kernel
name is the common user space name too, therefore matches what GParted
uses and pre-populating does avoid calling stat() at all for these
names.
Bug 767842 - File system usage missing when tools report alternate block
device names
Creation of every BlockSpecial object used to result in a stat() OS
call. On one of my test VMs debugging with 4 disks and a few partitions
on each, GParted refresh generated 541 calls to stat() in the
BlockSpecial(name) constructor. However there were only 45 unique
names. So on average each name was stat()ed approximately 12 times.
Cache the major, minor number associated with each name after starting
with a cleared cache for each GParted refresh. This reduces these
direct calls to stat() to just the 45 unique names.
Bug 767842 - File system usage missing when tools report alternate block
device names
The FS_Info cache is loaded from "blkid" output and compares block
special names. Therefore switch to using BlockSpecial objects so that
comparisons are performed by the major, minor device number instead.
Bug 767842 - File system usage missing when tools report alternate block
device names
FS_Info module caches the output from blkid as a single string and uses
regular expressions to find the line matching the requested block
special file name. This is not compatible with using BlockSpecial
objects to represent block special files, and perform matching by major,
minor device number. Therefore parse the blkid output into a vector of
structures containing the needed fields, ready for switching to
BlockSpecial objects in the following patch.
Interface to the module remains unchanged.
Bug 767842 - File system usage missing when tools report alternate block
device names
Vol_id was removed from udev 142, released 2009-05-13, and udev switched
to using blkid instead [1]. All currently supported distributions use
later versions of udev (or systemd after the udev merge), except for
RedHat / CentOS 5 with udev 095. However RedHat / CentOS 5 does provide
blkid and vol_id is found in udev specific /lib/udev directory not on
the PATH. Therefore effectively vol_id is not available on any
supported distribution and blkid is always available. Therefore remove
use of vol_id from the FS_Info module. Less code to refactor and test
in following changes.
[1] delete vol_id and require util-linux-ng's blkid
http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=f07996885dab45102492d7f16e7e2997e264c725
Bug 767842 - File system usage missing when tools report alternate block
device names
The SWRaid_Info cache is loaded from "mdadm" command output and
/proc/mdstat file. It contains the member name which is used to access
the cache, therefore switch to using BlockSpecial objects so that
comparison is performed using the major, minor device number.
Bug 767842 - File system usage missing when tools report alternate block
device names
The LUKS_Info module cache is loaded from "dmsetup" command and compares
block special files, therefore switch to using BlockSpecial objects so
that comparisons are performed by major, minor device number.
Bug 767842 - File system usage missing when tools report alternate block
device names
Small optimisation which avoids constructing an extra BlockSpecial
object when determining if a btrfs member is mounted. Rather than
extracting the name from the BlockSpecial object in
btrfs::get_mount_device() and re-constructing another BlockSpecial
object from that name in GParted_Core::is_dev_mounted(), pass the
BlockSpecial object directly.
Bug 767842 - File system usage missing when tools report alternate block
device names
There are no known errors which affect the remaining caches in GParted.
However those caches which compare block special devices will be changed
to use BlockSpecial objects so comparison is by major, minor device
number rather than by name.
Change btrfs member cache loaded from "btrfs filesystem show" output to
use BlockSpecial objects.
Bug 767842 - File system usage missing when tools report alternate block
device names
On some distributions having btrfs on top of LUKS encrypted partitions,
adding a second device and removing the first device used to mount the
file system causes GParted to no longer be able to report the file
system as busy or the mount points themselves.
For example, on CentOS 7, create a single btrfs file system and mount
it. The provided /dev/mapper/sdb1_crypt name is reported, via
/proc/mounts, as the mounting device:
# cryptsetup luksFormat --force-password /dev/sdb1
# cryptsetup luksOpen /dev/sdb1 sdb1_crypt
# mkfs.btrfs -L encrypted-btrfs /dev/mapper/sdb1_crypt
# mount /dev/mapper/sdb1_crypt /mnt/1
# ls -l /dev/mapper
total 0
lrwxrwxrwx. 1 root root 7 Jul 2 14:15 centos-root -> ../dm-1
lrwxrwxrwx. 1 root root 7 Jul 2 14:15 centos-swap -> ../dm-0
crw-------. 1 root root 10, 236 Jul 2 14:15 control
lrwxrwxrwx. 1 root root 7 Jul 2 15:14 sdb1_crypt -> ../dm-2
# fgrep btrfs /proc/mounts
/dev/mapper/sdb1_crypt /mnt/1 btrfs rw,seclabel,relatime,space_cache 0 0
Add a second device to the btrfs file system:
# cryptsetup luksFormat --force-password /dev/sdb2
# cryptsetup luksOpen /dev/sdb2 sdb2_crypt
# btrfs device add /dev/mapper/sdb2_crypt /mnt/1
# ls -l /dev/mapper
...
lrwxrwxrwx. 1 root root 7 Jul 2 15:12 sdb2_crypt -> ../dm-3
# btrfs filesystem show /dev/mapper/sdb1_crypt
Label: 'encrypted-btrfs' uuid: 45d7b1ef-820c-4ef8-8abd-c70d928afb49
Total devices 2 FS bytes used 32.00KiB
devid 1 size 1022.00MiB used 12.00MiB path /dev/mapper/sdb1_crypt
devid 2 size 1022.00MiB used 0.00B path /dev/mapper/sdb2_crypt
Remove the first mounting device from the btrfs file system. Now the
non-canonical name /dev/dm-3 is reported, via /proc/mounts, as the
mounting device:
# btrfs device delete /dev/mapper/sdb1_crypt /mnt/1
# btrfs filesystem show /dev/mapper/sdb2_crypt
Label: 'encrypted-btrfs' uuid: 45d7b1ef-820c-4ef8-8abd-c70d928afb49
Total devices 1 FS bytes used 96.00KiB
devid 2 size 1022.00MiB used 144.00MiB path /dev/mapper/sdb2_crypt
# fgrep btrfs /proc/mounts
/dev/dm-3 /mnt/1 btrfs rw,seclabel,relatime,space_cache 0 0
# ls -l /dev/dm-3
brw-rw----. 1 root disk 253, 3 Jul 2 15:12 /dev/dm-3
GParted loads the mount_info mapping from /proc/mounts and with it the
/dev/dm-3 name. When GParted is determining if the encrypted btrfs file
system is mounted or getting the mount points it is using the
/dev/mapper/sdb2_crypt name. Therefore no information is found and the
file system is incorrectly reported as unmounted.
Fix by changing mount_info and fstab_info to use BlockSpecial objects
instead of strings so that matching is performed by major, minor device
numbers rather than by string compare. Note that as BlockSpecial
objects are used as the key of std::map [1] mappings operator<() [2]
needs to be provided to order the key values.
[1] std::map
http://www.cplusplus.com/reference/map/map/
[2] std::map::key_comp
http://www.cplusplus.com/reference/map/map/key_comp/
Bug 767842 - File system usage missing when tools report alternate block
device names
In some cases creating an LVM2 Physical Volume on top of a DMRaid array
reports no usage information and this partition warning:
Unable to read the contents of this file system!
Because of this some operations may be unavailable.
The cause might be a missing software package.
The following list of software packages is required for lvm2
pv file system support: lvm2.
For example on Ubuntu 14.04 LTS (with GParted built with
--enable-libparted-dmraid) create an LVM2 PV in a DMRaid array
partition. GParted uses this command:
# lvm pvcreate -M 2 /dev/mapper/isw_bacdehijbd_MyArray0p2
But LVM reports the PV having a different name:
# lvm pvs
PV VG Fmt Attr PSize PFree
/dev/disk/by-id/dm-name-isw_bacdehijbd_MyArray0p2 lvm2 a-- 1.00g 1.00g
This alternate name is loaded into the LVM2_PV_Info module cache. Hence
when GParted queries partition /dev/mapper/isw_bacdehijbd_MyArray0p2 it
has no PV information against that name and reports unknown usage.
However they are actually the same block special device; major 252,
minor 2:
# ls -l /dev/mapper/isw_bacdehijbd_MyArray0p2
brw-rw---- 1 root disk 252, 2 Jul 2 11:09 /dev/mapper/isw_bacdehijbd_MyArray0p2
# ls -l /dev/disk/by-id/dm-name-isw_bacdehijbd_MyArray0p2
lrwxrwxrwx 1 root root 10 Jul 2 11:09 /dev/disk/by-id/dm-name-isw_bacdehijbd_MyArray0p2 -> ../../dm-2
# ls -l /dev/dm-2
brw-rw---- 1 root disk 252, 2 Jul 2 11:09 /dev/dm-2
To determine if two names refer to the same block special device their
major, minor numbers need to be compared, instead of string comparing
their names.
Implement class BlockSpecial which encapsulates the name and major,
minor numbers for a block special device. Also performs comparison as
needed. See bug 767842 comments 4 and 5 for further investigation and
decision for choosing to implement a class.
Replace name strings in the LVM2_PV_Info module with BlockSpecial
objects performing correct block special device comparison.
Bug 767842 - File system usage missing when tools report alternate block
device names
Now Device and Partition objects only have a single path,
get_alternate_paths() is never called. Remove the method and population
of the private alternate_paths_cache member that went with it.
Bug 767842 - File system usage missing when tools report alternate block
device names
To reflect that there is now only a single path in the Partition object
now. Also get rid of the now unneeded optional clear_paths parameter
which was only relevant when there was a vector of paths.
Bug 767842 - File system usage missing when tools report alternate block
device names
Now that Partition objects only have a single path, rather than a list
of paths, stop performing unnecessary actions in calibrate_partitions()
which added alternate paths reported from libparted. Just left with
setting the partition path name correctly, when the path name doesn't
exist. Happens when the path is set to "Copy of /dev/SRC" when the
partition was newly created by a copy-paste into unallocated space
earlier in the sequence of operations now being applied.
Bug 767842 - File system usage missing when tools report alternate block
device names
Change from a vector of paths to a single path member in the Partition
object. Remove add_paths() and get_paths() methods. Keep add_path()
and get_path().
Bug 767842 - File system usage missing when tools report alternate block
device names
To reflect that there is only a single path in the Device object now.
Also get rid of the now unneeded optional parameter which was only
relevant when there was a vector of paths.
Bug 767842 - File system usage missing when tools report alternate block
device names
Background
GParted stored a list of paths for Device and Partition objects. It
sorted this list [1][2] and treated the first specially as that is what
get_path() returned and was used almost everywhere; with the file system
specific tools, looked up in various *_Info caches, etc.
[1] Device::add_path(), ::add_paths()
[2] Partition::add_path(), ::add_paths()
Mount point display [3] was the only bit of GParted which really worked
with the path list. Busy file system detection [4] just used the path
provided by libparted, or for LUKS /dev/mapper/* names. It checked that
single path against the mounted file systems found from /proc/mounts,
expanded with additional block device names when symlinks were
encountered.
[3] GParted_Core::set_mountpoints() -> set_mountpoints_helper()
[4] GParted_Core::set_device_partitions() -> is_busy()
GParted_Core::set_device_one_partition() -> is_busy()
GParted_Core::set_luks_partition() -> is_busy()
Having the first path, by sort order, treated specially by being used
everywhere and virtually ignoring the others was wrong, complicated to
remember and difficult code with. As all the additional paths were
virtually unused and made no difference, remove them. The "improved
detection of mountpoins, free space, etc.." benefit from commit [5]
doesn't seem to exist. Therefore simplify to a single path for Device
and Partition objects.
[5] commit 6d8b169e73
changed the way devices and partitions store their device paths.
Instead of holding a 'realpath' and a symbolic path we store paths
in a list. This allows for improved detection of mountpoins, free
space, etc..
This patch
Simplify the Device object from a vector of paths to a single path.
Remove add_paths() and get_paths() methods. Keep add_path() and
get_path() for now.
Bug 767842 - File system usage missing when tools report alternate block
device names
Recognise GRUB2 core.img boot code written to a partition without a file
system. Such setups are possible/likely with GPT partitioned disks as
there is a specific partition type reserved for it [1][2]:
21686148-6449-6E6F-744E-656564454649 (BIOS Boot partition)
[1] GUID Partition Table, Partition types
https://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs
[2] BIOS boot partition
https://en.wikipedia.org/wiki/BIOS_boot_partition
Bug 766989 - zfsonline support - need file system name support for ZFS
type codes
Composing these operations caused GParted to abort on an assert failure:
(1) Delete an existing partition,
(2) Create a new partition,
(3) Delete new partition.
This is the equivalent issue as fixed in the previous commit, except with
the delete operation rather than the check operation:
Prevent assert failure from OperationCheck::get_partition_new() (#767233)
# ./gpartedbin
======================
libparted : 2.4
======================
**
ERROR:OperationDelete.cc:41:virtual GParted::Partition& GParted::OperationDelete::get_partition_new(): assertion failed: (false)
Aborted (core dumped)
# gdb ./gpartedbin core.19232 --batch --quiet --ex backtrace -ex quit
[New Thread 19232]
[New Thread 19234]
[Thread debugging using libthread_db enabled]
Core was generated by `./gpartedbin'.
Program terminated with signal 6, Aborted.
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x000000361f233dc5 in abort () at abort.c:92
#2 0x0000003620a67324 in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x50fcc0 "virtual GParted::Partition& GParted::OperationDelete::get_partition_new()", message=0x1b55f60 "assertion failed: (false)") at gtestutils.c:1358
#3 0x0000003620a678f0 in g_assertion_message_expr (domain=0x0, file=0x50fa68 "OperationDelete.cc", line=41, func=0x50fcc0 "virtual GParted::Partition& GParted::OperationDelete::get_partition_new()", expr=<value optimized out>) at gtestutils.c:1369
#4 0x000000000049aa0d in GParted::OperationDelete::get_partition_new (this=0x1b321b0) at OperationDelete.cc:41
#5 0x00000000004c6700 in GParted::Win_GParted::activate_delete (this=0x7ffc91403670) at Win_GParted.cc:2068
...
As before the crash is happened in Win_GParted::activate_delete() as it
was going through the list of operations removing those which applied to
the never created partition. It came across the delete operation of an
existing partition and called get_partition_new(). As partition_new was
not set or used by the delete operation this asserted false and crashed
GParted.
Unlike the check operation case, the delete operation doesn't have a
partition afterwards. (As GParted represents unallocated space with
partition objects then the delete operation could be populated with a
new partition by constructing the correctly merged unallocated space
partition object, but that is what OperationDelete::apply_to_visual()
does and having a place holder doesn't seem like the right thing to do).
Instead exclude delete operations, on existing partitions, when looking
for operations applying to this not yet created partition as they are
mutually exclusive.
Bug 767233 - GParted core dump on assert failure in
OperationDelete::get_partition_new()
Composing these operations caused GParted to abort on an assert failure:
(1) Check an existing partition,
(2) Create a new partition,
(3) Delete new partition.
# ./gpartedbin
======================
libparted : 2.4
======================
**
ERROR:OperationCheck.cc:40:virtual GParted::Partition& GParted::OperationCheck::get_partition_new(): assertion failed: (false)
Aborted (core dumped)
# gdb ./gpartedbin core.8876 --batch --quiet --ex backtrace -ex quit
[New Thread 8876]
[New Thread 8879]
[Thread debugging using libthread_db enabled]
Core was generated by `./gpartedbin'.
Program terminated with signal 6, Aborted.
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x000000361f233dc5 in abort () at abort.c:92
#2 0x0000003620a67324 in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x50f400 "virtual GParted::Partition& GParted::OperationCheck::get_partition_new()", message=0x1d37a00 "assertion failed: (false)") at gtestutils.c:1358
#3 0x0000003620a678f0 in g_assertion_message_expr (domain=0x0, file=0x50f1a8 "OperationCheck.cc", line=40, func=0x50f400 "virtual GParted::Partition& GParted::OperationCheck::get_partition_new()", expr=<value optimized out>) at gtestutils.c:1369
#4 0x0000000000498e21 in GParted::OperationCheck::get_partition_new (this=0x1d1bb30) at OperationCheck.cc:40
#5 0x00000000004c66ec in GParted::Win_GParted::activate_delete (this=0x7fff031c3e30) at Win_GParted.cc:2068
...
When Win_GParted::activate_delete() was stepping through the operation
list removing operations (2 & 3 in the above recreation steps) which
related to the new partition never to be created it called
get_partition_new() on all operations in the list. This included
calling get_partition_new() on the check operation (1 in the above
recreation steps). As partition_new was not set or used by the check
operation get_partition_new() asserted false and crashed GParted.
Fix by populating the partition_new member in OperationCheck objects,
thus allowing get_partition_new() to be called on the object. As a
check operation doesn't change any partition boundaries or file system
attributes, just duplicate the new partition from the original
partition.
Bug 767233 - GParted core dump on assert failure in
OperationDelete::get_partition_new()
E2fsprogs 1.42 adds ext4 64bit feature [1] allowing volume sizes larger
than 16 TiB. However only enable large volumes from e2fsprogs 1.42.9
when a large number of 64bit bugs were fixed [2]. (Also RHEL / CentOS 7
use e2fsprogs 1.42.9 and always enable 64bit feature by default).
[1] Release notes, E2fsprogs 1.42 (November 29, 2011)
http://e2fsprogs.sourceforge.net/e2fsprogs-release.html#1.42
"This release of e2fsprogs has support for file systems > 16TB.
Online resize requires kernel support which will hopefully be in
Linux version 3.2. Offline support is not yet available for > 16TB
file systems, but will be coming".
[2] Release notes, E2fsprogs 1.42.9 (December 28, 2013)
http://e2fsprogs.sourceforge.net/e2fsprogs-release.html#1.42.9
"Fixed a large number of bugs in resize2fs, e2fsck, debugfs, and
libext2fs to correctly handle bigalloc and 64-bit file systems.
There were many corner cases that had not been noticed in previous
uses of these file systems, since they are not as common. Some of
the bugs could cause file system corruption or data loss, so users
of 64-bit or bigalloc file systems are strongly urged to upgrade to
e2fsprogs 1.42.9".
Bug 766910 - Multiple boot loaders don't work on 64bit EXT4 file systems
Calling libparted ped_geometry_new() creates a new PedGeometry object
from malloced memory, however the corresponding ped_geometry_destroy()
is never called to destroy the object and free the memory.
Perform a resize of a FAT file system when running GParted under
valgrind identifies several memory blocks leaked via ped_geometry_new()
from resize_move_filesystem_using_libparted(). One such example:
# valgrind --track-origins=yes --leak-check=full ./gpartedbin
...
==32069== 32 bytes in 1 blocks are definitely lost in loss record 5,419 of 11,542
==32069== at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==32069== by 0x8ECD8C5: ped_malloc (libparted.c:231)
==32069== by 0x8ED23C1: ped_geometry_new (geom.c:79)
==32069== by 0x4764F3: GParted::GParted_Core::resize_move_filesystem_using_libparted(GParted::Partition const&, GParted::Partition const&, GParted::OperationDetail&) (GParted_Core.cc:2666)
==32069== by 0x478007: GParted::GParted_Core::resize_filesystem(GParted::Partition const&, GParted::Partition const&, GParted::OperationDetail&, bool) (GParted_Core.cc:2990)
==32069== by 0x478440: GParted::GParted_Core::maximize_filesystem(GParted::Partition const&, GParted::OperationDetail&) (GParted_Core.cc:3037)
==32069== by 0x4769A0: GParted::GParted_Core::resize(GParted::Partition const&, GParted::Partition const&, GParted::OperationDetail&) (GParted_Core.cc:2746)
==32069== by 0x47582B: GParted::GParted_Core::resize_move(GParted::Partition const&, GParted::Partition&, GParted::OperationDetail&) (GParted_Core.cc:2457)
==32069== by 0x46DDB2: GParted::GParted_Core::apply_operation_to_disk(GParted::Operation*) (GParted_Core.cc:767)
...
There is also a leak of a PedGeometry object from
resize_move_partition(). Fix by calling ped_geometry_destroy() to
delete all the allocated PedGeometry objects and free the memory.
Bug 767009 - PedGeometry objects are memory leaked
When replacing the list of paths for the other partition object involved
in either a Resize/Move or Format operation in apply_operation_to_disk()
should replace the whole list of partitions not just replace with the
first path. Copy the whole path list is the correct action. It makes
no material difference because secondary partition paths are only used
to discover mount points during refresh phase and not at the apply
phase, where only the primary path is used.
Bug 766349 - Resolve code ugliness with partition path getting set to
"copy of /dev/SRC"
Quoting the relevant comments from GParted_Core::calibrate_partition():
Re-add the real partition path from libparted.
When creating a copy operation by pasting into unallocated space the
list of paths for the partition object was set to
["Copy of /dev/SRC"] because the partition didn't yet exist before
the operations were applied. Additional operations on that new
partition also got the list of paths set to ["Copy of /dev/SRC"].
This re-adds the real path to the start of the list making it
["/dev/NEW", "Copy of /dev/SRC"]. This provides the real path for
file system specific tools used during those additional operations
such mkfs for the format operation or fsck and others for the
resize/move operation.
FIXME: Having this work just because "/dev/NEW" happens to sort
before "Copy of /dev/SRC" is ugly! Probably have a separate display
path which can be changed at will without affecting the list of real
paths for the partition.
Having a separate display path is overly complicated and unnecessary.
Just replace the list of paths with the real one reported by libparted
if it contained "Copy of /dev/SRC", determined by checking if the file
exists. Otherwise continue to add the libparted name as an alternate
path. Whole disk devices can never be named "Copy of /dev/SRC" because
they are not partitioned so never created or deleted by GParted, only
ever written to, hence don't need the extra exists test logic.
Bug 766349 - Resolve code ugliness with partition path getting set to
"copy of /dev/SRC"
When composing a copy operation it always named the destination
partition as "copy of /dev/SRC". For the case of pasting into
unallocated space creating a new partition this was the right thing to
do as the partition doesn't yet exist so the path is not yet known.
However for the case of pasting into an existing partition the path is
known and replacing it with "copy of /dev/SRC" is wrong. No other
operation when operating on an existing partition changes it path.
Given a set of existing partitions, sdb1 to sdb4, compose a set of copy
operations as: copy sdb1 to sdb2, copy sdb2 to sdb3 and copy sdb3 to
sdb4. The displayed partitions before being applied become:
/dev/sdb1
copy of /dev/sdb1
copy of copy of /dev/sdb1
copy of copy of copy of /dev/sdb1
And the pending operations are named:
Copy /dev/sdb1 to /dev/sdb2
Copy copy of /dev/sdb1 to /dev/sdb3
Copy copy of copy of /dev/sdb1 to /sev/sdb4
This is perverse. In the case of pasting into an existing partition
keep the real path name. This keeps the partitions being displayed as:
/dev/sdb1
/dev/sdb2
/dev/sdb3
/dev/sdb4
And the pending operations named as:
Copy /dev/sdb1 to /dev/sdb2
Copy /dev/sdb2 to /dev/sdb3
Copy /dev/sdb3 to /dev/sdb4
Much more understandable.
Also switch to an upper case "C" in "Copy of /dev/SRC" as the temporary
path name when pasting into unallocated space. Finally update the
comment in calibrate_partition() to describe the remaining cases when
re-adding the path is still required.
Bug 766349 - Resolve code ugliness with partition path getting set to
"copy of /dev/SRC"
Make the code a little more self documenting by adding the symbolic
constants:
SETTLE_DEVICE_APPLY_MAX_WAIT_SECONDS
SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS
which highlight that settle_device() is called in two different
contexts, device probe and apply operations, with two different timeout
values.
File system specific commands sometimes fail reporting that the
partition specific /dev entry doesn't exist. Example failing check
operation details:
Check and repair file system (ext4) on dev/sdb4
calibrate /dev/sdb4
path: /dev/sdb4 (partition)
start: 4196352
end: 6293503
size: 2097152 (1.00 GiB)
check file system on /dev/sdb4 for errors and (if possible) fix them
e2fsck -f -y -bv -C 0 /dev/sdb4
e2fsck 1.42.9 (28-Dec-2013)
e2fsck: No such file or directory while trying to open /dev/sdb4
Possibly non-existent device?
This has been reproduced on CentOS 7. Debugging shows that the
libparted calls used to re-read the partition details in
GParted_Core::calibrate_partition() leads to udev removing and re-adding
all the partition /dev entries for the disk.
# udevadm monitor &
# gpartedbin
...
16.480662 +12.618659 calibrate_partition() calling get_device("/dev/sdb", lp_device) ...
16.483644 +0.002982 calibrate_partition() get_device() returned
16.483678 +0.000034 calibrate_partition() calling get_disk(lp_device, lp_disk) ...
16.618113 +0.134435 calibrate_partition() get_disk() returned
KERNEL[19275.707968] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
16.618561 +0.000448 destroy_device_and_disk() calling ped_disk_destroy(lp_disk) ...
16.618584 +0.000023 destroy_device_and_disk() ped_disk_destroy() returned
16.618591 +0.000007 destroy_device_and_disk() calling ped_device_destroy(lp_disk) ...
16.618602 +0.000011 destroy_device_and_disk() ped_device_destroy() returned
16.618687 +0.000085 calibrate_partition() return true
16.618851 +0.000164 execute_command() e2fsck -f -y -v -C 0 /dev/sdb4
KERNEL[19275.708389] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[19275.708500] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[19275.708643] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
KERNEL[19275.768278] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
KERNEL[19275.771171] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
KERNEL[19275.771360] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[19275.771542] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[19275.775858] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [19275.820153] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [19275.823152] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [19275.828275] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
16.742735 +0.123884 execute_command() exit status 8
UDEV [19275.841425] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
UDEV [19275.905478] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
UDEV [19276.013580] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [19276.034728] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [19276.174840] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
UDEV [19276.237105] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
So exactly when GParted is running the external e2fsck command, udev is
in the middle of removing and re-adding all the /dev partition entries
for the disk. Hence the above failure reporting that /dev/sdb4 didn't
exist. This error depends on the timing between GParted running the
external file system specific command and udev removing and re-adding
the entries, so sometimes it works and sometimes it fails.
Further debugging showed that simply opening and closing the whole disk
device read-write triggers the same removing and re-adding of all the
partition /dev entries with udev >= 219. Opening the whole disk device
read-write is what libparted has always done until this post
libparted 3.2 patch to make it open read-only when probing:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=44d5ae0115c4ecfe3158748309e9912c5aede92d
libparted: Use read only when probing devices on linux (#1245144)
To fix this simply wait for udev devices to settle in
calibrate_partitions(). The longest I have seen udev take to do this is
0.80 seconds in a VM. Wait up to 10 seconds as is done in commit() ->
commit_to_os(), also called when applying operations.
On configurations which don't have this issue execution of udevadm
settle, which will return immediately, adds at most 0.1 seconds to the
time taken for the calibrate step. This won't be noticed in the time
taken of the operation details so there is no point in trying to avoid
executing udevadm settle when not needed.
Bug 762941 - Operations sometimes failing with: No such file or
directory
Minor issues:
1) In the while loop reading from /proc/partitions into variable line,
just after the sscanf() call the variable was re-purposed to hold the
device name making the code unnecessarily hard to follow.
2) Variable c_str was a fixed sized buffer holding the device name read
from /proc/partitions.
3) Variable c_str name provides no meaning as to what data it holds.
4) Return value from all the Utils::regexp_label() calls is converted
from Glib::ustring to std::string to be stored in device variable.
Resolve by using Utils::regexp_label() to extract the device name from
each line in /proc/partitions and store in the variable device, already
used for this purpose and now changed to type Glib::ustring.
realpath(3) manual page says:
BUGS
The POSIX.1-2001 standard version of this function is broken by
design, since it is impossible to determine a suitable size for
the output buffer, resolved_path. According to POSIX.1-2001 a
buffer of size PATH_MAX suffices, but PATH_MAX need not be a
defined constant, and may have to be obtained using pathconf(3).
And asking pathconf(3) does not really help, since, on the one
hand POSIX warns that the result of pathconf(3) may be huge and
unsuitable for mallocing memory, and on the other hand
pathconf(3) may return -1 to signify that PATH_MAX is not
bounded. The resolved_path == NULL feature, not standardized in
POSIX.1-2001, but standardized in POSIX.1-2008, allows this
design problem to be avoided.
The resolved_path == NULL feature of realpath() has existed as a Glibc
extension since realpath() was first added to Glibc 1.90, released in
June 1996. Therefore it can be used unconditionally.
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=fa0bc87c32d02cd81ec4d0ae00e0d943c683e6e1
Bug 764369 - Use realpath() safely
Mike Fleetwood