Move the code which reads the Unicode label from FS_Info::get_label()
into new function run_blkid_update_cache_one_label() which also replaces
the non-reversibly encoded copy loaded during the initial cache load.
This is mainly a bit of code refactoring ready for the following change.
It deliberately keeps the initial loaded labels so that reading
/etc/fstab and decoding LABEL=<label> to block special device names via
FS_Info::get_path_by_label() continues to works, at least for ASCII only
labels.
Bug 786502 - Support reading Unicode labels when file system specific
tools aren't available
Make especially the Volume Identifier length limit code simpler to
understand and therefore easier to maintain.
Bug 784533 - Add support for UDF file system
UDF label is stored in the Logical Volume Identifier which has space for
either 126 Latin1 or 63 UCS-2 characters. For compatibility reasons
with older versions of blkid, the possibly truncated UDF label is also
stored in the Volume Identifier which only has space for 30 Latin1 or 15
UCS-2 characters.
Because versions of mkudffs prior to 1.1 damage the label if it contains
non-ASCII characters, make sure GParted does not call such versions of
mkudffs with a non-ASCII character label.
Bug 784533 - Add support for UDF file system
Add support for detecting UDF file systems and formatting hard disks
with revision 2.01 UDF file systems using udftools. Formatting optical
disks or any other media types is not supported yet. Changing label or
UUID after formatting is not supported as the tools do not yet exist.
Bug 784533 - Add support for UDF file system
For large output a lot of time is used copying capturebuf to callerbuf
to provide a Glib::ustring copy of the buffer for the update callback.
However update callbacks are only used when commands are run to apply
operations by FileSystem::execute_command() and their output is
incrementally displayed in the UI. Whereas update callbacks are never
used when commands are used to query information via
Utils::execute_command().
Stop performing interim copying of capturebuf to callerbuf when there
are no update callbacks registered as it is unnecessary.
Time to read portions of the recorded fsck.fat output via
fat16::set_used_sectors() and intermediate copies aren't required:
1 MiB 10 MiB 122 MiB
old code : 0.074 sec 1.41 sec 210 sec [3:30]
new code : 0.063 sec 0.56 sec 6.57 sec
Bug 777973 - Segmentation fault on bad disk
If PipeCapture reads a NUL byte in the middle of what is expected to be
a multi-byte UTF-8 character then PipeCapture either returns the
captured characters to the previous update or loops forever depending on
whether the end of the stream is encountered before the read buffer is
full or not. This is equivalent to saying whether the NUL byte occurs
within the last 512 bytes of the output or not.
This is caused by a bug in g_utf8_get_char_validated() reporting that a
partial UTF-8 character has been found when the NUL byte is encountered
in the middle of a multi-byte character even though more bytes are
available in the length specified buffer. g_utf8_get_char_validated()
is always stopping at the NUL byte assuming it is working with a NUL
terminated string.
Workaround this by checking for g_utf8_get_char_validated() claiming a
partial UTF-8 character has been found when in fact there are at least
enough bytes in the read buffer to instead determine that it is really
an invalid UTF-8 character.
Reference:
Bug 780095 - g_utf8_get_char_validated() stopping at nul byte even
for length specified buffers
https://bugzilla.gnome.org/show_bug.cgi?id=780095
Bug 777973 - Segmentation fault on bad disk
A user had a very corrupted FAT file system such that fsck.fat produced
122 MiB of output. GParted has to read this output to get the file
system usage information. However GParted takes more than 48 hours to
read the 122 MiB of output, while using 100% CPU time and is
unresponsive for the duration.
Modified fsck.fat to output just the first 1 MiB of output and used perf
to capture performance data from GParted reading that output:
# perf -g -F 1999 -- ./gpartedbin
# perf report --stdio
67.84% Glib::ustring::replace [4.23s]
17.67% g_utf8_pointer_to_offset [1.10s]
8.48% g_utf8_offset_to_pointer [0.53s]
[ 6.01% (everything else) ] [0.38s]
[100.00% TOTAL ] [6.24s]
And to read the first 10 MiB of output the performance figures are:
92.95% Glib::ustring::replace [257.44s]
4.35% g_utf8_pointer_to_offset [ 12.05s]
2.13% g_utf8_offset_to_pointer [ 5.90s]
[ 0.58% (everything else) ] [ 1.61s]
[100.00% TOTAL ] [277.00s]
See how the total time is increasing non-linearly, 44 times longer for
only 10 times as much data. This is because of the exponential increase
in time spent in Glib::ustring::replace.
After a lot of experimentation I came to the conclusion that
Glib::ustrings are not appropriate for storing and editing large buffers
of data, sizes megabytes and above. The issues are that iterators are
invalid after the content changes and replacing UTF-8 characters by
index gets exponentially slower as the size of the string increases.
Hence the > 48 hours of 100% CPU time to read and apply the line
discipline to the 122 MiB of fsck.fat output. See code comment for a
more detailed description of the issues found.
Rewrote OnReadable() to use Glib::ustrings as little as possible.
Instead using buffers and vectors of fixed width data types allowing for
fast access using pointers and indexes (converted to pointers by the
compiler with simple arithmetic). Again see code comment for a more
detailed description of the implementation.
Repeating the performance capture with the new code for the first 1 MiB
of fsck.fat output:
63.34% memcpy [0.35s]
[ 36.66% (everything else) ] [0.21s]
[100.00% TOTAL ] [0.56s]
And for the first 10 MiB of fsck.fat output:
96.66% memcpy [63.60s]
[ 3.34% (everything else) ] [ 2.20s]
[100.00% TOTAL ] [65.80s]
Simple timings taken to read portions of the fsck.fat output (when not
using perf):
1 MiB 10 MiB 122 MiB
old code : 6.2 sec 277 sec > 48 hours
(4:37)
new code : 0.6 sec 66 sec 17262 sec
(1:06) (4:47:42)
Performance of the code is still non-linear because of the assignment
of the ever growing capturebuf to callerbuf for every block of input
read. This is required to generate a consistent Glib::ustring copy of
the input for the update callback. However this is much faster than
before, and I have a plan for further improvements.
Bug 777973 - Segmentation fault on bad disk
As the source code is managed in GIT and there is a .gitignore file in
the top level directory specifying file names to exclude from version
control, then the old per-directory .cvsignore files for CVS are
redundant.
Add the only missing and applicable entry from src/.cvsignore of '.libs'
to .gitignore and remove all the .cvsignore files.
Using the default MiB alignment, creating an MSDOS logical partition
between two other existing logical partitions fails with this error
dialog:
(-) <b>An error occurred while applying the operations</b>
See the details for more information.
<b>IMPORTANT</b>
If you want support, you need to provide the saved details!
See http://gparted.org/save-details.htm for more information.
[ OK ]
and these operation details:
+ libparted messages
- Unable to satisfy all constraints on the partition.
This bug was introduced by this commit included in GParted 0.23.0:
90e3ed68fc
Shallow copy Device object into Operation object (#750168)
The commit message claimed that the deep copied Partition objects inside
the Device inside the Operation object are never accessed. This turned
out not to be true. Win_GParted::Add_Operation() uses them as part of
snap_to_alignment() which updates requested partition boundaries to
account for alignment requirements and the space needed for EBR
(Extended Boot Record) preceding logical partitions.
In this case the new logical partition was trying to be created over the
top of the EBR for the following logical partition because
snap_to_alignment() wasn't aware of its existence.
Fix by making Add_Operation() and snap_to_alignment() refer to the
current device, as displayed in the UI, rather than the shallow copy
included in the Operation object. Hopefully now it is true that the
not copied vector of Partition objects in the Device object in each
Operation object are never accessed.
Bug 779339 - enforce at least 1 MiB "free space following"
The Operation class already provided find_index_extended() method and
was used in the Operation and derived classes where required. It
returns the index to the extended partition in the PartitionVector
object, or -1 when no extended partition exists.
There were several cases of the same functionality being open coded in
GParted_Core and Win_GParted. Therefore move the implementation to
find_extended_partition() in PartitionVector compilation unit and use
this implementation everywhere.
The clear_mountpoints parameter has never been used since
add_mountpoint*() were first added [1][2]. clear_mountpoints() method
[3] is available to provide this functionality and used. Therefore
removed unused parameter and code.
[1] add_mountpoints() added 2006-03-15
9532c3cad1
Made Partition::mountpoints private
[2] add_mountpoint() added 2011-12-16
208083f11d84dbd4f186271a3cdbf5170db259f8b8
Display LVM2 VGNAME as the PV's mount point (#160787)
[3] clear_mountpoint() added 2006-03-19
ad9f2126e7
fixed issues with copying (see also #335004) cleanups + added FIXME added
Now that resizing of encrypted file systems is implemented add growing
of the open LUKS mapping as part of the check repair operation.
Resizing an encrypted file system requires the LUKS mapping to be open
to access the file system within; therefore it also requires libparted
and kernel support for online partition resizing. This limits resizing
to the latest distributions with libparted >= 3.2 and kernel >= 3.6.
However growing an open LUKS mapping as part of a check repair operation
doesn't require resizing the partition. Therefore route via offline
grow of LUKS to avoid those extra, unnecessary requirement. This does
mean that offline LUKS grow artificially requires cryptsetup, but that is
not really significant as even opening LUKS requires cryptsetup.
So now checking an encrypted file system on even the oldest
distributions does:
1) runs FSCK on the encrypted file system;
2) grows the encryption volume to fill the partition;
3) grows the file system to fill the encryption mapping.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Moving of closed LUKS is simply enabled by luks .move capability being
set and requires no further coding.
Resizing of encrypted file systems requires both the LUKS mapping and
encrypted file system within to be resized in the right order for both
shrinking and growing. To keep the code simple split resizing of plain
and encrypted into separate functions.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Changing the Resize/Move dialog code to also handle PartitionLUKS
objects was considered too complicated. Instead create an unencrypted
equivalent using clone_as_plain(), pass that to the Resize/Move dialog
and finally apply the change back using Partition*::resize().
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Add a resize() method to both Partition and PartitionLUKS classes. They
take a reference Partition object, and update the position, size and
file system usage of *this Partition to match. This is ready for taking
a partition returned from Resize/Move dialog and applying the change.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Implement a specialist PartitionLUKS clone method. Creates a new
Partition object which has the same space usage as the source encrypted
file system, but is a plain file system. Namely, the overhead of the
LUKS header has been added to the file system usage. This is ready for
feeding this representation of the partition to the Resize/Move dialog.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Already have:
Utils::get_filesystem_string(FS_EXT2) -> "ext2"
virtual Partition::get_filesystem_string() -> "ext2"
virtual PartitionLUKS::get_filesystem_string() -> "[Encrypted] ext2"
Add these:
Utils::get_encrypted_string() -> "[Encrypted]"
Utils::get_filesystem_string(false, FS_EXT2) -> "ext2"
Utils::get_filesystem_string(true, FS_EXT2) -> "[Encrypted] ext2"
This is ready for use of Utils::get_filesystem_string(true, FS_EXT2)
when composing the preview of a format of an encrypted file system by
Win_GParted::activate_format().
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
When composing, describing and implementing the operation just need the
code to query and set the Partition object directly containing the file
system, instead of the enclosing encryption mapping to make it work.
The operation details for setting a new UUID on an encrypted ext4 file
system become:
Set a new random UUID on [Encrypted] ext4 file system on /dev/sdb4
+ calibrate /dev/sdb4
+ Set UUID on /dev/mapper/sdb4_crypt to a new, random value
+ tune2fs -U random /dev/mapper/sdb4_crypt
tune2fs 1.41.12 (17-May-2010)
Also note the now documented rule in apply_operation_to_disk() which
says each operation must leave the status of the encryption mapping and
file system as it found it.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Provide and use a single interface for getting the file system string
for display, regardless of whether the partition is encrypted or the
encryption mapping is active or not.
Example return values for get_filesystem_string() for different types
and states of Partition objects:
1) Plain ext4 file system: -> "ext4"
2) Closed encrypted: -> "[Encrypted]"
3) Open encrypted ext4 file system: -> "[Encrypted] ext4"
This simplifies the code in TreeView_Detail::create_row() which sets the
file system type displayed in the main window. The same method will
then also be used when setting the operation description as each
operation is updated to handle encrypted file systems.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
The previous commit changed how the code behind the main window
retrieved the file system label for display. This is the relevant
changes in TreeView_Detail::create_row():
+ const Partition & filesystem_ptn = partition.get_filesystem_partition();
...
- Glib::ustring temp_filesystem_label = partition.get_filesystem_label();
+ Glib::ustring temp_filesystem_label = filesystem_ptn.get_filesystem_label();
treerow[treeview_detail_columns.label] = temp_filesystem_label;
In the case of an encrypted file system get_filesystem_label() is now
called on the Partition object directly rather than on the outer
Partition object containing the LUKS encryption.
The code behind the Information dialog always obtained and used the
Partition object directly containing the file system to call
get_filesystem_label() since read-only LUKS support was added.
Therefore the virtualised PartitionLUKS::get_filesystem_label() is no
longer needed, so remove it.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
There are multiple cases of code wanting to work with the Partition
object directly containing the file system, regardless of whether it is
within a PartitionLUKS object or not. The code had to do something
similar to this to access it:
const Partition * filesystem_ptn = &partition;
if ( partition.filesystem == FS_LUKS && partition.busy )
filesystem_ptn = &dynamic_cast<const PartitionLUKS *>( &partition )->get_encrypted();
...
// Access Partition object directly containing the file system
filesystem_ptn-> ...
Implement and use virtual accessor get_filesystem_partition() which
allows the code to be simplified like this:
const Partition & filesystem_ptn = partition.get_filesystem_partition();
...
// Access Partition object directly containing the file system
filesystem_ptn. ...
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
More recent versions of blkid don't report an ISO9660 file system on the
whole disk device if partitions can be reports for embedded partitions.
However when querying the whole disk device directly then the expected
ISO9660 file system is reported. For example on CentOS 7 with the
previous ISO images:
# wget http://git.kernel.org/cgit/utils/util-linux/util-linux.git/plain/tests/ts/isosize/sample.iso.gz
# dd if=/dev/zero bs=1M of=/dev/sdc
# zcat sample.iso.gz | dd of=/dev/sdc
# blkid -v
blkid from util-linux 2.23.2 (libblkid 2.23.0, 25-Apr-2013)
# blkid | fgrep /dev/sdc
/dev/sdc1: UUID="2013-01-04-22-05-45-00" LABEL="ARCH_201301" TYPE="iso9660" PTTYPE="dos"
# blkid /dev/sdc
/dev/sdc: UUID="2013-01-04-22-05-45-00" LABEL="ARCH_201301" TYPE="iso9660" PTTYPE="dos"
# wget http://cdimage.debian.org/debian-cd/8.6.0/amd64/iso-cd/debian-8.6.0-amd64-netinst.iso
# dd if=/dev/zero bs=1M of=/dev/sdc
# dd if=debian-8.6.0-amd64-netinst.iso bs=1M of=/dev/sdc
# blkid | fgrep /dev/sdc
/dev/sdc1: UUID="2016-09-17-14-23-48-00" LABEL="Debian 8.6.0 amd64 1" TYPE="iso9660" PTTYPE="dos"
/dev/sdc2: SEC_TYPE="msdos" UUID="17F3-1162" TYPE="vfat"
# blkid /dev/sdc
/dev/sdc: UUID="2016-09-17-14-23-48-00" LABEL="Debian 8.6.0 amd64 1" TYPE="iso9660" PTTYPE="dos"
This behavioural difference with blkid is probably as a result of newer
versions of udev informing the kernel of the partitions embedded within
the ISO9660 image, and not directly as a result of a change in blkid
itself. Older distributions don't have partition entries for the above
ISO images, but CentOS 7 (with udev 219) and later distributions do have
partition entries:
# fgrep sdc /proc/partitions
8 16 8388608 sdc
8 17 252928 sdc1
8 18 416 sdc2
Fix by ensuring that the blkid FS_Info cache has entries for whole disk
devices, even if each entry has all empty attributes because there is a
partition table and not a recognised file system.
Calling blkid on whole disk devices containing partition tables produces
output like this, with newer versions of blkid:
# blkid /dev/sda
/dev/sda: PTTYPE="dos"
# blkid /dev/sdb
/dev/sdb: PTTYPE="gpt"
This will be loaded into the FS_Info cache as a blank entry for the
device by run_blkid_load_cache(). There will be a path name but all the
other attributes will be blank because there are no TYPE, SEC_TYPE, UUID
or LABEL name value pairs. With older versions of blkid no output is
produced at all. In that case load_fs_info_cache_extra_for_path() will
create the same blank entry with just the path name defined.
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
Move code from GParted_Core::set_devices_thread() performing top level
population of each Device object during the scan of the drives into new
set_device_from_disk() method.
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
Requires blkid.
Note that FS_LUKS was also moved to more closely match the order in
include/Utils.h
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
FileSystem::resize() (and derived) is only ever called from a single
location and always with the fill_partition argument supplied [1].
Therefore providing a default for the argument is unnecessary. So
remove it.
[1] Since these two commits from 2006-06-17, before GParted 0.3.0, which
added resize_filesystem() and maximize_filesystem() methods to wrap
calls to p_filesystem->resize() into a single location:
08245cd08c
cleanups in the core and the fs'es (resize)
2d7fb5700b
more cleanups in the core and the fs'es (these changelogs are getting
It made the code look a little messy, is easily resolved in the build
system and made the dependencies more complicated than needed. Each
GParted header was tracked via multiple different names (different
numbers of "../include/" prefixes). For example just looking at how
DialogFeatures.o depends on Utils.h:
$ cd src
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/../include/Utils.h \
../include/../include/../include/../include/../include/../include/Utils.h \
../include/../include/../include/Utils.h \
After removing "../include/" from the GParted header #includes, just
need to add "-I../include" to the compile command via the AM_CPPFLAGS in
src/Makefile.am. Now the dependencies on GParted header files are
tracked under a single name (with a single "../include/" prefix). Now
DialogFeatures.o only depends on a single name to Utils.h:
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/Utils.h ../include/i18n.h \
Linux-swap is recreated as part of copy, resize and move operations and
the code was special cased to implement that by calling the linux-swap
specific resize method. However the displayed text always said "growing
file system" and then proceeded to recreate linux swap. Example
operation:
Copy /dev/sdb1 to /dev/sdb2
...
+ copy file system from /dev/sdb1 to /dev/sdb2
Partition copy action skipped because linux-swap file system does not contain data
+ grow file system to fill the partition
+ create new linux-swap file system
+ mkswap -L"" -U "77d939ef-54d6-427a-a2bf-a053da7eed4c" /dev/sdb2
Setting up swapspace version 1, size = 262140 KiB
LABEL=, UUID=77d939ef-54d6-427a-a2bf-a053da7eed4c
Fix by writing recreate_linux_swap_filesystem() method with better
messaging and use everywhere maximise_filesystem() was previously used
to recreate linux-swap. Also as this is a create step, erase the
partition first to prevent the possibility of any other file system
signatures being found afterwards. Now the operation steps are more
reflective of what is actually being performed.
Copy /dev/sdb1 to /dev/sdb2
...
+ copy file system from /dev/sdb1 to /dev/sdb2
Partition copy action skipped because linux-swap file system does not contain data
+ clear old file system signatures in /dev/sdb2
+ create new linux-swap file system
+ mkswap -L"" -U "77d939ef-54d6-427a-a2bf-a053da7eed4c" /dev/sdb2
Setting up swapspace version 1, size = 262140 KiB
LABEL=, UUID=77d939ef-54d6-427a-a2bf-a053da7eed4c
Bug 775932 - Refactor mostly applying of operations
resize_filesystem() was meeting two different needs:
1) when called with fill_partition = false it generated operation
details;
2) when called from maximize_filesystem() with fill_partition = true it
skipped generating any operation details;
then ran the switch statement to select the resize implementation. So
extract the common switch statement into new method
resize_filesystem_implement().
Then observe that the only time resize_filesystem() was called to grow
the file system was when re-creating linux-swap. Therefore change that
call to use maximize_filesystem() and rename to shrink_filesystem() and
modify the operation detail messages to match.
Bug 775932 - Refactor mostly applying of operations
Make the methods called below apply_operation_to_disk() follow a
standard naming convention:
* Contains "_partition"
Uses libparted to query or change the partition in the disk label
(partition table).
E.g.:
calibrate_partition()
create_partition()
delete_partition()
name_partition()
resize_move_partition()
set_partition_type()
* Contains "_filesystem"
Manipulates the file system within the partition, mostly using the
FileSystem and derived class methods.
E.g.:
create_filesystem()
remove_filesystem()
label_filesystem()
copy_filesystem()
erase_filesystem_signatures()
check_repair_filesystem()
resize_filesystem()
maximize_filesystem()
* Other
Compound method calling multiple partition and file system related
apply methods.
E.g.:
create()
format()
copy()
resize_move()
resize()
move()
Rename:
Delete() -> delete_partition()
change_uuid() -> change_filesystem_uuid()
Bug 775932 - Refactor mostly applying of operations
Until now an encryption mapping has been modelled as a Partition object
similar to a partition like this:
.encrypted.device_path = "/dev/sdb1"
.encrypted.path = "/dev/mapper/sdb1_crypt"
.encrypted.whole_device = false
.encrypted.sector_start = // start of the mapping in the partition
.encrypted.sector_end = // end of the mapping in the partition
However accessing device_path in the start to end sector range is not
equivalent to accessing the partition path as it doesn't provide access
to the encrypted data. Therefore existing functions which read and
write partition data (GParted file system copying and signature erasure)
via libparted using the device_path won't work and will in fact destroy
the encrypted data. This could be coded around with an extra case in
the device opening code, however it is not necessary.
An encrypted block special device /dev/mapper/CRYPTNAME looks just like
a whole disk device because it doesn't contain a partition and the file
system it contains starts at sector 0 and goes to the end. Therefore
model an encryption mapping in the same way a whole disk device is
modelled as a Partition object like this:
.encrypted.device_path = "/dev/mapper/sdb1_crypt"
.encrypted.path = "/dev/mapper/sdb1_crypt"
.encrypted.whole_device = true
.encrypted.sector_start = 0
.encrypted.sector_end = // size of the encryption mapping - 1
Now GParted file system copy and erasure will just work without any
change. Just need to additionally store the LUKS header size, which was
previous stored in the sector_start, for use in the
get_sectors_{used,unused,unallocated}() calculations.
Bug 775932 - Refactor mostly applying of operations
Split out the switch statement selecting the copy implementation and
associated copy file system operation detail message into a separate
copy_filesystem() method, matching how a number of other operations are
coded. This is why the previous copy_filesystem() methods needed
renaming.
Re-write the remaining copy() into if-operation-fails-return-false style
to simplify it. Re-write final complicated conditional check repair and
maximise file system into separate positive if conditions for swap and
larger partition to make it understandable.
The min_size parameter to copy() was queried from the partition_src
parameter also passed to copy(). Drop the parameter and query inside
copy() instead.
Bug 775932 - Refactor mostly applying of operations
Rename GParted_Core methods:
copy_filesystem(4 params) -> copy_filesystem_internal()
copy_filesystem(5 params) -> copy_filesystem_internal()
copy_filesystem(10 params) -> copy_blocks()
See the following commit for the desire to do this.
Bug 775932 - Refactor mostly applying of operations
Files were named Block_Copy and the class was named block_copy. Change
to the primary naming convention of CamelCase class name and matching
file names.
Also make CopyBlocks::copy_block() a private method as it is only used
internally within the class.
Bug 775932 - Refactor mostly applying of operations
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
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
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
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.
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
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) 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()
The previous commit (Fix crash reading NTFS usage when there is no
/dev/PTN entry) identified that the FileSystem member variable "index"
is too small on 64-bit machines. Also this member variable stores no
FileSystem class information and was being used as a local variable.
Replace with local variables of the of the correct type, wide enough to
store the npos not found value.
Bug 764658 - GParted crashes when reading NTFS usage when there is no
/dev/PTN entry
As part of the internal block copy operation 5 initial ranges of blocks
are copied using different block sizes to determine the fastest. Then
the remainder is copied using the fastest block size. Each of these
copies reports progress independently, so during the benchmarking phase
the progress bar flashes 5 times as it goes from 0 to 100% in a fraction
of a second, before showing the progress of the remainder.
This looks bad, so report a single progress bar for all the ranges of
blocks copied in a single copy operation.
Already have variables done and length which track progress within each
copied range; and total_done which records amount copied in previous
ranges. Just add total_length to allow overall progress to be reported.
Bug 762367 - Use a single progress bar for the whole of the internal
copy operation
Copying of ntfs is performed using ntfsclone, which writes progress
indication to standard output like this:
# ntfsclone -f /dev/sdb2 /dev/sdb1 2> /dev/null
NTFS volume version: 3.1
Cluster size : 4096 bytes
Current volume size: 21474832384 bytes (21475 MB)
Current device size: 21474836480 bytes (21475 MB)
Scanning volume ...
100.00 percent completed
Accounting clusters ...
Space in use : 1832 MB (8.5%)
Cloning NTFS ...
100.00 percent completed
Syncing ...
Add ntfsclone progress tracker for ntfsclone command. Deliberately
doesn't stop the progress bar. See comment in ntfs::clone_progress()
for the explanation.
Bug 762366 - Add progress bar to NTFS file system specific copy method
The timed progress tracking callback for execution of xfs copy follows
this pattern:
sigc::connection c;
...
c = Glib::signal_timeout().connect( ... sigc::mem_fun( *this, &xfs::copy_progress ) ..., 500 /*ms*/ );
... execute_command( ... );
c.disconnect();
As with output progress tracking callbacks for ext2/3/4 and ntfs file
system specific commands, pass the callback slot and a flag into
execute_command() and connect the timed callback inside. This
simplified the pattern to:
... execute_command( ...|EXEC_PROGRESS_TIMED,
static_cast<TimedSlot>( sigc::mem_fun( *this, &xfs::copy_progress ) ) );
NOTE:
The type of sigc::mem_fun() doesn't allow the compiler to choose between
the two overloaded variants of execute_command() with the fourth
parameter of either (full types without typedefs of StreamSlot and
TimedSlot respectively):
sigc::slot<void, OperationDetail *> stream_progress_slot
sigc::slot<bool, OperationDetail *> timed_progress_slot
Therefore have to cast the result of all callback slots to the relevant
type. Hence:
static_cast<StreamSlot>( sigc::mem_fun( *this, &{CLASS}::{NAME}_progress ) )
static_cast<TimedSlot>( sigc::mem_fun( *this, &xfs::copy_progress ) )
References:
* [sigc] Functor not resolving between overloaded methods with
different slot types
https://mail.gnome.org/archives/libsigc-list/2016-February/msg00000.html
* Bug 306705 - Can't overload methods based on different slot<>
parameters.
https://bugzilla.gnome.org/show_bug.cgi?id=306705
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
For the relevant stream from a file system specific command being
tracked, there were 2 callbacks attached: update_command_output() and
update_command_progress(). When called, update_command_progress() just
emitted signal_progress to call the file system specific progress
tracker callback. Like this:
signal_update.emit() -> update_command_output()
-> update_command_progress()
signal_progress.emit() -> {CLASS}::{NAME}_progress()
Instead just connect the file system specific progress tracker callback
directly to signal_update and bypass the unnecessary
update_command_progress() method and the signal_progress signal. Like
this:
signal_update.emit() -> update_command_output()
-> {CLASS}::{NAME}_progress()
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
All the output progress tracking callbacks for execution of ext2/3/4 and
ntfs file system specific commands followed this pattern:
sigc::connection c = signal_progress.connect( sigc::mem_fun( *this, &ext2::..._progress ) );
bool success = ! execute_command( ... );
c.disconnect();
return success;
Instead, pass the callback slot and a flag into execute_command() and
connect the callback inside. This simplifies the pattern to:
return ! execute_command( ...|EXEC_PROGRESS_STDOUT,
sigc::mem_fun( *this, &ext2::..._progress ) );
Note that as the progress tracking callbacks are only registered against
updates to the relevant stream from the tracked commands they won't be
called when the other stream is updated any more.
Also note that signal_progress is a member of the FileSystem class and
derived objects so lives as long as GParted is running, therefore the
progress tracking callbacks need explicitly disconnecting at the end of
execute_command(). However signal_update is a member of the PipeCapture
class of which the output and error local variables in execute_command()
are types. Therefore there is no need to explicitly disconnect the
signal_update callbacks as they will be destructed along with the
callback slots when they go out of scope at the end of the
execute_command() method.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Remove unused members: fraction and progress_text from the
OperationDetail class now that the ProgressBar class has superseded
their use. This also allows removal of timer_global member from the
copy_blocks class. Timer_global was only used to track the elapsed time
copying blocks and allow the remaining time to be estimated and written
into progress_text. The ProgressBar class also does this itself
internally.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Most of the file system specific command progress trackers followed this
pattern:
void {CLASS}::{NAME}_progress( OperationDetail *operationdetail )
{
ProgressBar & progressbar = operationdetail->get_progressbar();
// parse output for progress and target values
if ( // have progress and target values )
{
if ( ! progressbar.running() )
progressbar.start( target );
progressbar.update( progress );
operationdetail->signal_update( *operationdetail );
}
else if ( // found progress finished )
{
if ( progressbar.running() )
progressbar.stop();
operationdetail->signal_update( *operationdetail );
}
}
That is a lot of repetition handling progress bar updates and
OperationDetail object update signalling. Remove the need for direct
access to the single ProgressBar object and provide these two
OperationDetail methods instead:
// Start and update in one
run_progressbar( progress, target, optional text_mode );
stop_progressbar();
Now the file system specific command progress trackers can become:
void {CLASS}::{NAME}_progress( OperationDetail *operationdetail )
{
// parse output for progress and target values
if ( // have progress and target values )
{
operationdetail->run_progressbar( progress, target );
}
else if ( // found progress finished )
{
operationdetail->stop_progressbar();
}
}
Make ProgressBar::get_progressbar() a private method to enforce use of
the new way to access the progress bar via the run_progress() and
stop_progressbar() methods. Then make the Dialog_Progress a friend
class to OperationDetail so that the Apply pending operations dialog can
still access the single ProgressBar object for its querying needs.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Using e2image to copy a file system looks like this. (Intermediate
progress lines which are constantly overwritten are indicated with ">").
# e2image -ra -p /dev/sdb4 /dev/sdb5
e2image 1.42.13 (17-May-2015)
Scanning inodes...
> Copying 0 / 276510 blocks (0%)
> Copying 8845 / 276510 blocks (3%)
> Copying 48433 / 276510 blocks (18%)
> Copying 77135 / 276510 blocks (28%)
> Copying 111311 / 276510 blocks (40%)
> Copying 137039 / 276510 blocks (50%)
> Copying 166189 / 276510 blocks (60%) 00:00:03 remaining at 108.20 MB/s
> Copying 190285 / 276510 blocks (69%) 00:00:03 remaining at 106.19 MB/s
> Copying 209675 / 276510 blocks (76%) 00:00:02 remaining at 102.38 MB/s
> Copying 238219 / 276510 blocks (86%) 00:00:01 remaining at 103.39 MB/s
> Copying 256692 / 276510 blocks (93%) 00:00:00 remaining at 100.27 MB/s
Copied 276510 / 276510 blocks (100%) in 00:00:10 at 108.01 MB/s
Note that the copying figures are reported in file system block size
units and the progress information is written to stderr, hence needing
these two previous commits:
Record file system block size where known (#760709)
Call any FS specific progress trackers for stderr updates too (#760709)
Add progress tracking function for e2image command. Also tracks when
the text progress indicator has passed in the output so that the
progress bar can be stopped as well as started when needed.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Record the file system block size in the Partition object. Only
implemented for file systems when set_used_sectors() method has already
parsed the value or can easily parse the value from the existing
executed command(s).
Needed for ext2/3/4 copies and moves performed using e2image so that
they can be tracked in bytes by the ProgressBar class as e2image reports
progress in file system block size units.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
XFS uses a file system specific method to copy the partition using
"xfsdump | xfsrestore". Monitor progress by periodically querying the
destination file system usage and comparing to the source file system
usage. Use 0.5 seconds as the polling interval to match that used by
the internal block copying algorithm.
NOTE:
The number of used blocks in the source and destination file system will
be very close but may not match exactly. I have seen an XFS copy finish
with the following progress text:
1.54 GiB of 1.50 GiB copied (-00:00:02 remaining)
Allow the progress bar to overrun like this as it is informing the user
that it actually copied a little more data and took a little longer than
expected. Needs these two previous commits to correctly round and
format the negative time remaining:
Fix rounding of negative numbers (#760709)
Fix formatting of negative time values (#760709)
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Adapt the ext2 resize progress tracker to the new ProgressBar class.
Also update the progress function to track when text progress bars have
completely passed in the output so that the progress bar can be stopped
as well as started when needed.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
1) Multiple progress bars
The OperationDetail class contains member fraction which is used to feed
data to the current operation progress bar shown in the Applying pending
operations dialog. Dialog_Progress::on_signal_update() gets called for
every updated OperationDetail object and depending on whether fraction
is > 0.0 or not, switches between showing a growing or pulsing progress
bar. This leads to the conclusion that every OperationDetail object
currently being updated is effectively driving the single on screen
progress bar with different data.
The Copy_Blocks code is careful to update text and faction in a single
OperationDetail object and everything is good. The on screen progress
bar is switched into growing mode and then grows to 100%.
Since external command output is updated in real time [1] there are two
OperationDetail objects, one for stdout and one for stderr, which are
updated whenever data is read from the relevant stream. Also now that
progress is interpreted from some external command output [2][3][4] a
separate OperationDetail object is getting updated with the progress
fraction. (Actually the grandparent OperationDetail of the ones
receiving stdout and stderr updates as used by the file system specific
*_progress() methods). In the normal case of an external command
which is reporting it's progress two OperationDetails are constantly
being updated together, the OperationDetail object tracking stdout and
it's grandparent receiving progress fraction updates. This causes the
the code in Dialog_Progress::on_signal_update() to constantly switch
between growing and pulsing progress bar mode. The only reason this
doesn't flash the progress bar is because the stdout OperationDetail
object is updated first and before the 100 ms timeout fires to pulse the
bar, it's grandparent is updated with the new fraction to keep growing
the bar instead.
2) Common code
The Copy_Blocks code currently tracks the progress of blocks copied
against target amount, which it has to do anyway. That information is
then used to generate the text and fraction to update into the
OperationDetail object and drive the on screen progress bar. This same
level of tracking is wanted for the XFS and ext2/3/4 file system
specific copy methods.
Conclusion and solution
Having multiple sources of progress bar data is a problem and makes it
clear that there must be only one source of progress data. Also some
code can be shared for tracking the amount of blocks copied and
generating the display.
Therefore have a single ProgressBar object which is used everywhere.
This commit
It just creates a single ProgressBar object which is available via all
OperationDetail objects and Copy_Blocks is updated accordingly. Note
that the ProgressBar still contains debugging and that the GUI progress
bar of the current operation is still driven via the fraction member in
any OperationDetail object.
Referenced commits:
[1] 52a2a9b00a
Reduce threading (#685740)
[2] ae434579e1
Display progress for e2fsck (#467925)
[3] baea186138
Display progress for mke2fs (#467925)
[4] 57b028bb8e
Display progress during resize (#467925)
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Write a generic progress bar class. Has the following features:
* Has separate progress and target numbers, rather than a single
completion fraction, to enable the the next feature.
* Optionally generates text reporting the amount of data copied using
the progress and target numbers like this:
"1.00 MiB of 16.00 MiB copied"
* After running for 5 seconds, also add estimated remaining time.
(Waits to allow the data copying rate to settle down a little before
estimating the remaining time). Looks like this:
"1.00 MiB of 16.00 MiB copied (00:01:59) remaining)"
The ProgressBar class is not driving the visual progress bar yet. It
has just been added into the internal block copy algorithm and generates
debug messages showing the progress bar is operating correctly.
Debugging looks like this:
DEBUG: ProgressBar::start(target=2.0636e+09, text_mode=PROGRESSBAR_TEXT_COPY_BYTES)
DEBUG: ProgressBar::update(progress=1.30023e+08) m_fraction=0.0630081 m_text="124.00 MiB of 1.92 GiB copied"
DEBUG: ProgressBar::update(progress=2.67387e+08) m_fraction=0.129573 m_text="255.00 MiB of 1.92 GiB copied"
DEBUG: ProgressBar::update(progress=4.0475e+08) m_fraction=0.196138 m_text="386.00 MiB of 1.92 GiB copied"
...
DEBUG: ProgressBar::update(progress=1.13351e+09) m_fraction=0.549289 m_text="1.06 GiB of 1.92 GiB copied (00:00:04 remaining)"
DEBUG: ProgressBar::update(progress=1.26249e+09) m_fraction=0.611789 m_text="1.18 GiB of 1.92 GiB copied (00:00:04 remaining)"
DEBUG: ProgressBar::update(progress=1.39041e+09) m_fraction=0.67378 m_text="1.29 GiB of 1.92 GiB copied (00:00:03 remaining)"
...
DEBUG: ProgressBar::update(progress=1.97552e+09) m_fraction=0.957317 m_text="1.84 GiB of 1.92 GiB copied (00:00:00 remaining)"
DEBUG: ProgressBar::update(progress=2.0636e+09) m_fraction=1 m_text="1.92 GiB of 1.92 GiB copied"
DEBUG: ProgressBar::stop()
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
At the moment any messages for an encrypted file system aren't shown,
only messages from the outer PartitionLUKS object are shown. Also in
Win_GParted::activate_paste() the selected Partition object, possibly
a derived PartitionLUKS, is cloned and the messages cleared.
Therefore a set of accessor methods must be provided to query and modify
partition messages. Messages will be stored in the Partition object to
which they are added and retrieved from all. So in the case of a
derived PartitionLUKS they will be retrieved from the messages vector of
the PartitionLUKS object itself and the messages vector for the
encrypted file system it contains.
To replace code like this in GParted_Core:
partition_temp->messages = messages;
We might naturally provide a set_messages() method which assigns the
messages vector and is used like this:
partition_temp->set_messages( messages );
However on a PartitionLUKS object what should set_messages() do? By the
name it will replace any existing messages in the PartitionLUKS object
itself, but what should happen to the messages for the contained
encrypted Partition object? Should they be cleared or left alone?
Rather than implement set_messages() with unclear semantics implement
append_messages(), which in the PartitionLUKS object case will clearly
leave any messages for the contained encrypted Partition object alone.
Append_messages() is then used to add messages as the Partition or
PartitionLUKS objects when populating the data in GParted_Core.
Bug 760080 - Implement read-only LUKS support
For LUKS formatted partitions add an encryption section into the
Information dialog and display the type of encryption, path, UUID and
status of the encryption.
The file system section continues to display appropriate file system
details, including the partition graphic with the file system specific
border colour and correct usage. The details will either be of a plain
file system, an encrypted file system, or nothing when there is no open
dm-crypt mapping, leaving the encrypted file system inaccessible.
Should there be LUKS encryption directly within LUKS encryption then the
details of the inner encryption will be displayed in the file system
section. However this configuration will not be further supported by
GParted.
Bug 760080 - Implement read-only LUKS support
There is already the set of methods in the Partition class to report the
file system usage. Virtualise them and provide PartitionLUKS specific
implementations to calculate the usage of a file system wrapped in LUKS
encryption.
See the ascii art and comment in PartitionLUKS.cc for the details of
those calculations.
Bug 760080 - Implement read-only LUKS support
LUKS headers don't provide any concept of label. Also there is already
the method Partition::get_filesystem_label() for getting the *file
system* label, so virtualise it and provide an appropriate
implementation to get the label of an encrypted file system represented
within a derived PartitionLUKS object. This causes the label to be
displayed correctly in the main window.
It also happens to display the encrypted file system label in the
Information dialog for a LUKS formatted partition. However the whole
Information dialog will be addressed differently in a following commit.
Bug 760080 - Implement read-only LUKS support
In the File System column in the GUI, when there is an open dm-crypt
mapping, display the colour square for the encrypted file system within
and the text as "[Encrypted] FSTYPE". For closed mappings nothing can
be known about the encrypted file system within so continue to display a
purple square and the text "[Encrypted]".
Looks like:
Partition | File System
...
/dev/sdb3 # ext4
v /dev/sdb4 * # extended
/dev/sdb5 # [Encrypted]
/dev/sdb6 * # [Encrypted] unknown
/dev/sdb7 * # [Encrypted] ext4
Bug 760080 - Implement read-only LUKS support
Partition object represents a region of a disk and the file system
within. GParted always displays the colour base of the type of the file
system. Therefore remove the color member and always look it up from
the type of the file system as needed.
This makes one less member that will need virtual accessor methods with
different handling in the derived PartitionLUKS class.
Bug 760080 - Implement read-only LUKS support
When there exists an open dm-crypt mapping, populate the encrypted
Partition object representing the encrypted file system.
Bug 760080 - Implement read-only LUKS support
Absolute minimum implementation of a PartitionLUKS class which can be
constructed, polymorphically copied and destroyed. Contains an
"encrypted" member of type Partition to represent the encrypted file
system within the LUKS format.
Create PartitionLUKS objects instead of base Partition objects when a
LUKS formatted partition is found. Only the base Partition object
member values have been populated, and the "encrypted" member remains
blank at this point.
Bug 760080 - Implement read-only LUKS support
This is the equivalent change as made to set_mountpoints() in an earlier
commit. Change GParted_Core::set_used_sectors() from being called with
a vector of partitions and processing them all to being called per
partition. This is in preparation for calling set_used_sectors() on a
single Partition object inside a PartitionLUKS object.
Bug 760080 - Implement read-only LUKS support
Previously GParted_Core::set_mountpoints() was called with a vector of
partitions and processed them all. Now make set_mountpoints() process a
single partition and push the calls to it down one level from
set_devices_thread() into set_device_partitions() and
set_device_one_partition(). This is in preparation for having an
encrypted file system represented as a Partition object inside a
PartitionLUKS object and needing to call set_mountpoints() for the inner
single Partition object.
Bug 760080 - Implement read-only LUKS support
Only load the LUKS_Info cache of active dm-crypt mappings when the first
LUKS partition is encountered. Not needed from a performance point of
view as the longest that I have ever seen "dmsetup table --target crypt"
take to run is 0.05 seconds. Just means that the dmsetup command is
only run when there are LUKS partitions and the information is needed.
Bug 760080 - Implement read-only LUKS support
Populate the used, unused and unallocated figures in the Partition
object for a LUKS formatted partition. See comment in
luks::set_used_sectors() for the rational of what is used, unused and
unallocated.
As that rational mentions, a LUKS header does not store the size of the
encrypted data and is assumed to extend to the end of the partition by
the tools which start the mapping.
An underlying block device of 128 MiB (131072 KiB).
# sfdisk -s /dev/sde
131072
An active LUKS mapping at offset 2 MiB (4096 512-byte sectors) and
length 126 MiB (129024 KiB, 258048 512-byte sectors).
# sfdisk -s /dev/mapper/sde_crypt
129024
# cryptsetup status sde_crypt
/dev/mapper/sde_crypt is active.
type: LUKS1
cipher: aes-cbc-essiv:sha256
keysize: 256 bits
device: /dev/sde
offset: 4096 sectors
size: 258048 sectors
mode: read/write
No size/length reported when dumping the LUKS header, just (payload)
offset.
# cryptsetup luksDump /dev/sde
LUKS header information for /dev/sde
Version: 1
Cipher name: aes
Cipher mode: cbc-essiv:sha256
Hash spec: sha1
Payload offset: 4096
MK bits: 256
MK digest: 7f fb ba 40 7e ba e4 3b 2f c6 d0 93 7b f7 05 49 7b 72 d4 ad
MK salt: 4a 5b 54 f9 7b 67 af 6e ef 16 31 0a fe d9 7e 5f
c3 66 dc 8a ed e0 07 f4 45 c3 7c 1a 8d 7d ac f4
MK iterations: 37750
UUID: 0a337705-434a-4994-a842-5b4351cb3778
...
Shrink the LUKS mapping to 64 MiB (65536 KiB, 131072 512-byte sectors).
# cryptsetup resize --size 131072 sde_crypt
# sfdisk -s /dev/mapper/sde_crypt
65536
# cryptsetup status sde_crypt
/dev/mapper/sde_crypt is active.
type: LUKS1
cipher: aes-cbc-essiv:sha256
keysize: 256 bits
device: /dev/sde
offset: 4096 sectors
size: 131072 sectors
mode: read/write
Stop and start the LUKS mapping.
# cryptsetup luksClose sde_crypt
# cryptsetup luksOpen /dev/sde sde_crypt
The size of the LUKS mapping is back to 126 MiB (129024 KiB, 258048
512-byte sectors), extending to the end of the partition.
# sfdisk -s /dev/mapper/sde_crypt
129024
# cryptsetup status sde_crypt
/dev/mapper/sde_crypt is active.
type: LUKS1
cipher: aes-cbc-essiv:sha256
keysize: 256 bits
device: /dev/sde
offset: 4096 sectors
size: 258048 sectors
mode: read/write
Bug 760080 - Implement read-only LUKS support
Also load the starting offset and length of the active dm-crypt mapping
into the LUKS_Info module from the dmsetup output. This provides the
location and size of the encrypted data within the underlying block
device.
Note that dmsetup reports in units of 512 bytes sectors [1], the GParted
LUKS_Info module uses bytes and GParted Partition objects work in device
sector size units. However the actual sector size of a dm-crypt mapping
[2] is the same as that of the underlying block device [3].
# modprobe scsi_debug dev_size_mb=128 sector_size=4096
# fgrep scsi_debug /sys/block/*/device/model
/sys/block/sdd/device/model:scsi_debug
# parted /dev/sde print
Error: /dev/sde: unrecognised disk label
Model: Linux scsi_debug (scsi)
Disk /dev/sde: 134MB
[3] Sector size (logical/physical): 4096B/4096B
Partition Table: unknown
# cryptsetup luksFormat /dev/sde
# cryptsetup luksOpen /dev/sde sde_crypt
# parted /dev/mapper/sde_crypt print
Error: /dev/mapper/sde_crypt: unrecognised disk label
Model: Linux device-mapper (crypt) (dm)
Disk /dev/mapper/sde_crypt: 132MB
[2] Sector size (logical/physical): 4096B/4096B
Partition Table: unknown
# cryptsetup status sde_crypt
/dev/mapper/sde_crypt is active.
type: LUKS1
cipher: aes-cbc-essiv:sha256
keysize: 256 bits
device: /dev/sde
offset: 4096 sectors
size: 258048 sectors
mode: read/write
# dmsetup table --target crypt
...
sde_crypt: 0 258048 crypt aes-cbc-essiv:sha256 0000000000000000000000000000000000000000000000000000000000000000 0 8:64 4096
[1] Both cryptsetup and dmsetup report the offset as 4096 and the size/
length as 258048. 128 MiB / (4096+258048) = 512 byte units, even on a
4096 byte sector size device.
Update debugging of LUKS to this:
# ./gpartedbin
======================
libparted : 2.4
======================
DEBUG: /dev/sdb5: LUKS closed
DEBUG: /dev/sdb6: LUKS open mapping /dev/mapper/sdb6_crypt, offset=2097152, length=534773760
/dev/sde: unrecognised disk label
DEBUG: /dev/sde: LUKS open mapping /dev/mapper/sde_crypt, offset=2097152, length=132120576
Bug 760080 - Implement read-only LUKS support
Provide a minimal implementation of a luks file system class which only
does busy detection.
NOTE:
For now, read-only LUKS support, a LUKS partition will be busy when a
dm-crypt mapping exists. Later when read-write LUKS support is added
GParted will need to look at the busy status of the encrypted file
system within the open LUKS partition and map LUKS partition busy status
to encryption being open or closed.
Bug 760080 - Implement read-only LUKS support
Load basic details of active Device-mapper encryption mappings from the
kernel. Use dmsetup active targets.
# cryptsetup luksFormat /dev/sdb5
# cryptsetup luksFormat /dev/sdb6
# cryptsetup luksOpen /dev/sdb6 sdb6_crypt
# ls -l /dev/mapper/sdb6_crypt /dev/dm-0
lrwxrwxrwx. 1 root root 7 Nov 15 09:03 /dev/mapper/sdb6_crypt -> ../dm-0
brw-rw----. 1 root disk 253, 0 Nov 15 09:03 /dev/dm-0
# ls -l /dev/sdb6
brw-rw----. 1 root disk 8, 22 Nov 15 09:02 /dev/sdb6
# dmsetup table --target crypt
sdb6_crypt: 0 1044480 crypt aes-cbc-essiv:sha256 0000000000000000000000000000000000000000000000000000000000000000 0 8:22 4096
So far just load the mapping name and underlying block device reference
(path or major, minor pair).
Note that all supported kernels appear to report the underlying block
device as major, minor pair in the dmsetup output. Underlying block
device paths are added to the cache when found during a search to avoid
stat(2) call on subsequent searches for the same path.
Prints debugging to show results, like this:
# ./gpartedbin
======================
libparted : 2.4
======================
DEBUG: /dev/sdb5: LUKS closed
DEBUG: /dev/sdb6: LUKS open mapping /dev/mapper/sdb6_crypt
Bug 760080 - Implement read-only LUKS support
Renamed from DEV_MAP_PATH to DEV_MAPPER_PATH. Moved so that the
constant is logically intended for use outside of the DMRaid class.
Also specifically make the string constant have external linkage, rather
than the default internal (static) linkage for constants, so that there
is only one copy of the variable in the program, rather than one copy in
each compilation unit which included DMRaid.h. Namely DMRaid.cc and
GParted_Core.cc.
References:
[1] Proper way to do const std::string in a header file?
http://stackoverflow.com/questions/10201880/proper-way-to-do-const-stdstring-in-a-header-file
[2] What is external linkage and internal linkage in C++
http://stackoverflow.com/questions/1358400/what-is-external-linkage-and-internal-linkage-in-c/1358796#1358796
Bug 760080 - Implement read-only LUKS support
History:
1) The constructor was added by commit:
6d8b169e73
2006-03-14 21:37:47
changed the way devices and partitions store their devicepaths. Instead of
2) Removed from most of the file system specific ::Copy() methods by
commit:
ad9f2126e7
2006-03-19 15:30:20
fixed issues with copying (see also #335004) cleanups + added FIXME added
3) Removed from GParted_Core::copy() method by commit:
7bb7e8a84f
2006-05-23 22:17:34
Use ped_device_read and ped_device_write instead of 'dd' to copy
4) Finally removed from the last place in xfs::Copy() method by commit:
e414b71b73
2012-01-11 19:49:13
Update xfs resize and copy to use new helper functions
The Partition(path) constructor is no longer used. Remove.
When a base class destructor is virtual, derived class destructors are
also virtual [1] even if they don't have the virtual qualifier.
As the Operation destructor is virtual, derived Operation* classes
destructors are virtual too. Add virtual qualifier just to reflect what
the C++ language mandates the compiler implement.
[1] Derived class with non-virtual destructor
http://stackoverflow.com/questions/7403883/derived-class-with-non-virtual-destructor
The sector_size parameter is unnecessary as the value can be retrieved
from the sector size of the selected Partition object on which the
create new, copy & paste or resize/move operation is being performed.
For the create new and resize/move operations it is trivial as the
existing unallocated or in use Partition object on which the operation
is being perform already contains the correct sector size. For the copy
& paste operation, which can copy across disk devices of different
sector sizes, we merely have to use the sector size of the existing
selected (destination) Partition object rather than copied (source)
Partition object. Hence these relevant lines in the new code:
Dialog_Partition_Copy::set_data(selected_partition, copied_partition)
new_partition = copied_partition.clone();
...
new_partition->sector_size = selected_partition.sector_size;
The struct FS constructor initialised every member *except* filesystem
and busy. Then in *most* cases after declaring struct FS, assignments
followed like this:
FS fs;
fs.filesystem = FS_BTRFS;
fs.busy = FS::GPARTED;
But member busy wasn't always initialised.
Add initialisation of members filesystem and busy to the struct FS
constructor. Specify optional parameter to the constructor to set the
filesystem member, or when left off filesystem is initialised to
FS_UNKNOWN.
Final step for full polymorphic handling of Partition objects is to
implement a virtual copy constructor. C++ doesn't directly support
virtual copy constructors, so instead use the virtual copy constructor
idiom [1]. (Just a virtual method called clone() which is implemented
in every polymorphic class and creates a clone of the current object and
returns a pointer to it).
Then replace all calls to the (monomorphic) Partition object copy
constructor throughout the code, except in the clone() implementation
itself, with calls to the new virtual clone() method "virtual copy
constructor".
Also have to make the Partition destructor virtual too [2][3] so that
the derived class destructor is called when deleting using a base class
pointer. C++ supports this directly.
[1] Wikibooks: More C++ Idioms / Virtual Constructor
https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Virtual_Constructor
[2] When to use virtual destructors?
http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
[3] Virtuality
Guideline #4: A base class destructor should be either public and
virtual, or protected and nonvirtual
http://www.gotw.ca/publications/mill18.htm
Bug 759726 - Implement Partition object polymorphism
SQUASH: When first using pointers to Partition and calling delete
Copy assignment of Partition objects is now only performed in a few
places in the Operation and OperationResizeMove classes when updating
the displayed PartitionVector. (From Refresh_Visual() when each
operation is visually applied to the display_partitions vector; the
new_partition from the operation is copy assigned over the top of the
relevant existing partition in the display_partitions vector).
In general polymorphic copy assignment is complicated [1], and is now
unnecessary given the above limited use. All that is needed is a way to
polymorphically replace one Partition object with another in a
PartitionVector.
First, prevent further use of Partition object copy assignment by
providing a private declaration and no implementation, so the compiler
enforces this. Second implement and use PartitionVector method
replace_at() which replaces a pointer to one Partition object with
another at the specified index in the PartitionVector.
[1] The Assignment Operator Revisited
[Section:] Virtual assignment
http://icu-project.org/docs/papers/cpp_report/the_assignment_operator_revisited.html
Bug 759726 - Implement Partition object polymorphism
Now use a pointer to the Partition object in Dialog_Base_Partition class
and derived classes, Dialog_Partition_{Copy,New,Resize_Move}. This is
equivalent to how the Partition objects are managed in the Operation and
derived classes.
The Partition object is allocated and copy constructed in each derived
classes' set_data() method, called from each constructor and deallocated
in the destructors. Considering the remaining Big 3, these classes are
never copy constructed or copy assigned so provide private definitions
and no implementations so the compiler enforces this.
Bug 759726 - Implement Partition object polymorphism
Change Win_GParted::copied_partition from Partition object which is
copied by value into a pointer to a Partition object object which is
allocated, copy constructed and deleted. Required as part of the
polymorphic implementation of Partitions.
As before when managing the lifetime of pointers to objects in a class
the Big 3 of destructor, copy constructor and copy assignment operator
need to be considered. A destructor is added to finally delete
copied_partition. A single Win_GParted object is only ever created and
destroyed in main(). The class is never copy constructed or copy
assigned. Make the compiler enforce this with private declarations and
no implementations.
Bug 759726 - Implement Partition object polymorphism
Operation classes now internally use pointers to Partition objects and
take on management of their lifetimes. As before, with the
PartitionVector class, when storing pointers in a class the Big 3 of
destructor, copy constructor and copy assignment operator also have to
be considered.
First, all the Partition objects are allocated in the derived Operation*
class parameterised constructors and freed in the associated
destructors. However the Operation classes are never copy constructed
or copy assigned; they are only ever created and destroyed. Only
pointers to the derived Operations are copied into the vector of pending
operations. Therefore the copy construtor and copy assignment operator
aren't needed. To enforce this provide inaccessible private
declarations without any implementation so that the compiler will
enforce this [1][2].
This example code fragment:
1 OperationCheck o1( device, partition );
2 OperationCheck o2 = o1;
3 o2 = o1;
Does these OperationCheck calls:
1 Implemented parameterised construtor,
2 Disallowed copy constructor,
3 Disallowed copy assignment
Trying to compile the above code would fail with errors like these:
../include/OperationCheck.h: In member function 'void GParted::Win_GParted::activate_check()':
../include/OperationCheck.h:36:2: error: 'GParted::OperationCheck::OperationCheck(const GParted::OperationCheck&)' is private
OperationCheck( const OperationCheck & src ); // Not implemented copy constructor
^
test.cc:2:21: error: within this context
OperationCheck o2 = o1;
^
../include/OperationCheck.h:37:19: error: 'GParted::OperationCheck& GParted::OperationCheck::operator=(const GParted::OperationCheck&)' is private
OperationCheck & operator=( const OperationCheck & rhs ); // Not implemented copy assignment operator
^
test.cc:3:4: error: within this context
o2 = o1;
^
[1] Disable copy constructor
http://stackoverflow.com/questions/6077143/disable-copy-constructor
[2] Disable compiler-generated copy-assignment operator [duplicate]
http://stackoverflow.com/questions/7823845/disable-compiler-generated-copy-assignment-operator
Bug 759726 - Implement Partition object polymorphism
The Operation classes contain partition objects which are copied by
value. Need to replace these with pointers to Partition objects instead
and manage their lifetimes so that they can be used polymorphically.
First step is to protect the partition members partition_new,
partition_original, and for OperationCopy class only, partition_copied
within the Operation classes and provide accessor methods.
get_partition_new() and get_partition_original() accessors are
implemented in the Operation base class so all derived classes get an
implementation. get_partition_new() is also virtual so that
OperationCheck and OperationDelete can override the implementation and
assert that they don't use partition_new. get_partition_copied() is
provided for the OperationCopy class only so can only be accessed via an
OperationCopy type variable.
Bug 759726 - Implement Partition object polymorphism
Remove PartitionVector push_back() and insert() methods which copy
construct Partitions objects into the vector. All the code has already
been changed to dynamically allocate Partition objects and use the
adoption variants of these methods named, push_back_adopt() and
insert_adopt(). Remove the no longer used methods.
Bug 759726 - Implement Partition object polymorphism
GParted_Core and Operation classes both have an insert_unallocated()
method which do the same thing with very nearly identical code. Both
methods insert unallocated partitions into the vector of partitions
within the specified range of sectors to fill in any gaps larger than
1 MiB. The only difference was how the two methods got the device path;
the GParted_Core class method got it via a parameter and the Operation
class method got it by calling get_path() on its device member variable.
The GParted_Core insert_unallocated() method gets called during device
scanning and the Operation one gets called when constructing the visual
for a pending operation.
Consolidate down to a single insert_unallocated() implementation by
making the Operation class method call the GParted_Core class method.
Make the GParted_Core class method static and public so that it can be
called using the class name from outside the class.
Bug 759726 - Implement Partition object polymorphism
The current code uses push_back() and insert() to copy Partition objects
into the vector of pointers. This has a few issues:
1) Unnecessary copying of Partition objects;
2) Hides the nature of the PartitionVector class as a manager of
pointers to Partition objects by providing copy semantics to add
items. It is generally better to be explicit;
3) C++ doesn't provide polymorphic copy construction directly, but this
is easily worked around by following the Virtual Constructor idiom
[1], which would allow PartitionLUKS derived class objects to be
copied into the vector.
Add push_back_adopt() and insert_adopt() methods which add a pointer to
a Partition object into the PartitionVector adopting ownership.
[1] Wikibooks: More C++ Idioms / Virtual Constructor
https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Virtual_Constructor
Bug 759726 - Implement Partition object polymorphism
The PartitionVector class is now internally using pointers to Partition
objects and taking on management of their lifetimes. It therefore has
to implement the Big 3: destructor, copy constructor and copy assignment
operator [1][2]. This is because the implicitly-defined copy
constructor and assignment operator perform memberwise "shallow copying"
and the destructor does nothing. This not correct for classes which
contain non-class types such as raw pointers.
The semantics of the interface still copies each Partition object into
the PartitionVector when they are added with push_back() and insert().
Note that a PartitionVector object is explicitly copy assigned in
Win_GParted::Refresh_Visual(). They are also implicitly copied when
(1) the implementing vector is resized larger to allow it to hold more
pointers to Partition objects than it previously had capacity for; and
(2) a Partition object is copied including the logicals PartitionVector
member.
[1] The rule of three/five/zero
http://en.cppreference.com/w/cpp/language/rule_of_three
[2] Rule of Three
https://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29
Bug 759726 - Implement Partition object polymorphism
Lots of files which use the Partition class relied on the declaration
being included via other header files. This is bad practice.
Add #include "Partition.h" into every file which uses the Partition
class which doesn't already include it. Header file #include guards are
specifically to allow this.
get_partitions() method was returning a vector of partitions. However
the calling code only needed to know whether any partitions were found
or not. Replace with found_partitions() method reporting the needed
boolean.
Now use of std::vector<Partition> partitions is hidden within the
Dialog_Rescue_Data class implementation.
Bug 759726 - Implement Partition object polymorphism
Just creates PartitionVector class and includes it in partition.h so
that it is built and validated by the compiler. Not used anywhere yet.
Implementation strategy is to create a PartitionLUKS class derived from
the Partition class. This implies polymorphism of Partition objects,
which in C++ requires using pointers and references to objects, and not
using objects directly. (See C++ object slicing). Later this
PartitionVector class will be modified to use pointers to Partition
objects and act as the owner of the pointed to Partition objects.
Bug 759726 - Implement Partition object polymorphism
Capture and parse the progress reports of ntfsresize and resize2fs and
update the dialog progress bar.
Bug 467925 - gparted: add progress bar during operation
Return newly constructed partition object by reference rather than by
copy from the Copy, Resize/Move and New dialog classes. This is another
case of stopping copying partition objects in preparation for using
polymorphic Partition objects. In C++ polymorphism has to use pass by
pointer and reference and not pass by value, copying, to avoid object
slicing.
The returned reference to the partition is only valid until the dialog
object containing the new_partition member is destroyed. This is okay
because in all three cases the returned referenced partition is copied
into a context with new lifetime expectations before the dialog object
is destroyed.
Case 1: GParted_Core::activate_paste()
Referenced new_partition is copied in the OperationCopy constructor
before the dialog object goes out of scope.
Operation * operation = new OperationCopy( ...,
dialog.Get_New_Partition( ... ),
... );
Case 2: GParted_Core::activate_new()
Referenced new_partition is copied in the OperationCreate
constructor before the dialog object goes out of scope.
Operation * operation = new OperationCreate( ...,
dialog.Get_New_Partition( ... ) );
Case 3: GParted_Core::activate_resize()
Temporary partition object is copied from the referenced
new_partition before the dialog object goes out of scope.
Partition part_temp = dialog.Get_New_Partition( ... );
Bug 757671 - Rework Dialog_Partition_New::Get_New_Partition() a bit
This is just to make the parameter name in the Dialog_Partition_New
constructor and set_data() method match the name of the equivalent
parameter in the Dialog_Partition_Copy and Dialog_Partition_Resize_Move
classes. (All three classes inherit from Dialog_Base_Partition and have
similar interfaces).
Bug 757671 - Rework Dialog_Partition_New::Get_New_Partition() a bit
Automatically load the cache of SWRaid information for the first time if
any of the querying methods are called before the first explicit
load_cache() call. Means we can't accidentally use the class and
incorrectly find no SWRaid members when they do exist.
Bug 756829 - SWRaid member detection enhancements
Busy file systems are accessed via a mount point, LVM Physical Volumes
are activated via the Volume Group name and busy SWRaid members are
accessed via the array device, /dev entry. Therefore choose to show the
array device in the mount point field for busy SWRaid members.
The kernel device name for an SWRaid array (without leading "/dev/") is
the same as used in /proc/mdstat and /proc/partitions. Therefore the
array device (with leading "/dev/") displayed in GParted will match
between the mount point for busy SWRaid members and the array itself as
used in the device combo box.
# cat /proc/mdstat
Personalities : [raid1]
md1 : active raid1 sda1[2] sdb1[3]
524224 blocks super 1.0 [2/2] [UU]
...
# cat /proc/partitions
major minor #blocks name
8 0 33554432 sda
8 1 524288 sda1
...
8 16 33554432 sdb
8 17 524288 sdb1
...
9 1 524224 md1
...
Bug 756829 - SWRaid member detection enhancements
In cases where blkid wrongly reports a file system instead of an SWRaid
member (sometimes confused by metadata 0.90/1.0 mirror array or old
version not recognising SWRaid members), the UUID and label are
obviously wrong too. Therefore have to use the UUID and label returned
by the mdadm query command and never anything reported by blkid or any
file system specific command.
Example of blkid reporting the wrong type, UUID and label for /dev/sda1
and the correct values for /dev/sdb1:
# blkid | egrep 'sd[ab]1'
/dev/sda1: UUID="10ab5f7d-7d8a-4171-8b6a-5e973b402501" TYPE="ext4" LABEL="chimney-boot"
/dev/sdb1: UUID="15224a42-c25b-bcd9-15db-60004e5fe53a" UUID_SUB="0a095e45-9360-1b17-0ad1-1fe369e22b98" LABEL="chimney:1" TYPE="linux_raid_member"
# mdadm -E -s -v
ARRAY /dev/md/1 level=raid1 metadata=1.0 num-devices=2 UUID=15224a42:c25bbcd9:15db6000:4e5fe53a name=chimney:1
devices=/dev/sda1,/dev/sdb1
...
ARRAY /dev/md127 level=raid1 num-devices=2 UUID=8dc7483c:d74ee0a8:b6a8dc3c:a57e43f8
devices=/dev/sdb6,/dev/sda6
...
NOTES:
* In mdadm terminology the label is called the array name, hence name=
parameter for array md/1 in the above output.
* Metadata 0.90 arrays don't support naming, hence the missing name=
parameter for array md127 in the above output.
Bug 756829 - SWRaid member detection enhancements
Add active attribute to the cache of SWRaid members. Move parsing of
/proc/mdstat to discover busy SWRaid members into the cache loading
code. New parsing code is a little different because it is finding all
members of active arrays rather than determining if a specific member is
active.
Bug 756829 - SWRaid member detection enhancements
Detection of Linux SWRaid members currently fails in a number of cases:
1) Arrays which use metadata type 0.90 or 1.0 store the super block at
the end of the partition. So file system signatures in at least
linear and mirrored arrays occur at the same offsets in the
underlying partitions. As libparted only recognises file systems
this is what is detected, rather than an SWRaid member.
# mdadm -E -s -v
ARRAY /dev/md/1 level=raid1 metadata=1.0 num-devices=2 UUID=15224a42:c25bbcd9:15db6000:4e5fe53a name=chimney:1
devices=/dev/sda1,/dev/sdb1
...
# wipefs /dev/sda1
offset type
----------------------------------------------------------------
0x438 ext4 [filesystem]
LABEL: chimney-boot
UUID: 10ab5f7d-7d8a-4171-8b6a-5e973b402501
0x1fffe000 linux_raid_member [raid]
LABEL: chimney:1
UUID: 15224a42-c25b-bcd9-15db-60004e5fe53a
# parted /dev/sda print
Model: ATA VBOX HARDDISK (scsi)
Disk /dev/sda: 34.4GB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Number Start End Size Type File system Flags
1 1049kB 538MB 537MB primary ext4 boot, raid
...
2) Again with metadata type 0.90 or 1.0 arrays blkid may report the
contained file system instead of an SWRaid member. Have a single
example of this configuration with a mirrored array containing the
/boot file system. Blkid reports one member as ext4 and the other as
SWRaid!
# blkid | egrep 'sd[ab]1'
/dev/sda1: UUID="10ab5f7d-7d8a-4171-8b6a-5e973b402501" TYPE="ext4" LABEL="chimney-boot"
/dev/sdb1: UUID="15224a42-c25b-bcd9-15db-60004e5fe53a" UUID_SUB="0a095e45-9360-1b17-0ad1-1fe369e22b98" LABEL="chimney:1" TYPE="linux_raid_member"
Bypassing the blkid cache gets the correct result.
# blkid -c /dev/null /dev/sda1
/dev/sda1: UUID="15224a42-c25b-bcd9-15db-60004e5fe53a" UUID_SUB="d0460f90-d11a-e80a-ee1c-3d104dae7e5d" LABEL="chimney:1" TYPE="linux_raid_member"
However this can't be used because if a user has a floppy configured
in the BIOS but no floppy attached, GParted will wait for minutes as
the kernel tries to access non-existent hardware on behalf of the
blkid query. See commit:
18f863151c
Fix long scan problem when BIOS floppy setting incorrect
3) Old versions of blkid don't recognise SWRaid members at all so always
report the file system when found. Occurs with blkid v1.0 on
RedHat / CentOS 5.
The only way I can see how to fix all these cases is to use the mdadm
command to query the configured arrays. Then use this information for
first choice when detecting partition content, making the order: SWRaid
members, libparted, blkid and internal.
GParted shell wrapper already creates temporary blank udev rules to
prevent Linux Software RAID arrays being automatically started when
GParted refreshes its device information[1]. However an administrator
could manually stop or start arrays or change their configuration
between refreshes so GParted must load this information every refresh.
On my desktop with 4 internal hard drives and 3 testing Linux Software
RAID arrays, running mdadm adds between 0.20 and 0.30 seconds to the
device refresh time.
[1] a255abf343
Prevent GParted starting stopped Linux Software RAID arrays (#709640)
Bug 756829 - SWRaid member detection enhancements
Abstract code checking sector size and ensuring the first sector of a
candidate disk device can be read into new
GParted_Core::useable_device() method.
Bug 755495 - GParted allowing partitioning of large sector devices
specified on the command line, when built with old
libparted which doesn't support it
Libsigc++2 version 2.5.2 and later removed header file
<sigc++/class_slot.h>. Quoting the NEWS file for version 2.5.2:
Remove useless headers: sigc++/class_slot.h ...
Libsigc++2 version 2.5.2 NEWS file:
https://git.gnome.org/browse/libsigc++2/tree/NEWS?id=2.5.2
Bug 756035 - GParted does not compile with newer gtkmm libraries in
Fedora 23
GParted waits forever when attempting to set a FAT16/32 file system
label which contains prohibited characters [1][2]. This is because
mlabel asks a question and is waiting for input. Force cancelling the
operation doesn't work either as GParted sends signal 2 (interrupt i.e.
[Ctrl-C]) but mtools commands specifically ignores this and a number of
other signals. Have to kill mlabel with signal 9 (kill) to regain
control of GParted.
Mlabel command with prohibited characters in the label:
# export MTOOLS_SKIP_CHECK=1
# mlabel ::"MYLABEL/ " -i /dev/sdb10
Long file name "MYLABEL/ " contains illegal character(s).
a)utorename A)utorename-all r)ename R)ename-all
s)kip S)kip-all q)uit (aArRsSq):
Remove prohibited characters from FAT16/32 file systems labels when
creating and labelling them. Also upper case the label to meet label
requirements [1][2]. This silently corrects the label and the actual
label applied will be displayed when GParted refreshes after applying
the operation.
[1] Microsoft TechNet: Label
https://technet.microsoft.com/en-us/library/bb490925.aspx
[2] Replicated in Wikikedia: label (command)
https://en.wikipedia.org/wiki/Label_%28command%29
Bug 755608 - Labeling fat16/fat32 partitions hangs if certain characters
included in label
These member variables store no Operation class information and were
being used as local variables. Replace with local variables.
Also indent a code block within an if clause so that the compiler can
confirm that the new local variable isn't used uninitialised. Prevents
this compiler warning:
OperationResizeMove.cc: In member function 'void GParted::OperationResizeMove::apply_normal_to_visual(std::vector<GParted::Partition, std::allocator<GParted::Partition> >&)':
OperationResizeMove.cc:125: warning: 'index' may be used uninitialized in this function
Bug 755214 - Refactor operation merging
Since this commit earlier in the patchset the second optional parameter
of method Win_GParted::Add_Operation() is no longer used. Remove it.
Replace open coded merge of resize/move into create operation
(#755214)
Bug 755214 - Refactor operation merging
Move the code from OperationCreate::apply_to_visual() into new method
Operation::insert_new() in the parent class. This is in preparation for
the following commit.
Bug 755214 - Refactor operation merging
The apply_to_visual() method for the change UUID, format, label file
system and name partition operations duplicated identical code. This
code was just substituting the partition in the disk graphic vector with
the new partition recorded in the operation, as none of these operations
change the partition boundaries. Move this duplicate code into the
parent class in new method Operation::substitute_new().
Bug 755214 - Refactor operation merging
After previous commit "Replace open coded merge of resize/move into
create operation (#755214)" the second created partition would disappear
from the disk graphic in the following sequence: create new #1, create
new #2 leaving space preceding, resize #1 larger. The create new #2
operation still existed and was shown in the operation list. It was
just that it disappeared from the disk graphic.
Remember that when each operation is created it records the partition,
or the unallocated space, to which the operation is applied at the time
the operation is created in the partition_original member variable. In
the above sequence the resize #1 larger operation was merged back into
the create new #1 operation. When visually re-applying the create
new #1 operation to the disk graphic, it left a smaller unallocated
partition following it. This was smaller than the unallocated partition
recorded in the create new #2 operation, hence it failed to visually
re-apply to the disk graphic.
The insight to fix this is that it doesn't matter what size the
unallocated space was when the create new operation was constructed. It
only matters that the new partition to be created fits in the available
unallocated space currently in the disk graphic.
Bug 755214 - Refactor operation merging
Creation of the various operations involved various implicit rules about
how the different types of operations were merged in different cases.
This was open coded in each ::activate_*() method. Abstract this into
new merge_operations() method and make the merging rules explicitly
specified.
NOTE:
The removal of operation type checking in the MERGE_LAST_WITH_ANY cases
is not a problem because all the Operation*::merge_operations() methods
ensure the operation types match as part of the merge attempt.
Bug 755214 - Refactor operation merging
Rename Win_GParted::Merge_Operations() to merge_two_operations(). To
reflect what it does and in preparation for further refactoring of the
code.
Be more strict on the validation of the first and second indexes. The
first operation must also be before the second operation in the
operation[] vector. (It is actually a programming bug if first and
second fail validation. However so far g_assert() is only being used to
validate pointers, which if wrong would likely cause the program to
eventually crash when dereferenced later. In this case a bug would
merely cause the incorrectly specified pair of operations to not be
merged).
Move validate_display_partition_ptr() declaration in the header file to
be in the same ordering as it's definition in the source file.
Bug 755214 - Refactor operation merging
Win_GParted::Merge_Operations() method was modifying the internals of
Operation* objects; in particular the partition_new member variable.
This is breaking data hiding and encapsulation tenant of object oriented
programming.
Implement exactly the same operation merge semantics, but hide the
manipulation of the internals of the Operation* objects within the
Operation* classes themselves.
Bug 755214 - Refactor operation merging
As previous commit, display_partitions is now a Win_GParted member
variable so checking for the existence of an extended partition can be
localised where it is used.
Remove index_extended member variable and localise the same checking in
activate_new().
Now that display_partitions is a Win_GParted member variable and
therefore available throughout the class, since commit [1], calculation
of primary_count can be localised in max_amount_prim_reached() where it
is used.
Implements a FIXME and removes primary_count as a member variable.
[1] 545b75d957
Move vector of partition objects to a Win_GParted class member (#750168)
Command exit status is a 1 byte value between 0 and 255. [1][2] However
at the Unix API level the value is encoded as documented in the
waitpid(2) manual page. This is true for the Glib API too. [3] This is
why, for example, the comment in ext2::check_repair() reported receiving
undocumented exit status 256. It was actually receiving exit status 1
encoded as per the waitpid(2) method.
Add shell style exit status decoding [2] to execution of all external
commands. Return value from Utils::execute_command() and
FileSystem::execute_command() functions are now:
0 - 125 - Exit status from the command
126 - Error executing the command
127 - Command not found
128+N - Command terminated by signal N
255 - Unexpected waitpid(2) condition
Also adjust checking of the returned statuses as necessary.
[1] Advanced Bash-Scripting Guide: Appendix D. Exit Codes With Special
Meanings
http://www.linuxtopia.org/online_books/advanced_bash_scripting_guide/exitcodes.html
[2] Quote from the bash(1) manual page:
EXIT STATUS
... Exit statuses fall between 0 and 255, though as
explained below, the shell may use values above 125
specially. ...
... When a command terminates on a fatal signal N, bash uses
the value of 128+N as the exit status.
If a command is not found, the child process created to
execute it returns a status of 127. If a command is found
but is not executable, the return status is 126.
[3] Quote from the Glib Reference Manual, Spawning Processes section,
for function g_spawn_check_exit_status():
https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-check-exit-status
The g_spawn_sync() and g_child_watch_add() family of APIs return
an exit status for subprocesses encoded in a platform-specific
way. On Unix, this is guaranteed to be in the same format
waitpid() returns, ...
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
There has been an undocumented rule that external commands displayed in
the operation details, as part of file system manipulations, only get a
time and check mark displayed when multiple commands are needed, and not
otherwise. (GParted checks whether all commands are successful or not
regardless of whether a check mark is displayed in the operation details
or not).
EXCEPTION 1: btrfs resize
Since the following commit [1] from 2013-02-22, GParted stopped
displaying the timing for the btrfs resize command in the operation
details. It being part of a multi-command sequence to perform the step.
This is because FileSystem::execute_command() since the commit can only
check the exit status for zero / non-zero while timing and checking the
command status but btrfs resize needs to consider some non-zero statuses
as successful.
[1] 52a2a9b00a
Reduce threading (#685740)
EXCEPTION 2: ext2/3/4 move and copy using e2image
When use of e2image was added [2] the single command steps were timed
and check.
[2] 86111fe12a
Use e2image to move/copy ext[234] file systems (#721516)
EXCEPTION 3: fat16/32 write label and UUID
Uses Utils::execute_command() rather than FileSystem::execute_command()
so can be separately changed. See the following commit for resolution
of the final commands not yet timed and check mark displayed.
CHANGE:
Lets make a simpler rule of always displaying the time and a check mark
for all external commands displayed in the operation details. However
this makes several of the other single command actions need special exit
status handling because zero success, non-zero failure is not correct
for every case. Specifically affects resizing of reiserfs and check
repair of ext2/3/4, fat16/32, jfs and reiserfs.
After this change all external commands run as file system actions must
follow one of these two patterns of using the EXEC_CHECK_STATUS flag or
separately calling FileSystem::set_status() to register success or
failure of the command:
exit_status = execute_command(cmd, od, EXEC_CHECK_STATUS...);
or:
exit_status = execute_command(cmd, od, ...);
bool success = (exit_status == 0 || exit_status == OTHER_SUCCESS_VALUE...);
set_status(od, success );
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
Change the two optional boolean parameters into a single optional flags
parameter which uses symbolically defined names. Makes reading the
execute_command() calls much easier to understand. (Implemented as bit
field using the same technique as used for Glib::SpawnFlags [1]).
This changes the calls thus:
execute_command(cmd, od) -> (cmd, od)
execute_command(cmd, od, false) -> (cmd, od, EXEC_NONE) // [2]
execute_command(cmd, od, true ) -> (cmd, od, EXEC_CHECK_STATUS)
execute_command(cmd, od, false, true) -> (cmd, od, EXEC_CANCEL_SAFE)
execute_command(cmd, od, true , true) ->
(cmd, od, EXEC_CHECK_STATUS|EXEC_CANCEL_SAFE)
[1] SpawnFlags bitwise operators in
/usr/include/glibmm-2.4/glibmm/spawn.h.
[2] False and EXEC_NONE are the default values for the optional third
parameter before and after this change respectively and both mean
the same. This is being used in btrfs::resize() and being kept for
now despite it being the default.
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
Use of execute_command_timed() was removed by this commit from
2013-02-22:
52a2a9b00a
Reduce threading (#685740)
Bug 754684 - Updates to FileSystem:: and Utils::execute_command()
functions
Since GParted commit 52a2a9b "Reduce threading (#685740)", released in
GParted 0.15.0, application of operations occurs in the main thread
running the UI, therefore long running libparted actions such as
resizing a FAT16 or FAT32 file system hang the UI for as long as it take
to complete the operation.
https://git.gnome.org/browse/gparted/commit/?id=52a2a9b00a32996921ace055e71d0e09fb33c5fe
Though this problem exists for all libparted actions, it is particularly
noticeable when performing a large resize of fat16/fat32/hfs/hfs+ file
systems.
To address this significant cause of an unresponsive GUI, this
enhancement adds threading to the libparted ped_file_system_resize
function call.
Bug 737022 - UI hangs while running libparted operations such as
FAT16/FAT32 resizing
Previously on every refresh for every device, GParted was searching the
PATH to discover if the hdparm command existed. Stracing GParted showed
that calling Glib::find_program_in_path("hdparm") made the following OS
calls:
access("/usr/lib64/qt-3.3/bin/hdparm", X_OK) = -1 ENOENT (No such file or directory)
access("/usr/local/sbin/hdparm", X_OK) = -1 ENOENT (No such file or directory)
access("/usr/local/bin/hdparm", X_OK) = -1 ENOENT (No such file or directory)
access("/sbin/hdparm", X_OK) = 0
getuid() = 0
stat("/sbin/hdparm", {st_mode=S_IFREG|0755, st_size=137, ...}) = 0
stat("/sbin/hdparm", {st_mode=S_IFREG|0755, st_size=137, ...}) = 0
The Linux VFS is very fast but repeatedly doing this is wasteful.
Remember the result of searching the PATH for the hdparm command at
startup and refresh this when the [Rescan For Supported Actions] button
is pressed in the File System Support dialog. This is the same as
GParted already does for file system specific commands and their
capabilities.
Bug 751251 - Show serial number in device information
Run "hdparm -I /dev/DISK" to get the hard drive serial number of
every device which has one and display it in the Device Information.
The displayed value can either be the actual serial number, "none" or
blank. "none" means the device doesn't have a hard drive serial number,
such as for Linux software RAID arrays, BIOS fake RAID arrays or USB
flash drives. Blank means something went wrong getting the serial
number. Either it couldn't be found in the hdparm output or the hdparm
command wasn't installed.
Example real hard drive:
# hdparm -I /dev/sda
...
ATA device, with non-removable media
Model Number: SAMSUNG HM500JI
Serial Number: S1WFJDSZ123732
...
Example Linux software RAID array:
# hdparm -I /dev/md127
/dev/md127:
HDIO_DRIVE_CMD(identify) failed: Inappropriate ioctl for device
On my desktop with 4 internal hard drives 2 Linux software RAID arrays
on those hard drives, 2 USB flash drives and 1 USB hard drive attached,
running hdparm 9 times added 0.07 seconds to the device refresh time.
Bug 751251 - Show serial number in device information
Btrfs-progs 4.1, released June 2015, includes support for changing the
UUID of a btrfs file system using the btrfstune command. Check for
availability by looking for the -u option in the btrfstune help output.
Use btrfstune like this:
# umount /dev/sdb1
# btrfstune -f -u /dev/sdb1
Current fsid: e7ad5dba-d721-4f99-990b-1ba2901c8ad2
New fsid: 231563d9-e173-410d-b1da-d34c4319a423
Set superblock flag CHANGING_FSID
Change fsid in extents
Change fsid on devices
Clear superblock flag CHANGING_FSID
Fsid change finished
# echo $?
0
Bug 751337 - btrfstune in btrfs-progs 4.1 supports changing the file
system UUID
The lvm query commands were always run and the cache loaded even if
GParted, actually blkid, didn't identify any LVM2 PVs. (GParted uses
libparted and blkid to identify partition content and the lvm commands
to provide the needed configuration details).
Now implement complete lazy initialization of the cache. Never force
loading of the cache. The cache is only loaded when the first value is
accessed from it. When there are no LVM2 PVs, the cache is never
queried, so never loaded. All the needed infrastructure for delayed
loading was previously added by this commit from 2011-12-11:
ff8ad04120
Lazy initialize the cache from querying LVM2 PVs (#160787)
Every public member function which access values from the cache already
calls initialize_if_required(). Just need to replace force loading of
the cache with a function which just clears the cache.
On my desktop, only when there are no LVM2 PVs, not loading the cache
and therefore not executing these external commands in
load_lvm2_pv_info_cache() saves 1.0 seconds of the 3.7 seconds it takes
to perform the a refresh in GParted:
lvm vgscan
lvm pvs ... -o pv_name,...
lvm pvs ... -o vg_name,...
Bug 750582 - Refactor the LVM2_PV_Info module object interface and
internal cache representation
GParted used to cache the results of the "lvm pvs" commands used to query
the state of the Logical Volume Manager as a series of lines of text.
Then every time a particular value was queried GParted would split all
the lines of text into fields until the required value was found.
Stop this repeat splitting of cached lines of text. Instead parse the
lines of text into separate fields and store in structures of values of
the correct type in the cache.
Bug 750582 - Refactor the LVM2_PV_Info module object interface and
internal cache representation
The LVM2_PV_Info cache had a pretend multi-object interface, yet all the
data is static. An LVM2_PV_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
LVM2_PV_Info objects.
Bug 750582 - Refactor the LVM2_PV_Info module object interface and
internal cache representation
An LVM2_PV_Info object contains no member variables as all the data is
static (exists once in the program and accessed by all objects). The
constructor did nothing, except when passed true to load the cache.
Provide a separate load_cache() member function and remove the
constructors and destructor which do nothing. The C++ compiler will
provide a default constructor and destructor, which don't do anything as
there are no member variables to initialise and finalise.
This makes the interface a little easier to understand. Mostly a step
along the way of refactoring how the LVM2_PV_Info cache module works.
Bug 750582 - Refactor the LVM2_PV_Info module object interface and
internal cache representation
The copy, resize/move and new dialog classes (Dialog_Partition_Copy,
Dialog_Partition_Resize_Move and Dialog_Partition_New respectively) had
to be used like this:
construct dialog object passing some parameters
call Set_Data() to pass more parameters
run() dialog
call Get_New_Partition()
There is nothing in the classes which forces Set_Data() to be called,
but it must be called for the dialogs to work and prevent GParted from
crashing.
Make these class APIs safer by making it impossible to program
incorrectly in this regard. Move all the additional parameters from
each Set_Data() method to each constructor. The constructors just call
the now private set_data() methods.
The member variable was named selected_partition. It is assigned from
Win_GParted::selected_partition_ptr (which is a pointer to a const
partition object so is never updated). This gives connotations that it
won't be modified.
However it is updated freely as the new resultant partition object is
prepared before being returned from the dialog, most notable in the
Get_New_Partition() methods.
Therefore rename from selected_partition to new_partition.
The code goes like this:
Dialog_Partition_Copy::Get_New_Partition()
call Dialog_Base_Partition::Get_New_Partition()
Update this->selected_partition with results from running
the dialog.
return this->selected_partition by value.
Save value back to this->selected_partition.
Update this->selected_partition some more.
return this->selected_partition by value.
So there is an unnecessary copy of the partition object returned from
the base class Get_New_Partition() function back to the same variable in
the derived copy class Get_New_Partition() function.
Need to keep the base class Get_New_Partition() function as derived
class Dialog_Partition_Resize_Move uses that implementation as it
doesn't override it, and it's part of the interface.
Avoid this unnecessary copy by moving base class Get_New_Partition()
code into a new private function, called prepare_new_partition(), which
doesn't return anything. Then have Get_New_Partition() in both classes
just return the required partition object. Like this:
Dialog_Base_Partition::Get_New_Partition()
call prepare_new_partition()
return this->selected_partition by value.
Dialog_Partition_Copy::Get_New_Partition()
call Dialog_Base_Partition::prepare_new_partition()
Update this->selected_partition some more.
return this->selected_partition by value.
Bug 750168 - Reduce the amount of copying of partition objects
When Operation objects are created they take a copy of the Device object
to which the operation is to be applied. The Device object includes a
vector of all the contained Partition objects currently on the device,
so these get copied too.
These additional deep copied Partition objects in the Operation object
are never accessed. Therefore don't copy the contained Partition
objects when copying the Device object into the Operation object.
Bug 750168 - Reduce the amount of copying of partition objects
When opening the Manage Flags dialog, creation of the dialog object was
creating a copy of the selected partition object. If this was an
extended partition it also included recursively constructing the
contained logical partitions too.
Instead, replace the partition object in the DialogManageFlags class
with a reference to it.
Bug 750168 - Reduce the amount of copying of partition objects
When opening the Partition Information dialog, creation of the dialog
object was creating a copy of the partition object to be displayed. If
this was an extended partition it also included recursively constructing
the contained logical partitions too.
Instead, replace the partition object in the Dialog_Partition_Info class
with a reference to it.
NOTE:
In C++ a reference is really just a pointer under the hood. As such,
dereferences of a pointer to an object in the context of needing a
reference to the object doesn't copy the object. It merely initialises
the reference from the pointer.
Specifically, with this prototype:
Dialog_Partition_Info( const Partition & partition );
and the dialog object being constructed in Win_GParted::activate_info():
Dialog_Partition_Info dialog( *selected_partition_ptr );
the partition object is not copy constructed. A reference (pointer) to
it is merely passed to the dialog constructor.
Bug 750168 - Reduce the amount of copying of partition objects
Further ensure that a bug doesn't get introduced with the use of
selected_partition_ptr, by asserting that it points to a current
partition object in the vector of display partitions.
After deliberately breaking the code so that selected_partition_ptr
points to some other partition object, trying to display the Information
dialog causes this crash:
======================
libparted : 2.4
======================
**
ERROR:Win_GParted.cc:989:void GParted::Win_GParted::set_valid_operations(): assertion failed: (valid_display_partition_ptr( selected_partition_ptr ))
Aborted (core dumped)
At this point in the code:
973 void Win_GParted::set_valid_operations()
974 {
...
986 // No partition selected ...
987 if ( ! selected_partition_ptr )
988 return ;
>> 989 g_assert( valid_display_partition_ptr( selected_partition_ptr ) ); // Bug: Not pointing at a valid display partition object
Bug 750168 - Reduce the amount of copying of partition objects
Now that TreeView_Details and DrawingAreaVisualDisk classes store and
pass pointers to partition objects in the Gtk signal callbacks, change
the selected partition into a pointer too.
Bug 750168 - Reduce the amount of copying of partition objects
This stops copying of each displayed partition object into the
DrawingAreaVisualDisk class.
Bug 750168 - Reduce the amount of copying of partition objects
This stops copying of each displayed partition object into the
TreeView_Details class.
It also stops copy constructing lots of partition objects when just
clicking on a partition in the disk graphic. The disk graphic needs to
inform the main GUI and then the partition list which partition has been
selected. The call sequence goes like:
DrawingAreaVisualDisk::on_button_press_event(event)
Win_GParted::on_partition_selected(partition_ptr, src_is_treeview)
TreeView_Detail::set_selected(partition_ptr)
TreeView_Detail::set_selected(rows, partition_ptr,
inside_extended)
Relevant source and highlighted comparison line:
140 bool TreeView_Detail::set_selected( Gtk::TreeModel::Children rows,
141 const Partition * partition_ptr, bool inside_extended )
142 {
143 for ( unsigned int t = 0 ; t < rows .size() ; t++ )
144 {
>> 145 if ( static_cast<Partition>( rows[t][treeview_detail_columns.partition] ) == *partition_ptr )
146 {
147 if ( inside_extended )
148 expand_all() ;
149
150 set_cursor( static_cast<Gtk::TreePath>( rows[ t ] ) ) ;
151 return true ;
152 }
153
154 if ( set_selected( rows[t].children(), partition_ptr, true ) )
155 return true ;
156 }
157
158 return false ;
159 }
Then in this function the partition selected in the disk graphic
(partition_ptr parameter) is compared in turn with each partition object
stored in the Gtk::TreeView model to find the matching one to mark it as
selected. This mere act of accessing the partition object stored in a
row of the Gtk::TreeView model causes it to be copy constructed. So
clicking on the 5th partition in the disk graphic will copy construct
the first 5 partition objects just to do a compare to find the matching
one.
This is because it is not possible to get a reference from a
Gtk:TreeViewProxy in gtkmm. Merely accessing a value in a Gtk::TreeView
model takes a copy of that value.
Subject: get a reference from a Gtk::TreeValueProxy
http://comments.gmane.org/gmane.comp.gnome.gtkmm/2217http://marc.info/?t=104400417500001&r=1&w=4
Bug 750168 - Reduce the amount of copying of partition objects
Change from passing a reference to the selected partition, to passing a
pointer to the selected partition in the signal_partition_selected
callbacks between the disk graphic, partition list and core GUI modules.
This is an enabler for the following patches.
Bug 750168 - Reduce the amount of copying of partition objects
Win_GParted::Refresh_Visual() used a local variable containing a copy of
the vector of partitions in the current device to be displayed. After
visually applying pending operations it loaded copies of each partition
object into the GUI widgets to display the disk graphic and partition
list, DrawingAreaVisualDisk and TreeView_Details classes respectively.
When a partition is selected in the UI, again a partition object is
copied. Also several of the partition dialogs, including the
information dialog, take a copy of the partition object. All these are
copies of the same set of partition objects, those currently being
displayed in the UI.
Move the vector of displayed partitions from a local variable in
Refresh_Visual() to a Win_GParted member variable. This will allow for
the above cases to be changed to used pointers and references to the
same set of partition objects.
The valid lifetime of pointers to elements in this partition object
vector is from one refresh to the next, when the vector is cleared and
repopulated with a new set of partition objects. This is exactly what
is needed as the GUI widgets are reloaded on each refresh, the selected
partition is reset and none of the partition dialog objects exist.
Dialog objects being created and destroyed on each use.
On the other hand some copies of partition objects currently being
displayed, still need to be made because they have lifetimes which need
to last longer than the next call to Refresh_Visual(). Specifically the
source of the copy partition and the partition objects copied into the
in the list of pending operations.
Bug 750168 - Reduce the amount of copying of partition objects
BUF in the copy dialog class, Dialog_Partition_Copy, is use to adjust
limits in 2 cases:
1) Minimum size when copying an XFS file system
Minimum size was set to the used space + 2 * cylinder size (typically
plus ~16 MiB). This commit from 2004-12-20 added it:
a54b52ea33
xfs copy now uses xfsdump and xfsrestore. icw some hacks in the other 2
Issues:
* This is increasing the minimum XFS file system size when copying it,
which doesn't happen in the resize case for other file systems.
* It allows an XFS file system to be created which is smaller than the
minimum size allowed by GParted. Copying an empty XFS file system can
create a new file system as small as 26 MiB. This is smaller than the
minimum GParted allows of 32 MiB because that is the minimum
xfs_repair can handle.
Remove this addition when copying an XFS file system and enforce minimum
file system size.
2) Maximum size when copying a file system into empty space larger than
it's maximum size
Maximum size was set to maximum file system size - cylinder size
(typically minus ~8 MiB). Only applied to FAT16 which has a maximum
file system size set in and can be grown. Added by this commit from
2004-12-15:
10e8f3338d
:get_fs now returns a const reference. in copy and resizedialog
...
* in copy and resizedialog filesystems with MAX set now have a max size of MAX - one cylinder .
Issue:
* This is applying a lower maximum resize when copying the file system
compared to that when creating the file system.
NOTE:
GParted currently allows all file systems to be resize to any size,
regardless of the maximum file system size. This is probably an
oversight, but it does allow libparted to convert FAT16 to FAT32 file
system when resizing.
Remove this lower maximum file system size when copying and resizing,
compared to creating.
Bug 749867 - Some limits are adjusted by arcane cylinder size amount
when copying and resizing in a single operation
This commit from 2010-05-20 removed use of cylinder size increase in the
minimum, and cylinder size decrease in the maximum file system sizes
from the resize/move dialog.
e62a23b5b5
Add partition alignment option to align to MiB (#617409)
This cylinder size limit adjustments were being performed using the
Dialog_Base_Partition::BUF member variable. Now in the
Dialog_Partition_Resize_Move class it is never accessed, and only
unnecessarily set. Move BUF from the common base class into the
Dialog_Partition_Copy class where it is still used.
Bug 749867 - Some limits are adjusted by arcane cylinder size amount
when copying and resizing in a single operation
The function never modifies any member variables so make it a const
member function.
(FileSystem::get_custom_text() is a virtual function so can't be made
static).
Rename a couple of GParted_Core methods for consistency and to better
distinguish get_filesystem() from get_filesystems() which do completely
unrelated things.
get_filesystem() -> detect_filesystem()
recognise_filesystem_signature() -> detect_filesystem_internal()
Also make detect_filesystem() a static member method as it doesn't use
any member variables. Requirement cascades to get_partition_path().
GParted_Core methods:
flush_device()
get_device()
get_disk()
get_device_and_disk()
destroy_device_and_disk()
commit()
commit_to_os()
settle_device()
This group of methods only call libparted API functions and run external
executables. None of them access any GParted_Core member variables.
Make them all static member functions.
These member functions are only used within the GParted_Core class and
only operate on the static member variable FILESYSTEM_MAP.
Make both functions private and also make init_filesystems() static.
The FileSystem objects stored in the FILESYSTEM_MAP are allocated once
using new in init_filesystems() but never deleted.
Valgrind output fragment:
# valgrind --leak-check=full ./gparted
==29314== 353 (72 direct, 281 indirect) bytes in 1 blocks are definitely lost in loss record 6,287 of 6,905
==29314== at 0x4A075FC: operator new(unsigned long) (vg_replace_malloc.c:298)
>> ==29314== by 0x46EDA5: GParted::GParted_Core::init_filesystems() (GParted_Core.cc:106)
==29314== by 0x46EC5F: GParted::GParted_Core::GParted_Core() (GParted_Core.cc:96)
==29314== by 0x4A74F4: GParted::Win_GParted::Win_GParted(std::vector<Glib::ustring, std::allocator<Glib::ustring> > const&) (Win_GParted.cc:51)
==29314== by 0x4D600A: main (main.cc:56)
...
==29314== 161 (72 direct, 89 indirect) bytes in 1 blocks are definitely lost in loss record 6,119 of 6,905
==29314== at 0x4A075FC: operator new(unsigned long) (vg_replace_malloc.c:298)
>> ==29314== by 0x46F50C: GParted::GParted_Core::init_filesystems() (GParted_Core.cc:124)
==29314== by 0x46EC5F: GParted::GParted_Core::GParted_Core() (GParted_Core.cc:96)
==29314== by 0x4A74F4: GParted::Win_GParted::Win_GParted(std::vector<Glib::ustring, std::allocator<Glib::ustring> > const&) (Win_GParted.cc:51)
==29314== by 0x4D600A: main (main.cc:56)
GParted_Core.cc source:
102 void GParted_Core::init_filesystems()
103 {
104 FILESYSTEM_MAP[ FS_UNKNOWN ] = NULL ;
105 FILESYSTEM_MAP[ FS_CLEARED ] = NULL ;
>> 106 FILESYSTEM_MAP[ FS_BTRFS ] = new btrfs() ;
...
>> 124 FILESYSTEM_MAP[ FS_XFS ] = new xfs() ;
125 FILESYSTEM_MAP[ FS_BITLOCKER ] = NULL ;
Fix by deleting all FILESYSTEM_MAP pointers. Note that delete on a NULL
pointer is defined by C++ as a safe do nothing operation.
C++ FAQ / Do I need to check for null before delete p?
https://isocpp.org/wiki/faq/freestore-mgmt#delete-handles-null
Fixing this reduces the valgrind reported definitely lost memory blocks
count from 25 down to 6. 19 FileSystem objects deleted and 19 memory
blocks no longer lost.
Bug 749036 - FileSystem objects are memory leaked in init_filesystems()
Add a partition name entry box to the Create New Partition dialog. The
entry box is greyed out (not sensitive) for partition table types which
don't support partition naming. Currently only supported for GPTs. See
Utils::get_max_partition_name_length() for details.
There was a slightly wider gap between the file system combobox row and
the label entry row when there were only three widgets on the right hand
side of the dialog. This has been removed now that there are four
widgets so that they are all evenly spaced and they line up with the
four widgets on the left hand side.
So far the partition name can be entered and previewed, but isn't yet
applied to the disk.
Bug 746214 - Partition naming enhancements
Adding a partition name entry to the Create New Partition dialog will
need access to these two Device methods: partition_naming_supported()
and get_max_partition_length(). The Set_Data() function already takes
two parameters, only_unformatted and disktype, taken from Device member
variables.
Rather than add two more parameters to the Set_Data() function pass the
Device object instead, replacing the current only_unformatted and
disktype parameters.
Bug 746214 - Partition name enhancements
Rename Gtk::Entry object entry -> filesystem_label_entry in the
Dialog_Partition_New class. This is in preparation for the introduction
of the partition name entry box in the Create New Partition dialog.
Bug 746214 - Partition name enhancements
Previously partition naming had only been implemented for gpt. Make the
code ready to support naming of the other partition table types for
which libparted supports naming. Specifically: amiga, dvh, mac and
pc98 in addition to gpt. Document issues found with some of these
partition table types, which can relatively easily been worked around.
Leave support of naming for partition table types other than gpt
disabled, mostly just to reduce ongoing testing effort, at least until
there is any user demand for it.
Bug 746214 - Partition naming enhancements
resize_move() and move() stopped using the device parameter in this
commit from 2006-07-23:
d663c3c277
removed cylindersize buffering during resize from the filesystems. It is
create() stopped using the device parameter in this commit from 2006-03-19:
ad9f2126e7
fixed issues with copying (see also #335004) cleanups + added FIXME added
For reference most other operation methods had the device parameter
removed in this earlier commit from 2005-12-07:
642f0a145b
from now on each partition has a reference to it's device. make use of new
When the following conditions were met GParted would fail to recognise a
newly created whole disk device file system, and instead show an unknown
file system filling the disk:
1) Disk was previously partitioned and contained at least one partition.
2) Using libparted version 2.0 to 3.0 inclusive.
Initial status:
# blkid | fgrep sdc
# fgrep sdc /proc/partitions
8 32 976762584 sdc
8 33 104857600 sdc1
# parted /dev/sdc
GNU Parted 2.4
Using /dev/sdc
Welcome to GNU Parted! Type 'help' to view a list of commands.
(parted) print
Model: ATA ST1000LM024 HN-M (scsi)
Disk /dev/sdc: 1000GB
Sector size (logical/physical): 512B/4096B
Partition Table: msdos
Number Start End Size Type File system Flags
1 1049kB 107GB 107GB primary
When creating the loop partition table libparted would not inform the
kernel to delete the old partitions. /proc/partitions still contained
the details of the old partitions.
(parted) mktable loop
Warning: The existing disk label on /dev/sdc will be destroyed and
all data on this disk will be lost. Do you want to continue?
Yes/No? Yes
(parted) print
Model: ATA ST1000LM024 HN-M (scsi)
Disk /dev/sdc: 1000GB
Sector size (logical/physical): 512B/4096B
Partition Table: loop
Number Start End Size File system Flags
(parted) quit
# fgrep sdc /proc/partitions
8 32 976762584 sdc
8 33 104857600 sdc1
Creation of the whole disk device file system goes unnoticed by blkid
because the kernel and therefore blkid's cache have stale partition
information.
# mkfs.xfs -f /dev/sdc
# blkid | fgrep sdc
NOTE:
On a Linux Software RAID array, as opposed to a hard disk, blkid does
notice creation of the whole disk device file system. However the
kernel still has old partition details.
This was fixed in libparted 3.1 by commit:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=f5c909c0cd50ed52a48dae6d35907dc08b137e88
libparted: remove has_partitions check to allow loopback partitions
Fix by deleting old partitions before creating the loop table when
compiled with a broken version of libparted. The GParted UI provides
no feedback while a new partition table is created, and with some
versions of GTK the UI become unresponsive too, so it is important to be
as fast as possible. Evaluated three different methods, deleting 15 and
22 MSDOS partitions on a physical 5400 RPM hard drive using libparted
2.4:
M1) Delete and commit one partition at a time.
Takes up to 24 seconds to delete 15 partitions. With 22 partitions
libparted always reports finding some of the partitions busy and
unable to inform the kernel about the modifications.
Too slow and doesn't work.
M2) Delete all partitions in one go and commit once.
Takes up to 1.4 seconds to delete either 15 or 22 partitions. Never
removes partitions 17 and higher from the kernel.
Doesn't work.
M3) Write GPT table (letting libparted delete any old partitions).
Takes up to 0.8 seconds to delete either 15 or 22 partitions.
Fast and works.
Use method 3 - write a GPT table thus using libparted code to inform the
kernel of the old partition deletions.
Bug 743181 - Add unpartitioned drive read-write support
Older versions of blkid don't correctly distinguish between FAT16 and
FAT32 file systems when overwriting one with the other. This effects
GParted too with these file systems on whole disk devices where only
blkid is used to recognise the contents. See previous fix for why only
blkid is used in this case:
Avoid whole disk FAT being detected as MSDOS partition table
(#743181)
Example:
# blkid -v
blkid from util-linux 2.20.1 (liblkid 2.20.0, 19-Oct-2011)
# mkdosfs -F16 -I /dev/md1
# blkid | fgrep md1
/dev/md1: SEC_TYPE="msdos" UUID="7C23-95D9" TYPE="vfat"
# mkdosfs -F32 -I /dev/md1
# blkid | fgrep md1
/dev/md1: SEC_TYPE="msdos" UUID="7F93-98F4" TYPE="vfat"
So blkid recognised the UUID changed but didn't remove the SEC_TYPE for
the FAT32 file system. See FS_Info::get_fs_type() as it uses this to
distinguish between FAT16 and FAT32. This is a caching update bug in
blkid, because telling blkid not to use the cache gets the right
results:
# blkid -c /dev/null | fgrep md1
/dev/md1: UUID="7F93-98F4" TYPE="vfat"
With testing determined that blkid from util-linux 2.23 and later are
not affected and earlier versions are affected. Mostly recently known
affected distribution is Ubuntu 14.04 LTS with util-linux 2.20.1.
The straight forward fix would be to instruct blkid to not use its cache
with 'blkid -c /dev/null'. But using blkid's cache is needed to prevent
blkid hanging for minutes when trying to access a non-existent floppy
drive when the BIOS is set incorrectly. See commit:
18f863151c
Fix long scan problem when BIOS floppy setting incorrect
Instead, when using an older affected version of blkid and when blkid
cache reports a vfat file system, run blkid again bypassing the cache.
The device is known to exist and contain a vfat file system, just not
whether it is a FAT16 or FAT32 file system, so can't be a non-existent
floppy device and won't hang.
Bug 743181 - Add unpartitioned drive read-write support
When writing "loop" partition table over the top of some whole disk
device file system types GParted continued to show those whole disk
device file systems rather than the virtual unknown partition from the
"loop" partition table.
This affected btrfs, jfs, reiser4 and reiserfs. It occurred because of
several factors:
1) Libparted only zeroed the first and last 9.5 KiB (assuming 512 byte
sectors) of the device before writing a new partition table. See
ped_disk_clobber().
2) These file systems have their super blocks and therefore signatures
after the first 9.5 KiB.
3) Whole disk device file system detection is performed using blkid
before checking for a libparted "loop" partition table. See
GParted_Core::set_devices_thread().
Ref:
libparted 3.2: disk.c:ped_disk_clobber()
http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/disk.c?id=v3.2#n302
Fix by always erasing any possible file system signatures on the device
before creating a new "loop" partition table.
NOTE:
This is typically taking up to 0.5 seconds in my testing on a 5400 RPM
hard drive, during which time the GParted UI is hung and the create
partition table dialog shows the apply button pressed but no other
progress indication.
Bug 743181 - Add unpartitioned drive read-write support
get_device_and_disk() basically calls libparted to get a PedDevice
object representing a disk device and a PedDisk object representing a
partition table. Re-implement get_device_and_disk() using two separate
functions, get_device() and get_disk(), to get one of these objects
each.
No functionality changes with this commit. It enables future commits to
incrementally add support for whole disk devices into GParted without
needing libparted to recognise the contents and create a virtual "loop"
partition table.
Bug 743181 - Add unpartitioned drive read-write support
Need to be able to take different actions in the GParted_Core partition
manipulation methods and in Win_GParted UI methods to deal with
libparted supported partitions or whole disk devices without a partition
table. Add boolean whole_device to the partition object and set
appropriately to allow for this.
Bug 743181 - Add unpartitioned drive read-write support
Function Utils::get_filesystem_kernel_name() returns the name of the
file system as needed for use in the mount command:
mount -t TYPE DEVICE DIR
Needed because the kernel / mount name is 'hfsplus' where as libparted /
GParted, as reported by Utils::get_filesystem_string(), calls it 'hfs+'.
So far just added debugging when mounting a file system to test the
function works.
# ./gartedbin
======================
libparted : 2.1
======================
DEBUG: (hfsplus) # mount -v /dev/sdb5 "/mnt/5"
DEBUG: (nilfs2) # mount -v /dev/sdb1 "/mnt/1"
Bug 742741 - Nilfs2 file system is unusable on RHEL/CentOS 6
For file systems which libparted recognises, when found on the whole
disk device, it reports with partition table "loop" and a partition
covering the whole disk. GParted duly displays this to the user.
For file systems which libparted doesn't recognise it reports
"unrecognised disk label". As of the latest libparted 3.2, these file
system aren't recognised and can't currently be shown when on the whole
disk device:
BitLocker, Crypt LUKS, exFAT, F2FS, LVM2 Physical Volume,
Linux Software RAID, ReFS, Reiser 4
So only when libparted doesn't recognise a file system on the whole disk
device and GParted does, either via blkid or it's internal code, display
this with partition table "none".
Bug 741430 - GParted cannot recognise LVM signature on unpartitioned
drive
Move code which queries the file system label and UUID of a partition
into a separate helper function.
Bug 741430 - GParted cannot recognise LVM signature on unpartitioned
drive
Refactor GParted internal file system signature detection to remove code
duplication. There were 5 separate copies of code to: allocate a
buffer, open, read and close the device, free the buffer and compare the
signature.
Bug 741430 - GParted cannot recognise LVM signature on unpartitioned
drive
Embedded devices (Android) use GPT partition names to identify
partitions, instead of file system labels. Add support for viewing and
changing them.
As partition names are used to provide unique identification they are
never copied when copying the contents of one partition to another.
Note that GNU/Linux uses file system labels, UUIDs or device names for
identification during the boot process and afterwards so while partition
names can be used, they are optional and purely for user information.
Bug 741424 - Add support for GPT partition names
This and the following few commits rename variables, methods, classes,
etc from *label_partition* to *label_filesystem* so that the code also
reflects that it is the label of the file system that is being modified
and to separate it from the name partition operation about to be added.
enum OPERATION_LABEL_PARTITION -> OPERATION_LABEL_FILESYSTEM
Bug 741424 - Add support for GPT partition names
Only recognises ReFS file system. No other actions are supported.
Requires blkid from util-linux >= 2.24.
Bug #738471 - ReFS file system is not recognised
Helper to check whether a recognised file system type is supported by
GParted or not. Supported means there is an implementation class and
will appear in the File System Support dialog.
Make supported_filesystem() a static member function so that it can be
called without a class object so that GParted_Core::GParted_Core()
initialiser isn't called multiple times. This requires FILESYSTEM_MAP
to become a static member variable too.
Bug #738471 - ReFS file system is not recognised
get_filesystem_object() takes a constant reference to a FILESYSTEM, but
FILESYSTEM is just an enumeration. So that's a pointer to a constant
int. Just pass by value instead.
RHEL / CentOS 5.6 and later officially support ext4 file system [1].
From RHEL / CentOS 5.3 ext4 file system was included as a technology
preview. Ext4 file system tools are in a separate package e4fsprogs,
using uniquely named commands. The standard e2fsprogs commands only
support ext2 and ext3 file systems.
# mkfs.ext4 /dev/sdb3
# tune2fs -l /dev/sdb3
tune2fs 1.39 (29-May-2006)
tune2fs: Filesystem has unsupported feature(s) while trying to open /dev/sdb3
Couldn't find valid filesystem superblock.
# echo $?
1
# tune4fs -l /dev/sdb3
tune4fs 1.41.12 (17-May-2010)
Filesystem volume name: <none>
Last mounted on: <not available>
Filesystem UUID: ba4a9d58-7728-4b47-8a90-80e772615637
Filesystem magic number: 0xEF53
Filesystem revision #: 1 (dynamic)
Filesystem features: has_journal ext_attr resize_inode dir_index filetype extent flex_bg sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
...
For ext4 only, search for the e4fsprogs specific commands first and the
standard e2fsprogs commands second.
[1] RHEL 5.6 Release Notes, 5. Filesystems and Storage
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/5/html/5.6_Release_Notes/ar01s05.html
Bug #738706 - GParted doesn't support ext4 on RHEL/CentOS 5.x
The device combobox was getting drawn blank, then getting drawn again
with the selected device. This was happening because at the start of
Win_GParted::refresh_combo_devices() the GTK model behind the combobox,
liststore_devices, was cleared, changing the active item, causing the
combobox to get redrawn empty. After the GTK model had been repopulated
the active item was reset causing the comboxbox to get redrawn again,
now showing the selected device. Call flow:
Win_GParted::refresh_combo_devices()
liststore_devices->clear()
//Gtk::Combobox emits signal_change. Registered callbacks
//called.
Win_GParted::combo_devices_changed()
Win_GParted::Refresh_Visual()
...
...
combo_devices.set_active(current_device);
//Gtk::Combobox emits signal_change. Registered callbacks
//called.
Win_GParted::combo_devices_changed()
Win_GParted::Refresh_Visual()
...
This has always been the case, since the device combobox was first added
to GParted before version 0.1 by commit:
3a4b43e0ad
replaced deprecated OptionMenu with ComboBox ...
Fix by temporarily blocking the devices comboxbox from emitting
signal_changed while the GTK model behind the combobox is recreated.
However, since automatic selection of the largest free space was added
[1] in GParted 0.15.0, a more noticeable flashing redraw issue was
caused in which the partition graphic and partition list were both drawn
blank then redrawn fully populated. Some distributions were not
affected by this at all, some only experienced a single flash and others
suffered from two or more flashing redraws. Some affected
distributions: CentOS 5.10, 6.5, 7.0, Debian 6, Fedora 14, 19, 20,
Ubuntu 13.10, Xubuntu 14.04 LTS. Did not occur on Kubuntu 12.04 LTS.
[1] 5b53c12f6e
Select largest unallocated partition by default (#667365)
Bug #696149 - Double refresh of display introduced with default
unallocated space
Remove HAVE_LIBPARTED_3_1_0_PLUS definition and replace Autoconf
check for libparted >= 3.1
Currently uses custom check which compiles an executable to check for
libparted version >= 3.1 to determine the availability of the library
parted-fs-resize and the need to include the header <parted/filesys.h>.
Change to use a direct Autoconf check to determine the availability of
the ped_file_system_resize() function in the parted-fs-resize library.
Remove inclusion of the header <parted/filesys.h> as it has always been
included via <parted/parted.h>, at least as far back as parted 1.8.0,
and even in parted 3.0 when ped_file_system_resize() and libparted file
system resize capability didn't exist.
Bug #734718 - Update Autoconf version specific libparted checks and
defines to feature specific ones
While one partition is busy, reformat another partition from the command
line. Afterwards parted/libparted still detects the original file
system and GParted shows errors from the file system specific tools
reporting the new file system doesn't exist. Only limitation is that
the new new file system must be recognised by libparted (or by GParted's
fallback file system signature detection).
Case #1, File system reformatting:
# parted /dev/sdb print
Model: ATA SAMSUNG SSD UM41 (scsi)
Disk /dev/sdb: 8012MB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Number Start End Size Type File system Flags
1 1049kB 2149MB 2147MB primary ext2
2 2149MB 4296MB 2147MB primary ext2
# mount | fgrep sdb
/dev/sdb1 on /mnt/1 type ext2 (rw)
# mkfs.xfs -f /dev/sdb2
# blkid /dev/sdb2
/dev/sdb2: UUID="c31823a2-b81b-46fa-8246-0a59695e4834" TYPE="xfs"
# parted /dev/sdb print
Model: ATA SAMSUNG SSD UM41 (scsi)
Disk /dev/sdb: 8012MB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Number Start End Size Type File system Flags
1 1049kB 2149MB 2147MB primary ext2
2 2149MB 4296MB 2147MB primary ext2
# e2label /dev/sdb2
e2label: Bad magic number in super-block while trying to open /dev/sdb2
Couldn't find valid filesystem superblock.
# dumpe2fs /dev/sdb2
dumpe2fs 1.41.12 (17-May-2010)
dumpe2fs: Bad magic number in super-block while trying to open /dev/sdb2
Couldn't find valid filesystem superblock.
Case #2, Removing device from multi-device btrfs:
# btrfs filesystem show /dev/sdb1
Label: none uuid: a05db434-efd5-4e8c-902f-05f89a88b610
Total devices 2 FS bytes used 156.00KB
devid 2 size 2.00GB used 512.00MB path /dev/sdb2
devid 1 size 2.00GB used 240.75MB path /dev/sdb1
# mount /dev/sdb1 /mnt/1
# btrfs device delete /dev/sdb2
# btrfs filesystem show /dev/sdb1
Label: none uuid: a05db434-efd5-4e8c-902f-05f89a88b610
Total devices 1 FS bytes used 92.00KB
devid 1 size 2.00GB used 714.25MB path /dev/sdb1
# btrfs filesystem show /dev/sdb2
and GParted reports this error for partition /dev/sdb2:
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 btrfs
file system support: btrfs-tools.
This is another case of libparted reading from the whole disk device
(/dev/sdb) yet the file system tools use the partition specific block
device (/dev/sdb2), and the Linux buffer cache not providing cache
coherency. Previous scenario was fixed with:
797f0b8eeb
Flush device after wiping a file system (#688882)
This affects libparted 2.0 to 3.1 inclusive and is fixed by:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=fb99ba5ebd0dc34204fc9f1014131d5d494805bc
Revert "linux-commit: do not unnecessarily open partition device nodes"
Fix by calling ped_device_sync() to guarantee cache coherency for each
device during scanning.
Bug #723842 - GParted resizes the wrong filesystem (does not pass the
devid to btrfs filesystem resize)
With linux 3.5 and later, the device used to mount a btrfs file system
is updated in /proc/mounts when the previous mounting device is removed
from the file system. Most recent distributions make /etc/mtab a
symbolic link to /proc/mounts. However some still have /etc/mtab as a
plain file only updated by mount and umount, thus showing the old device
name which is no longer part of the file system.
On Ubuntu 13.10, which has /etc/mtab as a plain file managed by mount
and umount:
# mkfs.btrfs /dev/sdb1
# mount /dev/sdb1 /mnt/1
# btrfs device add /dev/sdb2 /mnt/1
# btrfs device delete /dev/sdb1 /mnt/1
# sync
# btrfs filesystem show /dev/sdb1
# btrfs filesystem show /dev/sdb2
Label: none uuid: e47775a6-e5ad-4fb4-9ea4-1570aa5b4009
Total devices 2 FS bytes used 28.00KB
devid 2 size 2.00GB used 272.00MB path /dev/sdb2
# fgrep btrfs /proc/mounts
/dev/sdb2 /mnt/1 btrfs rw,relatime,space_cache 0 0
# ls -l /etc/mtab
-rw-r--r-- 1 root root 842 Apr 15 19:41 /etc/mtab
# fgrep btrfs /etc/mtab
/dev/sdb1 /mnt/1 btrfs rw 0 0
This causes GParted to report /dev/sdb1 as busy and mounted at /mnt/1
when it is no longer mounted. This effects recent releases of Ubuntu,
13.04, 13.10 and 14.04.
Either /etc/mtab is a symlink and is identical to /proc/mounts or
/etc/mtab is a plain file with wrong information. Fix by not reading
mounted file systems from /etc/mtab.
However old distributions only contain 'rootfs' and '/dev/root' device
names for the / (root) file system with '/dev/root' being a block device
rather than a symlink to the true device. For example from CentOS 5.x:
# fgrep ' / ' /proc/mounts
rootfs / rootfs rw 0 0
/dev/root / ext3 rw,data=ordered 0 0
# ls -l /dev/root
brw------- 1 root root 8, 3 Jun 4 2013 /dev/root
This prevents identification, and therefore busy detection, of the
device containing the / (root) file system. Used to read /etc/mtab to
get the root file system device name.
# fgrep ' / ' /etc/mtab
/dev/sda3 / ext3 rw 0 0
# ls -l /dev/sda3
brw-r----- 1 root disk 8, 3 Jun 4 2013 /dev/sda3
As per commit:
409096f739
improved scanning for root mountpoint (/) ...
but, as discussed above, this contains an out of date device name after
the mounting device has been dynamically removed from a multi-device
btrfs, thus identifying the wrong device as busy. Instead fall back to
reading mounted file systems from the output of the mount command, but
only when required.
# mount | fgrep ' / '
/dev/sda3 on / type ext3 (rw)
Bug #723842 - GParted resizes the wrong filesystem (does not pass the
devid to btrfs filesystem resize)
Linux can only show a single device name in /proc/mounts and /etc/mtab
for each mounted btrfs, even if it is a multi-device file system. So
GParted only shows a mount point for one of the devices in the btrfs, no
matter how many devices are part of the file system.
# mkfs.btrfs /dev/sdb1 /dev/sdb2
# btrfs filesystem show /dev/sdb1
Label: none uuid: 36eb51a2-2927-4c92-820f-b2f0b5cdae50
Total devices 2 FS bytes used 156.00KB
devid 2 size 2.00GB used 512.00MB path /dev/sdb2
devid 1 size 2.00GB used 240.75MB path /dev/sdb1
# mount /dev/sdb1 /mnt/1
# grep btrfs /proc/mounts
/dev/sdb1 /mnt/1 btrfs rw,seclabel,relatime,ssd,space_cache 0 0
GParted only shows the mount point for /dev/sdb1 as /mnt/1, but nothing
for /dev/sdb2.
Make GParted report the same mount point for all devices included in a
multi-device btrfs file system.
Add btrfs specific get_mount_device() method to report the mounting
device, if any, for the btrfs file system in the occupying the device in
question. Uses the existing cache of btrfs file system device
membership. Also extract common code from GParted_Core::
set_mountpoints() into set_mountpoints_helper().
Bug #723842 - GParted resizes the wrong filesystem (does not pass the
devid to btrfs filesystem resize)
Busy detection of file systems works by checking if the device is
mounted (appears in the mount_info map). For a multi-device btrfs file
system this will only report one of the devices as busy, not all of
them.
# btrfs filesystem show /dev/sdb1
Label: none uuid: 36eb51a2-2927-4c92-820f-b2f0b5cdae50
Total devices 2 FS bytes used 156.00KB
devid 2 size 2.00GB used 512.00MB path /dev/sdb2
devid 1 size 2.00GB used 240.75MB path /dev/sdb1
# mount /dev/sdb1 /mnt/1
# grep btrfs /proc/mounts
/dev/sdb1 /mnt/1 btrfs rw,seclabel,relatime,ssd,space_cache 0 0
GParted will only report /dev/sdb1 as busy, but not /dev/sdb2.
Add btrfs specific is_busy() method which reports the device as busy if
any of the devices in the btrfs file system are mounted. This uses a
cache which maps device membership in all btrfs file systems. The cache
is cleared on GParted refresh and incrementally populated as each btrfs
partition is checked for busy status.
WARNING:
Removal of the mounting device from a btrfs file system makes it
impossible to determine whether the file system is mounted or not for
linux <= 3.4. This is because /proc/mounts continues to show the old
device which is no longer a member of the file system.
# btrfs device delete /dev/sdb1 /mnt/1
# sync
# grep btrfs /proc/mounts
/dev/sdb1 /mnt/1 btrfs rw,seclabel,relatime,ssd,space_cache 0 0
# btrfs filesystem show /dev/sdb1
# btrfs filesystem show /dev/sdb2
Label: none uuid: 36eb51a2-2927-4c92-820f-b2f0b5cdae50
Total devices 1 FS bytes used 28.00KB
devid 2 size 2.00GB used 1.02GB path /dev/sdb2
Fixed in linux 3.5 by commit:
Btrfs: implement ->show_devname
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9c5085c147989d48dfe74194b48affc23f376650
Bug #723842 - GParted resizes the wrong filesystem (does not pass the
devid to btrfs filesystem resize)
Add static member function GParted_Core::is_dev_mounted() so that other
modules can determine if a particular partition contains a mounted file
system or not.
Make it a static member function so that it can be called without
needing the gparted_core object. Extend to make the group of
manipulated variables (mount_info, fstab_info) and manipulating
functions (init_maps(), read_mountpoints_from_file(),
read_mountpoints_from_file_swaps(), get_all_mountpoints()) static too.
Bug #723842 - GParted resizes the wrong filesystem (does not pass the
devid to btrfs filesystem resize)
GParted's primary inbuilt busy detection method is "is the partition
mounted?". A custom method is used for LVM2 PV because its not a
mounted file system.
Make busy detection selectable per file system type.
.fs.busy = FS::NONE (default)
No busy detection.
.fs.busy = FS::GPARTED
Use internal GParted method which checks if the partition is
mounted.
.fs.busy = FS:EXTERNAL
Call the file system type's member function is_busy().
LVM2 PV busy detection changes from a special case to just electing to
call the lvm2_pv::is_busy() method. Linux Software RAID remains a
special case because it's only recognised, but not otherwise supported.
Bug #723842 - GParted resizes the wrong filesystem (does not pass the
devid to btrfs filesystem resize)
Fragment of debugging and valgrind output:
D: tid=2193 main()
...
D: tid=2202 GParted_Core::set_devices_thread()
...
D: tid=2202 Utils::execute_command(command="dumpe2fs -h /dev/sda1", output, error, use_C_locale=1)
D: tid=2202 this=0x13fef4a0 PipeCapture::PipeCapture()
D: tid=2202 this=0x13fef4f0 PipeCapture::PipeCapture()
D: tid=2202 this=0x13fef4a0 PipeCapture::connect_signal()
D: sourceid=77
D: tid=2202 this=0x13fef4f0 PipeCapture::connect_signal()
D: sourceid=78
D: tid=2193 data=0x13fef4a0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4a0 PipeCapture::OnReadable()
D: signal_update.emit()
D: return true
D: tid=2193 data=0x13fef4f0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4f0 PipeCapture::OnReadable()
D: signal_update.emit()
D: return true
D: tid=2193 data=0x13fef4a0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4a0 PipeCapture::OnReadable()
D: signal_update.emit()
D: return true
D: tid=2193 data=0x13fef4f0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4f0 PipeCapture::OnReadable()
D: signal_eof.emit()
D: return false
D: (!rc) &(pc->sourceid)=0x13fef518
D: tid=2193 data=0x13fef4a0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4a0 PipeCapture::OnReadable()
D: signal_update.emit()
D: return true
D: tid=2193 data=0x13fef4a0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4a0 PipeCapture::OnReadable()
D: signal_update.emit()
D: return true
D: tid=2193 data=0x13fef4a0 PipeCapture::_OnReadable()
D: tid=2193 this=0x13fef4a0 PipeCapture::OnReadable()
D: signal_eof.emit()
D: tid=2202 this=0x13fef4f0 PipeCapture::~PipeCapture()
D: sourceid=0
D: tid=2202 this=0x13fef4a0 PipeCapture::~PipeCapture()
D: sourceid=77
D: return false
D: (!rc) &(pc->sourceid)=0x13fef4c8
==2193== Thread 1:
==2193== Invalid write of size 4
==2193== at 0x490580: GParted::PipeCapture::_OnReadable(_GIOChannel*, GIOCondition, void*) (PipeCapture.cc:56)
==2193== by 0x38662492A5: g_main_context_dispatch (gmain.c:3066)
==2193== by 0x3866249627: g_main_context_iterate.isra.24 (gmain.c:3713)
==2193== by 0x3866249A39: g_main_loop_run (gmain.c:3907)
==2193== by 0x3D7FD45C26: gtk_main (gtkmain.c:1257)
==2193== by 0x469743: GParted::GParted_Core::set_devices(std::vector<GParted::Device, std::allocator<GParted::Device> >&) (GParted_Core.cc:155)
==2193== by 0x4A78F1: GParted::Win_GParted::menu_gparted_refresh_devices() (Win_GParted.cc:1259)
==2193== by 0x4A7886: GParted::Win_GParted::on_show() (Win_GParted.cc:1253)
==2193== by 0x3D82B2009C: Gtk::Widget_Class::show_callback(_GtkWidget*) (widget.cc:3855)
==2193== by 0x3867210297: g_closure_invoke (gclosure.c:777)
==2193== by 0x3867221B86: signal_emit_unlocked_R (gsignal.c:3516)
==2193== by 0x386722A0F1: g_signal_emit_valist (gsignal.c:3330)
==2193== Address 0x13fef4c8 is not stack'd, malloc'd or (recently) free'd
==2193==
PipeCapture.cc (with debugging):
46 gboolean PipeCapture::_OnReadable( GIOChannel *source,
47 GIOCondition condition,
48 gpointer data )
49 {
50 std::cout << "D: tid=" << (long int)syscall(SYS_gettid) << " data=" << data << " PipeCapture::_OnReadable()" << std::endl;
51 PipeCapture *pc = static_cast<PipeCapture *>(data);
52 gboolean rc = pc->OnReadable( Glib::IOCondition(condition) );
53 if (!rc)
54 {
55 std::cout << "D: (!rc) &(pc->sourceid)=" << &(pc->sourceid) << std::endl;
56 pc->sourceid = 0;
57 }
58 return rc;
59 }
The use after free across threads only happens when an external program
is being executed from a thread other than the main() thread. This is
because by default glib registered callbacks are run by the glib main
loop, which is only called from the main() thread with Gtk::Main::run().
Event sequence:
tid=2193 tid=2202
main()
...
GParted_Core::set_devices()
Glib::Thread::create(... set_devices_thread ...)
Gtk::Main::run() GParted_Core::set_devices_thread()
...
Utils::execute_command("dumpe2fs ... /dev/sda1" ...)
Glib::spawn_async_with_pipes()
PipeCapture outputcapture(out, output)
outputcapture.connect_signal()
//Glib main loop runs callback
PipeCapture::_OnReadable()
pc->OnReadable()
//output read
signal_update.emit()
return true
...
//Glib main loop runs callback
PipeCapture::_OnReadable()
pc->OnReadable()
//eof reached
[1] signal_eof.emit()
return status.exit_status
[2] PipeCapture::~PipeCapture()
[3] return false
[4] pc->sourceid = 0
What is happening is that the PipeCapture destructor [2] is running in
the set_devices_thread() thread and freeing the object's memory as soon
as signal_eof.emit() [1] has been called. Then signal_eof.emit()
returns back to OnReadable() which then returns false [3] back to the
_OnReadable() callback function which then assigns 0 to sourceid member
variable [4] in the already freed object, detected by valgrind as:
Invalid write of size 4
at ... GParted::PipeCapture::_OnReadable(...) (PipeCapture.cc:56)
This is happening because PipeCapture member variable sourceid is being
saved, in a different thread, just so the _OnReadable() callback can be
removed. However a glib IOChannel callback, type GIOFunc(), returning
false will be automatically removed.
GLib Reference Manual 2.26 / IO Channels
https://developer.gnome.org/glib/2.26/glib-IO-Channels.html#GIOFunc
GIOFunc()
Returns : the function should return FALSE if the event source
should be removed
Therefore fix by just not saving the event sourceid at all, and not
calling g_source_remove() to manually remove the callback, but instead
letting glib automatically remove the callback when it returns false.
Bug #731752 - Write after free cross thread race in
PipeCapture::_OnReadable()
OperationDetail was storing its children in a std::vector. This means they
can be moved around in memory arbitrarily, going through indeterminate
lifetimes. This is generally a bad thing for any non trivial object and
in the case of OperationDetail, it created havoc with the way it maintains
pointers between parent/child objects for signal connections. It will now
keep only pointers to children in a std::vector instead, so their lifetime
can be controlled, fixing various crashes.
Bug 729139 - Refactor OperationDetail to address random behavior
As part of the work on bug 652044 - uses deprecated APIs, selectable
vertical alignment was defaulted to ALIGN_CENTER for all labels. The
relevant commits can be viewed in comment 26 of said bug report.
https://bugzilla.gnome.org/show_bug.cgi?id=652044#c26
For multi-line labels a vertical ALIGN_CENTER value is not
consistently aesthetically pleasing. This becomes obvious when a
single-line heading label is paired with a multi-line value label.
To improve the aesthetics, a vertical alignment of ALIGN_TOP is
preferred.
Hence re-add the ability to optionally specify a vertical alignment for
labels. If a yalign value is not specified a default vertical alignment
of ALIGN_CENTER is used.
Make the dialog resizable, add a vertical scrollbar to the information
and messages section, and set the initial height to ensure the dialog
fits entirely on an 800x600 screen.
A default height is required because some window managers, such as
fluxbox used in GParted Live, only permit resizing the height by using
the bottom corners of the dialog. If the dialog is too large for the
screen then the user would not be able to resize it.
Note that two default initial heights are used in an effort to minimize
the amount of extra whitespace.
Bug 690542 - Partition Information Dialog Warning not readable
The code used to unnecessarily destroy and re-create the file system
objects on every scan for file system support tools.
Instead only create the file system objects once and just call each
object's get_filesystem_support() method on each rescan.
Prior to commit:
1f3b11748e
Remove GParted_Core::p_filesystem (#683149)
set_proper_filesystems() used to set GParted_Core::p_filesystem member
variable to one of the FileSystem objects, but that was just treating it
like a local variable. After the commit local variables named
p_filesystem were used where required and set_proper_filesystem() became
a function which did nothing other than call get_filesystem_object().
Now remove set_proper_filesystem() altogether and use
get_filesystem_object() in its place.
Use e2image features added in e2fsprogs 1.42.9 to move/copy
an ext[234] file system more efficiently by skipping unused blocks.
Fall back to the internal copy algorithm if e2image is not found
or does not support move/copy.
Bug #721516 - Use e2image to move/copy ext[234] filesystems
Only recognises partitions containing BitLocker Disk Encryption content.
No other actions are supported.
Bug #723232 - BitLocker Disk Encryption not recognised
Restore the order of the source files so that they are once again
compiled in order A-Z, a-z. Order is obtained with:
fgrep .cc src/Makefile.am | LANG=C sort
fgrep .h include/Makefile.am | LANG=C sort
Make the dialog resizable, add a vertical scrollbar and set the minimum
(and therefore initial) height to 500 pixels. This is so that the
dialog entirely fits on an 800x600 screen, thus allowing the rescan
button to be pressed.
100 pixel difference is to account for the size of the top and bottom
GNOME 2 panels and two sets of title bars. Two sets of title bars
because the window manager tries to place the top of dialog title bars
in line with the bottom of the main window title bar.
Bug #342682 - too much information in 'features' dialog
Make the legend always shown, ready for when the dialog is resizable.
Change the widget containing the legend from an expander to a frame
widget. Set the frame to be borderless using a bold label as
recommended in the GNOME Human Interface Guidelines 2.2.1 / Controls /
Frames and Separators.
https://developer.gnome.org/hig-book/2.32/controls-frames.html.en
Bug #342682 - too much information in 'features' dialog
This is part of parent bug:
Bug #721455 - Obsolete info in license text on multiple modules
and GNOME Goal:
https://wiki.gnome.org/Initiatives/GnomeGoals/Proposals
* verify all source files to make sure they have a license and a
copyright, and that both are up-to-date
Bug #721565 - License text contains obsolete FSF postal address
In the Create Partition Table dialog display the entries in the combobox
in order.
Previously the default of MSDOS or GPT was moved to the first item in
the combobox. Now the partition table types remain in order with just
either MSDOS or GPT being selected as as the default as required.
The partition table types are displayed in the order supplied by
libparted, which is alphabetic except with "loop" last.
Bug #711098 - Default partition table can not handle > 2 TiB disks
MSDOS partition table is limited to addressing 2^32 sectors, limiting
disks using 512 byte sectors to 2 TiB in size. Fdisk reports the
following warning on disks 2 TiB and larger.
# truncate -s 2T /var/tmp/loop-2T
# losetup /dev/loop0 /var/tmp/loop-2T
# fdisk /dev/loop0
WARNING: The size of this disk is 2.2 TB (2199023255552 bytes).
DOS partition table format can not be used on drives for volumes
larger than (2199023255040 bytes) for 512-byte sectors. Use parted(1) and GUID
partition table format (GPT).
(Fdisk arguably reports this warning one sector too early. Anyway for
safety and consistency GParted will use this limit too). Continue to
use MSDOS as the default partition table type for disks smaller than 2
TiB and use GPT as the default for disks 2 TiB and larger. This
maximises compatibility.
Also remove the advanced expander and always show the partition table
list box.
Bug #711098 - Default partition table can not handle > 2 TiB disks
Many file systems are capable of growing while mounted, and a few can
even shrink. This support must be explicitly enabled at configure time
with the --enable-online-resize flag and depends on a patched libparted.
Also requires kernel >= 3.6 for partition resizing, even if the
partition is in use (BLKPG_RESIZE_PARTITION).
Thanks to Mike Fleetwood for double check mark idea instead of a second
column to show the online grow/shrink.
Bug #694622 - Add support for online resize
Read the contents of /proc/mdstat file to determine if a device is a
member of of an active RAID array.
$ cat /proc/mdstat
Personalities : [raid1]
md1 : active raid1 sda1[2] sdb1[3]
524224 blocks super 1.0 [2/2] [UU]
md2 : active raid1 sdb2[2] sda2[3](F)
5238720 blocks super 1.1 [2/1] [U_]
md3 : active raid1 sdb3[1]
10477440 blocks super 1.1 [2/1] [_U]
bitmap: 1/1 pages [4KB], 65536KB chunk
md4 : inactive sda4[0](S)
1048564 blocks super 1.2
unused devices: <none>
There are 5 example Linux Software RAID arrays, md1 to md5. All are
RAID1 mirrors with 2 members, in various states.
Array Members Status
md1 sda1, sdb2 Fully operational.
md2 sda2, sdb2 Member sda2 marked as faulty. (Device sda2 is
still in use).
md3 sda3, sdb3 Member sda3 has been removed. (Device sda3 is
not in use).
md4 sda4, sdb4 Incremental start of member sda4 only. (Neither
member devices is in use).
md5 sda5, sdb5 Array stopped. (Neither member device is in
use).
Also disable "Unmount" in the partition menu for active RAID array
members.
Bug #709640 - Linux Swap Suspend and Software RAID partitions not
recognised
Recognise in kernel, Linux Swap Suspend partitions. (When hibernated
the kernel write the RAM out to swap space and changes the magic string
from swap space to suspend). Recognition required either
libparted >= 1.8.8.1 or blkid from util-linux >= 2.15 or before that
blkid from e2fsprogs >= 1.39.
Recognise Linux Software RAID partitions. Recognition requires blkid
from util-linux >= 2.15.
Example:
# blkid /dev/sda10 /dev/sda11
/dev/sda10: ... TYPE="swsuspend"
/dev/sda11: ... TYPE="linux_raid_member"
# parted /dev/sda print
Model: ATA SAMSUNG HM500JI (scsi)
Disk /dev/sda: 500GB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Number Start End Size Type File system Flags
...
10 361GB 362GB 1074MB logical swsusp
11 362GB 363GB 1074MB logical raid
Bug #709640 - Linux Swap Suspend and Software RAID partitions not
recognised
Step 3 of 3:
Now that all label widgets are created with ALIGN_LEFT and ALIGN_CENTER
alignment remove the x_align and y_align parameters from mk_label() and
always use this alignment. Also specify this alignment via floats
rather than enumerators, one of which was deprecated.
Bug #652044 - uses deprecated APIs
Add concept of cursor position within the current line, separate from
the end of the buffer. This is so that programs which output a text
progress bar using backspace, such as resize2fs -p, are displayed
correctly.
Bug #709276 - Percentage indicator for subcommand
For active swap space read the usage from /proc/swaps. (Linux kernel
uses units of 1 KiB). By definition inactive swap space is 100% free.
$ cat /proc/swaps
Filename Type Size Used Priority
/dev/sda2 partition 5242876 430552 -1
Always set fs.read = FS::EXTERNAL even if /proc/swaps doesn't exist so
that an attempt is made to open the file generating a specific error, in
addition to the generic error.
open("/proc/swaps", O_RDONLY): No such file or directory
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 linux-
swap file system support: util-linux.
Closes Bug #708107 - Usage of swap space is not reported
Currently the btrfs command outputs figures to 2 decimal places followed
by an SI multiplier, e.g. 1.00GB.
This patch to btrfs-progs has been included in the integration
repository and will likely be included in the official btrfs-progs
repository at some point. It changes btrfs-progs to use IEC
multipliers, e.g. 1.00GiB. In fact multipliers already aren't used for
figures less than 1024.
[PATCH] btrfs-progs: use IEC units for size
http://permalink.gmane.org/gmane.comp.file-systems.btrfs/26888https://patchwork.kernel.org/patch/2825841/
Make GParted capable of also accepting IEC prefix multipliers, just "B"
for bytes and no multiplier, as well as an optional space between the
number and multiplier. Therefore accept values like these:
1.00GB 1.00 GB
1.00GiB 1.00 GiB
1073741824B 1073741824 B
1073741824
Closes Bug #706914 - Prepare for btrfs tools using IEC prefix
multipliers
With recent btrfs-progs, GParted failed to format a btrfs file system
over the top of an existing one. Make btrfs failed with this error:
# mkfs.btrfs /dev/sdb1
...
/dev/sdb1 appears to contain an existing filesystem (btrfs).
Use the -f option to force overwrite.
With this commit to btrfs-progs on 2013-02-19, mkfs.btrfs checks for
existing file system signatures, including all mirror copies of btrfs
super blocks, before writing to the partition.
http://git.kernel.org/cgit/linux/kernel/git/mason/btrfs-progs.git/commit/?id=2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6
btrfs-progs: require mkfs -f force option to overwrite filesystem or partition table
Make GParted clear all the mirror copies of the btrfs file system super
blocks as erase_filesystem_signatures() is intended to prevent detection
of old signatures. This also avoids having to determine if the -f
option to mkfs.btrfs is available before trying to use it.
Closes Bug #705426 - Formatting Existing BTRFS Partition as BTRFS Fails
Because mkfs.btrfs Is Not Run with "-f"
Dosfstools >= 3.0.18, released June 2013, renamed the programs thus:
dosfslabel becomes fatlabel,
dosfsck becomes fsck.fat,
and mkdosfs becomes mkfs.fat.
Dosfstools creates symbolic links for the old names for backward
compatibility, but unfortunately the Debian dosfstools-3.0.22-1
(experimental) package doesn't include those symbolic links. This
causes create, check and read unmounted FAT16/32 file systems to not be
supported.
Make GParted look for the new names first and the old names second.
Closes Bug #704629 - Program name changes in dosfstools 3.0.18+ break
FAT16/32 support
It was difficult to retrieve whether a filesystem's label can be set on reformat.
The read_label flag can't be used as it decides whether to use the logic in the filesystem class
rather than the fallback in GParted::set_device_partitions, to determine the label of a partition.
The create_with_label flag is NONE for file systems that we cannot format with a
label (or that we cannot format at all).
The value is usually EXTERNAL for file systems that we can format with a label.
The read-only functionality is unused and the readonly parameter is
always false in copy_filesystem() and copy_blocks() methods. This has
been the case since the copy simulation was dropped by commit:
b9b4b2e55d
Remove simulation pass ( read test ) on move
Include guards need to be unique within GParted code and all included
library header files.
http://en.wikipedia.org/wiki/Include_guard#Difficulties
Use this model for all include guards:
#ifndef GPARTED_FILE_NAME_H
#define GPARTED_FILE_NAME_H
...
#endif /* GPARTED_FILE_NAME_H */
Closes Bug #539297 - Make include guards unique
It was possible to make GParted crash by adding a label, check or new
UUID operation and then applying the operation before the view of
pending operations had finished fully opening. The operation would be
successfully applied but GParted would crash afterwards.
The fault was that Add_Operation() still enabled the Undo and Apply
buttons and processed the GTK event loop before merging the list of
pending operations. Faulty code flow went like this:
activate_*()
Add_Operation()
Add operation to the operations[] vector
Enable Undo and Apply buttons
Refresh_Visual()
Process GTK event loop
Process Apply button callback applying operations,
refreshing display and clearing operations[] vector
Merge operations in the operations[] vector
<< Core dump here >>
Merge_Operations()
Refresh_Visual()
This faulty code flow came about when merging of operations was added
and it didn't appreciate that the operations[] vector could have been
processed and cleared by Add_Operations() before the merge step.
Relevant commit:
b10349ae37
Merge overlapping operations (#438573)
Fragment of code in the label operation case:
2454 void Win_GParted::activate_label_partition()
2455 {
...
2472 Add_Operation( operation ) ;
2473
2474 // Verify if the two operations can be merged
2475 for ( unsigned int t = 0 ; t < operations .size() - 1 ; t++ )
2476 {
2477 if ( operations[ t ] ->type == OPERATION_LABEL_PARTITION )
2478 {
2479 if ( Merge_Operations( t, operations .size() - 1 ) )
2480 break;
2481 }
2482 }
Commentary in the crashing label operation case:
2472 The pending operation was already applied when Add_Operation()
returned resulting in the operations[] vector being cleared
setting its size to 0.
2475 The return type of operations.size() is an unsigned integral, so
the upper limit of the for loop is t < 0UL - 1. Assuming a
32-bit machine that's t < 4294967295.
2477 operations[] vector is access from out of bounds offset 0
upwards until unallocated memory is accessed resulting in a core
dump.
Fix this by not enabling the Undo and Apply buttons and processing the
GTK event loop until after merging of operations has been performed.
Fixed code flow goes like this:
activate_*()
Add_Operation()
Add operation to the operations[] vector
Merge operations in the operations[] vector
Merge_Operations()
show_operationslist()
Enable Undo and Apply buttons
Refresh_Visual()
Process GTK event loop
Process Apply button callback applying operations,
refreshing display and clearing operations[] vector
Not allowing the operations list to be process until after the merge
step is the be correct ordering. This also prevents the new operation
from flashing up in the operations list and then immediately
disappearing if merged. In the case of adding the first operation,
delaying enabling the Undo and Apply buttons is enough as the buttons
were previously disabled preventing the operation being applied before
the merge. In the case of adding further operations, processing of the
GTK event loop must also be delayed until after the merge to prevent the
operations being applied before the merge. Although that window of
opportunity would only be microseconds.
Bug #699452 - Crash when applying operations before pending operations
fully displayed
Mlabel sometimes writes uninitialised memory at the end of the label.
This causes mlabel, and therefore GParted, to display extra junk at the
end of the label. Depending on the bytes written GParted may also show
the following error on stdout:
(gpartedbin:18116): glibmm-CRITICAL **:
unhandled exception (type Glib::Error) in signal handler:
domain: g_convert_error
code : 1
what : Invalid byte sequence in conversion input
This is caused by a bug in mlabel, believed fixed in mtools 4.0.14.
Effects at least Fedora 14, RHEL/CentOS 6.x and Debian 6. (Use label
"1234567890" on Debian 6 to reproduce). Reproduction steps:
# mkdosfs -F16 /dev/sda7
mkdosfs 3.0.9 (31 Jan 2010)
# export MTOOLS_SKIP_CHECK=1
# mlabel ::123456 -i /dev/sda7
# mlabel -s :: -i /dev/sda7
Volume label is 123456~1t
It is not possible to detect which characters are junk so they can't be
trimmed. Instead just space pad labels so that at least newly written
labels aren't effected. (Fat labels are space padded on the disk by
definition anyway).
Bug #700228 - FAT16/32 labels are sometimes shown corrupted
There was virtually no difference between the separate modules for fat16
and fat32. Remove module fat32 and patch fat16 to serve both file
system subtypes. This is equivalent to what was previously done for
ext[234] by commit:
38dc55d49c
Combine duplicate code for ext[234]
Rename the libsigc++ signals to signal_update and signal_eof to match
the naming used for signals in GParted.
fgrep 'sigc::signal' include/*.h
Also explicitly use the emit() method rather than using the object
operator(). This again is to match the convention in GParted and make
it more obvious what is happening.
fgrep '.emit(' include/*.h
The previous commit missed one glibmm GSource wrapper in the form of the
io watch for the PipeCapture class. Convert this one to use glib
directly as well.
Bug #697727 - Segfault in livecd Gparted v 0.15.0-3 when copying
partition
The glibmm GSource wrappers have a bug where they do not do
reference counting properly, and have a race condition where
the background thread can try to touch the source after the
main thread has already processed and destroyed it. This
results in writes to freed memory and sometimes this causes
crashes or other erratic behavior. Avoid using the glibmm
wrappers and use glib directly. See bug #561885 for details
of the glibmm bug.
Bug #697727 - Segfault in livecd Gparted v 0.15.0-3 when copying partition
These functions in GParted_Core:
open_device()
open_device_and_disk()
close_disk()
close_device_and_disk()
call the following functions in the libparted API:
ped_device_get()
ped_disk_new()
ped_disk_destroy()
ped_device_destroy()
which don't open or close anything. Instead they allocate and
deallocate PedDevice and PedDisk memory structures which describe block
devices and partition tables respectively.
Rename functions:
open_device_and_disk() -> get_device_and_disk()
close_device_and_disk() -> destroy_device_and_disk()
and merge open_device() and open_device() as each only wrapped one
libparted function and was only called from a single place.
The wipefs command has the following significant limitations which were
worked around in previous commits:
1) Wasn't available in the earliest distributions supported by GParted;
2) Had to be called 3 times to erase vfat (fat16/32) signatures in all
but the most recent versions.
This meant we had all the code to clear file system signatures without
using the wipefs command as well as extra complexity of using wipefs
too. So just remove use of the wipefs command.
Bug #688882 - Improve clearing of file system signatures
Before util-linux 2.21.0, released Feb 2012, wipefs only cleared one of
the three vfat (fat16/fat32) signatures it can be detected by each time
wipefs was run. Also if a nilfs2 file system was created before all
three signatures were cleared the partition was still recognised as a
vfat file system, albeit a corrupted one, rather than as a nilfs2 file
system.
Old wipefs clearing vfat signatures:
# wipefs --version
wipefs from util-linux 2.20.1
# wipefs -a /dev/sda7
8 bytes were erased at offset 0x52 (vfat)
they were: 46 41 54 33 32 20 20 20
# wipefs -a /dev/sda7
1 bytes were erased at offset 0x0 (vfat)
they were: eb
# wipefs -a /dev/sda7
2 bytes were erased at offset 0x1fe (vfat)
they were: 55 aa
New wipefs clearing vfat signatures:
# wipefs --version
wipefs from util-linux 2.21.2
# wipefs -a /dev/sda12
8 bytes were erased at offset 0x00000052 (vfat): 46 41 54 33 32 20 20 20
1 bytes were erased at offset 0x00000000 (vfat): eb
2 bytes were erased at offset 0x000001fe (vfat): 55 aa
Workaround by calling "wipefs -a" three times if the output indicated
only one vfat signature was cleared.
Bug #688882 - Improve clearing of file system signatures
Add "cleared" to the bottom of list of file system formats available in
the Create new Partition dialog and in the Format to --> (file system
list) menu. This clears existing file system signatures in the newly
created partitions and existing partitions respectively.
Bug #688882 - Improve clearing of file system signatures
Move some code into new create_format_menu_add_item() sub-function which
adds one file system entry to the Partition --> Format to -->
(file system list) menu.
Bug #688882 - Improve clearing of file system signatures
Previously the function erase_filesystem_signatures() was used to clear
file system signatures when a new partition was created and when an
existing partition was formatted with a file system. However this was
only available with libparted <= 2.4 and then only for the file systems
which libparted supports.
Having multiple different file system signatures on a partition leads to
misidentification of file system. For example creating a nilfs2 over
the top of a fat32 file system is detected as a fat32, not nilfs2. This
shows that old file system signatures must be cleared before a new file
system is created.
Fix by always using "wipefs -a /dev/PARTITION" command to clear all old
file system signatures rather than libparted API calls. Failure from
wipefs is only considered a warning so doesn't fail the file system
creation. (This doesn't yet fully meet the "MUST be cleared"
requirement above. Will be fully met later in this patchset). Output
from the wipefs command is displayed as a new sub-step which looks like
this:
v Format /dev/sda7 as xfs 00:00:05
> calibrate /dev/sda14 00:00:01
v clear old file system signatures in /dev/sda7 00:00:01 [NEW]
> wipefs -a /dev/sda7 [NEW]
> set partition type on /dev/sda7 00:00:02
v create new xfs file system 00:00:01
> mkfs.xfs -f -L "" /dev/sda7
Also signatures are only cleared immediately before a new file system is
written and not when an unformatted partition is created. This allows
recovery from accidental partition deletion by re-creating the deleted
partition as unformatted.
Bug #688882 - Improve clearing of file system signatures
Only supports detection and creation of f2fs file systems. Requires
f2fs-tools and a blkid with f2fs support, util-linux > 2.22.2.
f2fs-tools v1.1.0 only supports file system creation.
Currently requires util-linux directly from the git repository as f2fs
support was only committed on 5 Feb 2013 and it has not yet been
released.
Closes Bug #695396 - Please apply f2fs patch
Mike Fleetwood