Struct FS and struct FS_Limits are strongly related to the FileSystem
class, both being return values from members and associated with storing
file system attributes. Move their definitions from Utils.h into
FileSystem.h.
There are too many different types of things named "filesystem" in the
GParted code with the potential to cause confusion. Namely:
std::vector<FS> FILESYSTEMS
Vector of file system capabilities.
class FileSystem Base class interfacing to file system
specific executables for querying and
modification.
enum FILESYSTEM Symbolic constants representing each file
system type.
Many recent written or re-written functions already used a variable
named fstype. Rename enum FILESYSTEM to enum FSType to clearly
distinguish it from the other things with very similar names. Only
changing the name of the enumeration, not the name of variables of that
type too because that is a lot more lines of code and those can be
changed when the relevant code is re-written.
Try to format an existing partition with a file system which doesn't
fit. The error dialog reporting the partition as too small or too large
always claimed the file system was encrypted, whether it was or not.
For example trying to format a 128 MiB partition as btrfs produces this
error dialog:
(-) Cannot format this file system to [Encrypted] btrfs
A [Encrypted] btrfs file system requires
a partition of at least 256.00 MiB.
[ OK ]
This commit:
88136c96d7
Extend functions generating encrypted file system string (#774818)
just completely missed handling the case for non-encrypted file systems
in Utils::get_filesystem_string(). Add the missed code.
There are multiple repetitions of the same code getting a FileSystem
object, checking for NULL and then calling the file system specific
get_filesystem_limits(). Extract that into a common function.
GParted_Core::get_filesystem_limits() can't use the file system from the
passed Partition object because that is the current file system which
will be different from the intended file system for new and format
operations. So would look up the wrong derived FileSystem specific
object and call the wrong get_filesystem_limits(). Hence still needing
fstype as a separate parameter to pass the intended file system.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
UDF file system minimum and maximum size limits are defined in terms of
numbers of file system blocks. So when resizing an existing file system
compute the byte size limits from the existing UDF file system's block
size. Alternatively when creating a new UDF file system use the
device's sector size as the multiplier instead.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
As described in the previous commit, this is so that file system
specific implementations can dynamically determine size limits based on
Partition object attributes: such as the device sector size and the file
system block size. (Assuming set_used_sectors() sets
partition.fs_block_size for the type of file system in question).
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Background information about UDF is that when creating a file system
it's block size must match the underlying device's sector size. For
optical media like CDs and DVDs that is 2K. For hard drives that is
usually 512 bytes or 4K. However if a UDF file system has been copied
from a device with a different sector size the UDF block size won't
match the sector size. Linux will happily mount such UDF file system.
Therefore the derived udf::get_filesystem_limits() will need access to
the file system block size when determining the size limits of an
existing UDF file system being resized and use the device sector size
when a new UDF file system is being created. All this can be queried
from an appropriate Partition object passed to get_filesystem_limits().
All the calls to get_filesystem_limits() have an appropriate Partition
object available already, except in Win_GParted::activate_reformat()
when composing a format operation. Or more correctly
activate_reformat() constructs temp_ptn, a suitable Partition object,
including with fs_block_size member defaulting to -1 indicating not a
resize, but not until after the file system size limits had been checked
and get_filesystem_limits() called.
Therefore reorder the code in activate_paste() so that the file system
size limits are checked after the wanted Partition object has been
created. No functional change with this commit.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
All the code has been switched to call get_filesystem_limits() and use
struct FS_Limits. Remove struct FS members .MIN & .MAX.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Change Dialog_Partition_New to use a fs_limits rather than struct FS
and .MIN and .MAX. No passing of struct FS_Limits required. Just use
the FILESYSTEMS vector of struct FS to provide the file system type and
look up it's size limits each time the selection changes.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Refactor Win_GParted::activate_resize() to query the file system size
limits using the new get_filesystem_limits() method and pass those
limits into the dialog class as struct FS_Limits.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Changes the internal code in Dialog_Partition_Resize_Move to use
fs_limits instead of fs.MIN and fs.MAX. The limits are still passed
into the constructor via struct FS and it's members .MIN and .MAX but
immediately used to assign to fs_limits.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Refactor Win_GParted::activate_paste() to query the file system size
limits using the new get_filesystem_limits() method and pass those
limits into the the dialog class as struct FS_Limits.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Adds working copy fs_limits member into common Dialog_Base_Partition
class. Changes the internal code in Dialog_Partition_Copy class to use
fs_limits instead of fs.MIN and fs.MAX. The limits are still passed
into the constructor via object of struct FS and it's members .MIN and
.MAX but immediately used to assign to the fs_limits member.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Duplicate the assignment of file system size limits into
struct FS_Limits, matching the fixed values currently assigned to
struct FS members .MIN and .MAX.
Bug 787204 - Minimum and maximum size of the UDF partition/disk
Those external tools were introduced in version 2.0 of udftools package
and can show or change UDF label, UDF uuid and can provide information
needed for counting total/free sectors.
Bug 792052 - Add support for changing UDF label/uuid and show disk usage
Attempt to grow a partition to more than twice it's size. If committing
that change to the partition fails in such a way that the new larger
partition boundaries are not written to the disk drive then rolling back
will fail with libparted error:
Can't have overlapping partitions.
Example operation details:
Grow /dev/sdb8 from 1.00 GiB to 2.20 GiB
* calibrate /dev/sdb8 (SUCCESS)
* check file system on /dev/sdb8 for errors and (if poss...(SUCCESS)
* grow partition from 1.00 GiB to 2.20 GiB (ERROR)
* attempt to rollback failed change to the partition (ERROR)
original start: 7350272
original end: 9447423
original size: 2097152 (1.00 GiB)
* libparted messages (ERROR)
Can't have overlapping partitions.
What happened is that resize_move_partition() passed the new Partition
object to resize_move_partition_implement() as the source partition for
the rollback, and than called ped_disk_partition_by_sector() with a
sector in the middle to identify the partition to be changed. However
the new partition was never written to the drive so in the middle was
outside the old smaller partition. Therefore libparted identified empty
space after the partition, rather than the partition itself, as the
intended target so when ped_disk_set_partition_geom() was called it
reported error "Can't have overlapping partitions" because it thought
another partition was being created with the same boundaries as the old
partition, rather than the boundaries of the old partition being
updated.
The same error also occurs when rolling back a failed partition change
as part of a move operation when the middle of the new partition falls
outside of the boundaries of the old partition.
Fix by making a temporary Partition object from the intersection of the
old and new partition boundaries just to be used to identify the
partition being changed to libparted. As this is only rolling back a
single step adjusting the partition boundaries as part of a resize
and/or move operation, the old and new partition boundaries must
intersect (and in fact that intersection contains the file system data).
Bug 791875 - Rollback specific failed partition change steps
The general rule is that:
1) For a partition change step BEFORE a file system change step,
rollback on failure;
2) For a partition change step AFTER a file system change step, don't
rollback on failure.
Examining every case where resize_move_partition() is called and whether
rollback on failure is wanted or not:
* In resize_move()
Resize / move extended partition. No associated file system change.
NO ROLLBACK
Just to keep possibly applied operation.
* #1 in move()
Making all encompassing partition before moving file system.
ROLLBACK
To restore partition boundaries back to those of the file system.
* #2 in move()
Recreating original partition boundaries after file system move
failed or was cancelled and has been rolled back.
NO ROLLBACK
To keep updated partition boundaries to match restored file system
data.
* #3 in move()
Replacing all encompassing partition with final partition after
successful file system move.
NO ROLLBACK
Keep new partition boundaries to match moved file system.
* #1 in resize_encryption()
Making the partition larger before growing closed LUKS encrypted
data.
ROLLBACK
Restore partition boundaries back to those of the closed LUKS
encrypted data.
* #2 in resize_encryption()
Shrinking the partition after open LUKS mapping has been shrunk, but
before swap is re-created (smaller).
NO ROLLBACK
Difficult case because the partition shrink is in the middle of a
LUKS shrink and a swap shrink (re-create). If swap was actually
shrunk like other types of file system, rather than re-created, then
the operation sequence would be (1) shrink swap, (2) shrink LUKS
encryption, (3) shrink partition. In this hypothetical case and the
actual case no rollback is preferred to try to keep the new
partition boundaries match the shrunk open LUKS encryption mapping.
* #3 in resize_encryption()
Grow the partition before growing open LUKS mapping and re-creating
swap larger.
ROLLBACK
Restore partition boundaries back to those of the smaller open LUKS
encryption mapping.
* #4 in resize_encryption()
Shrink the partition after shrinking the file system and open LUKS
encryption mapping.
NO ROLLBACK
Keep new smaller partition boundaries to match shrunk encrypted file
system.
* #5 in resize_encryption()
Grow the partition before growing the open LUKS encryption mapping
and file system.
ROLLBACK
Restore partition boundaries back to those of the not yet grown
encrypted file system.
* #1 in resize_plain()
Resize partition before re-creating swap a different size.
ROLLBACK
Restore partition boundaries back to those of the not yet resized
(re-created) swap space.
* #2 in resize_plain()
Shrink partition after shrinking the file system.
NO ROLLBACK
Keep new smaller partition boundaries to match shrunk file system.
* #3 in resize_plain()
Grow partition before growing the file system.
ROLLBACK
Restore partition boundaries back to those of the not yet grown
file system.
Removes the default value from the rollback_on_fail parameter so
rollback or not has to be explicitly specified for every call of
resize_move_partition().
Bug 791875 - Rollback specific failed partition change steps
Even after implementing a fix for bug 790418 "Unable to inform the
kernel of the change message may lead to corrupted partition table"
GParted/libparted can still encounter errors informing the kernel of the
new partition layout. This has been seen with GParted on CentOS 7 with
libparted 3.1.
In such a case the partition has been successfully written to the disk
but just informing the kernel failed. This is a problem because when a
partition is being moved in advance of a file system move step, failure
to inform the kernel leaves the partition boundaries not matching the on
disk limits of the file system. For a move to the left this leaves the
partition reported as unknown, apparently losing the user's data.
For example start with a 512 MiB partition containing an XFS file
system. This is recognised by blkid and parted, hence also by GParted.
# blkid /dev/sdb1
/dev/sdb1: UUID=... TYPE="xfs" PARTUUID="37965980-01"
# parted /dev/sdb unit s print
Model: ATA VBOX HARDDISK (scsi)
Disk /dev/sdb: 16777216s
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Disk Flags:
Number Start End Size Type File system Flags
1 1048576s 2097151s 1048576s primary xfs
Now move the partition 100 MiB to the left and have it fail to inform
the kernel after the first partition change step. Operation details:
Move /dev/sdb1 to the left (ERROR)
* calibrate /dev/sdb1 (SUCCESS)
* check file system on /dev/sdb1 for errors and (if poss...(SUCCESS)
* grow partition from 512.00 MiB to 612.00 MiB (ERROR)
old start: 1048576
old end: 2097151
old size: 1048576 (512.00 MiB)
requested start: 843776
requested end: 2097151
requested size: 1253376 (612.00 MiB)
* libparted messages (ERROR)
Error informing the kernel about modifications to partition
/dev/sdb1 -- Device or resource busy. This means Linux won't
know about any changes you made to /dev/sdb1 until you reboot
-- so you shouldn't mount it or use it in any way before
rebooting. Failed to add partition 1 (resource temporarily
unavailable)
Now because the start of the partition is 100 MiB before the start of
the file system, the file system is no longer recognised, and apparently
the user's data has been lost.
# blkid /dev/sdb1
/dev/sdb1: PARTUUID="37965980-01"
# parted /dev/sdb unit s print
...
Number Start End Size Type File system Flags
1 843776s 2097151s 1253376s primary
It doesn't matter why updating the partition failed, even if it was
because of an error writing to the disk. Rollback of the change to the
partition should be attempted. The worst case scenario is that rollback
of the change fails, which is the equivalent to how the code worked
before this patch set.
However in other cases where the partition boundaries are being updated
after a file system move or shrink step then the partition should be
updated to match the new location of the file system itself. And no
rollback is wanted. If the failure was only informing the kernel then
in fact the partition has actually been updated on disk after all.
So each partition resize/move step needs examining on a case by case
basis to decide if rolling back the change to the partition is wanted or
not.
This patch only adds partition change rollback into
resize_move_partition(). Rollback remains disabled until all cases are
examined in the following patch.
Bug 791875 - Rollback specific failed partition change steps
Extract common code which updates a DMRaid device mapper entry into a
sub-function. This will also be needed when adding rollback of a
partition change on failure.
Bug 791875 - Rollback specific failed partition change steps
Extract the code which actually implements the partition change into a
sub-function ready for adding rollback of the change on failure.
Bug 791875 - Rollback specific failed partition change steps
This is not required, but it is more logical to have an OperationDetail
object created and it's final status set in the same function rather
than split between caller and callee. So move creation of
"copy %1 using a block size of %2" OperationDetail objects into
GParted_Core::copy().
Also introduces a couple of variables to remove some recomputation:
benchmark_od & remaining_length.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Performing a copy or move operation which uses GParted's internal copy
routine triggered the new GParted bug message. Example operation
details:
Copy /dev/sdb8 to /dev/sdb (start at 4.51 GiB) (SUCESSS)
* calibrate /dev/sdb8 (SUCCESS)
* check file system on /dev/sdb8 for errors and (if possib...(SUCCESS)
* create empty partition (SUCCESS)
* set partition type on /dev/sdb9 (SUCCESS)
* copy file system from /dev/sdb8 to /dev/sdb9 (SUCCESS)
using internal algorithm
copy 1.00 GiB
* finding optimal block size
* copy 16.00 MiB using a block size of 1.00 MiB (SUCCESS)
16.00 MiB of 16.00 MiB copied
GParted Bug: Adding more information to the result...(WARNING)
0.797269 seconds
* copy 16.00 MiB using a block size of 2.00 MiB (SUCCESS)
* copy 16.00 MiB using a block size of 4.00 MiB (SUCCESS)
* copy 16.00 MiB using a block size of 8.00 MiB (SUCCESS)
* copy 16.00 MiB using a block size of 16.00 MiB (SUCCESS)
optimal block size is 1.00 MiB
* copy 944.00 MiB using a block size of 1.00 MiB (SUCCESS)
This is because when performing the initial benchmarking copies the time
taken by each copy is added to the operation detail results in the
calling GParted_Core::copy_blocks() after the final status was set in
CopyBlocks::copy() with set_success_and_capture_errors(). Fix by
setting the final status in the parent function after adding the time to
the benchmark copies.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
To be consistent with all previous bug messages being translatable.
Also only mark the bug as a warning instead of an error because the bug
doesn't cause any disk drive operations to fail.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
There is still another subtle issue. When GParted_Core::commit() closes
the device, the kernel initiates a second set of events which removes
and re-adds the partitions again. Need to wait for these to complete
to prevent any following step failing with missing partition device
nodes.
Bug 790418 - "Unable to inform the kernel of the change" message may
lead to corrupted partition table
Operations involving modifications to a partition are sometimes failing
with a libparted error informing the kernel about modifications to
partitions. For example I encountered these errors when just creating a
fourth partition on CentOS 7 in a VirtualBox VM. Operation results:
Create Primary Partition #1 (ext4, 4.73 GiB) on /dev/sdb (ERROR)
* create empty partition (ERROR)
* libparted messages (ERROR)
* Error informing the kernel about modification to partition
/dev/sdb1 -- Device or resource busy. This means Linux won't
know about any changes you made to /dev/sdb1 until you reboot
-- so you shouldn't mount it or use it in any way before
rebooting.
* Failed to add partition 1 (Resource temporarily unavailable)
Those two libparted messages were presented in "Libparted Error" dialogs
and [Cancel] was selected both times.
Libparted Error
(-) Error informing the kernel about modifications to partition
/dev/sdb1 -- Device or resource busy. This means Linux won't
know about any changes you made to /dev/sdb1 until you reboot --
so you shouldn't mount it or use it in any way before rebooting.
[ Cancel ] [ Ignore ]
Libparted Error
(-) Failed to add partition 1 (Resource temporarily unavailable)
[ Retry ] [ Cancel ]
This is the edited output showing GParted print debugging, stracing of
GParted and monitoring of udev events for this case.
# ./gpartedbin /dev/sdb
======================
libparted : 3.1
======================
...
24.541604 +23.923435 create_partition() start (new_partition, optdet, min_size=0) new_partition.device_path="/dev/sdb"
24.556101 +0.014497 create_partition() type=PED_PARTITION_NORMAL
24.556354 +0.000253 commit() start (lp_disk) lp_disk->dev->path="/dev/sdb"
D: strace pid 18054. Press [Return] to continue.
^Z
[1]+ Stopped ./gpartedbin /dev/sdb
# udevadm monitor &
[2] 18124
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent
# strace -p 18054 -e open,ioctl,write,close &
[3] 18129
strace: Process 18054 attached
# fg %1
./gpartedbin /dev/sdb
128.175811 +103.619457 commit() calling ped_disk_commit_to_dev(lp_disk) ...
open("/dev/sdb", O_RDWR) = 6
ioctl(6, BLKFLSBUF) = 0
write(6, "\372\270\0\20\216\320\274\0\260\270\0\0\216\330\216\300\373\276\0|\277\0\6\271\0\2\363\244\352!\6\0"..., 512) = 512
ioctl(6, BLKFLSBUF) = 0
close(6)
128.181352 +0.005542 commit() ped_disk_commit_to_dev(lp_disk) returned true
128.181475 +0.000122 commit_to_os() start (lp_disk, timeout=10) lp_disk->dev->path="/dev/sdb"
128.181527 +0.000052 commit_to_os() calling ped_disk_commit_to_os(lp_disk) ...
open("/dev/sdb", O_RDWR) = 6
ioctl(6, BLKFLSBUF) = 0
open("/sys/block/sdb/ext_range", O_RDONLY) = 7
close(7) = 0
KERNEL[1158935.380543] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
KERNEL[1158935.380565] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[1158935.380578] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
ioctl(6, BLKPG, {BLKPG_DEL_PARTITION, flags=0, datalen=152, data={start=0, length=0, pno=1, devname="", volname=""}}) = -1 ENXIO (No such device or address)
ioctl(6, BLKPG, {BLKPG_DEL_PARTITION, flags=0, datalen=152, data={start=0, length=0, pno=2, devname="", volname=""}}) = -1 ENXIO (No such device or address)
ioctl(6, BLKPG, {BLKPG_DEL_PARTITION, flags=0, datalen=152, data={start=0, length=0, pno=3, devname="", volname=""}}) = -1 ENXIO (No such device or address)
...
KERNEL[1158935.380977] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
KERNEL[1158935.381296] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
KERNEL[1158935.381367] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[1158935.381432] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[1158935.382992] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
ioctl(6, BLKPG, {BLKPG_DEL_PARTITION, flags=0, datalen=152, data={start=0, length=0, pno=62, devname="", volname=""}}) = -1 ENXIO (No such device or address)
ioctl(6, BLKPG, {BLKPG_DEL_PARTITION, flags=0, datalen=152, data={start=0, length=0, pno=63, devname="", volname=""}}) = -1 ENXIO (No such device or address)
ioctl(6, BLKPG, {BLKPG_DEL_PARTITION, flags=0, datalen=152, data={start=0, length=0, pno=64, devname="", volname=""}}) = -1 ENXIO (No such device or address)
ioctl(6, BLKPG, {BLKPG_ADD_PARTITION, flags=0, datalen=152, data={start=1048576, length=1073741824, pno=1, devname="/dev/sdb1", volname=""}}) = -1 EBUSY (Device or resource busy)
write(2, "Error informing the kernel about"..., 251) = 251
Error informing the kernel about modifications to partition
/dev/sdb1 -- Device or resource busy. This means Linux won't know
about any changes you made to /dev/sdb1 until you reboot -- so you
shouldn't mount it or use it in any way before rebooting.
UDEV [1158935.384641] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
UDEV [1158935.390203] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
UDEV [1158935.390243] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [1158935.462866] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [1158935.469207] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [1158935.471512] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
UDEV [1158935.492173] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
write(2, "Failed to add partition 1 (Resou"..., 60) = 60
Failed to add partition 1 (Resource temporarily unavailable)
close(6)
KERNEL[1158955.730960] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
KERNEL[1158955.731095] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[1158955.731314] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[1158955.731397] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
KERNEL[1158955.731817] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
KERNEL[1158955.731981] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
KERNEL[1158955.732166] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
KERNEL[1158955.732232] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
KERNEL[1158955.733955] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
148.533154 +20.351627 commit_to_os() ped_disk_commit_to_os(lp_disk) returned false
UDEV [1158955.738262] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
UDEV [1158955.738460] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [1158955.738525] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
148.537648 +0.004494 execute_command() udevadm settle --timeout=10
UDEV [1158955.740864] remove /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [1158955.760192] change /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb (block)
UDEV [1158955.801211] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb4 (block)
UDEV [1158955.815262] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb3 (block)
UDEV [1158955.815314] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb2 (block)
UDEV [1158955.828134] add /devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdb/sdb1 (block)
148.630797 +0.093149 execute_command() exit status 0
148.630882 +0.000085 commit_to_os() return false
D: stop strace pid 18054. Press [Return] to continue.
^Z
[1]+ Stopped ./gpartedbin /dev/sdb
# kill %3
strace: Process 18054 detached
[3]- Done strace -p 18054 -e open,ioctl,write,close
# kill %2
[2] Done udevadm monitor
# fg %1
./gpartedbin /dev/sdb
173.700143 +25.069261 commit() return false
173.700470 +0.000327 create_partition() return false
What happens is that GParted calls ped_disk_commit_to_dev() which opens
the device, writes the updated partition table and closes the device.
When the device closes the kernel initiates asynchronous uevents and
user space udev rules which remove and re-add all the partitions. In
the mean time GParted calls ped_disk_commit_to_os() to inform the kernel
of the changes to the partition table. This involves opening the
device, using ioctl() to remove all possible partitions [1] and re-add
needed partitions. It finds partitions 1 to 3 already removed and
accepts this along with all other non-existent partitions up to 64.
When it tries to re-add partition 1 the ioctl() BLKPG_ADD_PARTITION call
returns EBUSY. Presumably because the partition is in use by udev which
is in the process of running the user space rules associated with
removing and re-adding it. Then ped_disk_commit_to_os() closes the
device which initiates a second round of asynchronous uevents and user
space udev rules removing and re-adding all the partitions again.
So in summary the kernel and udev are removing and re-adding the
partitions exactly when libparted is trying to do exactly the same
thing!
[1] The algorithm in libparted 3.1 is to try to remove all possible
partitions, 64 for this kernel, followed by re-adding the needed
partitions.
parted/libparted/arch/linux.c:_disk_sync_part_table()
http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/arch/linux.c?h=v3.1#n2541
Partprobe has had exactly the same issue with failing to inform the
kernel about modifications to the partition table [2]. This was fixed
in libparted post v3.2 release by this commit [3].
[2] rhbz#1339705 - ceph-disk prepare: Error: partprobe /dev/vdb failed :
Error: Error informing the kernel about modifications to partition
/dev/vdb1 -- Device or resource busy.
https://bugzilla.redhat.com/show_bug.cgi?id=1339705
[3] partprobe: Open the device once for probing
Previously there were 3 open/close pairs for the device, which may
result in triggering extra udev actions. Instead, open it once at
the start of process_dev and close it at the end.
http://git.savannah.gnu.org/cgit/parted.git/commit/?id=cfafa4394998a11f871a0f8d172b13314f9062c2
Implement the same fix as implemented for partprobe. Hold a file handle
open which libparted can use internally to avoid having to open() and
close() the device itself twice, once for each of the calls
ped_disk_commit_to_dev() and ped_disk_commit_to_os(). This avoids the
first close() initiating the kernel and udev to remove and re-add the
partitions exactly when ped_disk_commit_to_os() is trying to do the same
thing.
Bug 790418 - "Unable to inform the kernel of the change" message may
lead to corrupted partition table
The includes were missed being removed by this earlier refactoring
commit which reduced direct access to the single ProgressBar object:
b1313281bd
Simplify use of the progress bar via OperationDetail (#760709)
All libparted messages were reported as informational, even for a step
which failed. Instead identify libparted messages as either
informational or errors depending on whether this step was successful
or not respectively.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Resizing any unmounted file system which has to be mounted to be resized
triggered the new GParted bug message. However the operation did
complete successfully. Example operation details:
Grow /dev/sdb8 from 1.00 GiB to 1.50 GiB (SUCCESS)
* calibrate /dev/sdb
* check file system on /dev/sdb8 for errors and (if possib...(SUCCESS)
* grow partition from 1.00 GiB to 1.50 GiB (SUCCESS)
* grow file system to fill the partition (SUCCESS)
* mkdir -v /tmp/gparted-wvH0Ez (SUCCESS)
* GParted Bug: Adding another child after no_more_chil...(ERROR)
* Created directory /tmp/gparted-wvH0Ez
* mount -v -t btrfs '/dev/sdb8' '/tmp/gparted-wvH0Ez' (SUCCESS)
* btrfs filesystem resize 1:max '/tmp/gparted-wvH0Ez' (SUCCESS)
* umount -v '/tmp/gparted-wvH0Ez' (SUCCESS)
* rmdir -v /tmp/gparted-wvH0Ez (SUCCESS)
* GParted Bug: Adding another child after no_more_chil...(ERROR)
* Removed directory /tmp/gparted-wvH0Ez
This is because set_success_and_capture_errors() was called first and
the child details added after. Reverse this ordering to fix.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Transition all remaining code, DMRaid and file system code, to use the
new method of reporting success of a step and automatic error
collection. None of this code calls libparted so can't generate any
libparted exceptions. This is just for consistency so all code follows
the same pattern using set_success_and_capture_errors().
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Refactor nested if-then-else into a sequence of if fail return early.
Makes the code simpler to understand and converts separate
OperationDetail::set_status() calls for success or error into a single
call using ternary conditional matching how it is or was done everywhere
else. This is also ready for status and error capture refactoring.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Transition GParted block copying code and partition manipulation code,
which uses libparted API, to the new method of reporting success of a
step and automatic error collection. Libparted exceptions are now
reported with the step at which they occurred.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
Just copies the callback into each newly added child detail. As there
are no more uses of set_success_and_capture_errors() yet, libparted
errors are still only captured once at the top-level of each operation.
Bug 790842 - Report libparted messages into operation details at the
point at which they occur
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
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
Add double quote (") to the list of prohibited FAT label characters,
previously missed [1][2].
Also add single quote (') because mlabel encoded it in a way that both
Windows and blkid don't understand, although mlabel can correctly decode
it itself.
# export MTOOLS_SKIP_CHECK=1
# mlabel ::"MIKE'S" -i /dev/sdf1
# mlabel -s :: -i /dev/sdf1
Volume label is MIKE'S (abbr=MIKE_S~1???)
# blkid -o value -s LABEL /dev/sdf1
MIKE_S~1???
(8-bit characters in the above output have been replaced with
question marks (?) just to keep this commit message as 7-bit ASCII).
Finally exclude ASCII control characters below SPACE (0x00 to 0x1F) as
they also cause mlabel to ask a question and wait for input in the same
way that prohibited characters do. As discussed in the previous commit
[1] the only way to stop GParted waiting forever is to manually kill
mlabel with signal 9 (KILL).
# mlabel ::"^A" -i /dev/sdf1
Long file name "^A" contains illegal character(s).
a)utorename A)utorename-all r)ename R)ename-all
s)kip S)kip-all q)uit (aArRsSq):
[1] 584137b32b
Remove prohibited characters from FAT16/32 labels (#755608)
[2] Microsoft TechNet: Label
https://technet.microsoft.com/en-us/library/bb490925.aspx
Bug 787202 - Update list of prohibited fat label characters
So far GParted is still loading the default non-reversible encoded
labels from blkid in the initial loading of the FS_Info module cache.
This encoded label is used to match LABEL=<label> when reading
/etc/fstab, via the get_path_by_label() call, so works for ASCII only
labels. This prevents GParted enabling the "mount on >" partition menu
item when non-ASCII labels are used.
To fix this:
1) Stop reading the labels the wrong way.
Via the blkid command used to initially load the FS_Info module cache
and is subject to default non-reversible encoding of non-printable
ASCII bytes.
2) Read all the labels the right way, but only when needed.
Only when /etc/fstab file contains LABEL=<label> and
get_path_by_label() is called, read all the labels from blkid without
encoding them via run_blkid_update_cache_one_label().
3) Return label from the cache.
get_label() returns the cached label, loading it into the cache first
if needed with run_blkid_update_cache_one_label().
In the worst case scenario of having a LABEL=<label> in /etc/fstab blkid
will be run for every partition containing a recognised file system to
read the label. On my desktop with 5 hard drives, 4 SWRaid arrays and
31 recognised file systems running 'blkid -o value -s LABEL ...' 31
times took 0.074 seconds of a total scan time of 9.072 seconds. Less
that 1% of the total scanning time. When LABEL=<label> is not used in
/etc/fstab individual blkid executions are only used to read labels for
file systems where there is no file system specific tool available
reducing the impact further. Blkid itself caches the data in it's
blkid.tab cache file rather than reading all file systems on each
invocation. Also the Linux file system cache will already contain the
blkid executable file, needed libraries files and the blkid.tab cache
file itself. Hence why repeated execution of blkid is so fast.
Further to the updated comment in set_partition_label_and_uuid().
Matching LABEL=<label> from /etc/fstab uses the label obtained from
blkid run in the C locale so this kind of assumes it returns the label
correctly and it does for my limited testing on Unicode enabled
desktops. Just not sure if it would be true for all cases in all
locales compared to the FS specific command run in the users default
locale.
Bug 786502 - Support reading Unicode labels when file system specific
tools aren't available
Move the code which reads the Unicode label from FS_Info::get_label()
into new function run_blkid_update_cache_one_label() which also replaces
the non-reversibly encoded copy loaded during the initial cache load.
This is mainly a bit of code refactoring ready for the following change.
It deliberately keeps the initial loaded labels so that reading
/etc/fstab and decoding LABEL=<label> to block special device names via
FS_Info::get_path_by_label() continues to works, at least for ASCII only
labels.
Bug 786502 - Support reading Unicode labels when file system specific
tools aren't available
For file systems which don't provide a specific tool to read labels
(exfat, f2fs, hfs, hfsplus, udf and ufs) or when the tools aren't
installed for any other file system, reading labels falls back to using
the blkid command. Blkid encodes non-ASCII bytes in it's output [1].
For example blkid reports a short Unicode label like this:
# blkid /dev/sdb12
/dev/sdb12: LABEL="M-PM-^ZM-PM->M-QM-^HM-PM-:M-PM-0" TYPE="hfsplus"
This shows encoding using 'M-' for bytes above 127 and caret '^' for
control characters. Unfortunately neither 'M-' or '^' are encoded by
blkid so it is impossible to distinguish between the original label
containing either of these sequences or encoded non-printable bytes.
See this Bugzilla's bug 786502 entry for more details. Unfortunately
this makes the default output format from blkid unsuitable for reading
Unicode character labels.
Instead instruct blkid to print values without encoding them using the
'-o value' option. This just produces a list of new line separated
values without being able to identify which TYPE, UUID or LABEL belongs
to which device. So query just the label for each block device one at
a time. This method has the advantage of also working with all versions
of blkid, from version 1.0.0 (12-Feb-2003) in CentOS 5 to the latest
version 2.30.1 (20-Jul-2017) in Fedora 26.
# blkid -o value -s LABEL /dev/sdb12 | hexdump -C
00000000 d0 9a d0 be d1 88 d0 ba d0 b0 0a |...........|
0000000b
(Using hexdump -C just so that this commit message only contains ASCII
characters).
Therefore this commit changes FS_Info::get_label() to query blkid as
shown above. Note that GParted_Core::set_partition_label_and_uuid()
only calls get_label() when there is no file system specific tool
available (or the tool failed).
The FS_Info cache still contains copies of the labels subject to blkid
encoding, and that will be addressed in following commits.
[1] blkid.c v2.30.1 safe_print()
https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/misc-utils/blkid.c?h=v2.30.1#n111
Bug 786502 - Support reading Unicode labels when file system specific
tools aren't available
When udftools are not installed and the mkudffs program isn't found
GParted would report this error during startup:
# ./gpartedbin
======================
libparted : 3.1
======================
Failed to execute child process "mkudffs" (No such file or directory)
Only run mkudffs to check for an old version when the program is found.
Bug 786050 - GParted reports failed to execute child process "mkudffs"
when it is not installed
Make especially the Volume Identifier length limit code simpler to
understand and therefore easier to maintain.
Bug 784533 - Add support for UDF file system
UDF label is stored in the Logical Volume Identifier which has space for
either 126 Latin1 or 63 UCS-2 characters. For compatibility reasons
with older versions of blkid, the possibly truncated UDF label is also
stored in the Volume Identifier which only has space for 30 Latin1 or 15
UCS-2 characters.
Because versions of mkudffs prior to 1.1 damage the label if it contains
non-ASCII characters, make sure GParted does not call such versions of
mkudffs with a non-ASCII character label.
Bug 784533 - Add support for UDF file system
Add support for detecting UDF file systems and formatting hard disks
with revision 2.01 UDF file systems using udftools. Formatting optical
disks or any other media types is not supported yet. Changing label or
UUID after formatting is not supported as the tools do not yet exist.
Bug 784533 - Add support for UDF file system
Btrfstune changed it's command line help output from being written on
stderr to being written on stdout in the following commit first included
in btrfs-progs 4.7.2 released 05-Sep-2016. This breaks GParted
detection of the change UUID capability. Found on the very latest
distributions: up to date Arch Linux and Fedora 26 Beta. Fix this.
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git/commit/?id=57d1cbd867311d99a2ae5e3cdcffd04
btrfs-progs: print help test to stdout
Bug 784467 - No longer detecting btrfs change UUID capability
Internal file system detection is broken for detection of LVM2 PVs
because it reports finding LVM2 PV even when only the first magic is
found. Prepare a partition like this:
# lvm pvcreate /dev/sdb1
# hexdump -C /dev/sdb1 > hexdump-1.txt
Clear the second magic:
# python
f = open("/dev/sdb1","w")
f.seek(0x218)
f.write("\x00"*4)
f.close()
quit()
# hexdump -C /dev/sdb1 > hexdump-2.txt
# diff -u hexdump-[12].txt
00000200 4c 41 42 45 4c 4f 4e 45 01 00 00 00 00 00 00 00 |LABELONE........|
-00000210 58 69 83 e1 20 00 00 00 4c 56 4d 32 20 30 30 31 |Xi.. ...LVM2 001|
+00000210 58 69 83 e1 20 00 00 00 00 00 00 00 20 30 30 31 |Xi.. ....... 001|
^^ ^^ ^^ ^^ ^^^^
00000220 52 4b 31 73 50 77 49 66 6a 72 55 4c 6a 4d 30 58 |RK1sPwIfjrULjM0X|
GParted still detects this as an LVM2 PV even though lvm and blkid do
not.
Correct logic error and only perform memcmp() with the second magic when
the second signature is non-NULL.
Bug 783997 - GParted detecting LVM2 PV regardless whether second magic
matches or not
For large output a lot of time is used copying capturebuf to callerbuf
to provide a Glib::ustring copy of the buffer for the update callback.
However update callbacks are only used when commands are run to apply
operations by FileSystem::execute_command() and their output is
incrementally displayed in the UI. Whereas update callbacks are never
used when commands are used to query information via
Utils::execute_command().
Stop performing interim copying of capturebuf to callerbuf when there
are no update callbacks registered as it is unnecessary.
Time to read portions of the recorded fsck.fat output via
fat16::set_used_sectors() and intermediate copies aren't required:
1 MiB 10 MiB 122 MiB
old code : 0.074 sec 1.41 sec 210 sec [3:30]
new code : 0.063 sec 0.56 sec 6.57 sec
Bug 777973 - Segmentation fault on bad disk
For large output PipeCapture spends all it's time copying the capturebuf
to callerbuf to provide a consistent view for any registered update
callbacks. This overhead is dependant of the size of the ever growing
captured output and the number of times OnReadable() is called.
Therefore increase the maximum read size to exponentially reduce this
overhead.
Time taken to read varying amounts of fsck.fat output with various
read buffer sizes:
1 MiB 10 MiB 122 MiB
512b : 0.60 sec 65 sec [1:05] 17262 sec [4:47:42]
4096b : 0.19 sec 13 sec 2157 sec [ 35:57]
64K : 0.07 sec 1.4 sec 210 sec [ 3:30]
Note that this is only increasing the maximum size that can be read from
the output of the external command. If the command produces it's output
slowly, such as the with progress reporting commands like mkfs.ext4,
then only the available number of bytes is read reporting the next
progress increment. However if the command produces it's output
quickly, such as when testing this bug using a modified fsck.fat
concatenating the 122 MiB of pre-recorded output, then full buffer reads
are performed.
To ensure that a single call to OnReadable() couldn't block the UI too
long, the time taken for OnReadable() to process a full buffer of
various sizes was recorded as:
512b : 0.031 milliseconds
4096b : 0.188 milliseconds
64K : 3.576 milliseconds
Adding this amount of processing time in the UI under normal
circumstances is not a problem.
As the captured output increases, the time taken by OnReadable() becomes
dominated by the time taken to copy the ever increasing capture buffer
to handle it's expansion and to copy it to the caller buffer for the
update callback. At the end of the 122M captured fsck.fat output
OnReadable() takes 350 milliseconds per call. This is not a problem
because this is an extreme case in which GParted is already hung and
increasing the buffer size is reducing the overall hang time from over
4 hours to a few minutes.
Bug 777973 - Segmentation fault on bad disk
If PipeCapture reads a NUL byte in the middle of what is expected to be
a multi-byte UTF-8 character then PipeCapture either returns the
captured characters to the previous update or loops forever depending on
whether the end of the stream is encountered before the read buffer is
full or not. This is equivalent to saying whether the NUL byte occurs
within the last 512 bytes of the output or not.
This is caused by a bug in g_utf8_get_char_validated() reporting that a
partial UTF-8 character has been found when the NUL byte is encountered
in the middle of a multi-byte character even though more bytes are
available in the length specified buffer. g_utf8_get_char_validated()
is always stopping at the NUL byte assuming it is working with a NUL
terminated string.
Workaround this by checking for g_utf8_get_char_validated() claiming a
partial UTF-8 character has been found when in fact there are at least
enough bytes in the read buffer to instead determine that it is really
an invalid UTF-8 character.
Reference:
Bug 780095 - g_utf8_get_char_validated() stopping at nul byte even
for length specified buffers
https://bugzilla.gnome.org/show_bug.cgi?id=780095
Bug 777973 - Segmentation fault on bad disk
If PipeCapture reads a NUL character, a valid UTF-8 character, it causes
GParted to allocate all available memory and crash. The while loop in
PipeCapture::OnReadable() loops forever reading the same NUL character
from readbuf because g_utf8_find_next_char() doesn't advance past it.
Hence an infinite number of NUL characters are added to the current
line, linevec.
Workaround this by checking for this failure case of
g_utf8_find_next_char() and increment past the NUL character.
This is actually a bug recently fixed in glib 2.49.3 released
2016-07-17. References:
* Bug 547200 - g_utf8_find_next_char() issues
https://bugzilla.gnome.org/show_bug.cgi?id=547200
* https://git.gnome.org/browse/glib/commit/?id=e0e652e4032a181d4f0b0a12aeddf0678b7a3c04
Fix a corner-case in g_utf8_find_next_char
In the case that *p is '\0', we should return p + 1, not p.
This change allows to simplify g_utf8_find_next_char a bit.
Bug 777973 - Segmentation fault on bad disk
A user had a very corrupted FAT file system such that fsck.fat produced
122 MiB of output. GParted has to read this output to get the file
system usage information. However GParted takes more than 48 hours to
read the 122 MiB of output, while using 100% CPU time and is
unresponsive for the duration.
Modified fsck.fat to output just the first 1 MiB of output and used perf
to capture performance data from GParted reading that output:
# perf -g -F 1999 -- ./gpartedbin
# perf report --stdio
67.84% Glib::ustring::replace [4.23s]
17.67% g_utf8_pointer_to_offset [1.10s]
8.48% g_utf8_offset_to_pointer [0.53s]
[ 6.01% (everything else) ] [0.38s]
[100.00% TOTAL ] [6.24s]
And to read the first 10 MiB of output the performance figures are:
92.95% Glib::ustring::replace [257.44s]
4.35% g_utf8_pointer_to_offset [ 12.05s]
2.13% g_utf8_offset_to_pointer [ 5.90s]
[ 0.58% (everything else) ] [ 1.61s]
[100.00% TOTAL ] [277.00s]
See how the total time is increasing non-linearly, 44 times longer for
only 10 times as much data. This is because of the exponential increase
in time spent in Glib::ustring::replace.
After a lot of experimentation I came to the conclusion that
Glib::ustrings are not appropriate for storing and editing large buffers
of data, sizes megabytes and above. The issues are that iterators are
invalid after the content changes and replacing UTF-8 characters by
index gets exponentially slower as the size of the string increases.
Hence the > 48 hours of 100% CPU time to read and apply the line
discipline to the 122 MiB of fsck.fat output. See code comment for a
more detailed description of the issues found.
Rewrote OnReadable() to use Glib::ustrings as little as possible.
Instead using buffers and vectors of fixed width data types allowing for
fast access using pointers and indexes (converted to pointers by the
compiler with simple arithmetic). Again see code comment for a more
detailed description of the implementation.
Repeating the performance capture with the new code for the first 1 MiB
of fsck.fat output:
63.34% memcpy [0.35s]
[ 36.66% (everything else) ] [0.21s]
[100.00% TOTAL ] [0.56s]
And for the first 10 MiB of fsck.fat output:
96.66% memcpy [63.60s]
[ 3.34% (everything else) ] [ 2.20s]
[100.00% TOTAL ] [65.80s]
Simple timings taken to read portions of the fsck.fat output (when not
using perf):
1 MiB 10 MiB 122 MiB
old code : 6.2 sec 277 sec > 48 hours
(4:37)
new code : 0.6 sec 66 sec 17262 sec
(1:06) (4:47:42)
Performance of the code is still non-linear because of the assignment
of the ever growing capturebuf to callerbuf for every block of input
read. This is required to generate a consistent Glib::ustring copy of
the input for the update callback. However this is much faster than
before, and I have a plan for further improvements.
Bug 777973 - Segmentation fault on bad disk
Seems more logical to initially clear the output capture buffer in a
single location in the PipeCapture class which reads the command output
into said buffer, rather than each calling site before the PipeCapture
objects are constructed.
Bug 777973 - Segmentation fault on bad disk
A user had a very corrupted FAT file system and when GParted was reading
the file system usage it would core dump. The tip of the stack trace
was:
#0 Glib::ustring_Iterator<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::operator++()
at /usr/include/glibmm-2.4/glibmm/ustring.h line 957
#1 Glib::ustring_Iterator<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::operator++(int)
at /usr/include/glibmm-2.4/glibmm/ustring.h line 965
#2 GParted::PipeCapture::OnReadable(Glib::IOCondition)
at PipeCapture.cc line 63
#3 GParted::PipeCapture::_OnReadable(_GIOChannel*, GIOCondition, void*)
at PipeCapture.cc line 45
GParted uses 'fsck.fat -n -v /dev/PTN' to get the file system usage.
However because the file system was so corrupted it was reporting every
file name as being damaged, including the file names being reported as
binary data. PipeCapture::OnReadable() reads up to 512 bytes at a time
and then uses a Glib::ustring iterator to loop over the UTF-8
characters, but a Glib::ustring iterator is explicitly not capable of
reading binary data [1]. With invalid UTF-8 bytes the code continued to
read beyond the end of the string until GParted crashed with a
segmentation violation.
Fix by accessing the read string by index instead of by iterator.
[1] Quote from the Glib::ustring_Iterator<T> Class Template Reference:
https://developer.gnome.org/glibmm/stable/classGlib_1_1ustring__Iterator.html
"The Glib::ustring iterated over must contain only valid UTF-8 data.
If it does not, operator++(), operator--() and operator*() may make
accesses outside the bounds of the string."
Bug 777973 - Segmentation fault on bad disk
As the source code is managed in GIT and there is a .gitignore file in
the top level directory specifying file names to exclude from version
control, then the old per-directory .cvsignore files for CVS are
redundant.
Add the only missing and applicable entry from src/.cvsignore of '.libs'
to .gitignore and remove all the .cvsignore files.
Attempting to grow an already mounted xfs where the mount point
contained spaces failed like this:
Grow /dev/sdb4 from 1.00 GiB to 1.50 GiB
+ calibrate /dev/sdb4
+ grow partition from 1.00 GiB to 1.50 GiB
+ grow file system to fill the partition
+ xfs_growfs /tmp/File System Label
Usage: xfs_growfs [options] mountpoint
...
Apply the rule and quote all uses of mount points within command lines.
This is also applied to copying xfs file systems even though it was safe
because it only ever used GParted generated mount points.
Also for the xfs copy operation switch unmounting of partitions to
specify mount points instead of partitions. This is just to be
consistent with how it is done in all the online file system resizing
code.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
The current nilfs2 resizing code is safe because it never passes a user
influenced mount point into a command line. Regardless, apply the same
simple rule to always quote mount points when used in command lines.
WARNING:
Nilfs-resize is broken and can't actually resize a file system mounted
with spaces in the mount point anyway!
# mkdir "/tmp/File System Label"
# mount -v -t nilfs2 /dev/sdb3 "/tmp/File System Label"
mount.nilfs2: started nilfs_cleanerd
# nilfs-resize -v -y /dev/sdb3
Error: cannot open NILFS on /dev/sdb3.
# echo $?
1
# strace nilfs-resize -v -y /dev/sdb3
...
stat("/dev/sdb3", {st_mode=S_IFBLK|0660, st_rdev=makedev(8, 19), ...}) = 0
open("/dev/sdb3", O_RDONLY) = 3
...
open("/proc/mounts", O_RDONLY) = 4
read(4, "sysfs /sys sysfs rw,seclabel,nos"..., 1024) = 1024
...
close(4) = 0
open("/tmp/File\\040System\\040Label", O_RDONLY) = -1 ENOENT (No such file or directory)
...
# fgrep /dev/sd /proc/mounts
/dev/sda3 / ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sda1 /boot ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sdb3 /tmp/File\040System\040Label nilfs2 rw,relatime 0 0
So it looks like nilfs-resize (or a library it uses) can't decode the
octal characters '\040' used to encode spaces in the mount point as
reported in /proc/mounts.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
Attempting to grow an already mounted jfs where the mount point
contained spaces failed like this:
Grow /dev/sdb2 from 1.00 GiB to 1.50 GiB
+ calibrate /dev/sdb2
+ grow partition from 1.00 GiB to 1.50 GiB
+ grow file system to fill the partition
+ mount -v -t jfs -o remount,resize /dev/sdb2 /tmp/File System Label
Usage:
mount [-lhV]
mount -a [options]
mount [options] [--source[ <source> | [--target] <directory>
mount [options] <source> <directory>
mount <operation> <mountpoint> [<target>]
...
Apply the rule to always enclose mount points within double quotes
within command lines.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
A user had a btrfs file system mounted by automounter on a mount point
like "/mount/$USER/File System Label" which included white space
characters. Resizing the file system while online failed like this:
Grow /dev/sdb1 from 1.00 GiB to 1.50 GiB
+ calibrate /dev/sdb1
+ grow partition from 1.00 GiB to 1.50 GiB
+ grow file system to fill the partition
+ btrfs filesystem resize 1:max /mount/USER/File System Label
btrfs filesystem resize: too many arguments
usage: btrfs filesystem resize [devid:][+/-]<newsize>[kKmMgGtTpPeE]|[devid:]max <path>
So mount points not created by GParted should be considered under user
control and need quoting when used as parameters in command lines.
Strictly speaking, mount points created by GParted itself, by
FileSystem::mk_temp_dir(), are safe and don't need quoting. However it
is simpler and safer just to quote all uses of mount points in command
lines, rather than risk missing some.
Bug 782681 - btrfs partitions mounted with whitespace cannot be resized
Using the default MiB alignment, creating an MSDOS logical partition
between two other existing logical partitions fails with this error
dialog:
(-) <b>An error occurred while applying the operations</b>
See the details for more information.
<b>IMPORTANT</b>
If you want support, you need to provide the saved details!
See http://gparted.org/save-details.htm for more information.
[ OK ]
and these operation details:
+ libparted messages
- Unable to satisfy all constraints on the partition.
This bug was introduced by this commit included in GParted 0.23.0:
90e3ed68fc
Shallow copy Device object into Operation object (#750168)
The commit message claimed that the deep copied Partition objects inside
the Device inside the Operation object are never accessed. This turned
out not to be true. Win_GParted::Add_Operation() uses them as part of
snap_to_alignment() which updates requested partition boundaries to
account for alignment requirements and the space needed for EBR
(Extended Boot Record) preceding logical partitions.
In this case the new logical partition was trying to be created over the
top of the EBR for the following logical partition because
snap_to_alignment() wasn't aware of its existence.
Fix by making Add_Operation() and snap_to_alignment() refer to the
current device, as displayed in the UI, rather than the shallow copy
included in the Operation object. Hopefully now it is true that the
not copied vector of Partition objects in the Device object in each
Operation object are never accessed.
Bug 779339 - enforce at least 1 MiB "free space following"
A regression which prevented growing a primary partition that had
unallocated space between it and the following extended partition was
introduced with the following commit:
Create and use general find_extended_partition() function
aa98107706
To fix the regression, restore the logic that checked for a logical
partition before seeking the index of the extended partition.
Bug 778700 - Unable to grow partition even though unallocated space is
adjacent
Increase the size of the Name Partition dialog, matching the change made
to the Label File System dialog in the previous commit. The code for
the Name Partition dialog was basically copied from the Label File
System dialog.
Bug 778003 - The "Label File System" dialog is too small
On Arch Linux with XFCE 4.12 and Fedora 24 with GNOME 3.20 and later;
the Label File System dialog is too small. The problem is that the
label entry box clips the Cancel and OK buttons.
Stop specifying the dialog height, instead letting it fit the combined
height of all the widgets automatically.
Also make the dialog wider and the label entry box wider so that longer
device names can be shown in the title before they are truncated.
Bug 778003 - The "Label File System" dialog is too small
Change the error message after it raised a question with the translators
and to better align with the operation only being described as a check
in the GParted Manual and in the application UI.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
The Operation class already provided find_index_extended() method and
was used in the Operation and derived classes where required. It
returns the index to the extended partition in the PartitionVector
object, or -1 when no extended partition exists.
There were several cases of the same functionality being open coded in
GParted_Core and Win_GParted. Therefore move the implementation to
find_extended_partition() in PartitionVector compilation unit and use
this implementation everywhere.
The clear_mountpoints parameter has never been used since
add_mountpoint*() were first added [1][2]. clear_mountpoints() method
[3] is available to provide this functionality and used. Therefore
removed unused parameter and code.
[1] add_mountpoints() added 2006-03-15
9532c3cad1
Made Partition::mountpoints private
[2] add_mountpoint() added 2011-12-16
208083f11d84dbd4f186271a3cdbf5170db259f8b8
Display LVM2 VGNAME as the PV's mount point (#160787)
[3] clear_mountpoint() added 2006-03-19
ad9f2126e7
fixed issues with copying (see also #335004) cleanups + added FIXME added
Shrinking LUKS encryption is only possible while the mapping is open and
active. Therefore the File System Support dialog shows Cross + Tick for
this operation. Add this new combination to the legend.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Switch the remaining create and delete operation description generation
to use the virtual Partition get_filesystem_string() method.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Now that resizing of encrypted file systems is implemented add growing
of the open LUKS mapping as part of the check repair operation.
Resizing an encrypted file system requires the LUKS mapping to be open
to access the file system within; therefore it also requires libparted
and kernel support for online partition resizing. This limits resizing
to the latest distributions with libparted >= 3.2 and kernel >= 3.6.
However growing an open LUKS mapping as part of a check repair operation
doesn't require resizing the partition. Therefore route via offline
grow of LUKS to avoid those extra, unnecessary requirement. This does
mean that offline LUKS grow artificially requires cryptsetup, but that is
not really significant as even opening LUKS requires cryptsetup.
So now checking an encrypted file system on even the oldest
distributions does:
1) runs FSCK on the encrypted file system;
2) grows the encryption volume to fill the partition;
3) grows the file system to fill the encryption mapping.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
While a device-mapper encryption mapping can only be resized while
active, a LUKS volume can inherently be grown while offline because it
doesn't store a size and when started fills the partition. This doesn't
even need the cryptsetup command to do the resizing (just to open the
LUKS volume afterwards which GParted doesn't yet support). Implement
offline growing of LUKS volumes.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Ensure pre-conditions of always or never being passed a Partition object
containing an open LUKS encryption are met for resizing file system and
LUKS encryption methods.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Moving of closed LUKS is simply enabled by luks .move capability being
set and requires no further coding.
Resizing of encrypted file systems requires both the LUKS mapping and
encrypted file system within to be resized in the right order for both
shrinking and growing. To keep the code simple split resizing of plain
and encrypted into separate functions.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Changing the Resize/Move dialog code to also handle PartitionLUKS
objects was considered too complicated. Instead create an unencrypted
equivalent using clone_as_plain(), pass that to the Resize/Move dialog
and finally apply the change back using Partition*::resize().
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Add a resize() method to both Partition and PartitionLUKS classes. They
take a reference Partition object, and update the position, size and
file system usage of *this Partition to match. This is ready for taking
a partition returned from Resize/Move dialog and applying the change.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Implement a specialist PartitionLUKS clone method. Creates a new
Partition object which has the same space usage as the source encrypted
file system, but is a plain file system. Namely, the overhead of the
LUKS header has been added to the file system usage. This is ready for
feeding this representation of the partition to the Resize/Move dialog.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
A partition containing LUKS encryption can only be moved when closed and
the Device Mapper encryption mapping only exists to be resized when
open. As GParted can't yet open or close LUKS encryption these
restrictions have to be adhered to when composing operations. Also as
encrypted partitions are only being resized when open, additionally
libparted and the kernel have to both be capable of resizing a partition
while in use.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Ensure pre-condition of never being passed a Partition object containing
an open LUKS encryption mapping is met for copy operation related
methods.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Users will expect to see that copying of LUKS is available in the File
System Support dialog, even if technically what is implemented is
copying of the file system within an open encryption mapping. There is
no other reason to do this change as these two previous commits have
fully enabled copying of encrypted content:
Implement copy operation of encrypted file systems (#774814)
Preview copy operation of encrypted file systems (#774818)
Set LUKS .copy capability so that the dialog shows copying availability,
but then disallow copying of closed LUKS. (Checking for the capability
and performing copying the content of an open LUKS encryption mapping is
inherent in Win_GParted::set_valid_operations() and GParted_Core::copy()
in the way that they access the block device containing the file system,
whether encrypted or not).
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Implement the copy operation by making the copy code work with the
Partition object directly containing the file system, instead of the
enclosing PartitionLUKS object containing the LUKS encryption mapping.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Implement composing of the copy paste operation for encrypted file
systems.
Copying a closed LUKS partition would duplicate the LUKS header
containing the UUID, passphrase and master encryption key. From a
security point of view having additional copies of encrypted data with
the same master key is an extra risk, but it all depends on what is
going to happen with that copy. The Cryptsetup FAQ [1] talks about how
to make a backup at the file system level and block level, preferring
file system level with separate encryption if needed. It strongly
recommends separate encryption if the copy is removable or going
off-site [2]. Also in the case of cloning the data, cloning the LUKS
container is strongly discouraged [3].
Therefore copying of encrypted file systems will be implemented by
copying the file system inside an open LUKS encryption mapping and not
by copying a closed LUKS partition.
Also, while creating new LUKS encryption is not yet supported, copying
an encrypted file system into a new partition will not be permitted as
that will always decrypt the data. An encrypted file system will be
allowed to be copied into an existing plain partition, decrypting the
data, or into an existing open encrypted partition, keeping it
encrypted. Pasting over the top of a closed encrypted partition will
remove the LUKS encryption. (This is planned to be removed when
creating and removing LUKS encryption is implemented as part of full
LUKS read-write support).
Remember that when pasting into an existing partition the file system
must fit within the available space and that encryption has overhead
from the LUKS header. Therefore copying from a plain partition into a
partition of the same size with open an encryption mapping will not fit
for space reasons.
[1] The Cryptsetup FAQ, Backup and data Recovery section
https://gitlab.com/cryptsetup/cryptsetup/wikis/FrequentlyAskedQuestions
[2] 6.7 Does a backup compromise security?
"If you do network-backup or tape-backup, I strongly recommend to go the
filesystem backup path with independent encryption, as you typically
cannot reliably delete data in these scenarios, especially in a cloud
setting."
[3] 6.15 Can I clone a LUKS container?
"You can, but it breaks security, because the cloned container has the
same header and hence the same master key. You cannot change the master
key on a LUKS container, even if you change the passphrase(s), the
master key stays the same. That means whoever has access to one of the
clones can decrypt them all, completely bypassing the passphrases.
The right way to do this is to first luksFormat the target container,
then to clone the contents of the source container, with both containers
mapped, i.e. decrypted. You can clone the decrypted contents of a LUKS
container in binary mode, although you may run into secondary issues
with GUIDs in filesystems, partition tables, RAID-components and the
like. These are just the normal problems binary cloning causes.
Note that if you need to ship (e.g.) cloned LUKS containers with a
default passphrase, that is fine as long as each container was
individually created (and hence has its own master key). In this case,
changing the default passphrase will make it secure again."
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Ensure pre-condition of never being passed a Partition object containing
an open LUKS encryption mapping is met for format operation related
methods.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Formatting an existing encrypted partition will format the file system
within the encrypted mapping. Formatting over the top of a closed
encrypted partition will remove the encryption. (The latter is planned
to be prevented when creating and removing LUKS encryption is
implemented as part of full LUKS read-write support).
Composing the format operation inside an open LUKS encryption mapping
also has to account for the size of that mapping and construct a
PartitionLUKS object containing the new file system. Implementing the
operation itself is as simple as passing the Partition object directly
containing the file system, instead of the outer PartitionLUKS object.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Already have:
Utils::get_filesystem_string(FS_EXT2) -> "ext2"
virtual Partition::get_filesystem_string() -> "ext2"
virtual PartitionLUKS::get_filesystem_string() -> "[Encrypted] ext2"
Add these:
Utils::get_encrypted_string() -> "[Encrypted]"
Utils::get_filesystem_string(false, FS_EXT2) -> "ext2"
Utils::get_filesystem_string(true, FS_EXT2) -> "[Encrypted] ext2"
This is ready for use of Utils::get_filesystem_string(true, FS_EXT2)
when composing the preview of a format of an encrypted file system by
Win_GParted::activate_format().
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Adapts the swapon/swapoff, activate/deactivate Volume Group and mount/
unmount file system code to work with the Partition object directly
containing the file system.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Ensure pre-condition of never being passed a Partition object containing
an open LUKS encryption mapping is met for check operation related
methods.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Just update the code describing and implementing the check operation.
No need to update the code composing the operation in
Win_GParted::activate_check() as that is incredibly simple, just cloning
the Partition object and doesn't need changing.
Note that for encrypted file systems this does:
1) runs FSCK on the encrypted file system;
2) grows the file system to fill the encrypted mapping.
At this time it does not grow the encryption mapping to fill the
partition. That will be added after resize/move has been implemented.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Ensure pre-condition of never being passed a Partition object containing
an open LUKS encryption mapping is met for label file system operation
related methods.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Again, just need to change the code when composing, describing and
implementing the operation to query and set the Partition object
directly containing the file system, instead of the enclosing encryption
mapping to make it work.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Ensure pre-condition of never being passed a Partition object containing
an open LUKS encryption mapping is met for change UUID operation related
methods.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
When composing, describing and implementing the operation just need the
code to query and set the Partition object directly containing the file
system, instead of the enclosing encryption mapping to make it work.
The operation details for setting a new UUID on an encrypted ext4 file
system become:
Set a new random UUID on [Encrypted] ext4 file system on /dev/sdb4
+ calibrate /dev/sdb4
+ Set UUID on /dev/mapper/sdb4_crypt to a new, random value
+ tune2fs -U random /dev/mapper/sdb4_crypt
tune2fs 1.41.12 (17-May-2010)
Also note the now documented rule in apply_operation_to_disk() which
says each operation must leave the status of the encryption mapping and
file system as it found it.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
Provide and use a single interface for getting the file system string
for display, regardless of whether the partition is encrypted or the
encryption mapping is active or not.
Example return values for get_filesystem_string() for different types
and states of Partition objects:
1) Plain ext4 file system: -> "ext4"
2) Closed encrypted: -> "[Encrypted]"
3) Open encrypted ext4 file system: -> "[Encrypted] ext4"
This simplifies the code in TreeView_Detail::create_row() which sets the
file system type displayed in the main window. The same method will
then also be used when setting the operation description as each
operation is updated to handle encrypted file systems.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
The previous commit changed how the code behind the main window
retrieved the file system label for display. This is the relevant
changes in TreeView_Detail::create_row():
+ const Partition & filesystem_ptn = partition.get_filesystem_partition();
...
- Glib::ustring temp_filesystem_label = partition.get_filesystem_label();
+ Glib::ustring temp_filesystem_label = filesystem_ptn.get_filesystem_label();
treerow[treeview_detail_columns.label] = temp_filesystem_label;
In the case of an encrypted file system get_filesystem_label() is now
called on the Partition object directly rather than on the outer
Partition object containing the LUKS encryption.
The code behind the Information dialog always obtained and used the
Partition object directly containing the file system to call
get_filesystem_label() since read-only LUKS support was added.
Therefore the virtualised PartitionLUKS::get_filesystem_label() is no
longer needed, so remove it.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
There are multiple cases of code wanting to work with the Partition
object directly containing the file system, regardless of whether it is
within a PartitionLUKS object or not. The code had to do something
similar to this to access it:
const Partition * filesystem_ptn = &partition;
if ( partition.filesystem == FS_LUKS && partition.busy )
filesystem_ptn = &dynamic_cast<const PartitionLUKS *>( &partition )->get_encrypted();
...
// Access Partition object directly containing the file system
filesystem_ptn-> ...
Implement and use virtual accessor get_filesystem_partition() which
allows the code to be simplified like this:
const Partition & filesystem_ptn = partition.get_filesystem_partition();
...
// Access Partition object directly containing the file system
filesystem_ptn. ...
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
In the main window, display the busy status of the file system only
according to whether it is mounted or not, ignoring the status of the
encryption mapping.
Bug 774818 - Implement LUKS read-write actions NOT requiring a
passphrase
More recent versions of blkid don't report an ISO9660 file system on the
whole disk device if partitions can be reports for embedded partitions.
However when querying the whole disk device directly then the expected
ISO9660 file system is reported. For example on CentOS 7 with the
previous ISO images:
# wget http://git.kernel.org/cgit/utils/util-linux/util-linux.git/plain/tests/ts/isosize/sample.iso.gz
# dd if=/dev/zero bs=1M of=/dev/sdc
# zcat sample.iso.gz | dd of=/dev/sdc
# blkid -v
blkid from util-linux 2.23.2 (libblkid 2.23.0, 25-Apr-2013)
# blkid | fgrep /dev/sdc
/dev/sdc1: UUID="2013-01-04-22-05-45-00" LABEL="ARCH_201301" TYPE="iso9660" PTTYPE="dos"
# blkid /dev/sdc
/dev/sdc: UUID="2013-01-04-22-05-45-00" LABEL="ARCH_201301" TYPE="iso9660" PTTYPE="dos"
# wget http://cdimage.debian.org/debian-cd/8.6.0/amd64/iso-cd/debian-8.6.0-amd64-netinst.iso
# dd if=/dev/zero bs=1M of=/dev/sdc
# dd if=debian-8.6.0-amd64-netinst.iso bs=1M of=/dev/sdc
# blkid | fgrep /dev/sdc
/dev/sdc1: UUID="2016-09-17-14-23-48-00" LABEL="Debian 8.6.0 amd64 1" TYPE="iso9660" PTTYPE="dos"
/dev/sdc2: SEC_TYPE="msdos" UUID="17F3-1162" TYPE="vfat"
# blkid /dev/sdc
/dev/sdc: UUID="2016-09-17-14-23-48-00" LABEL="Debian 8.6.0 amd64 1" TYPE="iso9660" PTTYPE="dos"
This behavioural difference with blkid is probably as a result of newer
versions of udev informing the kernel of the partitions embedded within
the ISO9660 image, and not directly as a result of a change in blkid
itself. Older distributions don't have partition entries for the above
ISO images, but CentOS 7 (with udev 219) and later distributions do have
partition entries:
# fgrep sdc /proc/partitions
8 16 8388608 sdc
8 17 252928 sdc1
8 18 416 sdc2
Fix by ensuring that the blkid FS_Info cache has entries for whole disk
devices, even if each entry has all empty attributes because there is a
partition table and not a recognised file system.
Calling blkid on whole disk devices containing partition tables produces
output like this, with newer versions of blkid:
# blkid /dev/sda
/dev/sda: PTTYPE="dos"
# blkid /dev/sdb
/dev/sdb: PTTYPE="gpt"
This will be loaded into the FS_Info cache as a blank entry for the
device by run_blkid_load_cache(). There will be a path name but all the
other attributes will be blank because there are no TYPE, SEC_TYPE, UUID
or LABEL name value pairs. With older versions of blkid no output is
produced at all. In that case load_fs_info_cache_extra_for_path() will
create the same blank entry with just the path name defined.
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
ISO9660 images can contain embedded partitions which when encountered on
whole disk drives causes libparted to report various warnings and
errors. For example on CentOS 6 with upgraded libparted 2.4 the
following errors and warnings are encountered with various ISO images.
(Deliberately using an older distribution with older blkid to avoid
another issue with blkid addressed in the following patch).
Libparted error message 1:
# wget http://git.kernel.org/cgit/utils/util-linux/util-linux.git/plain/tests/ts/isosize/sample.iso.gz
# dd if=/dev/zero bs=1M of=/dev/sdc
# zcat sample.iso.gz | dd of=/dev/sdc
# blkid -v
blkid from util-linux-ng 2.17.2 (libblkid 2.17.0, 22-Mar-2010)
# blkid | fgrep /dev/sdc
/dev/sdc: LABEL="ARCH_201301" TYPE="iso9660"
# ./gpartedbin /dev/sdc
======================
libparted : 2.4
======================
Invalid partition table - recursive partition on /dev/sdc.
Libparted Error
(-) Invalid partition table - recursive partition on /dev/sdc.
[ Cancel ] [ Ignore ]
Libparted error message 2:
# wget http://cdimage.debian.org/debian-cd/8.6.0/amd64/iso-cd/debian-8.6.0-amd64-netinst.iso
# dd if=/dev/zero bs=1M of=/dev/sdc
# dd if=debian-8.6.0-amd64-netinst.iso bs=1M of=/dev/sdc
# blkid | fgrep /dev/sdc
/dev/sdc: LABEL="Debian 8.6.0 amd64 1" TYPE="iso9660"
# ./gpartedbin /dev/sdc
======================
libparted : 2.4
======================
/dev/sdc contains GPT signatures, indicating that it has a GPT
table. However, it does not have a valid fake msdos partition
table, as it should. Perhaps it was corrupted -- possibly by a
program that doesn't understand GPT partition tables. Or perhaps
you deleted the GPT table, and are now using an msdos partition
table. Is this a GPT partition table?
Libparted Warning
/!\ /dev/sdc contains GPT signatures, indicating that it has a GPT
table. However, it does not have a valid fake msdos partition
table, as it should. Perhaps it was corrupted -- possibly by a
program that doesn't understand GPT partition tables. Or
perhaps you deleted the GPT table, and are now using an msdos
partition table. Is this a GPT partition table?
[ Yes ] [ No ]
These messages are because GParted is calling ped_disk_new() to attempt
to read the partition table even before it has tried to recognise the
ISO9660 file system on the whole disk drive. Full call chain is:
set_devices_thread()
set_device_from_disk()
get_device_and_disk()
get_disk()
ped_disk_new()
Fix this by delaying the call to ped_disk_new() until after whole disk
drive recognition has been performed. Replace combined
get_device_and_disk() with separate get_device() and only call
get_disk() after no whole disk drive file system has been recognised.
This is similar to how calibrate_partition() and
erase_filesystem_signatures() are structured to also handle whole disk
drive file systems.
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
Move code from GParted_Core::set_devices_thread() performing top level
population of each Device object during the scan of the drives into new
set_device_from_disk() method.
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
Requires blkid.
Note that FS_LUKS was also moved to more closely match the order in
include/Utils.h
Bug 771244 - gparted does not recognize the iso9660 file system in
cloned Ubuntu USB boot drives
Glibc 2.25 is deprecating <sys/types.h> including <sys/sysmacros.h>.
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=dbab6577c6684c62bd2521c1c29dc25c3cac966f
Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
Building on Fedora Rawhide with Glibc 2.24.90 produces these warnings:
BlockSpecial.cc:64:13: warning: In the GNU C Library, "major" is defined
by <sys/sysmacros.h>. For historical compatibility, it is
currently defined by <sys/types.h> as well, but we plan to
remove this soon. To use "major", include <sys/sysmacros.h>
directly. If you did not intend to use a system-defined macro
"major", you should undefine it after including <sys/types.h>.
m_major = major( sb.st_rdev );
^~~~~~~~~~~~~~~~~~~~
BlockSpecial.cc:65:13: warning: In the GNU C Library, "minor" is defined
by <sys/sysmacros.h>. For historical compatibility, it is
currently defined by <sys/types.h> as well, but we plan to
remove this soon. To use "minor", include <sys/sysmacros.h>
directly. If you did not intend to use a system-defined macro
"minor", you should undefine it after including <sys/types.h>.
m_minor = minor( sb.st_rdev );
^~~~~~~~~~~~~~~~~~~~
Code needing major and minor macros should include <sys/sysmacros.h>
directly. As both Glibc and musl-libc have always provided this header
and GParted is a Linux only application, just always include the header
and don't bother with an autoconf check for its existence.
Bug 776173 - Missing sys/sysmacros.h #include, needed for experimental
glibc versions
The details for a file system shrink operation look like this with the
"shrink file system" message being missing:
Shrink /dev/sdb1 from 128.00 MiB to 100.00 MiB
+ calibrate /dev/sdb1
+ check file system on /dev/sdb1 for errors and (if possible) fix them
+ e2fsck -f -y -v -C 0 /dev/sdb1
+ resize2fs -p /dev/sdb1 102400K
+ shrink partition from 128.00 MiB to 100.00 MiB
This earlier commit [1] in the series dropped the message while moving
code from resize_filesystem() to shrink_filesystem(). Re-add the
message back.
[1] a0158abbeb
Refactor resizing file system apply methods (#775932)
Bug 775932 - Refactor mostly applying of operations
Explain when and why shrinking file systems is disallowed.
Unnecessary history. Check added in 2004-12-15:
d100935b55
:Set_Valid_Operations()
Comment later added in 2006-07-30:
677a21f50a
improved errorhandling a bit. At the initialscan we store errors/warnings
It made the code look a little messy, is easily resolved in the build
system and made the dependencies more complicated than needed. Each
GParted header was tracked via multiple different names (different
numbers of "../include/" prefixes). For example just looking at how
DialogFeatures.o depends on Utils.h:
$ cd src
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/../include/Utils.h \
../include/../include/../include/../include/../include/../include/Utils.h \
../include/../include/../include/Utils.h \
After removing "../include/" from the GParted header #includes, just
need to add "-I../include" to the compile command via the AM_CPPFLAGS in
src/Makefile.am. Now the dependencies on GParted header files are
tracked under a single name (with a single "../include/" prefix). Now
DialogFeatures.o only depends on a single name to Utils.h:
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/Utils.h ../include/i18n.h \
Simplify conditions checking whether activate/deactivate of an LVM2 PV
is possible. Excluding extended partition type was unnecessary as it
only matters that the file system type is LVM2 PV or not.
Also remove activate/deactivate from the comment above as that check
only determines if the busy state of file systems and swap space can be
toggled.
Linux-swap is recreated as part of copy, resize and move operations and
the code was special cased to implement that by calling the linux-swap
specific resize method. However the displayed text always said "growing
file system" and then proceeded to recreate linux swap. Example
operation:
Copy /dev/sdb1 to /dev/sdb2
...
+ copy file system from /dev/sdb1 to /dev/sdb2
Partition copy action skipped because linux-swap file system does not contain data
+ grow file system to fill the partition
+ create new linux-swap file system
+ mkswap -L"" -U "77d939ef-54d6-427a-a2bf-a053da7eed4c" /dev/sdb2
Setting up swapspace version 1, size = 262140 KiB
LABEL=, UUID=77d939ef-54d6-427a-a2bf-a053da7eed4c
Fix by writing recreate_linux_swap_filesystem() method with better
messaging and use everywhere maximise_filesystem() was previously used
to recreate linux-swap. Also as this is a create step, erase the
partition first to prevent the possibility of any other file system
signatures being found afterwards. Now the operation steps are more
reflective of what is actually being performed.
Copy /dev/sdb1 to /dev/sdb2
...
+ copy file system from /dev/sdb1 to /dev/sdb2
Partition copy action skipped because linux-swap file system does not contain data
+ clear old file system signatures in /dev/sdb2
+ create new linux-swap file system
+ mkswap -L"" -U "77d939ef-54d6-427a-a2bf-a053da7eed4c" /dev/sdb2
Setting up swapspace version 1, size = 262140 KiB
LABEL=, UUID=77d939ef-54d6-427a-a2bf-a053da7eed4c
Bug 775932 - Refactor mostly applying of operations
Resizing a file system only checked the .grow and .shrink support
capabilities of the file system, even when perform online resizing.
This wasn't a issue because .online_grow and .online_shrink were always
set from the offline .grow and .shrink capabilities respectively, but
only when online resizing was possible.
However the Device Mapper encryption mapping used for LUKS can only be
resized when online, and not when offline. Therefore the correct
.online_grow and .online_shrink capabilities needs to be checked to
prevent a LUKS resize step failing as not implemented.
Bug 775932 - Refactor mostly applying of operations
resize_filesystem() was meeting two different needs:
1) when called with fill_partition = false it generated operation
details;
2) when called from maximize_filesystem() with fill_partition = true it
skipped generating any operation details;
then ran the switch statement to select the resize implementation. So
extract the common switch statement into new method
resize_filesystem_implement().
Then observe that the only time resize_filesystem() was called to grow
the file system was when re-creating linux-swap. Therefore change that
call to use maximize_filesystem() and rename to shrink_filesystem() and
modify the operation detail messages to match.
Bug 775932 - Refactor mostly applying of operations
Attempting to copy or move linux-swap partition was possible when the
mkswap command was unavailable. However as linux-swap is re-created as
part of these operations the mkswap command is a mandatory requirement.
Example copy operation results:
Copy /dev/sdb1 to /dev/sdb2
+ calibrate /dev/sdb1
+ check file system on /dev/sdb1 for errors and (if possible) fix them
+ create empty partition
+ set partition type on /dev/sdb2
+ copy file system from /dev/sdb1 to /dev/sdb2 [SUCCESS]
Partition copy action skipped because linux-swap file system does not contain data
+ grow file system to fill the partition [WARNING]
growing is not available for this file system
Because mkswap was not available the grow action didn't re-create
the linux-swap so the newly copied partition is left without any content
and therefore reported as an unknown partition.
Fix by making copying and moving linux-swap depend on mkswap being
available.
Bug 775932 - Refactor mostly applying of operations
Make the methods called below apply_operation_to_disk() follow a
standard naming convention:
* Contains "_partition"
Uses libparted to query or change the partition in the disk label
(partition table).
E.g.:
calibrate_partition()
create_partition()
delete_partition()
name_partition()
resize_move_partition()
set_partition_type()
* Contains "_filesystem"
Manipulates the file system within the partition, mostly using the
FileSystem and derived class methods.
E.g.:
create_filesystem()
remove_filesystem()
label_filesystem()
copy_filesystem()
erase_filesystem_signatures()
check_repair_filesystem()
resize_filesystem()
maximize_filesystem()
* Other
Compound method calling multiple partition and file system related
apply methods.
E.g.:
create()
format()
copy()
resize_move()
resize()
move()
Rename:
Delete() -> delete_partition()
change_uuid() -> change_filesystem_uuid()
Bug 775932 - Refactor mostly applying of operations
The initial check in each of GParted_Core::resize() and move() check for
impossible conditions. (See resize_move() where both functions are
called for confirmation). The old messages could have suggested the
user somehow composed the operation incorrectly if they had ever been
seen.
Make it clear these failures are programming bugs, thus expecting the
user to raise a bug report should they ever be seen. While for now the
checks are obviously impossible they document preconditions for the
functions. More precondition checks are expected to be added later.
Bug 775932 - Refactor mostly applying of operations
Simplify the move() function. Change it into an if-operation-fails-
return-false style and re-write the final overly complicated conditional
check repair and maximise file system.
The final condition said if the file system was linux-swap or the new
partition was larger, perform a check repair and maximize file system.
However this is a move only step with a check at the top of the move()
function ensuring the Partition objects are the same size. So that
simplifies to only checking for linux-swap. As the context is a move
and linux-swap is recreated, performing a check repair first is
unnecessary, so remove that too.
Bug 775932 - Refactor mostly applying of operations
Make the logic at the resize_move_partition() call sites a little
simpler by not having to avoid calling it for whole disk device
partitions. Make it a successful non-operation in that case.
Bug 775932 - Refactor mostly applying of operations
Make the logic at the check_repair_filesystem() call sites a little
easier by not having to avoid calling it for online file systems. The
function handles that itself as a silent non-operation.
Bug 775932 - Refactor mostly applying of operations
Make the logic at the set_partition_type() call sites a little simpler
by not having to avoid calling it for whole disk device Partition
objects. Make set_partition_type() handle those itself as a silent
non-operation.
This is similar to how calibrate_partition() could be called on an
UNALLOCATED Partition object, and update_bootsector() is called on
non-NTFS file system Partition objects. Both are successful and silent
non-operations.
Bug 775932 - Refactor mostly applying of operations
Until now an encryption mapping has been modelled as a Partition object
similar to a partition like this:
.encrypted.device_path = "/dev/sdb1"
.encrypted.path = "/dev/mapper/sdb1_crypt"
.encrypted.whole_device = false
.encrypted.sector_start = // start of the mapping in the partition
.encrypted.sector_end = // end of the mapping in the partition
However accessing device_path in the start to end sector range is not
equivalent to accessing the partition path as it doesn't provide access
to the encrypted data. Therefore existing functions which read and
write partition data (GParted file system copying and signature erasure)
via libparted using the device_path won't work and will in fact destroy
the encrypted data. This could be coded around with an extra case in
the device opening code, however it is not necessary.
An encrypted block special device /dev/mapper/CRYPTNAME looks just like
a whole disk device because it doesn't contain a partition and the file
system it contains starts at sector 0 and goes to the end. Therefore
model an encryption mapping in the same way a whole disk device is
modelled as a Partition object like this:
.encrypted.device_path = "/dev/mapper/sdb1_crypt"
.encrypted.path = "/dev/mapper/sdb1_crypt"
.encrypted.whole_device = true
.encrypted.sector_start = 0
.encrypted.sector_end = // size of the encryption mapping - 1
Now GParted file system copy and erasure will just work without any
change. Just need to additionally store the LUKS header size, which was
previous stored in the sector_start, for use in the
get_sectors_{used,unused,unallocated}() calculations.
Bug 775932 - Refactor mostly applying of operations
Split out the switch statement selecting the copy implementation and
associated copy file system operation detail message into a separate
copy_filesystem() method, matching how a number of other operations are
coded. This is why the previous copy_filesystem() methods needed
renaming.
Re-write the remaining copy() into if-operation-fails-return-false style
to simplify it. Re-write final complicated conditional check repair and
maximise file system into separate positive if conditions for swap and
larger partition to make it understandable.
The min_size parameter to copy() was queried from the partition_src
parameter also passed to copy(). Drop the parameter and query inside
copy() instead.
Bug 775932 - Refactor mostly applying of operations
Rename GParted_Core methods:
copy_filesystem(4 params) -> copy_filesystem_internal()
copy_filesystem(5 params) -> copy_filesystem_internal()
copy_filesystem(10 params) -> copy_blocks()
See the following commit for the desire to do this.
Bug 775932 - Refactor mostly applying of operations
Files were named Block_Copy and the class was named block_copy. Change
to the primary naming convention of CamelCase class name and matching
file names.
Also make CopyBlocks::copy_block() a private method as it is only used
internally within the class.
Bug 775932 - Refactor mostly applying of operations
These two methods had a lot of repeated and common code. Both determine
if the partition has any pending operations, notify the user that
changing the busy status can not be performed, and report any errors
when changing the status.
Extract the common code into sub-functions check_toggle_busy_allowed()
and show_toggle_failure_dialog() to handle showing the message dialogs.
Also refactor toggle_busy_state() to make it clear that it handles 5
cases of swapon, swapoff, activate VG, deactivate VG and unmount file
system.
Also remove out of date comment near the top of toggle_busy_state()
stating there can only be pending operations for inactive partitions is
out of date. Some file systems can be resized while online and
partition naming is allowed whatever the busy status of the file system.
Bug 775932 - Refactor mostly applying of operations
The primary reason to refactor unmount_partition() is to pass the
Partition object to be unmounted, rather than use member variable
selected_partition_ptr so that it doesn't have to handle the differences
between encrypted and non-encrypted Partition objects. The calling
function can deal with that instead. Then there were lots of small
reasons to change almost every other line too:
* Return success or failure rather than updating a passed pointer with
the result. Leftover from when the function used to be a separate
thread:
commit 52a2a9b00a
Reduce threading (#685740)
* Pass updated error string by reference rather than by pointer. Likely
another leftover.
* Stop borrowing the updated error string as a local variable for the
error output from the umount command. Use new umount_error local
variable instead. Was bad practice making the code harder to
understand.
* Rename failed_mountpoints to skipped_mountpoints to better reflect
that it contains the mount points not attempted to be unmounted
because two or more file systems are mounted at that point.
* Rename errors to umount_errors to better reflect it contains the
errors from each failed umount command.
* Document the reason for mount points being skipped.
* Update the skipped mount points message to state definitely why they
could not be unmounted rather than stating most likely.
* Simplify logic processing the error cases and return value.
* Made static because it no longer accesses any class members.
* Remove out dated "//threads.." comment from the header. Another
leftover from when the function use to be a separate thread.
Bug 775932 - Refactor mostly applying of operations
Reorder the parameters into the same order in which they occur in the
row, i.e. Name first, then Mount Point and finally Label. Rename local
variables in load_partitions(1 param) and parameters of
load_partitions(5 params) prefixing with "show_" to make it clearer the
variables track if that column will be displayed or not.
create_row() populates the values for each row to be displayed in the UI
from the relevant Partition object. However load_partitions(5 params)
independently decided if the Name, Mount Point and Label columns were
empty and should be displayed.
Getting the mount point value is more complex for encrypted file systems
because it has to call get_mountpoints() on the inner encrypted
Partition object. load_partitions(5 params) didn't account for this.
Fix by making create_row() both copy the values into each row and at the
same time check if they are empty to decide if they should be displayed
or not.
Bug 775475 - Mount Point column displayed for encrypted file systems
even when empty
Running Device > Attempt Data Rescue... hangs the GParted UI for as long
as gpart takes to scan the whole disk device looking for file system
signatures.
Originally when gpart support was added by commit [1] a separate thread
was created to run gpart. Then most threading was removed by commit [2]
which left gpart running in the main thread blocking the UI.
[1] ef37bdb7de
Added support to lost data recovery using gpart
[2] 52a2a9b00a
Reduce threading (#685740)
guess_partition_table() hand codes using Glib to run the gpart command
asynchronously reading standard output, but it just doesn't run the Gtk
main loop to process events, hence the UI hangs. Instead just use
Utils::execute_command() which handles everything already. It runs the
commands asynchronously, reading output and if being run in the main
thread also calls the Gtk main loop to keep the UI responsive.
Bug 772123 - GParted is unresponsive while gpart is running
If the GParted main window is closed before the initial device load
completed gpartedbin never exits. The main window closes but the
process sits there idle forever. Subsequently running GParted reports
this error:
# gparted
The process gpartedbin is already running.
Only one gpartedbin process is permitted.
If the user is running GParted from a desktop menu they will never see
this error so they will never know why GParted won't start any more.
More technically, it is if the main window is closed before the
Win_GParted::on_show() callback completes.
I assume the Gtk main loop doesn't setup the normal quit handling until
the on_show() callback finishes drawing the main window for the first
time. Following this hint [1], move the initial device load from the
on_show() callback to immediately after it completes by using a run once
idle callback setup in on_show().
This looks exactly the same to the user except now gpartedbin exits when
the main window is closed during the initial device load. Note that
GParted finished the device load before exiting. This is exactly the
same as happens when exiting during subsequent device refreshes.
[1] How to know when a Gtk Window is fully shown?
http://stackoverflow.com/questions/14663212/how-to-know-when-a-gtk-window-is-fully-shown
"If you want to know when your main window appears the first time,
it is far easier (and saner) add a g_idle_add after your show_all
call."
Bug 771816 - GParted never exits if the main window is closed before the
initial device load completes
Again this is to handle a long list of mount points for a single
partition. To prevent the file system status value of "Mounted on [list
of 20 mount points]" causing the dialog to become wider than the screen,
line wrap the text. This instead makes the dialog taller, which already
automatically scrolls vertically as needed.
Bug 771693 - Mount Point column is wider than the screen on openSUSE
using btrfs root with lots of mounted subvolumes
openSUSE 42.1 with default btrfs root installation is heavily using
subvolumes. (Think of btrfs as a storage pool and subvolumes as
individual file systems within the pool for a rough approximation).
Thus the root partition is mounted on many mount points:
# df -k
Filesystem 1K-blocks Used Available Use% Mounted on
...
/dev/sda2 19445760 5820080 13157104 31% /
/dev/sda2 19445760 5820080 13157104 31% /var/lib/libvirt/images
/dev/sda2 19445760 5820080 13157104 31% /var/lib/mysql
/dev/sda2 19445760 5820080 13157104 31% /.snapshots
/dev/sda2 19445760 5820080 13157104 31% /home
/dev/sda2 19445760 5820080 13157104 31% /var/opt
/dev/sda2 19445760 5820080 13157104 31% /usr/local
/dev/sda2 19445760 5820080 13157104 31% /var/tmp
/dev/sda2 19445760 5820080 13157104 31% /var/lib/named
/dev/sda2 19445760 5820080 13157104 31% /var/lib/mariadb
/dev/sda2 19445760 5820080 13157104 31% /var/spool
/dev/sda2 19445760 5820080 13157104 31% /var/lib/pgsql
/dev/sda2 19445760 5820080 13157104 31% /var/lib/mailman
/dev/sda2 19445760 5820080 13157104 31% /srv
/dev/sda2 19445760 5820080 13157104 31% /opt
/dev/sda2 19445760 5820080 13157104 31% /var/crash
/dev/sda2 19445760 5820080 13157104 31% /tmp
/dev/sda2 19445760 5820080 13157104 31% /boot/grub2/i386-pc
/dev/sda2 19445760 5820080 13157104 31% /var/log
/dev/sda2 19445760 5820080 13157104 31% /boot/grub2/x86_64-efi
As the Mount Point column contains all 20 mount points it makes the
column wider than the screen, requiring lots of horizontal scrolling to
see the following columns. (Truncated to just 7 mounts in this
example here).
Partition | File System | Mount Point
/dev/sda1 * | # swap |
/dev/sda2 * | # btrfs | /, /.snapshots, /boot/grub2/i386-pc, /boot/grub2/x86_64-efi, /home, /opt, /srv
Fix by making the Mount Point column resizable and display truncated
text with ellipsis. The column now takes the initial and minimum width
from the width of the "Mount Point" column header text.
Partition | File System | Mount Point | Size
/dev/sda1 * | # swap | | 1.45 GiB
/dev/sda2 * | # btrfs | /, /.snapsho... | 18.54 GiB
Bug 771693 - Mount Point column is wider than the screen on openSUSE
using btrfs root with lots of mounted subvolumes
This commit stopped setting the text colours in the Partition, File
System and Mount Point columns to avoid hard coding text colours making
them impossible to read when using GNOME's High Contrast Inverse theme:
ff2a6c00dd
Changes post gparted-0.3.6 - code recreation from Source Forge
* src/TreeView_Detail.cc: Removed text_color hard coding
- Removed hard coding of Partition and Filesystem text_color
which was based on if partition was TYPE_UNALLOCATED.
- Removed hard coding of Mount text_color which was based
on if partition was busy. Lock symbol provides this indicator.
- Closes GParted bug #413810 - Don't hardcode text colour in
partition list
Now remove the remaining vestiges left behind. Remove the unused color
text and mount_text_color columns from the tree model. Also remove
setting of the column attributes which set the colour of the text in the
tree view from those unused columns in the tree model.
Unnecessary history. Added by:
b179990dc9
show greyed-out mountpoint of unmounted partitions in the treeview
as an improved way to identify partitions
Bug #333027 - Displaying unmounted partitions' default mount points
in grey
and by commit only in CVS history:
Bart Hakvoort <...> 2004-08-22 15:06:45
Made text in Partition column darkgrey for unallocated. this offers
more visual difference between partitions and unallocated space
Trying to create a new partition table on a device with active
partitions reports the number of active partitions in the error dialog.
However when there is a busy logical partition the number of reported
busy partitions will be one less than the number of partitions in the
main UI showing the busy symbol.
GParted considers extended partitions as busy when any of the logical
partitions it contains as busy. Display in the main UI reflects this.
Fix Win_GParted::active_partitions_on_device_count() to not exclude
extended partitions from the count.
Found that in some cases usage of active encrypted swap was not working,
but only for the first encrypted swap partition. This only failed on
the first Device Mapper device, dm-0:
# ls -l /dev/mapper/ /dev/dm-*
brw-rw---- 1 root disk 254, 0 Oct 4 20:58 /dev/dm-0
brw-rw---- 1 root disk 254, 1 Oct 4 20:58 /dev/dm-1
/dev/mapper/:
total 0
crw------- 1 root root 10,236 Oct 4 19:48 control
lrwxrwxrwx 1 root root 7 Oct 4 20:58 sdb1_crypt -> ../dm-0
lrwxrwxrwx 1 root root 7 Oct 4 20:58 sdb2_crypt -> ../dm-1
# cat /proc/swaps
Filename Type Size Used Priority
/dev/sda1 partition 1524732 92356 -1
/dev/dm-0 partition 1046524 0 -2
/dev/dm-1 partition 1046524 0 -3
Was failing because the minor number of dm-0 was 0, causing BlockSpecial
operator==() to fall back to name comparison rather than major, minor
number, and GParted name /dev/mapper/sdb1_crypt doesn't match /dev/dm-0.
Found on openSUSE and Ubuntu which don't use LVM by default and don't
already have dm-0 used as an LVM Logical Volume which GParted doesn't
support.
The LINUX ALLOCATED DEVICES document [1] says block special device 0, 0
(major, minor) is not used "reserved as null device number". (Not to
be confused with 1, 3 /dev/null the Null device). All other
non-negative pairs are valid block special device numbers. Therefore
update BlockSpecial operator==() accordingly; compare by major, minor
number when either is greater than 0 falling back to string compare
otherwise. This still fits in with the BlockSpecial() constructor using
major, minor numbers 0, 0 to represent plain files.
[1] LINUX ALLOCATED DEVICES
https://www.kernel.org/doc/Documentation/devices.txt
Bug 771670 - Usage of active encrypted swap is not shown
GParted does not show the usage of active encrypted swap partitions,
instead showing partition warning "Unable to read the contents of this
file system! ...". OS setup:
# ls -l /dev/mapper/sdb4_crypt /dev/dm-3
brw-rw----. 1 root disk 253, 3 Sep 14 07:26 /dev/dm-3
lrwxrwxrwx. 1 root root 7 Sep 14 07:26 /dev/mapper/sdb4_crypt -> ../dm-3
# mkswap -L encrypted_swap /dev/mapper/sdb4_crypt
# swapon /dev/mapper/sdb4_crypt
# cat /proc/swaps
Filename Type Size Used Priority
/dev/sda2 partition 2097148 237632 -1
/dev/dm-3 partition 1046524 0 -2
This is because the code was performing a string compare between the
canonical /dev/mapper/sdb4_crypt name GParted is using and the /dev/dm-3
name reported by the kernel via /proc/swaps. Fix by creating
BlockSpecial objects from the names and compare those so that comparison
is done correctly using major, minor numbers.
Bug 771670 - Usage of active encrypted swap is not shown
The comment became completely unnecessary with the transfer of
mount_info and fstab_info into separate Mount_Info module by commit:
63ec73dfda
Split mount_info and fstab_info maps into separate Mount_Info module
It was never necessary to clear one of the mappings at the end of the
device refresh because it was reloaded at the start of the next device
refresh anyway and it is only a small amount of memory.
Have an unmounted file system within an open encrypted mapping and an
entry in /etc/fstab for the file system like this:
# lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
...
sdb 8:16 0 8G 0 disk
+-sdb1 8:17 0 1G 0 part
+-sdb1_crypt 253:0 0 1022M 0 crypt
# blkid | grep sdb1
/dev/sdb1: TYPE="crypto_LUKS" ...
/dev/mapper/sdb1_crypt: TYPE="ext4" ...
# ls -l /dev/mapper/sdb1_crypt /dev/dm-0
brw-rw----. 1 root disk 253, 0 Sep 12 19:09 /dev/dm-0
lrwxrwxrwx. 1 root root 7 Sep 12 19:09 /dev/mapper/sdb1_crypt -> ../dm-0
# grep sdb1 /etc/fstab
/dev/mapper/sdb1_crypt /mnt/1 ext4 defaults 0 0
The mount point will be shown twice for the partition:
/mnt/1, /mnt/1
This is because add_node_and_mountpoint() adds two entries for both the
symbolic and real block special names:
map["/dev/mapper/sdb1_crypt"] = ["/mnt/1"]
map["/dev/dm-0"] = ["/mnt/1"]
This was needed for the old code which used string compare to match
block devices so that the mount point could be looked up by either name.
However since bug 767842 introduced major, minor number comparison it
became unnecessary. As both names refer to the same device the mount
point gets added twice to the same entry. Hence display of the double
mount.
map[BlockSpecial{"/dev/mapper/sdb1_crypt", 253, 0}] =
["/mnt/1", "/mnt/1"]
It is always going to be the case that the symbolic link and real block
special names have the same major, minor numbers. That was the
requirement of the BlockSpecial class and the reason for using stat() to
lookup the numbers. Therefore adding entries for both names will always
add duplicate entries. Fix by stop using realpath() to lookup the real
name and adding the duplicate entry.
Introduced by:
a800ca8b68
Add BlockSpecial into mount_info and fstab_info (#767842)
Bug 771323 - GParted is showing duplicate mount points for unmounted
encrypted file systems
The GParted_Core::mount_info and GParted_Core::fstab_info maps and the
methods that manipulate them are self-contained. Therefore move them to
a separate Mount_Info module and reduce the size of the monster
GParted_Core slightly.
The FS_Info module has a pseudo multi-object interface and used the
constructor to load the cache. However all the data in the class is
static. An FS_Info object doesn't contain any member variables, yet was
needed just to call the member functions.
Make all the member functions static removing the need to use any
FS_Info objects and provide an explicit load_cache() method.
Following earlier commit "Pre-populate BlockSpecial cache while reading
/proc/partitions (#767842)" load_proc_partitions_info_cache() has
extracted just the name field from each line of /proc/partitions.
Therefore simplify the regular expressions matching each type of whole
disk device to just matching in the name field rather than matching in
the whole line.
The Proc_Partitions_Info has a pseudo multi-object interface and uses
the constructor to load the cache. However all the data in the class is
static. A Proc_Partitions_Info object doesn't contain any member
variables, yet was needed just to call the member functions.
Make all the member functions static removing the need to use any
Proc_Partitions_Info objects and provide and explicit load_cache()
method.
Vol_id has been retired and removed from all supported distributions.
See earlier commit "Remove use of retired vol_id from FS_Info module
(#767842)" for more details. Therefore remove it's use from GParted
entirely.
GParted is already reading /proc/partitions to get whole disk device
names. The file also contains the major, minor device number of every
partition. Use this information to pre-populate the cache in the
BlockSpecial class.
# cat /proc/partitions
major minor #blocks name
8 0 20971520 sda
8 1 512000 sda1
8 2 20458496 sda2
...
9 3 1047552 md3
259 2 262144 md3p1
259 3 262144 md3p2
...
253 0 18317312 dm-0
253 1 2097152 dm-1
253 2 8383872 dm-2
253 3 1048576 dm-3
Note that for Device-Mapper names (dm-*) the kernel is not using the
canonical user space names (mapper/*). There is no harm in
pre-populating the cache with these names and will help if tools report
them too. It is just that for DMRaid, LVM and LUKS, GParted uses the
canonical /dev/mapper/* names so will still have to call stat() once for
each such name.
For plain disks (sd*) and Linux Software RAID arrays (md*) the kernel
name is the common user space name too, therefore matches what GParted
uses and pre-populating does avoid calling stat() at all for these
names.
Bug 767842 - File system usage missing when tools report alternate block
device names
Creation of every BlockSpecial object used to result in a stat() OS
call. On one of my test VMs debugging with 4 disks and a few partitions
on each, GParted refresh generated 541 calls to stat() in the
BlockSpecial(name) constructor. However there were only 45 unique
names. So on average each name was stat()ed approximately 12 times.
Cache the major, minor number associated with each name after starting
with a cleared cache for each GParted refresh. This reduces these
direct calls to stat() to just the 45 unique names.
Bug 767842 - File system usage missing when tools report alternate block
device names
The FS_Info cache is loaded from "blkid" output and compares block
special names. Therefore switch to using BlockSpecial objects so that
comparisons are performed by the major, minor device number instead.
Bug 767842 - File system usage missing when tools report alternate block
device names
FS_Info module caches the output from blkid as a single string and uses
regular expressions to find the line matching the requested block
special file name. This is not compatible with using BlockSpecial
objects to represent block special files, and perform matching by major,
minor device number. Therefore parse the blkid output into a vector of
structures containing the needed fields, ready for switching to
BlockSpecial objects in the following patch.
Interface to the module remains unchanged.
Bug 767842 - File system usage missing when tools report alternate block
device names
Vol_id was removed from udev 142, released 2009-05-13, and udev switched
to using blkid instead [1]. All currently supported distributions use
later versions of udev (or systemd after the udev merge), except for
RedHat / CentOS 5 with udev 095. However RedHat / CentOS 5 does provide
blkid and vol_id is found in udev specific /lib/udev directory not on
the PATH. Therefore effectively vol_id is not available on any
supported distribution and blkid is always available. Therefore remove
use of vol_id from the FS_Info module. Less code to refactor and test
in following changes.
[1] delete vol_id and require util-linux-ng's blkid
http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=f07996885dab45102492d7f16e7e2997e264c725
Bug 767842 - File system usage missing when tools report alternate block
device names
The SWRaid_Info cache is loaded from "mdadm" command output and
/proc/mdstat file. It contains the member name which is used to access
the cache, therefore switch to using BlockSpecial objects so that
comparison is performed using the major, minor device number.
Bug 767842 - File system usage missing when tools report alternate block
device names
The LUKS_Info module cache is loaded from "dmsetup" command and compares
block special files, therefore switch to using BlockSpecial objects so
that comparisons are performed by major, minor device number.
Bug 767842 - File system usage missing when tools report alternate block
device names
Small optimisation which avoids constructing an extra BlockSpecial
object when determining if a btrfs member is mounted. Rather than
extracting the name from the BlockSpecial object in
btrfs::get_mount_device() and re-constructing another BlockSpecial
object from that name in GParted_Core::is_dev_mounted(), pass the
BlockSpecial object directly.
Bug 767842 - File system usage missing when tools report alternate block
device names
There are no known errors which affect the remaining caches in GParted.
However those caches which compare block special devices will be changed
to use BlockSpecial objects so comparison is by major, minor device
number rather than by name.
Change btrfs member cache loaded from "btrfs filesystem show" output to
use BlockSpecial objects.
Bug 767842 - File system usage missing when tools report alternate block
device names
On some distributions having btrfs on top of LUKS encrypted partitions,
adding a second device and removing the first device used to mount the
file system causes GParted to no longer be able to report the file
system as busy or the mount points themselves.
For example, on CentOS 7, create a single btrfs file system and mount
it. The provided /dev/mapper/sdb1_crypt name is reported, via
/proc/mounts, as the mounting device:
# cryptsetup luksFormat --force-password /dev/sdb1
# cryptsetup luksOpen /dev/sdb1 sdb1_crypt
# mkfs.btrfs -L encrypted-btrfs /dev/mapper/sdb1_crypt
# mount /dev/mapper/sdb1_crypt /mnt/1
# ls -l /dev/mapper
total 0
lrwxrwxrwx. 1 root root 7 Jul 2 14:15 centos-root -> ../dm-1
lrwxrwxrwx. 1 root root 7 Jul 2 14:15 centos-swap -> ../dm-0
crw-------. 1 root root 10, 236 Jul 2 14:15 control
lrwxrwxrwx. 1 root root 7 Jul 2 15:14 sdb1_crypt -> ../dm-2
# fgrep btrfs /proc/mounts
/dev/mapper/sdb1_crypt /mnt/1 btrfs rw,seclabel,relatime,space_cache 0 0
Add a second device to the btrfs file system:
# cryptsetup luksFormat --force-password /dev/sdb2
# cryptsetup luksOpen /dev/sdb2 sdb2_crypt
# btrfs device add /dev/mapper/sdb2_crypt /mnt/1
# ls -l /dev/mapper
...
lrwxrwxrwx. 1 root root 7 Jul 2 15:12 sdb2_crypt -> ../dm-3
# btrfs filesystem show /dev/mapper/sdb1_crypt
Label: 'encrypted-btrfs' uuid: 45d7b1ef-820c-4ef8-8abd-c70d928afb49
Total devices 2 FS bytes used 32.00KiB
devid 1 size 1022.00MiB used 12.00MiB path /dev/mapper/sdb1_crypt
devid 2 size 1022.00MiB used 0.00B path /dev/mapper/sdb2_crypt
Remove the first mounting device from the btrfs file system. Now the
non-canonical name /dev/dm-3 is reported, via /proc/mounts, as the
mounting device:
# btrfs device delete /dev/mapper/sdb1_crypt /mnt/1
# btrfs filesystem show /dev/mapper/sdb2_crypt
Label: 'encrypted-btrfs' uuid: 45d7b1ef-820c-4ef8-8abd-c70d928afb49
Total devices 1 FS bytes used 96.00KiB
devid 2 size 1022.00MiB used 144.00MiB path /dev/mapper/sdb2_crypt
# fgrep btrfs /proc/mounts
/dev/dm-3 /mnt/1 btrfs rw,seclabel,relatime,space_cache 0 0
# ls -l /dev/dm-3
brw-rw----. 1 root disk 253, 3 Jul 2 15:12 /dev/dm-3
GParted loads the mount_info mapping from /proc/mounts and with it the
/dev/dm-3 name. When GParted is determining if the encrypted btrfs file
system is mounted or getting the mount points it is using the
/dev/mapper/sdb2_crypt name. Therefore no information is found and the
file system is incorrectly reported as unmounted.
Fix by changing mount_info and fstab_info to use BlockSpecial objects
instead of strings so that matching is performed by major, minor device
numbers rather than by string compare. Note that as BlockSpecial
objects are used as the key of std::map [1] mappings operator<() [2]
needs to be provided to order the key values.
[1] std::map
http://www.cplusplus.com/reference/map/map/
[2] std::map::key_comp
http://www.cplusplus.com/reference/map/map/key_comp/
Bug 767842 - File system usage missing when tools report alternate block
device names
In some cases creating an LVM2 Physical Volume on top of a DMRaid array
reports no usage information and this partition warning:
Unable to read the contents of this file system!
Because of this some operations may be unavailable.
The cause might be a missing software package.
The following list of software packages is required for lvm2
pv file system support: lvm2.
For example on Ubuntu 14.04 LTS (with GParted built with
--enable-libparted-dmraid) create an LVM2 PV in a DMRaid array
partition. GParted uses this command:
# lvm pvcreate -M 2 /dev/mapper/isw_bacdehijbd_MyArray0p2
But LVM reports the PV having a different name:
# lvm pvs
PV VG Fmt Attr PSize PFree
/dev/disk/by-id/dm-name-isw_bacdehijbd_MyArray0p2 lvm2 a-- 1.00g 1.00g
This alternate name is loaded into the LVM2_PV_Info module cache. Hence
when GParted queries partition /dev/mapper/isw_bacdehijbd_MyArray0p2 it
has no PV information against that name and reports unknown usage.
However they are actually the same block special device; major 252,
minor 2:
# ls -l /dev/mapper/isw_bacdehijbd_MyArray0p2
brw-rw---- 1 root disk 252, 2 Jul 2 11:09 /dev/mapper/isw_bacdehijbd_MyArray0p2
# ls -l /dev/disk/by-id/dm-name-isw_bacdehijbd_MyArray0p2
lrwxrwxrwx 1 root root 10 Jul 2 11:09 /dev/disk/by-id/dm-name-isw_bacdehijbd_MyArray0p2 -> ../../dm-2
# ls -l /dev/dm-2
brw-rw---- 1 root disk 252, 2 Jul 2 11:09 /dev/dm-2
To determine if two names refer to the same block special device their
major, minor numbers need to be compared, instead of string comparing
their names.
Implement class BlockSpecial which encapsulates the name and major,
minor numbers for a block special device. Also performs comparison as
needed. See bug 767842 comments 4 and 5 for further investigation and
decision for choosing to implement a class.
Replace name strings in the LVM2_PV_Info module with BlockSpecial
objects performing correct block special device comparison.
Bug 767842 - File system usage missing when tools report alternate block
device names
Now Device and Partition objects only have a single path,
get_alternate_paths() is never called. Remove the method and population
of the private alternate_paths_cache member that went with it.
Bug 767842 - File system usage missing when tools report alternate block
device names
To reflect that there is now only a single path in the Partition object
now. Also get rid of the now unneeded optional clear_paths parameter
which was only relevant when there was a vector of paths.
Bug 767842 - File system usage missing when tools report alternate block
device names
Now that Partition objects only have a single path, rather than a list
of paths, stop performing unnecessary actions in calibrate_partitions()
which added alternate paths reported from libparted. Just left with
setting the partition path name correctly, when the path name doesn't
exist. Happens when the path is set to "Copy of /dev/SRC" when the
partition was newly created by a copy-paste into unallocated space
earlier in the sequence of operations now being applied.
Bug 767842 - File system usage missing when tools report alternate block
device names
Change from a vector of paths to a single path member in the Partition
object. Remove add_paths() and get_paths() methods. Keep add_path()
and get_path().
Bug 767842 - File system usage missing when tools report alternate block
device names
To reflect that there is only a single path in the Device object now.
Also get rid of the now unneeded optional parameter which was only
relevant when there was a vector of paths.
Bug 767842 - File system usage missing when tools report alternate block
device names
Background
GParted stored a list of paths for Device and Partition objects. It
sorted this list [1][2] and treated the first specially as that is what
get_path() returned and was used almost everywhere; with the file system
specific tools, looked up in various *_Info caches, etc.
[1] Device::add_path(), ::add_paths()
[2] Partition::add_path(), ::add_paths()
Mount point display [3] was the only bit of GParted which really worked
with the path list. Busy file system detection [4] just used the path
provided by libparted, or for LUKS /dev/mapper/* names. It checked that
single path against the mounted file systems found from /proc/mounts,
expanded with additional block device names when symlinks were
encountered.
[3] GParted_Core::set_mountpoints() -> set_mountpoints_helper()
[4] GParted_Core::set_device_partitions() -> is_busy()
GParted_Core::set_device_one_partition() -> is_busy()
GParted_Core::set_luks_partition() -> is_busy()
Having the first path, by sort order, treated specially by being used
everywhere and virtually ignoring the others was wrong, complicated to
remember and difficult code with. As all the additional paths were
virtually unused and made no difference, remove them. The "improved
detection of mountpoins, free space, etc.." benefit from commit [5]
doesn't seem to exist. Therefore simplify to a single path for Device
and Partition objects.
[5] commit 6d8b169e73
changed the way devices and partitions store their device paths.
Instead of holding a 'realpath' and a symbolic path we store paths
in a list. This allows for improved detection of mountpoins, free
space, etc..
This patch
Simplify the Device object from a vector of paths to a single path.
Remove add_paths() and get_paths() methods. Keep add_path() and
get_path() for now.
Bug 767842 - File system usage missing when tools report alternate block
device names
Recognise GRUB2 core.img boot code written to a partition without a file
system. Such setups are possible/likely with GPT partitioned disks as
there is a specific partition type reserved for it [1][2]:
21686148-6449-6E6F-744E-656564454649 (BIOS Boot partition)
[1] GUID Partition Table, Partition types
https://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs
[2] BIOS boot partition
https://en.wikipedia.org/wiki/BIOS_boot_partition
Bug 766989 - zfsonline support - need file system name support for ZFS
type codes
Composing these operations caused GParted to abort on an assert failure:
(1) Delete an existing partition,
(2) Create a new partition,
(3) Delete new partition.
This is the equivalent issue as fixed in the previous commit, except with
the delete operation rather than the check operation:
Prevent assert failure from OperationCheck::get_partition_new() (#767233)
# ./gpartedbin
======================
libparted : 2.4
======================
**
ERROR:OperationDelete.cc:41:virtual GParted::Partition& GParted::OperationDelete::get_partition_new(): assertion failed: (false)
Aborted (core dumped)
# gdb ./gpartedbin core.19232 --batch --quiet --ex backtrace -ex quit
[New Thread 19232]
[New Thread 19234]
[Thread debugging using libthread_db enabled]
Core was generated by `./gpartedbin'.
Program terminated with signal 6, Aborted.
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x000000361f233dc5 in abort () at abort.c:92
#2 0x0000003620a67324 in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x50fcc0 "virtual GParted::Partition& GParted::OperationDelete::get_partition_new()", message=0x1b55f60 "assertion failed: (false)") at gtestutils.c:1358
#3 0x0000003620a678f0 in g_assertion_message_expr (domain=0x0, file=0x50fa68 "OperationDelete.cc", line=41, func=0x50fcc0 "virtual GParted::Partition& GParted::OperationDelete::get_partition_new()", expr=<value optimized out>) at gtestutils.c:1369
#4 0x000000000049aa0d in GParted::OperationDelete::get_partition_new (this=0x1b321b0) at OperationDelete.cc:41
#5 0x00000000004c6700 in GParted::Win_GParted::activate_delete (this=0x7ffc91403670) at Win_GParted.cc:2068
...
As before the crash is happened in Win_GParted::activate_delete() as it
was going through the list of operations removing those which applied to
the never created partition. It came across the delete operation of an
existing partition and called get_partition_new(). As partition_new was
not set or used by the delete operation this asserted false and crashed
GParted.
Unlike the check operation case, the delete operation doesn't have a
partition afterwards. (As GParted represents unallocated space with
partition objects then the delete operation could be populated with a
new partition by constructing the correctly merged unallocated space
partition object, but that is what OperationDelete::apply_to_visual()
does and having a place holder doesn't seem like the right thing to do).
Instead exclude delete operations, on existing partitions, when looking
for operations applying to this not yet created partition as they are
mutually exclusive.
Bug 767233 - GParted core dump on assert failure in
OperationDelete::get_partition_new()
Composing these operations caused GParted to abort on an assert failure:
(1) Check an existing partition,
(2) Create a new partition,
(3) Delete new partition.
# ./gpartedbin
======================
libparted : 2.4
======================
**
ERROR:OperationCheck.cc:40:virtual GParted::Partition& GParted::OperationCheck::get_partition_new(): assertion failed: (false)
Aborted (core dumped)
# gdb ./gpartedbin core.8876 --batch --quiet --ex backtrace -ex quit
[New Thread 8876]
[New Thread 8879]
[Thread debugging using libthread_db enabled]
Core was generated by `./gpartedbin'.
Program terminated with signal 6, Aborted.
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
#0 0x000000361f2325e5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x000000361f233dc5 in abort () at abort.c:92
#2 0x0000003620a67324 in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x50f400 "virtual GParted::Partition& GParted::OperationCheck::get_partition_new()", message=0x1d37a00 "assertion failed: (false)") at gtestutils.c:1358
#3 0x0000003620a678f0 in g_assertion_message_expr (domain=0x0, file=0x50f1a8 "OperationCheck.cc", line=40, func=0x50f400 "virtual GParted::Partition& GParted::OperationCheck::get_partition_new()", expr=<value optimized out>) at gtestutils.c:1369
#4 0x0000000000498e21 in GParted::OperationCheck::get_partition_new (this=0x1d1bb30) at OperationCheck.cc:40
#5 0x00000000004c66ec in GParted::Win_GParted::activate_delete (this=0x7fff031c3e30) at Win_GParted.cc:2068
...
When Win_GParted::activate_delete() was stepping through the operation
list removing operations (2 & 3 in the above recreation steps) which
related to the new partition never to be created it called
get_partition_new() on all operations in the list. This included
calling get_partition_new() on the check operation (1 in the above
recreation steps). As partition_new was not set or used by the check
operation get_partition_new() asserted false and crashed GParted.
Fix by populating the partition_new member in OperationCheck objects,
thus allowing get_partition_new() to be called on the object. As a
check operation doesn't change any partition boundaries or file system
attributes, just duplicate the new partition from the original
partition.
Bug 767233 - GParted core dump on assert failure in
OperationDelete::get_partition_new()
E2fsprogs 1.42 adds ext4 64bit feature [1] allowing volume sizes larger
than 16 TiB. However only enable large volumes from e2fsprogs 1.42.9
when a large number of 64bit bugs were fixed [2]. (Also RHEL / CentOS 7
use e2fsprogs 1.42.9 and always enable 64bit feature by default).
[1] Release notes, E2fsprogs 1.42 (November 29, 2011)
http://e2fsprogs.sourceforge.net/e2fsprogs-release.html#1.42
"This release of e2fsprogs has support for file systems > 16TB.
Online resize requires kernel support which will hopefully be in
Linux version 3.2. Offline support is not yet available for > 16TB
file systems, but will be coming".
[2] Release notes, E2fsprogs 1.42.9 (December 28, 2013)
http://e2fsprogs.sourceforge.net/e2fsprogs-release.html#1.42.9
"Fixed a large number of bugs in resize2fs, e2fsck, debugfs, and
libext2fs to correctly handle bigalloc and 64-bit file systems.
There were many corner cases that had not been noticed in previous
uses of these file systems, since they are not as common. Some of
the bugs could cause file system corruption or data loss, so users
of 64-bit or bigalloc file systems are strongly urged to upgrade to
e2fsprogs 1.42.9".
Bug 766910 - Multiple boot loaders don't work on 64bit EXT4 file systems
Calling libparted ped_geometry_new() creates a new PedGeometry object
from malloced memory, however the corresponding ped_geometry_destroy()
is never called to destroy the object and free the memory.
Perform a resize of a FAT file system when running GParted under
valgrind identifies several memory blocks leaked via ped_geometry_new()
from resize_move_filesystem_using_libparted(). One such example:
# valgrind --track-origins=yes --leak-check=full ./gpartedbin
...
==32069== 32 bytes in 1 blocks are definitely lost in loss record 5,419 of 11,542
==32069== at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==32069== by 0x8ECD8C5: ped_malloc (libparted.c:231)
==32069== by 0x8ED23C1: ped_geometry_new (geom.c:79)
==32069== by 0x4764F3: GParted::GParted_Core::resize_move_filesystem_using_libparted(GParted::Partition const&, GParted::Partition const&, GParted::OperationDetail&) (GParted_Core.cc:2666)
==32069== by 0x478007: GParted::GParted_Core::resize_filesystem(GParted::Partition const&, GParted::Partition const&, GParted::OperationDetail&, bool) (GParted_Core.cc:2990)
==32069== by 0x478440: GParted::GParted_Core::maximize_filesystem(GParted::Partition const&, GParted::OperationDetail&) (GParted_Core.cc:3037)
==32069== by 0x4769A0: GParted::GParted_Core::resize(GParted::Partition const&, GParted::Partition const&, GParted::OperationDetail&) (GParted_Core.cc:2746)
==32069== by 0x47582B: GParted::GParted_Core::resize_move(GParted::Partition const&, GParted::Partition&, GParted::OperationDetail&) (GParted_Core.cc:2457)
==32069== by 0x46DDB2: GParted::GParted_Core::apply_operation_to_disk(GParted::Operation*) (GParted_Core.cc:767)
...
There is also a leak of a PedGeometry object from
resize_move_partition(). Fix by calling ped_geometry_destroy() to
delete all the allocated PedGeometry objects and free the memory.
Bug 767009 - PedGeometry objects are memory leaked
When replacing the list of paths for the other partition object involved
in either a Resize/Move or Format operation in apply_operation_to_disk()
should replace the whole list of partitions not just replace with the
first path. Copy the whole path list is the correct action. It makes
no material difference because secondary partition paths are only used
to discover mount points during refresh phase and not at the apply
phase, where only the primary path is used.
Bug 766349 - Resolve code ugliness with partition path getting set to
"copy of /dev/SRC"
Mike Fleetwood