Replace the explicit adding of libparted exception messages with a
callback to do it instead, and fire the callback just once per operation
by only changing the very top-level OperationDetail to use the new
set_success_and_capture_errors(). Therefore this still produces exactly
the same operation details with libparted messages at the end of each
operation.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
All code implementing a step of an operation follows this pattern:
od.add_child(OperationDetail("Step heading"));
od.get_last_child().add_child(OperationDetail("More details"));
// Do step
success = ...
od.get_last_child().set_status(success ? STATUS_SUCCESS
: STATUS_ERROR);
At this point any libparted messages reported via exceptions need to be
added into the OperationDetail tree. Also adding further children into
the tree after collecting those errors needs to be prohibited (as much
as the previous patch prohibited it).
Add a new method which will replace the final set_status() call above
like this which set the status, captures the errors and flags that
further children shouldn't be added:
...
od.get_last_child().set_success_and_capture_errors(status);
It emits a callback to capture the errors to provide flexibility and so
that the OperationDetail class doesn't have to get into the details of
how GParted_Core saves libparted exception messages.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Want functionality to prevent further child details being added to an
OperationDetail. This is so that the captured libparted error messages
are always the last child in the list, and more details (at that point
in the tree) can't be added.
For example we want GParted to report like this:
Move /dev/sdb3 to the right and shrink it from 1.14 GiB to...(SUCCESS)
...
* shrink partition from 1.14 GiB to 1.00 GiB (SUCCESS)
* old start: 4464640
old end: 6856703
old size: 2392064 (1.14 GiB)
* new start: 4464640
new end: 6561791
new size: 2097152 (1.00 GiB)
* libparted messages (INFO)
* DEBUG: GParted generated synthetic libparted excepti...
and not like this:
Move /dev/sdb3 to the right and shrink it from 1.14 GiB to...(SUCCESS)
...
* shrink partition from 1.14 GiB to 1.00 GiB (SUCCESS)
* old start: 4464640
old end: 6856703
old size: 2392064 (1.14 GiB)
* libparted messages (INFO)
* DEBUG: GParted generated synthetic libparted excepti...
* new start: 4464640
new end: 6561791
new size: 2097152 (1.00 GiB)
So actually preventing the addition of more child details would stop
users seeing information they should see. So instead just report a bug
message into the operation details. This doesn't stop anything, but the
bug message will be seen and allow us to fix GParted.
So far nothing is enforced. This patch just adds the mechanism to
report a bug when a new child detail is added when prohibited.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
PATCH SET SUMMARY:
Libparted exception messages are reported into the operation details at
the end of each separate operation. For operations which involve
multiple steps of partition manipulation there is no way to identify
which exceptions occurred with which steps.
Example resize/move operation in which multiple libparted exceptions
were raised:
Move /dev/sdb to the right and shrink it from 1.15 GiB to ...(ERROR)
* calibrate /dev/sdb3 (SUCCESS)
* check file system on /dev/sdb3 for errors and (if possib...(SUCCESS)
* e2fsck -f -y -v -C 0 '/dev/sdb3' (SUCCESS)
* shrink file system (SUCCESS)
* resize2fs -p 'dev/sdb3' 1048576K (SUCCESS)
* shrink partition from 1.14 GiB to 1.00 GiB (SUCCESS)
* check file system on /dev/sdb3 for errors and (if possib...(SUCCESS)
* e2fsck -f -y -v -C 0 '/dev/sdb3' (SUCCESS)
* grow partition from 1.00 GiB to 1.12 GiB (SUCCESS)
* move file system to the right (SUCCESS)
* e2image -ra -p -O 134217728 '/dev/sdb3' (SUCCESS)
* shrink partition from 1.12 GiB to 1.00 GiB (ERROR)
* libparted messages (INFO)
* DEBUG: GParted generated synthetic libparted exception...
* Error informing the kernel about modifications to part...
* Error informing the kernel about modifications to part...
* DEBUG: GParted generated synthetic libparted exception...
* DEBUG: GParted generated synthetic libparted exception...
But there is no way to know which of the libparted steps: 1 calibrate or
3 partition resize steps encountered which exceptions.
Fix this by reporting the libparted messages into the operation details
at the point at which they occur. Then the above example would become:
Move /dev/sdb to the right and shrink it from 1.15 GiB to ...(ERROR)
* calibrate /dev/sdb3 (SUCCESS)
* check file system on /dev/sdb3 for errors and (if possib...(SUCCESS)
* e2fsck -f -y -v -C 0 '/dev/sdb3' (SUCCESS)
* shrink file system (SUCCESS)
* resize2fs -p 'dev/sdb3' 1048576K (SUCCESS)
* shrink partition from 1.14 GiB to 1.00 GiB (SUCCESS)
* libparted messages (INFO)
* DEBUG: GParted generated synthetic libparted excepti...
* check file system on /dev/sdb3 for errors and (if possib...(SUCCESS)
* e2fsck -f -y -v -C 0 '/dev/sdb3' (SUCCESS)
* grow partition from 1.00 GiB to 1.12 GiB (SUCCESS)
* libparted messages (INFO)
* Error informing the kernel about modifications to pa...
* Error informing the kernel about modifications to pa...
* DEBUG: GParted generated synthetic libparted excepti...
* move file system to the right (SUCCESS)
* e2image -ra -p -O 134217728 '/dev/sdb3' (SUCCESS)
* shrink partition from 1.12 GiB to 1.00 GiB (ERROR)
* libparted messages (ERROR)
* DEBUG: GParted generated synthetic libparted excepti...
THIS PATCH:
Small change so that setting the status of an OperationDetail to N/A,
warning, also stops the execution timer if it was running. Matching
what happens when the status is set to either success or error.
This is to avoid having to set status twice, first time just to stop the
timer, and second time to set it to the desired status when reporting a
warning.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Fix up following switch from whole_device flag to TYPE_UNPARTITIONED.
Also calibrate the type for whole disk device partitions.
Bug 788308 - Remove whole_device partition flag
So they display as previously; as all grey in the graphic and with only
the correct attributes shown.
Fix up following switch from whole_device flag to TYPE_UNPARTITIONED.
Bug 788308 - Remove whole_device partition flag
Fix up following switch from whole_device flag to TYPE_UNPARTITIONED.
Again use FS_UNALLOCATED to determine if a partition represents
unallocated space and creation of a new partition should be allowed.
This is so that trying to create a new partition on a whole disk device
shows the "No partition table found on device /dev/DEV" warning again.
Bug 788308 - Remove whole_device partition flag
After the change from whole_device flag to TYPE_UNPARTITIONED,
unallocated whole disk devices are no longer automatically selected
because the partition type is no longer TYPE_UNALLOCATED. Fix by
checking for file system type FS_UNALLOCATED when identifying the
largest unallocated space.
Bug 788308 - Remove whole_device partition flag
Following the switch from whole_device flag to TYPE_UNPARTITIONED,
unallocated space can be found in two partition types:
TYPE_UNALLOCATED
TYPE_UNPARTITIONED (only when filesystem == FS_UNALLOCATED)
As the file system in both cases is FS_UNALLOCATED check for that when
determining whether a partition is unallocated or not.
Bug 788308 - Remove whole_device partition flag
Remove whole_device flag and replace with new partition type
TYPE_UNPARTITIONED. Minimally adapt the remaining code to compile and
run.
Bug 788308 - Remove whole_device partition flag
Update the partition can be named and partition flags can be managed
checks from being disallowed on unallocated partition types, to being
allowed on primary, logical and extended partition types. This is in
preparation for the introduction of new unallocated partition type.
Bug 788308 - Remove whole_device partition flag
Now when refreshing the devices, GParted_Core::read_label() calls
GParted_Core::get_fs() with parameter FS_EXTENDED.
Before this change, get_fs() would fail to find file system capabilities
set for FS_EXTENDED and construct a not supported capabilities set and
return that.
Afterwards, find_supported_filesystems() creates a not supported
capabilities set from the NULL pointer for FS_EXTENDED and adds this
entry into the FILESYSTEMS vector. Then get_fs() finds that not
supported capabilities set for FS_EXTENDED in the FILESYSTEMS vector and
returns that.
This makes no functional difference. It just seems right as other
unsupported but used file system types have entries in FILESYSTEM_MAP.
See this earlier commit doing the same thing:
7870a92b80
Add FILESYSTEM_MAP[FS_UNALLOCATED] entry
For example GParted_Core::read_label() is already called for empty
partitions where .filesystem = FS_UNKNOWN. In that case get_fs()
returns an unsupported capability set with all capabilities set to
FS::NONE. The same would be true extended partitions where .filesystem
= FS_EXTENDED too; get_fs() would return an unsupported capability set
with all capabilities set to FS::NONE.
Therefore the if not extended partition condition around the switch
statements is not necessary in GParted_Core::read_label(), read_uuid(),
label_filesystem() and change_uuid(). Remove.
This also achieves removal of some uses of partition type enumerations
in advance of the introduction of new partition type TYPE_UNPARTITIONED.
Bug 788308 - Remove whole_device partition flag
PATCHSET OVERVIEW:
When unpartitioned drive read-write support was added this commit added
a whole_device flag:
5098744f9a
Add whole_device flag to the partition object (#743181)
Using a whole_device flags now seems not the correct way to model
unpartitioned drives. GParted models an uninitialised drive as:
.path = _("uninitialized")
.type = TYPE_UNALLOCATED
.whole_device = true
.filesystem = FS_UNALLOCATED
and a whole drive file system, using ext4 for example, as:
.path = "/dev/sdb"
.type = TYPE_PRIMARY
.whole_device = true
.filesystem = FS_EXT4
No partitioning changed yet the type of the partition in the model
changed between TYPE_UNALLOCATED and TYPE_PRIMARY depending on whether
the whole drive contains a recognised file system or not.
The partition object describing a file system within a LUKS encryption
mapping is another case of the model not matching reality.
.path = /dev/mapper/crypt_sdb1_crypt
.type = TYPE_PRIMARY
.whole_device = true
.filesystem = FS_EXT4
There is no partition table within the encryption mapping, the file
system fills it, but GParted records it as a primary partition.
Make TYPE_UNALLOCATED and TYPE_PRIMARY be reserved for representing
unallocated space and primary partitions within a partitioned disk drive
and introduce new TYPE_UNPARTITIONED for all cases of an unpartitioned
whole disk drive.
The GParted UI does differentiate between an unallocated whole disk
device and anything else by requiring a partition table to be created
first, even if that is just the loop partition table. That
determination can simply look for the partition object containing file
system type FS_UNALLOCATED instead.
THIS PATCH:
Create set_unpartitioned() helper method to set a partition object to
represent a whole disk drive and use everywhere such an object is
modelled. This matches what existing methods Set_Unallocated() and
indeed Set() do for unallocated space and any type of partition
respectively.
For now the partition type is still set to either TYPE_UNALLOCATED or
TYPE_PRIMARY so the rest of the code base remains the same.
TYPE_UNPARTITIONED will be introduced later.
Bug 788308 - Remove whole_device partition flag
Missed when adding partition naming at creation time in enhancement:
Bug 746214 - Partition naming enhancements
starting with this commit:
f804bc3244
Allow partition naming on busy partitions (#746214)
There were a few cases of creating a local string variable from a
literal and then passing the variable to execute_command() like this:
Glib::ustring cmd = "whatever";
Utils::execute_command( cmd, ... );
This creates an unnecessary local variable. Instead pass the string
literal directly to Utils::execute_command() like this:
Utils::execute_command( "whatever", ... );
This also make the code a little bit more grep friendly.
No other commands run by GParted are niced, so stop nicing commands run
from the DMRaid module.
I think nicing of possibly long running file system modification
commands would have made virtually no difference because "nice -n 19"
lowered the CPU priority, but such command would be I/O bound.
History:
Nicing of file system modification commands was added by this commit
from 2006-08-21:
82e6f6b132
added nice -n 19, so that all extensive filesystem operations will be
Nicing of DMRaid operations was copied into the DMRaid module when it
was added here in 2009-03-14:
5865c92dc0
Added new class for dmraid support
Nicing was removed from file system modification commands with this
commit from 2013-02-22:
52a2a9b00a
Reduce threading (#685740)
Bug 788007 - Remove minor bits of legacy from DMRaid module
The -P option was first added to dmraid-1.0.0.rc15 released 2006-09-17
[1]. dmraid-1.0.0.rc16 (a later release) was included in Debian 6 and
Ubunbu 12.04 LTS. dmraid-1.0.0.rc13 was included in RedHat/CentOS 5
however the -P option was back ported into RedHat/CentOS 5.1.
All of these distributions are so old as to be no longer supported and
yet they provided the dmraid -P option. Therefore backward
compatibility for dmraid without the -P option is no longer required.
So remove it.
[1] old dmraid source code releases
http://people.redhat.com/heinzm/sw/dmraid/src/old/
Bug 788007 - Remove minor bits of legacy from DMRaid module
Trying to set a file system label to (including the double quotes):
" --help "
fails. For example labelling an ext4 file system would try to run this
command:
# e2label /dev/sdb1 "" --help ""
Usage: e2label device [newlabel]
# echo $?
1
Alternatively trying to create a file system with a label of just a
double quote also fails. The Applying Pending Operations dialog waits
forever and won't cancel or force cancel. Have to use the window
manager close window button to close the dialog. Also GParted reports
this error to the console:
(gpartedbin:9648): glibmm-CRITICAL **:
unhandled exception (type Glib::Error) in signal handler:
domain: g-shell-error-quark
code : 0
what : Text ended before matching quote was found for ". (The text was 'mkfs.xfs -f -L """ /dev/sdb2')
Command strings are parsed and split into argv array by function
Glib::shell_parse_argv() which calls internal glib function
tokenize_command_line() for shell tokenization. It expects the command
string to be properly quoted and escaped and after tokenization, calls
g_shell_unquote() on every parsed argument. So to prevent constructing
incorrect commands, every non-static string needs to be properly quoted.
GParted only puts labels and mount points into double quotes, but has
not escaped special characters in those values itself. This patch
fixes all these problems by using Glib::shell_quote() on all variable
values. Labels, mount points, paths and all others too.
Probably a better solution would be to use a new function which takes
argv array instead of one string with all the, correctly quoted and
escaped, arguments concatenated together.
Bug 787203 - Correctly quote and escape arguments of external programs
passed to execute_command()
Shell fragments must be properly quoted and escaped to prevent execution
of unintended commands derived from user controllable data. For
example:
$ printf '#!/bin/sh\necho test > out\n' > script.sh
$ chmod +x script.sh
$ truncate -s 20M 'jfs;script.sh'
$ mkfs.jfs -q 'jfs;script.sh'
$ ls -l out
ls: cannot access out: No such file or directory
$ sudo PATH=$PWD:$PATH /usr/local/bin/gparted 'jfs;script.sh'
$ sudo PATH=$PWD:$PATH /usr/local/bin/gparted 'jfs;script.sh'
$ ls -l out
-rw-r--r-- 1 root root 5 Sep 12 23:11 out
$ cat out
test
What is happening is that jfs::set_used_sectors() is using the device
name 'jfs;script.sh' without quoting it and passing it to the shell to
execute like this:
sh -c 'echo dm | jfs_debugfs jfs;script.sh'
which the shell duly executes as these two commands:
echo dm | jfs_debugfs jfs
script.sh
This could be a security related issue as "sh -c" is able to execute
arbitrary shell commands from the argument if if contains shell special
characters. Use Glib::shell_quote() [1] to quote and escape file names
and whole commands passed to the shell.
[1] Glib::shell_quote(const std::string & unquoted_string)
https://developer.gnome.org/glibmm/stable/group__ShellUtils.html
"Quotes a string so that the shell (/bin/sh) will interpret the
quoted string to mean unquoted_string.
If you pass a filename to the shell, for example, you should first
quote it with this function."
Bug 787203 - Correctly quote and escape arguments of external programs
passed to execute_command()
Naming a file system image file on the command line is shown by GParted
as unknown.
$ truncate -s 100M /tmp/fat.img
$ mkfs.vfat /tmp/fat.img
$ sudo ./gpartedbin /tmp/fat.img
Currently the FS_Info cache is loaded for all devices reported by
blkid (plus all whole disk devices identified from /proc/partitions even
if blkid reports nothing). However file system images named on the
command line are not queried so GParted can't identify them.
Fix by ensuring that the FS_Info blkid cache is loaded for all named
devices, including named file system image files.
Note that Mount_Info::load_cache() depends on the contents of the
FS_Info cache to lookup UUID= and LABEL= device names from /etc/fstab.
However only file systems in block devices can be mounted like this, and
never file system image files, so the fact that the cache may be
extended afterwards by FS_Info::load_cache_for_paths() does not matter.
History
Prior to version 0.22.0, when unpartitioned drive support was added,
GParted could recognise some file system image files using loop
partition handling in libparted. However libparted before version 3.2
reported the loop partition name as the whole disk device name appended
with "1" so all the query commands were provided a non-existent name to
use. Therefore no file system usage or the label was displayed.
Bug 787181 - Fix detection of file system images
Mike Fleetwood