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
Composing these operations caused GParted to abort on an assert failure:
(1) Delete an existing partition,
(2) Create a new partition,
(3) Delete new partition.
This is the equivalent issue as fixed in the previous commit, except with
the delete operation rather than the check operation:
Prevent assert failure from OperationCheck::get_partition_new() (#767233)
# ./gpartedbin
======================
libparted : 2.4
======================
**
ERROR:OperationDelete.cc:41:virtual GParted::Partition& GParted::OperationDelete::get_partition_new(): assertion failed: (false)
Aborted (core dumped)
# gdb ./gpartedbin core.19232 --batch --quiet --ex backtrace -ex quit
[New Thread 19232]
[New Thread 19234]
[Thread debugging using libthread_db enabled]
Core was generated by `./gpartedbin'.
Program terminated with signal 6, Aborted.
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x000000361f233dc5 in abort () at abort.c:92
#2 0x0000003620a67324 in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x50fcc0 "virtual GParted::Partition& GParted::OperationDelete::get_partition_new()", message=0x1b55f60 "assertion failed: (false)") at gtestutils.c:1358
#3 0x0000003620a678f0 in g_assertion_message_expr (domain=0x0, file=0x50fa68 "OperationDelete.cc", line=41, func=0x50fcc0 "virtual GParted::Partition& GParted::OperationDelete::get_partition_new()", expr=<value optimized out>) at gtestutils.c:1369
#4 0x000000000049aa0d in GParted::OperationDelete::get_partition_new (this=0x1b321b0) at OperationDelete.cc:41
#5 0x00000000004c6700 in GParted::Win_GParted::activate_delete (this=0x7ffc91403670) at Win_GParted.cc:2068
...
As before the crash is happened in Win_GParted::activate_delete() as it
was going through the list of operations removing those which applied to
the never created partition. It came across the delete operation of an
existing partition and called get_partition_new(). As partition_new was
not set or used by the delete operation this asserted false and crashed
GParted.
Unlike the check operation case, the delete operation doesn't have a
partition afterwards. (As GParted represents unallocated space with
partition objects then the delete operation could be populated with a
new partition by constructing the correctly merged unallocated space
partition object, but that is what OperationDelete::apply_to_visual()
does and having a place holder doesn't seem like the right thing to do).
Instead exclude delete operations, on existing partitions, when looking
for operations applying to this not yet created partition as they are
mutually exclusive.
Bug 767233 - GParted core dump on assert failure in
OperationDelete::get_partition_new()
Composing these operations caused GParted to abort on an assert failure:
(1) Check an existing partition,
(2) Create a new partition,
(3) Delete new partition.
# ./gpartedbin
======================
libparted : 2.4
======================
**
ERROR:OperationCheck.cc:40:virtual GParted::Partition& GParted::OperationCheck::get_partition_new(): assertion failed: (false)
Aborted (core dumped)
# gdb ./gpartedbin core.8876 --batch --quiet --ex backtrace -ex quit
[New Thread 8876]
[New Thread 8879]
[Thread debugging using libthread_db enabled]
Core was generated by `./gpartedbin'.
Program terminated with signal 6, Aborted.
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x000000361f233dc5 in abort () at abort.c:92
#2 0x0000003620a67324 in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x50f400 "virtual GParted::Partition& GParted::OperationCheck::get_partition_new()", message=0x1d37a00 "assertion failed: (false)") at gtestutils.c:1358
#3 0x0000003620a678f0 in g_assertion_message_expr (domain=0x0, file=0x50f1a8 "OperationCheck.cc", line=40, func=0x50f400 "virtual GParted::Partition& GParted::OperationCheck::get_partition_new()", expr=<value optimized out>) at gtestutils.c:1369
#4 0x0000000000498e21 in GParted::OperationCheck::get_partition_new (this=0x1d1bb30) at OperationCheck.cc:40
#5 0x00000000004c66ec in GParted::Win_GParted::activate_delete (this=0x7fff031c3e30) at Win_GParted.cc:2068
...
When Win_GParted::activate_delete() was stepping through the operation
list removing operations (2 & 3 in the above recreation steps) which
related to the new partition never to be created it called
get_partition_new() on all operations in the list. This included
calling get_partition_new() on the check operation (1 in the above
recreation steps). As partition_new was not set or used by the check
operation get_partition_new() asserted false and crashed GParted.
Fix by populating the partition_new member in OperationCheck objects,
thus allowing get_partition_new() to be called on the object. As a
check operation doesn't change any partition boundaries or file system
attributes, just duplicate the new partition from the original
partition.
Bug 767233 - GParted core dump on assert failure in
OperationDelete::get_partition_new()
When composing a copy operation it always named the destination
partition as "copy of /dev/SRC". For the case of pasting into
unallocated space creating a new partition this was the right thing to
do as the partition doesn't yet exist so the path is not yet known.
However for the case of pasting into an existing partition the path is
known and replacing it with "copy of /dev/SRC" is wrong. No other
operation when operating on an existing partition changes it path.
Given a set of existing partitions, sdb1 to sdb4, compose a set of copy
operations as: copy sdb1 to sdb2, copy sdb2 to sdb3 and copy sdb3 to
sdb4. The displayed partitions before being applied become:
/dev/sdb1
copy of /dev/sdb1
copy of copy of /dev/sdb1
copy of copy of copy of /dev/sdb1
And the pending operations are named:
Copy /dev/sdb1 to /dev/sdb2
Copy copy of /dev/sdb1 to /dev/sdb3
Copy copy of copy of /dev/sdb1 to /sev/sdb4
This is perverse. In the case of pasting into an existing partition
keep the real path name. This keeps the partitions being displayed as:
/dev/sdb1
/dev/sdb2
/dev/sdb3
/dev/sdb4
And the pending operations named as:
Copy /dev/sdb1 to /dev/sdb2
Copy /dev/sdb2 to /dev/sdb3
Copy /dev/sdb3 to /dev/sdb4
Much more understandable.
Also switch to an upper case "C" in "Copy of /dev/SRC" as the temporary
path name when pasting into unallocated space. Finally update the
comment in calibrate_partition() to describe the remaining cases when
re-adding the path is still required.
Bug 766349 - Resolve code ugliness with partition path getting set to
"copy of /dev/SRC"
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
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
The code currently allows attempting to mount and unmount a LUKS
partition. It is nonsense to directly try to mount and unmount a LUKS
partition and obviously doesn't work. For read-only LUKS support there
is no need to attempt to apply this to the encrypted file system within.
Therefore prevent these operations for LUKS partitions.
Bug 760080 - Implement read-only LUKS support
This patchset is adding read-only LUKS support. Creation of LUKS is
planned to be a tick box adding encryption in the Create New Partition
dialog. Therefore remove the greyed out crypt-luks entry in the Create
New Partition dialog and the Format menu.
Bug 760080 - Implement read-only LUKS support
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;
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
A number of methods in GParted_Core and Win_GParted were using local
Partition objects. Change them into pointers so that Partition object
polymorphism can be implemented.
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
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
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
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
This information is already documented in the existing comments
associated with the calls to merge_operations() and assignments to the
mergetype variables. The table just summaries the rules together in one
place.
Bug 755214 - Refactor operation merging
For the case of resizing/moving a new, not yet created partition,
activate_resize() open coded the merge operation. Again this code has
existed in GParted since before version 0.0.5 and the current code
history in Git.
Replace the necessary code so that an explicit merge_operations() call
is used instead; along with the other case of resizing/moving an
existing partition.
NOTES:
This commit changes the merge direction. The old coded merged forward
by removing the old create operation and adding a new create operation
with the new size. This was bad because with multiple pending create
operations, each merged resize operation reordered those create
operations. Then when the operations were applied the partitions were
created and therefore numbered in a different order to that shown in
disk graphic.
The new code merges backwards by updating the initial create operation
with the new size. This maintains the create operation order so that
when applied the partitions are numbered in the same order as shown in
the disk graphic.
Bug 755214 - Refactor operation merging
For the case of formatting a new, not yet created partition,
activate_format() open coded the merge operation. This code has existed
in GParted since before version 0.0.5 and the current code history in
Git.
Replace the necessary code so that an explicit merge_operations() call
is used instead; along with the other case of formatting an existing
partition.
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
... before refactoring the code.
See the commit message from 2011-10-05 for details of what operations,
available at that time, on the same partition can be merged and in what
cases:
b10349ae37
Merge overlapping operations (#438573)
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)
Perform a copy, reformat source and paste sequence in GParted. When the
source is a primary partition everything works as expected, with the
newly pasted partition reflecting the reformatted source. However when
the source is a logical partition GParted thinks it is pasting the
original source, rather than the reformatted source. The same is also
true for other file system manipulation operations: resize, file system
label and new UUID. It is just that reformatting the source to a
different file system type is the most obvious in the UI and causes the
most significantly wrong actions to be performed.
For example start with an ext4 logical partition, select it for copy,
format it to xfs and paste into a new partition. GParted thinks the
second operation will create a copy of an ext4 file system instead of
the xfs file system. When applied the operation details are:
Format /dev/sdd5 as xfs
+ calibrate /dev/sdd5
+ clear old file system signatures in /dev/sdd5
+ set partition type on /dev/sdd5
+ create new xfs file system
+ mkfs.xfs -f -L "" /dev/sdd5
Copy /dev/sdd5 to /dev/sdd (start at 131.00 MiB)
+ calibrate /dev/sdd5
+ check file system on /dev/sdd5 for errors and (if possible) fix them
+ e2fsck -f -y -v -C 0 /dev/sdd5
e2fsck: Subperblock invalid, trying backup blocks...
Resize inode not valid. Recreate? yes
...
/dev/sdd5: ***** FILE SYSTEM WAS MODIFIED *****
+ create empty partition
+ set partition type on /dev/sdd6
new partition type: ext4
+ copy file system of /dev/sdd5 to /dev/sdd6
using internal algorithm
...
GParted formatted sdd5 to xfs, but then the copy step ran e2fsck and
managed to resurrect the ext4 file system and then performed a block
copy of it to partition sdd6. The copy step should have ran xfs_repair
and used xfsdump | xfsrestore to copy the xfs file system. Afterwards
sdd5 contains both xfs and ext4 signatures.
# wipefs /dev/sdd5
offset type
----------------------------------------------------------------
0x438 ext4 [filesystem]
UUID: f0ed4247-76db-4d93-b3bc-c7da4a70f95e
0x0 xfs [filesystem]
UUID: 1ac8e7c3-0311-4c64-8e4a-b715a23ea0bd
This has been broken at least as far back as GParted 0.1.0.
Fix by simply refreshing the copy source partition object when it is a
logical partition too, as well as when it is a primary partition.
Bug 754827 - Copy, reformat source and paste a logical partition thinks
it's pasting the original file system
Opening the Resize/Move dialog on a logical partition causes GParted to
crash. This crash affects current GParted GIT HEAD, but does not affect
GParted 0.22.0. Git bisect identifies that it was broken with the
following commit:
Remove Set_Data() from the copy, resize/move and new dialog class APIs
7a4a375ed6
The problem was trying to treat the reference display_partitions_ref
like a pointer, and in particular on line 1732 trying to make it refer
to the a different vector of partitions, .logicals sub-vector.
1721 void Win_GParted::activate_resize()
1722 {
...
1726 std::vector<Partition> & display_partitions_ref = display_partitions;
1727 if ( selected_partition_ptr->type == TYPE_LOGICAL )
1728 {
1729 unsigned int ext = 0 ;
1730 while ( ext < display_partitions.size() && display_partitions[ext].type != TYPE_EXTENDED )
1731 ext++;
* 1732 display_partitions_ref = display_partitions[ext].logicals;
1733 }
1734
1735 Dialog_Partition_Resize_Move dialog( gparted_core.get_fs( selected_partition_ptr->filesystem ),
1736 *selected_partition_ptr,
1737 display_partitions_ref );
What was actually happening was that the .logicals sub-vector was being
copied, replacing the display_partitions vector and freeing the original
sub-vector. This left selected_partition_ptr pointing to the original
memory where the selected partition use to exist in the .logicals
sub-vector. At some point in the Dialog_Partition_Resize_Move class
*selected_partition_ptr was referenced, accessing the freed memory.
Crash soon followed.
Fix by using a pointer instead of a reference, which can be assigned to
point to a different object.
Bug 752587 - GParted crashing when opening Resize/Move dialog on
logical partition
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
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
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 first actions of Win_GParted::activate_resize() were to create a
copy of the vector of partitions for the currently displayed device and
visually apply any pending operations. Exactly this has already been
done in Win_GParted::Refresh_Visual() with the result now available in
the member variable display_partitions. Stop this unnecessary partition
object copying and processing by just using display_partitions member
variable instead.
Bug 750168 - Reduce the amount of copying of partition objects
Document how GParted displays partitions in the GUI and manages the
lifetime and ownership of that data.
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
Add Glib g_assert() to ensure that a bug doesn't get introduced which
allows a partition callback to be called without a partition being
selected first.
After deliberately breaking the code so that selected_partition_ptr is
not set, trying to display the Information dialog causes this crash:
# ./gpartedbin
======================
libparted : 2.4
======================
ERROR:Win_GParted.cc:1978:void GParted::Win_GParted::activate_info(): assertion failed: (selected_partition_ptr != NULL)
Aborted (core dumped)
At this point in the code:
1976 void Win_GParted::activate_info()
1977 {
>> 1978 g_assert( selected_partition_ptr != NULL ); // Bug: Partition callback without a selected partition
1979
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
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
Avoid long lines, long statements and repeated calls to
gparted_core.get_filesystem_object( selected_partition.filesystem ) by
storing the returned pointer in a local variable.
Needs the previous commit so that the the local variable can be a
pointer to a const FileSystem object instead of a pointer to a
(modifiable) FileSystem object.
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
Preview of the format operation cleared the partition name, yet when
applied, the partition name reappeared. Fix the preview to reflect
reality.
Bug 746214 - Partition naming 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
Allow partition names to be changed whether or not the partition is
busy, rather than only when not busy, because it doesn't effect the busy
file system or change the partition boundaries in any way.
Bug 746214 - Partition naming enhancements
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
Mike Fleetwood