These two methods had a lot of repeated and common code. Both determine
if the partition has any pending operations, notify the user that
changing the busy status can not be performed, and report any errors
when changing the status.
Extract the common code into sub-functions check_toggle_busy_allowed()
and show_toggle_failure_dialog() to handle showing the message dialogs.
Also refactor toggle_busy_state() to make it clear that it handles 5
cases of swapon, swapoff, activate VG, deactivate VG and unmount file
system.
Also remove out of date comment near the top of toggle_busy_state()
stating there can only be pending operations for inactive partitions is
out of date. Some file systems can be resized while online and
partition naming is allowed whatever the busy status of the file system.
Bug 775932 - Refactor mostly applying of operations
The primary reason to refactor unmount_partition() is to pass the
Partition object to be unmounted, rather than use member variable
selected_partition_ptr so that it doesn't have to handle the differences
between encrypted and non-encrypted Partition objects. The calling
function can deal with that instead. Then there were lots of small
reasons to change almost every other line too:
* Return success or failure rather than updating a passed pointer with
the result. Leftover from when the function used to be a separate
thread:
commit 52a2a9b00a
Reduce threading (#685740)
* Pass updated error string by reference rather than by pointer. Likely
another leftover.
* Stop borrowing the updated error string as a local variable for the
error output from the umount command. Use new umount_error local
variable instead. Was bad practice making the code harder to
understand.
* Rename failed_mountpoints to skipped_mountpoints to better reflect
that it contains the mount points not attempted to be unmounted
because two or more file systems are mounted at that point.
* Rename errors to umount_errors to better reflect it contains the
errors from each failed umount command.
* Document the reason for mount points being skipped.
* Update the skipped mount points message to state definitely why they
could not be unmounted rather than stating most likely.
* Simplify logic processing the error cases and return value.
* Made static because it no longer accesses any class members.
* Remove out dated "//threads.." comment from the header. Another
leftover from when the function use to be a separate thread.
Bug 775932 - Refactor mostly applying of operations
Reorder the parameters into the same order in which they occur in the
row, i.e. Name first, then Mount Point and finally Label. Rename local
variables in load_partitions(1 param) and parameters of
load_partitions(5 params) prefixing with "show_" to make it clearer the
variables track if that column will be displayed or not.
create_row() populates the values for each row to be displayed in the UI
from the relevant Partition object. However load_partitions(5 params)
independently decided if the Name, Mount Point and Label columns were
empty and should be displayed.
Getting the mount point value is more complex for encrypted file systems
because it has to call get_mountpoints() on the inner encrypted
Partition object. load_partitions(5 params) didn't account for this.
Fix by making create_row() both copy the values into each row and at the
same time check if they are empty to decide if they should be displayed
or not.
Bug 775475 - Mount Point column displayed for encrypted file systems
even when empty
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
Mike Fleetwood