Was attempting to erase the wrong key "key-long" for the test. It
should be erasing "key-too-long". Fix this. Given that that line in
the test was to erase a non-existent key and confirm failure; the test
passed before with the wrong non-existent key, and still passes with the
right non-existent key. Clearly a cut and paste error as a result of
copying the previous LongPassword test when initially added in:
c6657aab9e
Add unit tests for PasswordRAMStore module (#795617)
Separate udevsettle command was merged into udevadm back in udev 117
(released 2007-11-13) by:
225cb03bd8
udevadm: merge all udev tools into a single binary
The oldest supported distributions have these much newer versions of
udev:
Distro EOL udevadm -V
Debian 8 2020-Apr 215 (combined systemd and udev)
RHEL / CentOS 7 2024-Jun 219 (combined systemd and udev)
Ubuntu 16.04 LTS 2021-Apr 229 (combined systemd and udev)
Along with the previous patch "Avoid reading the same sector multiple
times in a row (!46)(#16)" internal detection of file system signatures
now reads the minimum number of sectors in increasing order.
As the code still considers the sector size, it now also reads less
sectors from devices with larger sectors to get all the signatures. So
on a 512 byte sector device it maximally seeks to and reads from these
byte offsets:
0, 512, 1024, 65536
and on a 4096 byte sector device, only from these byte offsets:
0, 65536
Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
Closes#16 - "invalid argument for seek()" error on very small (<=40KiB)
drives
GParted internal file system detection is reading the same sector
multiple times in a row. Add an optimisation to only read a sector if
it is different to the previously read sector.
Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
Closes#16 - "invalid argument for seek()" error on very small (<=40KiB)
drives
After the previous commit the lp_device structure pointer parameter is
only used to provide sector_size. Just pass that instead.
Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
Closes#16 - "invalid argument for seek()" error on very small (<=40KiB)
drives
GParted_Core::useable_device() doesn't change the pointed to PedDevice
structure. Pass it as const so the compiler enforces this.
Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
For every device that GParted scans, it determines if it can be used by
attempting to read the first sector. This is the sequence of events,
as reported in the previous two commit messages:
gparted |set_devices_thread(pdevices) pdevices->["/dev/sdb"]
...
gparted | useable_device(lp_device) lp_device->path="/dev/sdb"
gparted | ped_device_open()
libparted | open("/dev/sdb", O_RDWR) = 8
gparted | ped_device_read()
gparted | ped_device_close()
libparted | close(8)
udev(async)| KERNEL change /devices/.../sdb (block)
...
udev(async)| UDEV change /devices/.../sdb (block)
Note that these udev block device change events occur before the ones
waited for in the first commit "Wait for udev change on /dev/DISK when
querying whole device FS (!46)" so these were already waited for.
So because libparted only opens devices read-write this triggers a set
of udev change events for the whole block device, and if the device is
partitioned, also remove and re-add events for all the partitions.
Switch to using POSIX open(), read() and close() calls so read-only
access can be specified to avoid the unnecessary udev change events.
Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
After the previous fix, on my CentOS 7 VM, GParted is now often
reporting a warning that the /dev/PTN entry is missing when reading the
label of the file system in the first partition. This happens
regardless of the file system type.
Example error for EXT4:
e2label: No such file or directory while trying to open /dev/sdb1
Couldn't find valid file system superblock.
Example error for XFS:
/dev/sdb1: No such file or directory
fatal error -- couldn't initialize XFS library
As with the case in the previous commit, GParted gets the label via
blkid instead.
Again with debugging and sleeping in GParted, stracing the GParted
thread GParted_Core::set_devices_thread() and using 'udevadm monitor' to
watch udev events the following sequence of events is observed:
gparted |set_devices_thread(pdevices) pdevices->["/dev/sdb"]
gparted | ped_device_get("/dev/sdb")
libparted | open("/dev/sdb", O_RDONLY) = 8
libparted | close(8)
gparted | useable_device(lp_device) lp_device->path="/dev/sdb"
gparted | ped_device_open()
libparted | open("/dev/sdb", O_RDWR) = 8
gparted | ped_device_read()
gparted | ped_device_close()
libparted | close(8)
udev(async)| KERNEL remove /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL remove /devices/.../sdb/sdb2 (block)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| KERNEL add /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL add /devices/.../sdb/sdb2 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb1 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb2 (block)
udev(async)| UDEV change /devices/.../sdb (block)
udev(async)| UDEV add /devices/.../sdb/sdb1 (block)
udev(async)| UDEV add /devices/.../sdb/sdb2 (block)
gparted | set_device_from_disk(device, device_path="/dev/sdb")
gparted | get_device(device_path="/dev/sdb", ..., flush=true)
gparted | ped_device_get()
gparted | flush_device(lp_device) lp_device->path="/dev/sdb"
gparted | ped_device_open()
libparted | open("/dev/sdb", O_RDWR) = 8
gparted | ped_device_sync()
gparted | ped_device_close()
libparted | close(8)
udev(async)| KERNEL remove /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL remove /devices/.../sdb/sdb2 (block)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| KERNEL add /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL add /devices/.../sdb/sdb2 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb1 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb2 (block)
udev(async)| UDEV change /devices/.../sdb (block)
udev(async)| UDEV add /devices/.../sdb/sdb1 (block)
udev(async)| UDEV add /devices/.../sdb/sdb2 (block)
gparted | settle_device()
gparted | Utils::execute_command("udevadm settle --timeout=1")
gparted | detect_filesystem(lp_device, NULL, ...) lp_device->path="/dev/sdb"
gparted | detect_filesystem_internal(lp_device, NULL) lp_device->path="/dev/sdb"
gparted | ped_device_open()
libparted | open("/dev/sdb", O_RDWR) = 8
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_read()
gparted | ped_device_close()
libparted | close(8)
udev(async)| KERNEL remove /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL remove /devices/.../sdb/sdb2 (block)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| KERNEL add /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL add /devices/.../sdb/sdb2 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb1 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb2 (block)
udev(async)| UDEV change /devices/.../sdb (block)
udev(async)| UDEV add /devices/.../sdb/sdb1 (block)
udev(async)| UDEV add /devices/.../sdb/sdb2 (block)
gparted | get_disk(lp_device, lp_disk, strict=false)
gparted | ped_disk_new(lp_device) lp_device->path="/dev/sdb"
libparted | open("/dev/sdb", O_RDWR) = 8
libparted | close(8)
udev(async)| KERNEL remove /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL remove /devices/.../sdb/sdb2 (block)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| KERNEL add /devices/.../sdb/sdb1 (block)
udev(async)| KERNEL add /devices/.../sdb/sdb2 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb1 (block)
udev(async)| UDEV remove /devices/.../sdb/sdb2 (block)
udev(async)| UDEV change /devices/.../sdb (block)
udev(async)| UDEV add /devices/.../sdb/sdb1 (block)
udev(async)| UDEV add /devices/.../sdb/sdb2 (block)
gparted | set_device_partitions()
gparted | detect_filesystem()
gparted | set_partition_label_and_uuid(partition) partition.get_path()="/dev/sdb1"
gparted | read_label()
gparted | ext2::read_label()
gparted | Utils::execute_command("e2label /dev/sdb1")
So in this case for a partitioned drive, libparted ped_disk_new() is
opening and closing the device read-write and triggering the udev remove
and add partition block special entries exactly when the label is trying
to be read from the first partition, causing the device missing error.
The call chain is:
set_devices_thread()
set_device_from_disk()
get_disk()
ped_disk_new()
Looking at the source for ped_disk_new() [1] it is calling
ped_device_open() and ped_device_close(), hence why it is opening the
device read-write and triggering the udev events.
Looking back at commit "Wait for udev to recreate /dev/PTN entries when
calibrating (#762941)" [2] and re-testing it, the udev events were also
caused by ped_disk_new() with this call chain:
apply_operation_to_disk()
calibrate_partition()
get_disk()
ped_disk_new()
Fix this probe case and keep the fix for the previous calibrate case by
moving the waiting for udev events from calibrate_partition() into
get_disk(), right after ped_disk_new(). The maximum wait time is
shortened to the shorter 1 second probing timeout to avoid the longer
10 second apply timeout in this probing case. The calibrate case commit
message said "The longest I have seen udev take to do this is 0.80
seconds in a VM".
[1] parted/libparted/disk.c:ped_disk_new()
http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/disk.c?id=v3.2#n169
[2] fd9013d5f6
Wait for udev to recreate /dev/PTN entries when calibrating
(#762941)
Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
GParted nearly always shows this warning against a whole disk device
FAT32 file system on my development VMs:
plain floppy: device "/dev/sdb" busy (Resource temporarily
unavailable): Cannot initialize '::'
mlabel: Cannot initialize drive
However GParted does get the label via blkid instead. Occasionally
everything works correctly and there is no warning.
Never found a similar error for any other file system on a whole disk
device. The timing never seems to clash.
Remember that when a writable file descriptor of a disk device is
closed, udev events are triggered which update the kernel and /dev
entries for any partition changes. (This is in addition to coding which
has always existed in the partitioning tools, including fdisk and
parted, to inform the kernel, but not create /dev entries, of partition
changes).
With debugging and sleeping in GParted, stracing the GParted thread
GParted_Core::set_devices_thread() and using 'udevadm monitor' to watch
udev events the following sequence of events is observed:
gparted |set_devices_thread(pdevices) pdevices->["/dev/sdb"]
gparted | ped_device_get("/dev/sdb")
libparted | open("/dev/sdb", O_RDONLY) = 8
libparted | close(8)
gparted | useable_device(lp_device) lp_device->path="/dev/sdb"
gparted | ped_device_open()
libparted | open("/dev/sdb", O_RDWR) = 8
gparted | ped_device_read()
gparted | ped_device_close()
libparted | close(8)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| UDEV change /devices/.../sdb (block)
udev(async)| UDEV change /devices/.../sdb (block)
gparted | set_device_from_disk(device, device_path="/dev/sdb")
gparted | get_device(device_path="/dev/sdb", ..., flush=true)
gparted | ped_device_get()
gparted | flush_device(lp_device) lp_device->path="/dev/sdb"
gparted | ped_device_open()
libparted | open("/dev/sdb", O_RDWR) = 8
gparted | ped_device_sync()
gparted | ped_device_close()
libparted | close(8)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| KERNEL change /devices/.../sdb (block)
udev(async)| UDEV change /devices/.../sdb (block)
udev(async)| UDEV change /devices/.../sdb (block)
gparted | set_device_one_partition()
gparted | set_partition_label_and_uuid()
gparted | read_label()
gparted | fat16::read_label()
gparted | Utils::execute_command("mlabel -s :: -i /dev/sdb")
This sequence of events only shows which close()s by libparted trigger
udev events. It does not show that processing those events are
asynchronous and overlap with GParted executing mlabel, causing the
device busy error.
As libparted's ped_device_open() doesn't offer a way to specify that a
device will only be used for reading, it always opens it read-write
[1][2]. Hence this sequence in GParted_Core::flush_device():
ped_device_open()
ped_device_sync()
ped_device_close()
always triggers udev change events on the whole disk device. Fix by
waiting for those udev events to complete.
[1] libparted documentation, PedDevice, ped_device_open()
https://www.gnu.org/software/parted/api/group__PedDevice.html#ga7c87bd06e76553c8ff3262b5e6ca256
[2] parted/libparted/arch/linux.c:linux_open()
http://git.savannah.gnu.org/cgit/parted.git/tree/libparted/arch/linux.c?id=v3.2#n1628Closes!46 - Whole device FAT32 file system reports device busy warning
from mlabel
The last distribution to include the separate 'udevinfo' command was
RHEL / CentOS 5 with udev 095. All supported distributions have much
newer versions of udev and instead have the 'udevadm' command.
A user reported that GParted was slow to refresh on FAT32 file systems:
"can take very long, up to several minutes; can be reproduced by running
dosfsck manually". This is because the file system was large and almost
full, and GParted performs a file system check just to to report the
file system usage.
Created a 4 GiB FAT32 file system and almost filled it with 4 KiB files,
just over 970,000 files.
# df -k /mnt/2
Filesystem 1K-blocks Used Available Used% Mounted on
/dev/sdb2 4186108 39155384 270724 94% /mnt/2
# df -i /mnt/2
Filesystem Inodes IUsed IFree IUse% Mounted on
/dev/sdb2 0 0 0 - /mnt/2
# find /mnt/2 -type f -print | wc -l
971059
# find /mnt/2 -type d -print | wc -l
1949
Testing performance of the current fsck.fat:
# time fsck.fat -n -v /dev/sdb2 | \
> egrep 'bytes per logical sector|bytes per cluster|sectors total|clusters$'
512 bytes per logical sector
4096 bytes per cluster
8388608 sectors total
/dev/sdb2: 973008 files, 978846/1046527 clusters
real 0m11.552s
user 0m2.183s
sys 0m7.547s
Free sectors in the file system according to fsck.fat:
(1046527 - 978846) * 4096 / 512 = 541448 sectors
Repeating this test while also using 'blktrace /dev/sdb2' and Ctrl-C
around the test in a separate terminal, reports these numbers of I/Os
being performed:
Read requests Read bytes
15,563 165 MiB
Prior to this commit [1] from 0.0.9, GParted used libparted's
ped_file_system_get_resize_constraint() to report the minimum size to
which a FAT* file system can be resized. Use this test program [2] to
performance test this method:
# time ./fscons /dev/sdb2
dev=/dev/sdb2
sector_size=512
min_size=7909522
max_size=8388608
real 0m2.673s
user 0m0.070s
sys 0m1.834s
Free sectors in the file system according to libparted
ped_file_system_get_resize_constraint():
8388608 - 7909522 = 479086 sectors
blktrace reports these numbers of I/Os being performed:
Read requests Read bytes
7,821 71 MiB
So using libparted resize constraint is a bit faster but is still
reading too much data and is really too slow. Also when testing GParted
using this libparted method against a corrupted FAT32 file system, on
every refresh, one popup dialog is displayed for each error libparted
detects with the file system, each of which needs acknowledgement.
Example popup:
Libparted Error
\DIRNAME\FILENAME.EXT is 107724k, but is has 1920
clusters (122880k).
[ Cancel ][ Ignore ]
There could be a huge number of such errors in a corrupted file system.
Not really suitable for use by GParted.
Test the performance of mtools' minfo command to report the file system
figures:
# time minfo -i /dev/sdb2 :: | \
> egrep 'sector size:|cluster size:|small size:|big size:|free clusters='
sector size: 512 bytes
cluster size: 8 sectors
small size: 0 sectors
big size: 8388608 sectors
free clusters=67681
real 0m0.013s
user 0m0.004s
sys 0m0.019s
Free sectors in the file system according to minfo:
67681 * 8 = 541448 sectors
blktrace reports these numbers of I/Os being performed by minfo:
Read requests Read bytes
1 16 KiB
This matches with minfo just reading information from the BPB (BIOS
Parameter Block) [3] from sector 0 and the FS Information Sector [4]
usually in sector 1. Note that the free cluster figure reported by
minfo comes from the FS Information Sector because it only reports it
for FAT32 file systems, not for FAT16 file systems. Scanning the File
Allocation Table (FAT) [5] to count free clusters is exactly what mdir,
without the '-f' (fast) flag, does. Test the performance of mdir:
# export MTOOLS_SKIP_CHECK=1
# time mdir -i /dev/sdb2 ::/ | fgrep 'bytes free'
277 221 376 bytes free
real 0m0.023s
user 0m0.011s
sys 0m0.023s
Free sectors in the file system according to mdir:
277221376 / 512 = 541448 sectors
blktrace reports these number of I/Os being performed by mdir:
Read requests Read bytes
5 448 KiB
So minfo and mdir together provide the needed information and are 2 to 3
orders of magnitude faster because they only read the needed BPB and FAT
data from the drive. Use these together to read the file system usage.
[1] 61cd0ce778
lots of stuff and cleanups, including fixing getting used/unused
space of hfs/hfs+/fat16/fat32
[2] fscons.c
/* FILE: fscons.c
* SYNOPSIS: Report libparted's FS resize limits.
* BUILD: gcc -o fscons fscons.c -lparted -lparted-fs-resize
*/
int main(int argc, const char *argv[])
{
PedDevice* dev = NULL;
PedDisk* tab = NULL;
PedPartition* ptn = NULL;
PedFileSystem* fs = NULL;
PedConstraint* cons = NULL;
if (argc != 2)
{
fprintf(stderr, "Usage: fscons BLOCKDEV\n");
exit(1);
}
dev = ped_device_get(argv[1]);
if (dev == NULL)
{
fprintf(stderr, "ped_device_get(\"%s\") failed\n", argv[1]);
exit(1);
}
printf("dev=%s\n", dev->path);
printf("sector_size=%ld\n", dev->sector_size);
tab = ped_disk_new(dev);
if (tab == NULL)
{
fprintf(stderr, "ped_disk_new(dev) failed\n");
exit(1);
}
ptn = ped_disk_get_partition_by_sector(tab, 0);
if (ptn == NULL)
{
fprintf(stderr, "ped_disk_get_partition(tab, 0) failed\n");
exit(1);
}
fs = ped_file_system_open(&ptn->geom);
if (fs == NULL)
{
fprintf(stderr, "ped_file_system_open(&ptn->geom) failed\n");
exit(1);
}
cons = ped_file_system_get_resize_constraint(fs);
if (cons == NULL)
{
fprintf(stderr, "ped_file_system_get_resize_constraint(fs) failed\n");
exit(1);
}
printf("min_size=%ld\n", cons->min_size);
printf("max_size=%ld\n", cons->max_size);
ped_constraint_destroy(cons);
ped_file_system_close(fs);
ped_disk_destroy(tab);
ped_device_destroy(dev);
return 0;
}
[3] Design of the FAT file system, BIOS Parameter Block
https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system#BIOS_Parameter_Block
[4] Design of the FAT file system, FS Information Sector
https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system#FS_Information_Sector
[5] Design of the FAT file system, File Allocation Table
https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system#File_Allocation_Table
Bug 569921 - dosfsck -n delays device scan
When GParted is configured with `--disable-doc` the help documentation
is neither built, nor installed.
With this configuration the Help -> Contents menu displays a message
dialog indicating the "Documentation is not available".
On GNOME 3 no title is shown; however, on some other graphic toolkits
/ window managers an unspecified title is shown.
For example on GParted Live with Fluxbox the following is displayed:
Unnamed
-------
Documentation is not available
This build of gparted is configured without documentation.
Documentation is available at the project web site.
https://gparted.org
OK
To address the unspecified title on non-GNOME 3 graphic toolkits, add
a title that works with and without GNOME 3.
Closes!45 - Add missing window title to Help Contents dialog
All the dialogs which compose new or modified partition boundaries
create those partitions within the boundaries of the device. Therefore
trimming the partition boundaries to device boundaries never happens.
So replace this trimming with bug error reports.
Also add bug prefixes to the other error messages in
GParted_Core::valid_partition() too.
Closes#48 - Error when moving locked LUKS-encrypted partition
The previous commit moved the code from GParted_Core to
Dialog_Base_Partition class without making a single formatting change to
ensure the code was guaranteed to work the same within that larger
commit. Now reformat the code to current layout standards. Making it a
separate commit simplifies the effort for both changes and improves
bisectability.
Additionally to be sure there were no code changes,
Dialog_Base_Partition.cc was compiled to assembler code with and without
this change applied and the resultant assembler code compared. There
were no differences in the generated assembler code.
Start with the make generated g++ command for compiling
Dialog_Base_Partition.o; (1) remove the '-g' flag as inserted debugging
directives do differ; and (2) replace '-c -o Dialog_Base_Partition.o'
with '-S -o Dialog_Base_Partition.s' to produce assembler output instead
of object code.
Closes#48 - Error when moving locked LUKS-encrypted partition
Move snap_to_*() method calls from the point when all operations are
added to the list, to earlier when Resize/Move, Paste (into new) and
Create New dialogs are composing the new partition objects. In
particular for the Resize/Move operation, to just before updating the
file system usage.
This change finally resolves this bug.
Because of the dialog call chains into Dialog_Base_Partition,
snap_to_alignment() must be added into:
* Dialog_Base_Partition::prepare_new_partition() for the Resize/Move and
Paste (into new) dialogs; and
* Dialog_Partition_New::Get_New_Partition() for the Create New dialog.
Closes#48 - Error when moving locked LUKS-encrypted partition
The current device has to be passed to the dialog constructors and then
on to the Dialog_Base_Constructor. Note that the Dialog_Partition_New
constructor is already passed the current device, however it still needs
to pass it on to Dialog_Base_Constructor.
This is ready so that snap_to_*() methods can access the current device
when they are called from within these dialogs.
* Pass Parameter to Base Class Constructor while creating Derived class
Object
https://stackoverflow.com/questions/16585856/pass-parameter-to-base-class-constructor-while-creating-derived-class-objectCloses#48 - Error when moving locked LUKS-encrypted partition
Move setting of the new_partition object file system usage to after
everything else, specifically after free_space_before and strict_start.
This is because snap_to_*() use free_space_before and strict_start and
snap_to_alignment() is going to be called before the file system usage
is updated to avoid the error in this bug report.
Closes#48 - Error when moving locked LUKS-encrypted partition
None of the snap_to_*() methods report any errors so remove the unused
error parameter and return value.
Closes#48 - Error when moving locked LUKS-encrypted partition
PATCHSET OVERVIEW
A user had 2 adjacent partitions which were aligned to multiples of
33553920 bytes (32 MiB - 512 bytes), not MiB or cylinders. As far as
GParted is concerned this is not aligned. The second partition
contained closed LUKS encrypted data. Recreate this setup with:
# truncate -s 200G /tmp/disk.img
# losetup -f --show /tmp/disk.img
/dev/loop0
# sfdisk -u S /dev/loop0 << EOF
65535 2162655 83
2228190 78904140 83
EOF
# partprobe /dev/loop0
# echo -n badpassword | cryptsetup luksFormat /dev/loop0p2 -
When trying to move the second LUKS encrypted partition to the right by
any amount, with the default MiB alignment, GParted displays this error
dialog and fails to even queue the operation:
Could not add this operation to the list
A partition with used sectors (78907392) greater than its
length (78905344) is not valid
[ OK ]
Overview of the steps involved:
1. The Resize/Move dialog composed a new partition to start a whole
multiple of MiB after the end of the previous non-aligned partition.
The new partition also had it's size increased to a whole multiple of
MiB, to 78907392 sectors (38529 MiB) which was 1.59 MiB larger than
before. Neither the start or end of the new partition are aligned at
this point.
2. Win_GParted::activate_resize() applied the change back to the closed
LUKS partition object, additionally making the used sectors equal to
the partition size.
(To match the fact that when opened the LUKS mapping it will
automatically fill the new larger partition size).
3. GParted_Core::snap_to_mebibyte() then aligned the partition start and
end to whole MiB boundaries, reducing the partition size in the
process to 78905344 (38528 MiB).
4. GParted_Core::snap_to_alignment() reported the error saying that it
couldn't add the operation to the list because it was invalid to have
the file system used sectors larger than the partition size.
Fix this by having the snap to alignment adjustments applied before the
dialogs update any associated file system usage. Specifically the
Resize/Move, Paste (into new) and Create New dialogs as these are the
only ones which either create or modify partition boundaries.
Validation done by snap_to_alignment() will continue to occur at the
current point when the operation is added to the list.
THIS COMMIT
snap_to_alignment() is doing two different jobs, it is (1) optionally
adjusting the new partition boundaries for MiB or Cylinder alignment;
and (2) checking that the partition boundaries and file system usage are
valid.
Split those into two different functions (1) snap_to_alignment() and
(2) valid_partition(). For now valid_partition() still calls
snap_to_alignment() so there is no functional change with this commit.
Closes#48 - Error when moving locked LUKS-encrypted partition
We have to copy the dentry->d_name before calling closedir(). If not,
the string points to nothing and the test fails (It does not fail all
the time, but only by chance).
Confirmed using valgrind. Selected output from running the unit test
under valgrind:
$ valgrind --track-origins=yes ./test_blockSpecial
==25110== Memcheck, a memory error detector
...
==25110== Command: ./test_BlockSpecial
==25110==
Running main() from src/gtest_main.cc
[==========] Running 26 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 26 tests from BlockSpecialTest
...
[ RUN ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
==25110== Invalid read of size 1
==25110== at 0x4C2C9B2: strlen (vg_replace_strmem.c:458)
==25110== by 0x40E7C4: length (char_traits.h:259)
==25110== by 0x40E7C4: append (basic_string.h:1009)
==25110== by 0x40E7C4: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
==25110== by 0x40E7C4: get_link_name (test_BlockSpecial.cc:156)
==25110== by 0x40E7C4: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
=25110== Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
==25110== at 0x4C2ACBD: free (vg_replace_malloc.c:530)
==25110== by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
==25110== by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Block was alloc'd at
==25110== at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==25110== by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
==25110== by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Invalid read of size 1
==25110== at 0x4C2E220: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1022)
==25110== by 0x953A997: std::string::append(char const*, unsigned long) (in /usr/lib64/libstdc++.so.6.0.19)
==25110== by 0x40E7D2: append (basic_string.h:1009)
==25110== by 0x40E7D2: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
==25110== by 0x40E7D2: get_link_name (test_BlockSpecial.cc:156)
==25110== by 0x40E7D2: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
==25110== at 0x4C2ACBD: free (vg_replace_malloc.c:530)
==25110== by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
==25110== by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
==25110== Block was alloc'd at
==25110== at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==25110== by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
==25110== by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
==25110== by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
...
[ OK ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (50 ms)
Selected lines from test_BlockSpecial.cc:
132 static std::string get_link_name()
133 {
134 DIR * dir = opendir( "/dev/disk/by-id" );
...
141 bool found = false;
142 struct dirent * dentry;
143 // Silence GCC [-Wparentheses] warning with double parentheses
144 while ( ( dentry = readdir( dir ) ) )
145 {
146 if ( strcmp( dentry->d_name, "." ) != 0 &&
147 strcmp( dentry->d_name, ". " ) != 0 )
148 {
149 found = true;
150 break;
151 }
152 }
153 closedir( dir );
154
155 if ( found )
156 return std::string( "/dev/disk/by-id/" ) + dentry->d_name;
So the memory referred to by dentry was allocated on line 134, freed on
153 and accessed after freed on 156.
Closes!41 - Fix test (dentry->d_name is invalidated by closedir...)
After an NTFS file system has been resized the command GParted currently
uses to read the file system usage fails like this:
# ntfsinfo --mft /dev/sdb1
Volume is scheduled for check.
Please boot into Windows TWICE, or use the 'force' option.
NOTE: If you had not scheduled check and last time accessed this volume
using ntfsmount and shutdown system properly, then init scripts in your
distribution are broken. Please report to your distribution developers
(NOT to us!) that init scripts kill ntfsmount or mount.ntfs-fuse during
shutdown instead of proper umount.
Failed to open '/dev/sdb1'.
Fix by added the '--force' flag as described in the error message.
Closes#57 - NTFS Resize results in Partition Information Warning on
Refresh
GParted uses ntfsinfo to read the NTFS file system usage. However when
ntfsinfo fails with a non-zero exit status GParted reports stdout twice,
rather than stdout and stderr. Correct this.
Closes#57 - NTFS Resize results in Partition Information Warning on
Refresh
With the same case as from the previous commit, the very long "Mounted
on ..." text is now wrapped, but the text may not be left justified.
Slowly adjust the dialog width and see how the text wrapping is updated
to fit the size adjustment but the text is centred rather than left
justified.
This is because setting the halign property to Gtk::ALIGN_START does not
guarantee left alignment of text for wrapped or ellipsized Gtk::Labels.
Use the xalign property instead.
To set the xalign property there is a method in the GtkMisc (Gtk::Misc)
base class:
gtk_misc_set_alignment (Gtk::Misc::set_alignment)
However, GtkMisc (Gtk::Misc) was deprecated in Gtk 3.14 (Gtkmm 3.14)
and in Gtk 3.16 (gtkmm 3.16) set_alignment() was replaced with the
introduction of two new methods:
gtk_label_set_xalign (Gtk::Label::set_xalign)
gtk_label_set_yalign (Gtk::Label::set_yalign)
Add a check for Gtkmm method Gtk::Label::set_xalign() in configure.ac
and use it when available.
References:
[1] Gtk3 Reference Documentation - gtk_misc_set_alignment()
https://developer.gnome.org/gtk3/stable/GtkMisc.html#gtk-misc-set-alignment
"gtk_misc_set_alignment has been deprecated since version 3.14 and
should not be used in newly-written code. Use GtkWidget's alignment
("halign" and "valign") and margin properties or GtkLabel's
"xalign" and "yalign" properties."
[2] Gtkmm 3.16 Gtk::Misc Class Reference, set_alignment() method
https://developer.gnome.org/gtkmm/3.16/classGtk_1_1Misc.html#a52b2675874cf46a3097938756b9fe9e8
[3] GNOME BugZilla - EmptyBoxes: instructions_label's alignment is off
https://bugzilla.gnome.org/show_bug.cgi?id=735841
[4] Gtk commit from 2014-09-16:
GtkLabel: add x/yalign properties
https://gitlab.gnome.org/GNOME/gtk/commit/d39424fc
[5] Gtk3 Reference Documentation - gtk_label_set_xalign()
https://developer.gnome.org/gtk3/stable/GtkLabel.html#gtk-label-set-xalign
[6] Gtkmm 3.16 Gtk::Label Class Reference, set_xalign() method
https://developer.gnome.org/gtkmm/3.16/classGtk_1_1Label.html#acee7d4e87d7cc14080a7b8ded5f84e5eCloses!40 - Limit wrapping labels
Opening the Partition Information dialog for a file system mounted on a
very long mount point, or on openSUSE which mounts the OS from 10 btrfs
subvolumes from the same partition, will cause the dialog to be very
wide as the "Mounted on ..." text is not wrapped.
Back in Gtk2, when width_chars / max_width_chars were not set, wrapping
labels had a default width beyond which text wrapped onto a new line
[1].
For Gtk3 this default width was first reworked a bit [2], and then was
removed for the very early Gtk3 3.0.10 release [3].
It is recommended that applications explicitly set default values,
otherwise wrapping labels never wrap when requesting their natural
allocation.
References:
[1] Gtk 2.24.32 source code - gtk/gtklabel.c:2975
https://gitlab.gnome.org/GNOME/gtk/blob/2.24.32/gtk/gtklabel.c#L2975
"This long string gives a good enough length for any line to
have."
[2] Gtk commit from 2010-04-21:
680d7762ba
Make sure not to base the minimum size on "max-width-chars", only
the natural size.
"This string is just about long enough."
[3] Gtk commit from 2011-04-17:
c8ce1106c1
label: Don't try to guess a label's size
People should use window default sizes or label
width-chars/max-width-chars to find the ideal layout for a label
instead of relying on magic.
Closes!40 - Limit wrapping labels
Before Gtk 3.22 GtkScrolledWindow propagated natural size to its
Children and so on to descendants. In Gtk 3.22 this was changed to
always request the minimum size. This was done because it is believed
to be a safer default (gives a better behaviour) in case of dynamic
content inside the scrolled window, that is, content that may change
allocated size. [1][2][3]
When the scrolled window content is not dynamic the natural size is
preferable because it gives a better looking layout and without any
downside.
In the case of GParted content which is not dynamic, so request the
scrolled windows to allocate children at natural sizes for Gtk >= 3.22.
The benefits of natural size allocation are evident in presence of
wrapping labels (for example inside the "Partition Info" dialog), that
with the minimum size request likely end up taking a very small width.
References:
[1] Gtk commit from 2016-08-31:
GtkScrolledWindow: Make propagation of natural child sizes optional
0984d1622d
"Making propagation of child natural sizes mandatory (or default,
even) was evidently a mistake as this causes dynamic content in a
scrolled window to resize it's parent when the scrolled window is
competing for space with an adjacent widget."
[2] Gtk 3.22 Reference Documentation -
gtk_scrolled_window_set_propagate_natural_width
https://developer.gnome.org/gtk3/3.22/GtkScrolledWindow.html#gtk-scrolled-window-set-propagate-natural-width
[3] Gtkmm 3.24 Gtk::ScrolledWindow Class Reference,
set_propagate_natural_width() method
https://developer.gnome.org/gtkmm/3.24/classGtk_1_1ScrolledWindow.html#a2d4cb945688ecb8739efd70b18742779
[4] Gtkmm 3.21.6 NEWS
https://gitlab.gnome.org/GNOME/gtkmm/blob/3.21.6/NEWS
"ScrolledWindow: Added get/set_propagate_natural_height/width() and
the properties."
Closes!39 - Always request natural size inside Gtk::ScrolledWindow
Mathias L. Baumann