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
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
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
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.
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().
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()
Background:
GParted_Core::calibrate_partition() reloads the partition path name and
boundary to ensure they are correct before the operation is performed.
(See comments in calibrate_partition() for the reasons why this is
necessary). This also displays details of the partition being modified
in the operation details to inform the user.
The operation object contains these relevant member objects:
* partition_original
Partition before the operation is applied.
* partition_new
Partition as it is intended to be after the operation has been
applied.
* partition_copied (for the copy operation only)
Source partition being copied.
Issues:
GParted_Core::apply_operation_to_disk() was always calibrating partition
object partition_original, but for about half the operations
partition_original was not used and partition_new is used, so should be
calibrated instead.
Copy into an existing partition calibrated three partitions, the source,
destination before and destination after the operation was applied.
This doesn't really make sense in the operation details to the user.
They would expect to only see the source and destination partitions and
don't care about the distinction between the before and after
representation of the destination.
Minor issues:
The previous fix had to copy the correct partition path from the
calibrated partition_original object to the used partition_new object
for the format, label file system, name partition and change uuid
operations.
Calibrate was called for the create operation too, even though the
partition didn't yet exist. It was a no-operation.
Fix:
Stop always calibrating the partition_original object and instead
calibrate the correct partition object in each operation case. For the
copy into existing partition operation only calibrate the right two
partition objects as the user would expect.
Bug 746559 - Various operations fail when following paste into existing
partition
Format, label file system and new UUID operations would fail when
applied in a sequence to the destination partition following a previous
copy-paste operation.
Giving the copy of a file system a new label and a new UUID are the sort
of actions which should be performed when the disk containing the copy
remains attached to the same computer. This really should work.
Fragment of the failing operation details for a copy and label operation
sequence:
+ Copy /dev/sdb1 to /dev/sdb2
+ calibrate /dev/sdb2
+ calibrate copy of /dev/sdb1
+ calibrate /dev/sdb1
+ check the file system on /dev/sdb1 for errors and (if possible fix them
+ copy file system of /dev/sdb1 to /dev/sdb2
+ Set file system label "small-dst" on copy of /dev/sdb1
+ calibrate copy of /dev/sdb1
path: /dev/sdb2 (partition)
...
+ set file system label to "small-dst" on copy of /dev/sdb1
+ e2label copy of /dev/sdb1 "small-dst"
Usage: e2label device [newlabel]
This is failing because the file system specific command is passed
"copy of /dev/sdb1" as the device name. Code sequence:
1) OperationCopy::OperationCopy() sets the real path name of the
partition_new object to "copy of /dev/SRC" for display purposes.
2) GParted_Core::apply_operation_to_disk() calls calibrate_partition()
on partition_original object, restoring the real path name for
object partition_original.
3) apply_operation_to_disk() calls format(), label_filesystem() or
change_uuid() on the partition_new object, which still has the real
path name set to "copy of /dev/SRC". File system specific commands
fail with this as a path name.
Fix by copying the real path name from object partition_original to
partition_new, as is already done for the resize/move operation. Also
apply this fix to the name partition operation, because it uses the
partition_new object and so that it displays the real path name in the
operation details.
Bug 746559 - Various operations fail when following paste into existing
partition
When the partition is named in the Create New Partition dialog, set the
partition name as part of the create partition operation. Currently
this is only supported for GPTs. See
Utils::get_max_partition_name_length() for details.
Bug 746214 - Partition naming enhancements
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
This is a small tidy-up to remove Gtk::Entry method calls on the file
system label entry box in the Create New Partition dialog which serve no
purpose.
filesystem_label_entry.set_activates_default( true );
It trying to make the Create New Partition dialog automatically
close when Enter is pressed with focus in the label entry box.
However this doesn't work, presumably because the default widget for
the dialog is not the Add button. Remove.
filesystem_label_entry.set_text( partition.get_filesystem_label() );
Initialises the text in the entry box with the file system label
from the passed partition object. The label is blank and the entry
box defaults to blank. Achieves nothing. Remove.
filesystem_label_entry.select_region( 0, filesystem_label_entry.get_text_length() );
Highlights the empty text in the entry box. Achieves nothing.
Remove.
NOTE:
The same set of Gtk::Entry method calls in Dialog_FileSystem_Label() and
Dialog_Partition_Name, which are editing the existing file system label
and partition name respectively, do work and have a useful effect so
shouldn't be removed.
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
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
Attempting to create a new partition on a pc98 partition table fails
with the following libparted error:
The flag 'lvm' is not available for pc98 disk labels.
This has been broken since LVM2 Physical Volume read-write support was
first added in this commit:
c3ab62591b
Add creation of LVM2 PVs (#670171)
Fix by only clearing and setting the lvm partition flag when the type of
the partition table supports it. When creating a partition to contain
an LVM2 PV and the lvm flag is not support add the following message to
the operation results to explain that setting the lvm partition flag was
skipped and why:
Skip setting unsupported partition flag: lvm
Bug 746204 - Creating partitions on pc98 table fails with lvm flag not
available
Refactor GParted_Core::set_partition_type().
1) Set lp_partition variable earlier and use a single if lp_partition
set condition, rather than in both if conditions for the normal file
system case and the LVM2 Physical Volume case.
2) Stop calling Utils::get_filesystem_string() multiple times, instead
save the result in a local variable.
Tidies the code a little and reorders it in preparation for the
following fix to only set the lvm partition flag when support, making
that code change simpler.
Bug 746204 - Creating partitions on pc98 table fails with lvm flag not
available
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
Libparted 1.9.0 to 2.3 inclusive, recognises whole disk device FAT file
systems as MSDOS partition tables. This causes GParted to do the same.
# dd if=/dev/zero bs=1M of=/dev/md4
# mkdosfs -F32 -v -I /dev/md4
# blkid /dev/md4
/dev/md4: UUID="53FE-31F2" TYPE="vfat"
# parted /dev/md4
GNU Parted 2.1
Using /dev/md4
Welcome to GNU Parted! Type 'help' to view a list of commands.
(parted) print
Model: Unknown (unknown)
Disk /dev/md4: 536MB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Number Start End Size Type File system Flags
(parted) quit
# /tmp/parted24/bin/parted /dev/md4
GNU Parted 2.4
Welcome to GNU Parted! Type 'help' to view a list of commands.
(parted) print
Model: Linux Software RAID Array (md)
Disk /dev/md4: 536MB
Sector size (logical/physical): 512B/512B
Partition Table: loop
Number Start End Size File system Flags
1 0.00B 536MB 536MB fat32
(parted) quit
This was fixed in libparted 2.4 by commit:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=616a2a1659d89ff90f9834016a451da8722df509
libparted: avoid regression when processing a whole-disk FAT partition
Make GParted immune to this bug by moving blkid performed whole disk
device file system detection before libparted partition detection. Also
have to always erase old file system signatures on whole disk devices
when creating new partition tables to ensure that blkid doesn't detect
those old signatures before libparted has a chance to detect the new
partition table.
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
Creating a new partition table was getting libparted to read any
existing partition table before creating a new partition table on the
device. This is an unnecessary step, and if the device didn't already
contain a partition table also printed this error from libparted:
/dev/sdb: unrecognised disk label
Since get_device_and_disk() has been split into two, just call
get_device() instead to just populate the PedDevice object representing
the disk device. Removes a small unnecessary step.
Bug 743181 - Add unpartitioned drive read-write support
The preview of clearing a whole disk device file system was previewing
the same as formatting to all other file system types; as a cleared file
system spanning the whole disk device. However when implemented this
removes all signatures on the disk so it actually becomes an unallocated
and unpartitioned device. Make the preview match what happens in when
implemented.
GParted previously used mydevice.max_prims = -1 to represent an
unpartitioned device. It is now represented as:
mydevice.max_prims = 1
mydevice.disktype = _("unrecognized")
mydevice.partitions[0].type = TYPE_UNALLOCATED
mydevice.partitions[0].whole_device = true
mydevice.partitions[0].filesystem = FS_UNALLOCATED
and the check for an unpartitioned device in Win_GParted.cc becomes:
partitions[0].type == TYPE_UNALLOCATED && partitions[0].whole_device
Bug 743181 - Add unpartitioned drive read-write support
Previously GParted displayed a device containing the parted "loop"
partition table signature "GNU Parted Loopback 0" and nothing else, as
an unrecognised device.
Now make GParted display this as a virtual whole disk device partition
with unknown contents, complete with the unable to detect a file system
warning. This change then allows a whole disk device file system to be
created with the following two steps:
1) Create "loop" partition table on a device;
2) Format to required file system.
GParted represents a whole disk device file system as:
mydevice.max_prims = 1
mydevice.disktype = "none"
mydevice.partitions[0].type = TYPE_PRIMARY
mydevice.partitions[0].whole_device = true
mydevice.partitions[0].filesystem = FS_EXT4 (example)
Now represents just Parted's "loop" signature as:
mydevice.max_prims = 1
mydevice.disktype = "loop"
mydevice.partitions[0].type = TYPE_PRIMARY
mydevice.partitions[0].whole_device = true
mydevice.partitions[0].filesystem = FS_UNKNOWN
And as before, an unpartitioned device as:
mydevice.max_prims = -1
mydevice.disktype = _("unrecognized")
mydevice.partitions[0].type = TYPE_UNALLOCATED
mydevice.partitions[0].whole_device = true
mydevice.partitions[0].filesystem = FS_UNALLOCATED
Bug 743181 - Add unpartitioned drive read-write support
Only allow resizing, not moving of a whole disk device file system.
There is no actual partition to move and moving a file system away from
the start of a disk only makes it unrecognisable.
Also don't perform the partition resize step as there's no actual
partition to be resized. Only the file system is being resized.
(Libparted actually allows the virtual partition spanning a whole disk
device to be resized, implementing it as a no-operation, but only for
recognised file systems. For unrecognised file systems it fails with
"unrecognised disk label").
Note that the existing resize dialog was designed for resizing partition
boundaries, and their contained file systems, not for resizing file
systems within a fixed boundary. The difference is noticeable when
there is unallocated space because the file system doesn't fill the
whole disk device. The dialog starts resizing a virtual partition the
size of the whole disk device, not the actual size of the file system.
Leave addressing this for a possible future update.
Bug 743181 - Add unpartitioned drive read-write support
First, copying into a whole disk device fails on the set partition type
step. Fails with either libparted error "The flag 'lvm' is not
available for loop disk labels" or "unrecognised disk label" depending
whether libparted recognised the content and created a virtual partition
or not. (This is with libparted 2.4).
Fix by just skipping setting the partition type on whole disk devices.
Second, if any file system specific tools are used during the copy, they
will fail because they are passed the device name as "copy of /dev/SRC"
instead of "/dev/DST". Occurs when either the destination whole disk
device is not an identical size to the source so the file system check
and grow steps are added, or when file system specific tools are used to
copy the file system as with XFS or recent EXT2/3/4 tools.
Fix by re-adding the real partition path from libparted for whole disk
devices, as is already done for partitioned device names in
GParted_Core::calibrate_partition().
Bug 743181 - Add unpartitioned drive read-write support
Creation of reiserfs file system fails in GParted with the this error.
# mkreiserfs -f --label "" /dev/sdb < /dev/null
mkreiserfs 3.6.24
/dev/sdb is entire device, not just one partition!
Continue (y/n):
# echo $?
1
Add second force flag, -f, to the mkreiserfs command to make it work.
Bug 743181 - Add unpartitioned drive read-write support
Creation of ext2/3/4 and ntfs file systems fails in GParted on whole
disk devices with these errors.
# mkfs.ext4 -L "" /dev/sdb < /dev/null
mke2fs 1.42.9 (4-Feb-2014)
/dev/sdb is entire device, not just one partition!
Proceed anyway? (y,n)
# echo $?
1
# mkntfs -Q -v -L "" /dev/sdc
/dev/sdc is entire device, not just one partition.
Refusing to make a filesystem here!
# echo $?
1
Add force flag, -F, to both mkfs commands to make them work.
Bug 683643 - Doesn't properly support partitionless drives.
Formatting a whole disk device fails on the set partition type step with
libparted error "unrecognised disk label". This is because the previous
step just cleared the old file system signatures leaving libparted with
nothing to recognise. Therefore libparted doesn't present a virtual
"loop" partition table.
As there is no partition table, there's no partition and no partition
type. Just skip setting the partition type on whole disk devices.
Bug 743181 - Add unpartitioned drive read-write support
This enables Format to Cleared operation to succeed on whole disk device
file systems even when libparted doesn't recognise the file system.
(Turns out that making calibrate work in the previous commit happened to
make Format to Cleared operation succeed, but only if libparted
recognised the file system on the whole disk device).
Bug 743181 - Add unpartitioned drive read-write support
In the operational results of the calibrate step include the type of the
path GParted is working with, either partition or whole disk device. Do
the same for the create empty partition step too for consistency, even
though it only ever creates partitions. Looks like:
create empty partition
path: /dev/sdb3 (partition)
start: 2099200
end: 4196351
size: 2097152 (1.00 GiB)
calibrate /dev/sdc
path: /dev/sdc (device)
start: 0
end: 1953525167
size: 1953525168 (931.51 GiB)
Makes it explicit to the users what GParted has detected. Helps the
developers when looking at saved results to understand what decisions
were made and why specific steps were performed or not.
Bug 743181 - Add unpartitioned drive read-write support
This enables the Check, Label and New UUID operations to succeed on
whole disk device file systems even when libparted doesn't recognise the
file system.
This benefits reiser4 and lvm2 pv file systems with all versions of
libparted, current version is 3.2, and for nilfs2 with libparted < 2.4.
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
Mike Fleetwood