Since the only use of SWRaid_Info::get_uuid() assign the returned value
this doesn't actually save any copy construction. Do it for consistency
with the other get_*() methods in SWRaid_Info.
Closes!94 - Make more getter methods use return-by-constant-reference
Have to use a second constant reference variable array_path_2 in
GParted_Core::set_mountpoints() because by design C++ does not implement
rebinding of references [1].
[1] why doesn't C++ allow rebinding a reference?
https://stackoverflow.com/questions/27037744/why-doesnt-c-allow-rebinding-a-referenceCloses!94 - Make more getter methods use return-by-constant-reference
All uses of get_description() copy construct to a local variable, not
assign to a reference, so this doesn't save anything. It is just being
done to be consistent with making other getters return a constant
reference.
Closes!94 - Make more getter methods use return-by-constant-reference
Method already returned a constant reference. Change local variables to
constant references to avoid copy constructing them.
Closes!94 - Make more getter methods use return-by-constant-reference
A number of GParted methods named get_*() are returning properties and
are return-by-value. For objects this implies the returned value is
copy constructed and later destroyed when it goes out of scope. Change
them to use return-by-constant-reference to avoid unnecessary object
duplication. Just have to make sure the reference goes out of scope
before the referenced object is destroyed to avoid having a reference
(pointer) to freed memory. Assigning to a local variable instead of
of a local reference still duplicates the object so can be used when the
lifetime of the gotten object needs to be longer that the referenced
object.
Previously done for other getters here:
d948cbcb91
Make get_custom_text() and get_generic_text() return by reference
This change just makes Device::get_path() return a constant reference
and updates uses to match to avoid copy constructing the returned value.
Closes!94 - Make more getter methods use return-by-constant-reference
The first two clauses say is the partition object type unallocated
inside an extended partition, or a logical partition (which only occur
inside an extended partition). As these are the only two partition
object types which can occur inside an extended partition, this is
equivalent to saying is the partition object inside an extended
partition. Simplify.
The create new and paste into new dialogs are both composing a new
partition into the unallocated selected_partition they are passed. The
starting sector of this unallocated partition is the first sector the
newly composed partition could possibly have. To ensure it doesn't
overlay with the partition table or EBR (Extended Boot Records) it calls
MB_Needed_for_Boot_Record() to indicate if the first 1 MiB needs to be
reserved in the dialog or not.
Code:
Dialog_Partition_New::set_data(..., selected_partition, ...)
...
MIN_SPACE_BEFORE_MB = Dialog_Base_Partition::MB_Needed_for_Boot_Record(selected_partition);
Dialog_Partition_Copy::set_data(selected_partition, ...)
...
MIN_SPACE_BEFORE_MB = Dialog_Base_Partition::MB_Needed_for_Boot_Record(selected_partition);
However the resize/move dialog is different. It is passed the existing
selected_partition object and the vector of partitions from which it
determines if there is a previous unallocated partition object or not.
When there is no previous unallocated partition object, there is no need
to reserve 1 MiB because the selected_partition can't be moved further
to the left. In the other case when there is a previous unallocated
partition object the start of the existing selected_partition can be
moved to the left. However the code passes the selected_partition
object to MB_Needed_for_Boot_Record() so doesn't have the first sector
the newly composed partition could possibly have so doesn't reserve the
first 1 MiB to prevent it overlapping with the partition table at the
start of the drive. These two commits addressed this limitation:
* Ensure 1 MiB reserved when moving extended partition to start of drive
* Ensure 1 MiB reserved when moving partition to start of disk
Code:
Dialog_Partition_Resize_Move::set_data(selected_partition, ...)
new_partition = selected_partition.clone();
if (selected_partition.type == TYPE_EXTENDED)
Resize_Move_Extended(...);
else
Resoze_Move_Normal(...);
Dialog_Partition_Resize_Move::Resize_Move_Normal(...)
...
if (previous <= 0)
MIN_SPACE_BEFORE_MB = 0;
else
{
if (START < MEBIBYTE / new_partition->sector_size)
MIN_SPACE_BEFORE_MB = 1;
else
MIN_SPACE_BEFORE_MB = Dialog_Base_Partition::MB_Needed_for_Boot_Record(*new_partition);
}
Dialog_Partition_Resize_Move::Resize_Move_Extended(...)
...
if (previous <= 0)
MIN_SPACE_BEFORE_MB = 0;
else
{
if (START < MEBIBYTE / new_partition->sector_size)
MIN_SPACE_BEFORE_MB = 1;
else
MIN_SPACE_BEFORE_MB = Dialog_Base_Partition::MB_Needed_for_Boot_Record(*new_partition);
}
So instead pass the previous unallocated partition object as that
contains the left most start sector the newly composed partition could
possibly have, therefore the check for overlapping the partition table
in the first 1 MiB of the drive can succeed.
On an MSDOS partitioned drive, create an extended partition at least 2
MiBs from the start of the drive. Like this:
dd if=/dev/zero bs=1M of=dev/sdb
echo 4096,4096,5 | sfdisk -uS --force /dev/sdb
The resize/move dialog appears to allow the extended partition to be
moved and/or resized to the very start of the drive, over the top of the
partition table, without reserving the first 1 MiB. Ensure the dialog
reserves the first 1 MiB, like it does when moving normal partitions.
Reference:
ceab9bde57
Ensure 1 MiB reserved when moving partition to start of disk
This case is an extension of the setup in the previous commit. Add a
second partition several megabytes after the first. It looks like this:
[TABLE][PTN#1] [PTN#2]
<-- 1st MiB -><- several MiBs ->
Just need to make the gap 2 MiB or more so that it can be seen what the
resize/move dialog is allowing. Setup like this using an 8 MiB gap and
8 MiB partition #2.
For MSDOS use:
dd if=/dev/zero bs=1M of=/dev/sdb
(echo 1,2047; echo 18432,16384) | sfdisk -uS --force /dev/sdb
mkswap /dev/sdb2
For GPT use:
sgdisk --zap-all /dev/sdb
sgdisk --set-alignment=1 --new 1:34:2047 /dev/sdb
sgdisk --new 2:18432:34815 /dev/sdb
mkswap /dev/sdb2
In GParted try to move partition sdb2 to the left as much as possible,
or try to resize the start to the left as much as possible. GParted
insists on having a 1 MiB of padding before the start of sdb2, forcing
it to start at sector 4096, even though sector 2048 is free and aligns
to whole megabytes.
Delete the preceding partition. Now GParted allows sdb2 to be moved or
resize to start at sector 2048.
Fix another off by one error in the sector comparison for the
resizing/moving of partitions.
Closes#172 - GParted doesn't allow creation of a partition starting at
sector 2048 if there is a partition before it
[This commit message and test case is written assuming a drive with a
(logical) sector size of 512 bytes. GParted equally well works with
other sector sizes because the limit is expressed as 1 MiB / sector
size. Adjust the test case sector counts as needed when testing with
different sector sized drives.]
Prepare an MSDOS or GPT partitioned disk with the first partition within
the first 1 MiB.
For MSDOS use:
dd if=/dev/zero bs=1M of=/dev/sdb
echo 1,2047 | sfdisk -uS --force /dev/sdb
For GPT use:
sgdisk --zap-all /dev/sdb
sgdisk --set-alignment=1 --new 1:34:2047 /dev/sdb
In GParted create a new partition on /dev/sdb as near to the start of
the drive as possible. GParted insists on added an extra 1 MiB of space
before the new partition, making it start at sector 4096, even though
sector 2048 is free and aligns to whole megabytes.
Delete the preceding partition. Now GParted allows the new partition
to be created starting at sector 2048.
GParted only needs to add padding of 1 MiB to account for the partition
table at the start of the drive when the possible start is within the
first MiB, not already at the first MiB. Fix this off by one error in
the sector comparison.
The reason this has bug has never occurred before is because it is very
unusual to have the first partition entirely within the first 1 MiB.
Normally either the (start of) the drive is free so GParted creates an
unallocated partition object starting at sector 0, so adding 1 MiB of
padding to preserve the partition table is correct; or the first
partition starts at 1 MiB so the possible start for a second partition
is much later in the drive.
Closes#172 - GParted doesn't allow creation of a partition starting at
sector 2048 if there is a partition before it
Accessibility relations are essential for usage with screen readers. It
enables the screen reader to read the corresponding label along with the
value of a widget when it gains focus, rather than just the value of the
widget itself.
Test by running Orca screen reader and tab around the elements of the UI
and listen to what is read out. For example before it would be
"500 GiB", where as after it would be "Unused 500 GiB".
Closes!92 - Add accessibility relations
A user reported that scrolling the mouse wheel quickly crashes GParted
[1]. The minimum setup required to reproduce this crash is 3 drives
partitioned like this:
sda - any
sdb - some unallocated space
sdc - no unallocated space
Then move the pointer over the drive selection combobox and scroll the
mouse wheel quickly downwards.
The sequence of events goes like this:
1. The first scroll wheel down event causes the device combobox
selection to change to the second entry (sdb), which calls
combo_devices_changed() -> Refresh_Visual().
2. Refresh_Visual() sets selected_partition_ptr to point to the largest
unallocated space partition object in sdb.
3. The first Gtk event processing loop in Refresh_Visual() comes
across the next scroll wheel down event. This changes the selection
to the third entry (sdc), which makes a recursive call to
combo_devices_changed() -> Refresh_Visual().
4. Refresh_Visual() sets selected_partition_ptr to NULL as sdc has no
unallocated space and returns.
5. The first call to Refresh_Visual() resumes after the first Gtk event
loop, continuing the processing for drive sdb. As sdb has a
largest unallocated space partition (largest_unalloc_size >= 0),
DrawingAreaVisualDisk::set_selected() is called with the now
NULL selected_partition_ptr.
6. One of the DrawingAreaVisualDisk::set_selected() methods
dereferences the NULL pointer, crashing GParted.
As a comment says of selected_partition_ptr "Lifetime: Valid until the
next call to Refresh_Visual()." It just wasn't expected that the next
call to Refresh_Visual() would be half way through Refresh_Visual()
itself.
Considered but rejected fixes:
1. Remove automatic selection of the largest unallocated space.
Removes functionality.
2. Return at the top of Refresh_Visual() when called recursively.
This causes GParted to not update the main window with the latest
selected drive. In the above example the combobox would show sdc,
but the main window graphic and partition list would have only been
updated once showing sdb, the first scrolled selection.
Fix by only having a single Gtk event processing loop at the end of
Refresh_Visual() with the optional calls to select the largest
unallocated partition before that loop. This makes it impossible to
call the ::set_selected() methods with selected_partition_ptr modified
by a recursive call.
This fix reverts this earlier commit:
0fb8cce699
Reduce flashing redraw from automatic partition selection (#696149)
That commit was in GParted 0.20.0 when Gtk 2 was still used. Re-testing
now doesn't see any flashing redrawing from the automatic partition
selection, with GParted currently using Gtk 3.
[1] Debian bug #991998 - gparted segfaults if scrolling quickly the
device dropdown list
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991998Closes#165 - GParted segfaults if scrolling quickly in the device
dropdown
Compiling GParted on an older distribution with gtk+-3.0 < 3.22.0, where
HAVE_GTK_SHOW_URI_ON_WINDOW is undefined, produces this warning message:
Win_GParted.cc: In member function 'void GParted::Win_GParted::show_help(const Glib::ustring&, const Glib::ustring&)':
Win_GParted.cc:1822:56: warning: operation on 'gscreen' may be undefined [-Wsequence-point]
GdkScreen *gscreen = gscreen = gdk_screen_get_default();
^
Found on Ubuntu 16.04 LTS with gtk+ 3.18.0.
Stop double assigning to gscreen. Fixes commit:
26f4dc504a
Replace deprecated gtk_show_uri() method for help window (!82)
Creating a grep process to check if a particular mount is still mounted
is an unnecessary overhead. All that is needed is for the Mount_Info
module to refresh it's copy of /proc/mounts and query that.
To keep the code as simple as possible just reload the whole of the
Mount_Info module and query the mount cache to determine if the
particular block device is still mounted at this particular mount point.
This therefore re-reads /proc/mounts (necessary) and /proc/swaps and
/etc/fstab (unnecessary). This is still much less overhead than
creating a separate grep process.
Closes!89 - Fix unmount error when unmounting below a bind mount point
Bind mounts duplicate part of the file system hierarchy to an additional
mount point [1]. When mounting and unmounting a second file system
below a duplicating bind mount Linux automatically presents this lower
file system as being mounted multiple times. GParted displays these
multiple mount points. However using GParted to unmount this lower file
system reports an error that the second mount point is no longer
mounted, because all were unmounted by the first unmount command.
Setup:
1. Mount an upper file system
# mkdir /mnt/1
# mount /dev/sdb1 /mnt/1
# fgrep sdb /proc/mounts
/dev/sdb1 /mnt/1 ext4 rw,seclabel,relatime,data=ordered 0 0
2. Bind mount it to a second directory
# mkdir /mnt/b1
# mount --bind /mnt/1 /mnt/b1
# fgrep sdb /proc/mounts
/dev/sdb1 /mnt/1 ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sdb1 /mnt/b1 ext4 rw,seclabel,relatime,data=ordered 0 0
3. Mount a file system below the first
# mkdir /mnt/1/lower
# mount /dev/sdb2 /mnt/1/lower
# fgrep sdb /proc/mounts
/dev/sdb1 /mnt/1 ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sdb1 /mnt/b1 ext4 rw,seclabel,relatime,data=ordered 0 0
/dev/sdb2 /mnt/1/lower xfs rw,seclabel,relatime,attr2,inode64,noquota 0 0
/dev/sdb2 /mnt/b1/lower xfs rw,seclabel,relatime,attr2,inode64,noquota 0 0
Two mount records for sdb2 were added to /proc/mounts from one mount
command.
Then use GParted to unmount sdb2. It reports this error dialog:
+-------------------------------------+
| |
| Could not unmount /dev/sdb2 |
| |
| # umount -v '/mnt/b1/lower' |
| umount: /mnt/b1/lower: not mounted. |
+-------------------------------------+
| [ OK ] |
+-------------------------------------+
Fix by checking that the file system is still mounted before each
unmount attempt.
[1] mount (8), Description, Bind mount operation
https://man7.org/linux/man-pages/man8/mount.8.html#DESCRIPTIONCloses!89 - Fix unmount error when unmounting below a bind mount point
Extra hint of warning status being represented by enumeration constant
STATUS_N_A is no longer needed since commit:
8c5c13d613
Rename OperationDetailStatus STATUS_N_A to STATUS_WARNING
Implemented the second half of the solution described in the previous
commit. Resolve UUID= and LABEL= references when searching in the
Mount_Info cache so that mount points of encrypted file systems listed
in /etc/fstab can be found using the later added FS_Info details.
Closes#162 - It is no longer possible to mount a LUKS encrypted file
system
ISSUE DETAILS
GParted no longer enables Partition > Mount on, for unmounted encrypted
file systems listed in /etc/fstab.
Steps to reproduce:
1. Create LUKS mapping and open.
# cryptsetup luksFormat /dev/sdb1 -
# cryptsetup luksOpen /dev/sdb1 sdb1_crypt
2. Create any file system.
# mkfs.ext4 /dev/mapper/sdb1_crypt
# uuid=`blkid -o value -s UUID /dev/mapper/sdb1_crypt`
3. Add /etc/fstab entry.
# mkdir /mnt/1
# echo "UUID=$uuid /mnt/1 ext4 defaults 0 0" >> /etc/fstab
4. Run GParted and try Partition > Mount on.
With GParted >= 1.3 no mount point is available. With GParted <= 1.2
mount point /mnt/1 is available.
EXPLANATION
Up until GParted 1.2.0 it worked like this:
1. Ran blkid and loaded the details for every file system into the
FS_Info cache. This included results for file systems in open LUKS
mappings, such as /dev/mapper/sdb1_crypt in the above example.
2. Read /etc/fstab, resolved UUID= and LABEL= references into block
device names and added those into the Mount_Info cache.
3. Looped through all partitions adding mount points known by the
Mount_Info cache.
After the changes for issue #131 "GParted hangs when non-named device is
hung" and issue #148 "Encrypted file systems are no longer recognised"
it works like this instead:
1. Runs blkid for specified devices and partitions only and loads file
system details into the FS_Info cache. Does not include open LUKS
mappings so no results for those file systems.
2. Loading of /etc/fstab into the Mount_Info cache is unable to resolve
UUID= and LABEL= references for file systems in LUKS mappings, so
they aren't included.
3. No mount points known for encrypted file systems.
Note that currently when an encrypted file system is added into the data
model it extends the FS_Info cache <2>, but this is after the Mount_Info
cache has been loaded <1>. Call flow is like this:
GParted_Core::set_devices_thread()
FS_Info::clear_cache()
FS_Info::load_cache_for_paths()
1> Mount_Info::load_cache()
...
set_device_from_disk()
set_device_one_partition() / set_device_partitions()
set_luks_partition()
detect_filesystem_in_encryption_mapping()
2> FS_Info::load_cache_for_paths()
...
set_mountpoints()
partition.add_mountpoints(Mount_Info::get_fstab_mountpoints())
SOLUTION
Also save unresolved UUID= and LABEL= references from /etc/fstab into
the Mount_Info cache. Then when searching the Mount_Info /etc/fstab
cache resolve encountered UUID= and LABEL= references.
THIS COMMIT
Also save unresolved UUID= and LABEL= references into the Mount_Info
cache.
Closes#162 - It is no longer possible to mount a LUKS encrypted file
system
E2label works the same way whether an ext2/3/4 file system is mounted or
not, by directly reading and writing the superblock from the partition
block device. (Technically the superblock will already be in the kernel
device buffer cache because the kernel has the ext2/3/4 file system
mounted and a reference to the superblock in the device buffer cache).
This is safe and supported as confirmed here [1]. As this method has
always worked, even on the oldest distributions, unconditionally enable
this feature.
# mkfs.ext4 -L label_1 /dev/sdb3
# mount /dev/sdb3 /mnt/3
# e2label /dev/sdb3 label_2
# blkid /dev/sdb3
/dev/sdb3: LABEL="label_2" ...
[1] Is labelling a mounted ext2/3/4 file system safe and supported?
https://lore.kernel.org/linux-ext4/CAMU1PDgNvW_3Jr91iii-Nh=DCuRytVG8ka3-J+a43gKwigx8Yg@mail.gmail.com/T/#u
Bug 600496 - Allow changing ext2/3 label without unmounting
Closes#163 - Feature request: set label on a mounted btrfs
XFS also supports labelling of the file system while it is mounted.
This was added into Linux kernel 4.18 [1] and xfsprogs 4.17.0 [2].
These versions are newer than available in older but still supported
distributions so we'll need to detect versions before enabling support.
These are the oldest releases of distributions which meet the
requirements.
Distro EOL Linux kernel xfsprogs
Debian 10 2024-Jun 4.19.0 4.20.0
RHEL 8 2029-May 4.18.0 5.0.0
Ubuntu 20.04 LTS 2030-Apr 5.4 5.3.0
As with btrfs a mounted XFS is labelled via it's mount point:
# mkfs.xfs -L label_1 /dev/sdb2
# mount /dev/sdb2 /mnt/2
# xfs_io -c 'label -s mnt_label_2' /mnt/2
label = "mnt_label_2"
# blkid /dev/sdb2
/dev/sdb2: LABEL="mnt_label_2" ...
And cleared with:
# xfs_io -c 'label -c' /mnt/2
label = ""
Note that in some error situations xfs_io reports exit status zero and
writes the failure message to stdout.
# xfs_io -c 'label -s "mnt label 3"' /mnt/2
bad argument count 4 to label, expected between 0 and 3 arguments
# echo $?
0
Therefore determine success based on stdout starting with 'label = "'
reporting the new label or not.
Also note that as seen in this failure case, it is not possible to set
an XFS label which contains a space character using xfs_io. However
that is nothing new as that can't be done using the existing xfs_admin
command for an unmounted XFS either.
# umount /mnt/2
# xfs_admin -L 'umnt label 4' /dev/sdb2
Usage: xfs_admin [-efjlpuV] [-c 0|1] [-L label] [-U uuid] device
# echo $?
2
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7664b31975bd893190708e76b2c424328f0c49b
xfs: implement online get/set fs label
[2] https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/commit/?id=cfa10b0f972005b38ed294bca66cebf2f65298ec
xfs_io: add label command
Closes#163 - Feature request: set label on a mounted btrfs
Mike Fleetwood