Make the code a little more self documenting by adding the symbolic
constants:
SETTLE_DEVICE_APPLY_MAX_WAIT_SECONDS
SETTLE_DEVICE_PROBE_MAX_WAIT_SECONDS
which highlight that settle_device() is called in two different
contexts, device probe and apply operations, with two different timeout
values.
File system specific commands sometimes fail reporting that the
partition specific /dev entry doesn't exist. Example failing check
operation details:
Check and repair file system (ext4) on dev/sdb4
calibrate /dev/sdb4
path: /dev/sdb4 (partition)
start: 4196352
end: 6293503
size: 2097152 (1.00 GiB)
check file system on /dev/sdb4 for errors and (if possible) fix them
e2fsck -f -y -bv -C 0 /dev/sdb4
e2fsck 1.42.9 (28-Dec-2013)
e2fsck: No such file or directory while trying to open /dev/sdb4
Possibly non-existent device?
This has been reproduced on CentOS 7. Debugging shows that the
libparted calls used to re-read the partition details in
GParted_Core::calibrate_partition() leads to udev removing and re-adding
all the partition /dev entries for the disk.
# udevadm monitor &
# gpartedbin
...
16.480662 +12.618659 calibrate_partition() calling get_device("/dev/sdb", lp_device) ...
16.483644 +0.002982 calibrate_partition() get_device() returned
16.483678 +0.000034 calibrate_partition() calling get_disk(lp_device, lp_disk) ...
16.618113 +0.134435 calibrate_partition() get_disk() returned
KERNEL[19275.707968] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
16.618561 +0.000448 destroy_device_and_disk() calling ped_disk_destroy(lp_disk) ...
16.618584 +0.000023 destroy_device_and_disk() ped_disk_destroy() returned
16.618591 +0.000007 destroy_device_and_disk() calling ped_device_destroy(lp_disk) ...
16.618602 +0.000011 destroy_device_and_disk() ped_device_destroy() returned
16.618687 +0.000085 calibrate_partition() return true
16.618851 +0.000164 execute_command() e2fsck -f -y -v -C 0 /dev/sdb4
KERNEL[19275.708389] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[19275.708500] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[19275.708643] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
KERNEL[19275.768278] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
KERNEL[19275.771171] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
KERNEL[19275.771360] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[19275.771542] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[19275.775858] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [19275.820153] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [19275.823152] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [19275.828275] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
16.742735 +0.123884 execute_command() exit status 8
UDEV [19275.841425] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
UDEV [19275.905478] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
UDEV [19276.013580] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [19276.034728] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [19276.174840] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
UDEV [19276.237105] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
So exactly when GParted is running the external e2fsck command, udev is
in the middle of removing and re-adding all the /dev partition entries
for the disk. Hence the above failure reporting that /dev/sdb4 didn't
exist. This error depends on the timing between GParted running the
external file system specific command and udev removing and re-adding
the entries, so sometimes it works and sometimes it fails.
Further debugging showed that simply opening and closing the whole disk
device read-write triggers the same removing and re-adding of all the
partition /dev entries with udev >= 219. Opening the whole disk device
read-write is what libparted has always done until this post
libparted 3.2 patch to make it open read-only when probing:
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=44d5ae0115c4ecfe3158748309e9912c5aede92d
libparted: Use read only when probing devices on linux (#1245144)
To fix this simply wait for udev devices to settle in
calibrate_partitions(). The longest I have seen udev take to do this is
0.80 seconds in a VM. Wait up to 10 seconds as is done in commit() ->
commit_to_os(), also called when applying operations.
On configurations which don't have this issue execution of udevadm
settle, which will return immediately, adds at most 0.1 seconds to the
time taken for the calibrate step. This won't be noticed in the time
taken of the operation details so there is no point in trying to avoid
executing udevadm settle when not needed.
Bug 762941 - Operations sometimes failing with: No such file or
directory
Minor issues:
1) In the while loop reading from /proc/partitions into variable line,
just after the sscanf() call the variable was re-purposed to hold the
device name making the code unnecessarily hard to follow.
2) Variable c_str was a fixed sized buffer holding the device name read
from /proc/partitions.
3) Variable c_str name provides no meaning as to what data it holds.
4) Return value from all the Utils::regexp_label() calls is converted
from Glib::ustring to std::string to be stored in device variable.
Resolve by using Utils::regexp_label() to extract the device name from
each line in /proc/partitions and store in the variable device, already
used for this purpose and now changed to type Glib::ustring.
realpath(3) manual page says:
BUGS
The POSIX.1-2001 standard version of this function is broken by
design, since it is impossible to determine a suitable size for
the output buffer, resolved_path. According to POSIX.1-2001 a
buffer of size PATH_MAX suffices, but PATH_MAX need not be a
defined constant, and may have to be obtained using pathconf(3).
And asking pathconf(3) does not really help, since, on the one
hand POSIX warns that the result of pathconf(3) may be huge and
unsuitable for mallocing memory, and on the other hand
pathconf(3) may return -1 to signify that PATH_MAX is not
bounded. The resolved_path == NULL feature, not standardized in
POSIX.1-2001, but standardized in POSIX.1-2008, allows this
design problem to be avoided.
The resolved_path == NULL feature of realpath() has existed as a Glibc
extension since realpath() was first added to Glibc 1.90, released in
June 1996. Therefore it can be used unconditionally.
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=fa0bc87c32d02cd81ec4d0ae00e0d943c683e6e1
Bug 764369 - Use realpath() safely
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
On a 64-bit distribution, with an NTFS file system in a partition
without a /dev entry then GParted will crash when attempting to read
the file system usage. Not having a /dev entry for the partition is
rare and only known to occur for the disk devices used within Fake RAID
(dmraid) arrays, and then only on Ubuntu 12.04 LTS. Other/newer
distributions do create /dev entries for partitions found on disk
devices within Fake RAID arrays.
Create mirror Fake RAID array:
# dmraid -f isw -C MyArray --type 1 --disk /dev/sdc,/dev/sdd
# dmraid -ay
Create NTFS partition on the Fake RAID array. On refresh GParted
crashes:
# ./gpartedbin
(gpartedbin:590): glibmm-ERROR **:
unhandled exception (type std::exception) in signal handler:
what: basic_string::assign
Without a /dev/sdc1 device entry the ntfsresize command reports this:
# ntfsresize --info --force --no-progress-bar /dev/sdc1
ntfsresize v2015.3.14 (libntfs-3g)
ERROR(2): Failed to check '/dev/sdc1' mount state: No such file or directory
Probably /etc/mtab is missing. It's too risky to continue. You might try
an another Linux distro.
The problem code in ntfs::set_used_sectors():
145 index = output.find( "Cluster size" );
146 if ( index == output.npos ||
147 sscanf( output.substr( index ).c_str(), "Cluster size : %Ld", &S ) != 1 )
As "Cluster size" did not exist in the output find() returned the not
found token of string::npos [1], which in a 64-bit environment is
represented by 2^64-1 [2]. However it was saved in the variable index
of type unsigned integer, which is only a 32-bit integer, thus
truncating it to 2^32-1. Therefore the comparison failed and sscanf()
tried to parse the output starting at offset 2^32-1 which resulted in
the crash.
Introduced by commit:
324d99a172
Record file system block size where known (#760709)
Fix by following the same pattern of the other comparisons in
ntfs::set_used_sectors() which checks if index is less than the output
length.
References:
[1] std::string::find
http://www.cplusplus.com/reference/string/string/find/
[2] std::string::npos
http://www.cplusplus.com/reference/string/string/npos/
(Note that Glib::ustring is derived from std::string in the Standard C++
library and provides a compatible interface).
Bug 764658 - GParted crashes when reading NTFS usage when there is no
/dev/PTN entry
As with glibmm [1] the latest versions of libsigc++ also uses ISO C++
2011 features. The NEWS file [2] says:
2.5.1 (unstable):
* Use (and require) C++11
(Kjell Ahlstedt)
* Using C++11 lambda functions to create sigc::slots:
Avoid the need for SIGC_FUNCTORS_DEDUCE_RESULT_TYPE_WITH_DECLTYPE.
(Kjell Ahlstedt)
Without enabling C++11 compiler features, compilation of GParted with
libsigc++ 2.5.1 and later fails with errors such as theses:
/usr/include/sigc++-2.0/sigc++/trackable.h:40:3: warning: identifier 'noexcept' is a keyword in C++11 [-Wc++0x-compat]
trackable_callback(void* data, func_destroy_notify func) noexcept
^
[1] d6d7cb2bbf
Enable C++11 compilation when using glibmm 2.45.40 and later (#756035)
[2] libsigc++ 2.5.1 NEWS file
https://git.gnome.org/browse/libsigcplusplus/tree/NEWS?h=2.5.1
Bug 758545 - gparted-0.24.0 fails to build with gnome 3.18 mm packages
(on Gentoo)
GParted is allowing creation of a FAT32 formatted partition of any size.
However with a 512 byte sector size the maximum volume size of a FAT32
file system is reported to be 2 TiB.
* Wikipedia: File Allocation Table / FAT32
https://en.wikipedia.org/wiki/File_Allocation_Table#FAT32
"The boot sector uses a 32-bit field for the sector count, limiting
the FAT32 volume size to 2 TB for a sector size of 512 bytes and 16 TB
for a sector size of 4,096 bytes."
* Microsoft: Default cluster size for NTFS, FAT, and exFAT / Default
cluster sizes for FAT32
https://support.microsoft.com/en-us/kb/140365
Trying to create a FAT32 file system in a partition larger than 2 TiB
results in unallocated space being left after the file system.
Nuances:
[1] Larger sector sizes allow larger maximum volume sizes up to 16 TiB
with 4096 byte sectors.
[2] mkdosfs/mkfs.fat has an -S SECTOR_SIZE option which allows changing
the "logical" sector size of the file system allowing the maximum
volume to be proportionally increased.
[3] mkfs.fat appears to have an signed overflow bug when the size of the
partition is larger than maximum signed 32-bit integer of logical(?)
sectors. (2 TiB for a sector size of 512 bytes). It reports the
partition size as minus size and creates a 1 TiB file system.
GParted wants a single maximum file system size and the code is not
ready for a differing maximum file system size for different sector
sizes.
In fat16::create() could specify larger "logical" sector sizes to
mkfs.fat when the partition is larger than 2 TiB to allow maximum volume
size to be increased further. However that will take a lot of cross
platform testing to ensure that all sorts of devices support "logical"
sector sizes other than 512 bytes on devices with a hardware sector size
of 512 bytes. This is too much effort.
Therefore implement a single FAT32 maximum volume size of 2 TiB.
Bug 763896 - GParted not restricting creation of FAT32 volume to maximum
size of 2 TiB
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
Previously total_done was updated in copy_thread() after copying of the
blocks, but importantly before the last call to set_progress_info() to
update the progress bar. Having total_done varying during the copy of a
single range of blocks, single call to copy_blocks::copy(), is an
impediment to being able to report a single progress bar across the
whole internal copy operation.
Move updating of total_done to copy() immediately after copy_thread()
completes.
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
Previously the autoconf check for Gtk::Window::set_default_icon_name()
method was a compile test because the documentation reported the method
was available in gtkmm from 2.6 [1], however it wasn't available on
RHEL / CentOS 5.x with gtkmm 2.10.
Then commit [2] added detection and enabling of C++11 compilation, but
after the above autoconf check. So on Fedora 23 the compiler based
autoconf check for set_default_icon_name() method failed because C++11
compilation had not yet been enabled:
> checking for Gtk::Window::set_default_icon_name method... no
checking for gtk_show_uri function... yes
checking for Gtk::MessageDialog::get_message_area() method... yes
> checking for glibmm >= 2.45.40 which requires C++11 compilation... yes
> checking whether g++ supports C++11 features by default... no
> checking whether g++ supports C++11 features with -std=gnu++11... yes
The gtkmm source code reveals that set_default_icon_name() method was
only added in gtkmm 2.11.1 [3] so switch to a PKG_CHECK_EXISTS for this
version of gtkmm.
[1] gtkmm GTK::Window Class Reference
https://developer.gnome.org/gtkmm/3.6/classGtk_1_1Window.html#a533d03e9b92d8ccd142ab3a44005cae4
[2] Enable C++11 compilation when using glibmm 2.45.40 and later (#756035)
d6d7cb2bbf
[3] gtkmm NEWS file
https://git.gnome.org/browse/gtkmm/tree/NEWS?h=gtkmm-2.14.0#n565
Bug 762184 - Autoconf check for C++11 comes after compile test for
Gtk::Window::set_default_icon_name()
Mostly the code is explicit and calls the emit() method when emitting a
signal [1], like this:
signal_name.emit();
However there are a few cases which use the function call operator on
the signal object [2], like this:
signal_name();
The behaviour is identical [3] but it is preferred to be explicit that a
signal callback is being initiated, and it also makes them much easier
to search for too.
[1] List explicit emit() signal calls
fgrep '.emit(' src/*.cc
[2] List function call operator emitted signals
egrep "`sed -n '/sigc::signal/s/.*sigc::signal.*> *\([a-zA-Z_]*\).*/\1/p' include/*.h | tr '\n' '|' | sed 's/\(.*\).$/[^a-zA-Z_](\1)\\\(/'`" src/*.cc
[3] Quote from the libsigc++ Reference Manual, class sigc::signal
https://developer.gnome.org/libsigc++/stable/classsigc_1_1signal7.html#ab37db0ecc788824d0baa3c301efc8dcd
result_type sigc::signal<...>::operator()(...)
Triggers the emission of the signal (see emit())
For non-progress tracked external commands the command being executed is
displayed in the Apply pending operations dialog, just below the top
pulsing progress bar. However for progress tracked external commands
the description of the parent operation detail is displayed instead.
Example 1: non-progress tracked xfs check repair:
TreePath
Check and repair file system (xfs) on /dev/sdc1 0
+ calibrate /dev/sdc1 0:0
+ check file system on /dev/sdc1 for errors and (if po... 0:1
+ xfs_repair -v /dev/sdc1 0:1:0
+ [empty stdout] 0:1:0:0
+ [stderr]Phase 1 - find and verify superblock... 0:1:0:1
"xfs_repair -v /dev/sdc1" (TreePath 0:1:0) is shown because it is the
latest updated operation detail which is timed (set to status
executing).
Example 2: progress tracked ext4 copy using e2image:
TreePath
Copy /dev/sdc2 to /dev/sdc3 0
+ calibrate /dev/sdc2 0:0
+ check file system on /dev/sdc2 for errors and (if po... 0:1
+ set partition type on /dev/sdc3 0:2
+ copy file system of /dev/sdc2 to /dev/sdc3 0:3
+ e2image -ra -p /dev/sdc2 /dev/sdc3 0:3:0
+ [stdout]Scanning inodes... 0:3:0:0
+ [stderr]e2image 1.42.9 (28-Dec-2013)... 0:3:0:1
"copy file system of /dev/sdc2 to /dev/sdc3" (TreePath 0:3) is shown
because that operation detail is also timed and it is being constantly
updated by the progress bar updates via it.
Change execute_command() to update the progress bar via the operation
detail it creates to hold the command being executed, instead of the
parent operation detail, to resolve the above. Also replaces calling
operationdetail.get_last_child() throughout the method.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Remove starting and stopping the progress bar in xfs::copy(). The
progress bar will be automatically started in xfs::copy_progress()
callback when run_progressbar() is called; and automatically stopped in
FileSystem::execute_command() when it calls stop_progress() at the end.
Note that this will now not initialise the progress bar from zero
immediately that the XFS copy is started, but instead 0.5 seconds into
the copy when xfs::copy_progress() timed callback is called for the
first time.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
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
So far the signal_progress callback slot was only emitted when standard
output from the file system specific command was updated. This was okay
as all the commands until now wrote their progress information to
stdout. However e2image writes its progress information to stderr,
therefore also emit signal_progress when stderr is updated too.
This does mean that the file system specific *_progress() tracking
callbacks will be called when either of the OperationDetail objects
containing stdout or stderr are updated. Therefore the trackers may be
called when there is no update to the stream from which it is parsing
the progress information. This is not a problem as the tracker will
just update the progress bar with the same information it already has.
Also it won't happen much as only e2image is known to write to both
streams, and then only one line to stdout and the updated progress
information to stderr. This is just an observation and not an issue
which needs coding around.
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
... to display a negative sign before the hours, minutes and seconds.
Before:
Utils::format_time(-1) = "00:00:0-1"
Utils::format_time(-119) = "00:0-1:0-59"
After:
Utils::format_time(-1) = "-00:00:01"
Utils::format_time(-119) = "-00:01:59"
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Utils::round() was doing +0.5 then truncate. Correct for positive
values. Wrong for negative values.
E.G.
Utils::round(-1.4)
= trunc(-1.4 + 0.5)
= trunc(-0.9)
= 0
Round of -1.4 is definitely not 0. Fix this for negative values by
subtracting 0.5 then truncating.
Reference:
How can I convert a floating-point value to an integer in C?
https://www.cs.tut.fi/~jkorpela/round.html
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
When the ntfs resize operation had almost completed, percentage complete
was >= 99.9%, the progress tracker was passing 0.04 (4%) to the progress
bar. After reading the next chunk of output from the ntfsresize command
the last line contained this text:
" 4) set the bootable flag for the partit"
End of the ntfsresize command output for context:
Relocating needed data ...
100.00 percent completed
Updating $BadClust file ...
Updating $Bitmap file ...
Updating Boot record ...
Syncing device ...
Successfully resized NTFS on device '/dev/sdd4'.
You can go on to shrink the device for example with Linux fdisk.
IMPORTANT: When recreating the partition, make sure that you
1) create it at the same disk sector (use sector as the unit!)
2) create it with the same partition type (usually 7, HPFS/NTFS)
3) do not make it smaller than the new NTFS filesystem size
4) set the bootable flag for the partition if it existed before
Otherwise you won't be able to access NTFS or can't boot from the disk!
If you make a mistake and don't have a partition table backup then you
can recover the partition table by TestDisk or Parted's rescue mode.
This was occurring because *scanf() can't actually report failure to
match fixed text after conversion of the last variable. See code
comment in ntfs::resize_progress() for more details. Fix by using
.find() instead to match the required "percent completed" explicit text
of the progress information when it appears on the last line.
Bug 760709 - Add progress bars to XFS and EXT2/3/4 file system specific
copy methods
Adapt the ntfs resize progress tracker to use the new ProgressBar class.
Also make it track 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
Adapt the ext2 fsck progress tracker to use the new ProgressBar class.
Also make it track when the text progress bar has 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
Adapt the ext2 create file system progress tracker to used the new
ProgressBar class. Also make it track when the text progress indicator
completes 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
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
Change the Applying pending operations dialog so that it takes it source
of progress from the single ProgressBar object, rather than the fraction
value in every OperationDetail object. Also remove ProgressBar
debugging now that it is being used to drive the UI.
NOTE:
This temporarily causes the existing file system specific progress bars
to not be shown because they still update via the fraction member in
each OperationDetail object, rather than the new ProgressBar. This will
be corrected in following commits.
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
Mike Fleetwood