Network Block Devices are not displayed in GParted as partitionable
devices. They do appear in /proc/partitions, are reported by
fdisk -l [1] and by ped_device_probe_all() from libparted. Therefore
include them.
Create NBD device for testing:
# truncate -s 1G /tmp/disk-1G.img
# nbd-server -C /dev/null 9000 /tmp/disk-1G.img
# nbd-client localhost 9000 /dev/nbd0
After creating a couple of partitions for testing, the contents of
/proc/partitions looks like this:
# egrep 'name|nbd' /proc/partitions
major minor #blocks name
43 0 1048576 nbd0
43 1 262144 nbd0p1
43 2 785408 nbd0p2
Listing all disks using fdisk:
# fdisk -l
...
Disk /dev/nbd0: 1 GiB, 1073741824 bytes, 2097152 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x081b1cd1
Device Boot Start End Sectors Size Id Type
/dev/nbd0p1 2048 526335 524288 256M 83 Linux
/dev/nbd0p2 526336 2097151 1570816 767M 83 Linux
Temporarily apply this patch to GParted so that it ignores the devices it
currently selects from /proc/partitions to use what get_device_probe_all()
reports. GParted shows NBDs.
$ git diff --unified=1
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index 1629f94f..abea7a0b 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -172,3 +172,3 @@ void GParted_Core::set_devices_thread( std::vector<Device> * pdevices )
//try to find all available devices if devices exist in /proc/partitions
- std::vector<Glib::ustring> temp_devices = Proc_Partitions_Info::get_device_paths();
+ std::vector<Glib::ustring> temp_devices;
if ( ! temp_devices .empty() )
Tidy-up NBD device:
# nbd-client -d /dev/ndb0
# killall nbd-server
# rm /tmp/disk-1G.img
[1] man fdisk
"-l, --list
List the partition tables for the specified devices and then
exit. If no devices are given, the devices mentioned in
/proc/partitions (if this file exists) are used.
"
Closes#247 - GParted does not list NBD (Network Block Device) devices
in the GUI
... code pattern. This is to make the code easier to understand by not
having to remember if condition context for indented code over longer
distances. This has been done before. Here are just 2 examples:
[1] 75bda733bb
Refactor run_blkid_load_cache() into if fail return early (#131)
[2] 407e0ac6e3
Refactor fat16::read_label() into if fail return early pattern (!104)
Closes!119 - Tidy-ups for file system interface classes
Restructure the variable parsing code into "if leading text found then
scan the number" pattern.
Closes!119 - Tidy-ups for file system interface classes
Restructure the variable parsing code into "if leading text found then
scan the number" pattern.
Anchor leading text matches to the start of a new line in the output.
Closes!119 - Tidy-ups for file system interface classes
Restructure the variable parsing code into "if leading text found then
scan the number" pattern.
Closes!119 - Tidy-ups for file system interface classes
And restructure the variable parsing code into "if leading text found
then scan the number" pattern.
Anchor leading text matches to the start of a new line in the output.
Closes!119 - Tidy-ups for file system interface classes
FileSystem member variables T, N & S are being used like local variables
in many of the file system specific set_used_sectors() methods. They
are only used within each set_used_sectors() call and not used to
represent persistent information of a FileSystem interface class or to
pass information between separate methods. Therefore stop using them
and replace them with local variables instead.
This block of code finds a field in the output and scans the number:
Glib::ustring::size_type index = output.find("Block count:");
if (index >= output.length() ||
sscanf(output.substr(index).c_str(), "Block count: %lld", &T) != 1)
T = -1;
The if statement says "if leading text is not found or scanning the
number fails then assign -1". A sequence of two negatives leading to
assigning an error value is hard to understand. Instead this an
equivalent block from btrfs::set_used_sectors():
long long total_bytes = -1;
Glib::ustring::size_type index = output.find("\ntotal_bytes");
if (index < output.length())
sscanf(output.substr(index).c_str(), "\ntotal_bytes %lld", &total_bytes);
This assigns a default error value and the if statement says "if leading
text found then scan the number". Much simpler to understand.
Therefore change the code around to use this same pattern.
Anchor the leading text matches to the start of a new line in the
output where possible. Just because it's what some of the other file
system's set_used_sectors() methods do (btrfs, reiser4 and xfs) and it
seems like more robust text matching.
Closes!119 - Tidy-ups for file system interface classes
Replace floating point calculation to convert size and space figures
from file system block sized units to sectors with an integer
calculation. Do this for the same reasons discussed in commit "Stop
using floating point calculations in FS resize() methods" earlier in
this patchset. This will limit the largest file system that GParted can
read the usage of to 8 EiB - 1 bytes.
There is still a floating point calculation in btrfs::set_used_sectors()
which is being left because that is apportioning used space figure
between multiple devices.
Closes!119 - Tidy-ups for file system interface classes
A number of the file system specific resize() methods use floating point
calculations to convert from the new partition size in sectors to the
new file system size to be passed to the resize command in bytes or
kibibytes. This is bad because there could be rounding errors
converting from integer to floating point, performing the calculation
and converting back. Replace with integer only multiply and divide
calculations. Integer division always truncates [1] which is exactly
what is needed. The largest integer will be the size of the file system
in bytes held in a signed 64-bit long long, or Sector or Byte_Value
typedef of the same type. This will limit the size that a file system
can be shrunk to, to 8 EiB - 1 byte.
[1] C++ Arithmetic operators
https://en.cppreference.com/w/cpp/language/operator_arithmetic
"the algebraic quotient of integer division is truncated towards
zero (fractional part is discarded)"
Closes!119 - Tidy-ups for file system interface classes
The GUI displays the file system of an open encrypted file system as
"[Encrypted] FSTYPE" [1]. However saved details just writes the file
system type as "luks". Update saved details writing code to use the
same method the GUI currently uses [2].
[1] commit cb3cc505ce
Display "[Encrypted] FSTYPE" in the File System column (#760080)
[2] commit bd6fc67afb
Provide virtual Partition::get_filesystem_string() method (#774818)
gpart scans a drive trying to guess the location of partitions when an
MBR partition table is lost [1]. However the tool is unmaintained,
takes hours or days of 100% CPU time to scan a drive and provides no
progress indication [2][3][4]. We keep recommending killing the gpart
process and using TestDisk [5] instead.
Therefore remove Device > Attempt Data Rescue and the use of gpart from
GParted.
[1] Gpart
https://github.com/baruch/gpart
[2] Have you had a good or bad experience with Dev->Attempt Data Rescue?
http://gparted-forum.surf4.info/viewtopic.php?id=17992
No good, only bad experiences using gpart were reported.
[3] Gparted does not say anything
http://gparted-forum.surf4.info/viewtopic.php?id=17749
Forum user reported waiting 48 hours with no progress indication.
We recommended using TestDisk.
[4] How cancel Data Rescue process?
http://gparted-forum.surf4.info/viewtopic.php?id=18143
Forum user reported it will take 3 days to scan their external 480GB
drive. We recommended using TestDisk instead.
[5] TestDisk, Data Recovery
https://www.cgsecurity.org/wiki/TestDiskCloses!118 - Remove Attempt Data Rescue and use of gpart
In C++11, nullptr [1] is the strongly typed value to use instead of the
macro NULL [2]. Use everywhere [3][4].
[1] nullptr, the pointer literal (since C++11)
https://en.cppreference.com/w/cpp/language/nullptr
[2] NULL
https://en.cppreference.com/w/cpp/types/NULL
[3] Bjarne Stroustrup's C++ Style and Technique FAQ, Should I use NULL
or 0?
https://www.stroustrup.com/bs_faq2.html#null
"In C++, the definition of NULL is 0, so there is only an
aesthetic difference. I prefer to avoid macros, so I use 0.
Another problem with NULL is that people sometimes mistakenly
believe that it is different from 0 and/or not an integer. In
pre-standard code, NULL was/is sometimes defined to something
unsuitable and therefore had/has to be avoided. That's less
common these days.
If you have to name the null pointer, call it nullptr; that's
what it's called in C++11. Then, "nullptr" will be a keyword.
"
[4] What is nullptr in C++? Advantages, Use Cases & Examples
https://favtutor.com/blogs/nullptr-cpp
"Advantages of nullptr
...
Compatible: Null pointers are compatible with null pointer
constants in the C style (such as NULL and 0). This implies
that old C code that uses these constants and null pointers can
communicate with each other in C++.
"
Closes!117 - Require C++11 compilation
As discussed in the previous commit the oldest supported distributions
now provide gtkmm versions higher that 3.18.0 (which requires C++11
compilation). Therefore increase the minimum required version to gtkmm
3.18.0. This allows removal of HAVE_LABEL_SET_XALIGN autoconf
definition and associated fallback code.
Closes!117 - Require C++11 compilation
GParted crashes when blkid doesn't provide the UUID of the exfat
partition and its serial has preceding zeroes (and it isn't mounted).
blkid doesn't report UUID if the serial number is 0000-0000 (a.k.a.
0x0).
Steps to reproduce:
# truncate -s 4M /tmp/disk.img
# losetup -f --show /tmp/disk.img
/dev/loop0
# mkfs.exfat /dev/loop0
[...]
exFAT format complete!
# partprobe /dev/loop0
# blkid /dev/loop0
/dev/loop0: UUID="F7BF-ABFF" BLOCK_SIZE="512" TYPE="exfat" PTTYPE="dos"
# exfatlabel /dev/loop0 -i 0x0
exfatprogs version : 1.1.3
New volume serial : 0x0
# blkid /dev/loop0
/dev/loop0: BLOCK_SIZE="512" TYPE="exfat" PTTYPE="dos"
# gparted /dev/loop0
GParted 1.3.1
configuration --enable-libparted-dmraid --enable-online-resize
libparted 3.4
** (gpartedbin:94926): ERROR **: 10:45:01.894:
unhandled exception (type std::exception) in signal handler:
what: basic_string::assign: __pos (which is 18446744073709551615) > this->size() (which is 3)
Trace/breakpoint trap (core dumped)
# losetup -d /dev/loop0; rm /tmp/disk.img
As blkid doesn't report exfat UUID 0000-0000 FS_Info cache can't report
it so exfat::read_uuid() is called. Then `tune.exfat -i /dev/loop0` is
executed:
# tune.exfat -i /dev/loop0
exfatprogs version : 1.1.3
volume serial : 0x0
And exfat::serial_to_blkid_uuid() is called with "0x0", which causes a
crash. Fix by safely handling volume serial numbers of any length,
specifically shorter than 8 hexadecimal digits.
Closes!115 - Fix crash when dealing with 0000-0000 exfat UUID
get_text() only performs const access on the ProgressBar object so
return the member string by constant reference.
Previously done for other string returning getters, even though the
value is assigned to a variable and doesn't save anything:
1f6e81295b
Return constant reference from OperationDetail::get_description() (!94)
The only use of the reference returned from
OperationDetail::get_progressbar() is to call const methods
ProgressBar::running(), ::get_fraction() and ::get_text(). Therefore
make OperationDetail::get_progressbar() return a const reference.
As described in the previous commit "Clear progress bar text when
starting the bar (#230)" progress bar data is either reporting bytes
copied or fraction complete. The bytes copied case gets in progress
text like this:
544.00 MiB of 1.00 GiB copied (00:00:11 remaining)
But the fraction complete gets no text.
Now also generate time remaining text for progress bars only reporting
fraction complete. As with the bytes copied text only add the time
remaining estimate after 5 seconds have passed. Looks like:
(00:01:59 remaining)
This is most useful for NTFS partition copy and resize operations which
can take a while depending on the amount of data involved.
These operations use steps which generate progress bar bytes copied
information text:
* Partition move or copy using GParted's internal block copy
* EXT2/3/4 partition move or copy
* XFS partition copy
The bytes copied text looks like this (after the copy completes and the
time remaining is no longer included):
1.00 GiB of 1.00 GiB copied
And these operations use steps which generate progress bar information
without text (because the progress bar data only represents a fraction
complete):
* EXT2/3/4 partition resize
* EXT2/3/4 partition create
* EXT2/3/4 partition format
* EXT2/3/4 partition check
* NTFS partition resize
* NTFS partition copy
In the Applying pending operations dialog, while an operation is being
applied there are 2 progress bars. The top progress bar displays either
a pulse bar or the progress bar data for the current step. Additionally
for the relevant steps the progress bar generates the bytes copied text.
This text, when available, is displayed in small grey characters just
above the progress bar itself.
Copy a FAT partition and apply. Bytes copied text is displayed just
above the top progress bar. Copy an NTFS partition and apply. The left
behind bytes copied text from the previous operation is displayed,
instead of nothing.
Restart GParted and copy an NTFS partition and apply. As intended, this
time there is no bytes copied text displayed just above the top progress
bar.
As there is just a single ProgressBar object, single_progressbar, fix
this by clearing the progress bar text each time it is started.
Closes#230 - Missing progress bar text reset when applying operation
STAT_FORMATTED is only used inside snap_to_mebibyte() to suppress
enforcement that partition boundaries must not overlay the MBR or EBRs
when merely formatting existing partitions. However since commit [1],
snap_to_mebibyte() is only called inside the dialogs composing Create
New, Copy / Paste into New and Resize / Move operations and never when
composing a Format operation or any other operation which doesn't change
partition boundaries. Therefore remove STAT_FORMATTED.
[1] 7c94b7d920
Snap partition boundaries before dialogs update FS usage (#48)
The available number of sectors 'total_length' is calculated as one
sector too few. However this doesn't matter because when composing a
new partition in Dialog_Partition_New::Get_New_Partition() the dialog
fits the end of the partition to the end of the unallocated partition
'new_partition->sector_end' in which it is being created, not the
available space. Missed in earlier commit:
4f84cff781
cleanups
Anyway correct the calculation.
As a partition's position is completely defined by it's starting and
ending sector, it is aligned if both boundaries are aligned. The size
of a partition just depends on the starting and ending sector.
Therefore it is never necessary to also check if the size of the
partition is also an exact multiple of MiB. Remove this unnecessary
portion of the alignment check when resizing/moving a partition (or
copy/pasting into a new partition).
Now Win_GParted::m_display_device.partitions is an identical copy of
Win_GParted::display_partitions with the same lifetime. That's wasteful
and pointless. Therefore remove the later and use the former in it's
place.
Closes#227 - Unable to allocate 1 MiB between partitions when moving to
the right
The Create New and Paste dialogs also create partitions and have to
honour currently composed partitions while doing so. Therefore they
must have a Device object containing the currently composed partition
layout for passing into snap_to_alignment() and below. So copy the
current Device object when refreshing the visual at the same time
visual_partitions is generated and use in all 3 dialogs which compose
new partitions.
Note that Create New and Paste aren't subject to the same bug as Resize/
Move was because the code in snap_to_mebibyte() [1] checked the
partition object being composed has status STAT_REAL. This is true for
partition objects created by the Resize/Move dialog, but not true for
the Create New and Paste dialogs which set status to STAT_NEW and
STAT_COPY respectively instead.
[1] Dialog_Base_Partition::snap_to_mebibyte() lines 418 to 438
https://gitlab.gnome.org/GNOME/gparted/-/blob/GPARTED_1_5_0/src/Dialog_Base_Partition.cc#L418Closes#227 - Unable to allocate 1 MiB between partitions when moving to
the right
Start with 2 partitions next to each other, containing file systems that
GParted can move and resize.
EG:
|[#1 ext2 ][#2 swap ] |
Move the start of partition #2 to the right. Then attempt to move the
end of partition #1 to the right to meet it.
EG:
|[#1 ext2 ][#2 swap ] |
The Resize/Move dialog will allow the free space following to be set to
0 so partition #1 is again adjacent to partition #2, but after closing
the dialog a forced 1 MiB gap is added, shrinking the composed size of
partition #1 by that 1 MiB.
If instead the first operation to shrink and move partition #2 is
applied, then partition #1 can be successfully resize right up to
partition #2 without a 1 MiB gap.
Relevant call sequence:
Win_GParted::activate_resize()
Dialog_Partition_Resize_Move::Dialog_Partition_Resize_Move()
Dialog_Base_Partition::Get_New_Partition()
prepare_new_partition()
snap_to_alignment()
snap_to_mebibyte()
prepare_new_partition() created a new partition object to correctly
represent the resized/moved partition #1. However this code in
snap_to_mebibyte() [1] determined that the new location for partition #1
overlapped with where partition #2 currently is on disk, not where
partition #2 will be after the previous operation is applied, therefore
it forced a 1 MiB decrease in partition #1's size creating the gap.
This is because snap_to_mebibyte() is working with the on disk view of
the partitions obtained from devices[current_device] passed into the
Dialog_Partition_Resize_Move() constructor. Hence why applying the
operations one at a time doesn't suffer from the forced 1 MiB gap.
Fix this by creating a copy of the current device object, replacing the
on disk partition layout with the composed partition layout as displayed
in the UI. Then pass this into the Dialog_Partition_Resize_Move
constructor.
[1] Dialog_Base_Partition::snap_to_mebibyte() lines 418 to 438
https://gitlab.gnome.org/GNOME/gparted/-/blob/GPARTED_1_5_0/src/Dialog_Base_Partition.cc#L418Closes#227 - Unable to allocate 1 MiB between partitions when moving to
the right
The code in erase_filesystem_signatures() used libparted
ped_device_write() which allowed any sector in the whole disk device to
be written. The code only depended on calculations of somewhat
complicated zero offset ranges and the start partition offset to ensure
that it didn't zero sectors outside the target partition. The code
doesn't overwrite partition boundaries, but there have been updates and
bug fixes to the calculation code. To improve the safety create a
libparted geometry representing the partition, or whole disk device,
to be cleared and use ped_geometry_write() so that libparted enforces
writes are only within the partition boundary being erased.
Deliberately breaking erase_filesystem_signatures() code so that it
tries to write past the end of the partition produces this dialog:
Libparted Error
(-) Attempt to write sectors 1024000-1024007 outside of
partition on /dev/sdb.
[ Cancel ] [ Ignore ]
And trying to write before the start of the partition produces this
dialog:
Libparted Bug
(-) Assert (offset >= 0) at cs/geom.c:375 in function
ped_geometry_write() failed.
[ No ]
Followed by GParted aborting and producing a core dump. Not ideal from
libparted, but it does prevent GParted writing outside the partition
boundaries and only occurs in the case of a bug in
erase_filesystem_signatures() which is exercised on every Create and
Format Partition operation and now also unit tested. So not something
we will let through to the users.
User reported that GParted didn't clear a pdc (Promise FastTrack) RAID
signature [1]. Reproduce this issue by creating a 16 MiB - 512 byte
test image with Promise FastTrack RAID signatures at all recognised
offsets [2].
$ python << 'EOF'
signature = b'Promise Technology, Inc.'
import os
fd = os.open('/tmp/test.img', os.O_CREAT|os.O_WRONLY)
os.ftruncate(fd, 16*1024*1024 - 512)
for offset in [63, 255, 256, 16, 399, 591, 675, 735, 911, 974, 991, 951, 3087]:
os.lseek(fd, -(offset*512), os.SEEK_END)
os.write(fd, signature)
os.close(fd)
EOF
Then use GParted Format to > Cleared.
$ sudo ./gpartedbin /tmp/test.img
Afterwards blkid, and therefore GParted, still recognises this as a
Promise FastTrack RAID member.
$ blkid /tmp/test.img
/tmp/test.img: TYPE="promise_fasttrack_raid_member"
This is because the test image still contains multiple signatures.
$ hexdump -C /tmp/test.img | grep Promise
00e7e000 50 72 6f 6d 69 73 65 20 54 65 63 ... |Promise Technolo|
00fce000 50 72 6f 6d 69 73 65 20 54 65 63 ... |Promise Technolo|
00fdfe00 50 72 6f 6d 69 73 65 20 54 65 63 ... |Promise Technolo|
00ff8000 50 72 6f 6d 69 73 65 20 54 65 63 ... |Promise Technolo|
Used a test image not an exact multiple of MiBs because drives generally
aren't an exact MiB multiple in size either and as the clearing of ZFS
labels L2 and L3 by writes of zeros at the end of the drive is rounded
to 256 KiBs there will be sectors after that not zeroed where other
Promise signatures remain. The above signatures map back to these
sectors before the end:
16*1024*1024 - 512 = 16776704
512b sectors KiB
(0x00e7e000 - 16776704) / 512 = -3087 -1543.5
(0x00fce000 - 16776704) / 512 = -399 -199.5
(0x00fdfe00 - 16776704) / 512 = -256 -128
(0x00ff8000 - 16776704) / 512 = -63 -31.5
Promise FastTrack RAID signatures are always at multiples 512-byte
sectors (code uses left shift 9 to convert from sectors to byte offset)
[2].
Fix this by:
1. Replace existing zeroing of 3 ranges relative to the end of the
device to be a single range covering the ZFS labels L2 and L3 to the
end of the drive. This will also clear the SWRaid 0.90 & 1.0
super blocks, the Nilfs2 secondary super block, the Intel Software
RAID signature found not zeroed in the unaligned unit test case and
the above Promise FastTrack RAID signatures at -199.5 KiB and later.
2. Add zeroing of the final Promise FastTrack RAID signature at sector
-3087.
Performed a review of all the other ATARAID super blocks detected by
blkid (files *_raid.c) [3] and they are all located within the last 11
sectors so will be zeroed by case 1. above.
[1] GParted forum thread: How to remove a ataraid partition ?
http://gparted-forum.surf4.info/viewtopic.php?id=18104
[2] blkid from util-linux promise_raid.c:probe_pdcraid()
https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/libblkid/src/superblocks/promise_raid.c?h=v2.38.1#n27
[3] blkid RAID member detection (files *_raid.c)
https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/libblkid/src/superblocks/?h=v2.38.1Closes#220 - Format to Cleared not clearing "pdc" ataraid signature
As documented in the previous commit xfsprogs >= 5.19.0 refuses to
create an XFS file system smaller than 300 MiB.
$ truncate -s $((300*1024*1024-1)) test.img
$ ls -l test.img
-rw-r--r-- 1 auser auser 314572799 Dec 21 11:01 test.img
$ mkfs.xfs -V
mkfs.xfs version 6.0.0
$ mkfs.xfs test.img
Filesystem must be larger than 300MB.
...
$ echo $?
1
Successfully create an XFS file system at minimum size of 300 MiB.
$ truncate -s $((300*1024*1024)) test.img
$ ls -l test.img
-rw-r--r-- 1 auser auser 314572800 Dec 21 11:05 test.img
$ mkfs.xfs test.img
...
$ echo $?
0
$ blkid test.img
test.img: UUID="..." BLOCK_SIZE="512" TYPE="xfs"
Increase the GParted minimum XFS size to 300 MiB. For simplicity and
because the XFS developers said of smaller XFS file systems [1]:
"are known to have performance and redundancy problems that are not
present on the volume sizes that XFS is best at handling"
regardless of the version of mkfs.xfs used to create that XFS then apply
to all versions of xfsprogs.
[1] https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/commit/?id=6e0ed3d19c54603f0f7d628ea04b550151d8a262
mkfs: stop allowing tiny filesystems
Closes#217 - GitLab CI test job failing with new mkfs.xfs error
"Filesystem must be larger than 300MB."
GParted's check operation is a check and if possible repair. For most
file system types GParted already requests that the file system is
repaired. fsck.exfat -y flag has been available since the first release
of exfatprogs 1.0.1 [1] so unconditionally add this.
[1] exfatprogs 1.0.1 fsck/fsck.c:main() case 'y':
https://github.com/exfatprogs/exfatprogs/blob/1.0.1/fsck/fsck.c#L1231!109 - Enable repair when checking exfat file systems
The code was overly complicated in how it converted to the 32-bit little
endian on-disk representation of the Hidden Sectors field. It did:
1. Formatted the partition start sector as a hexadecimal string.
2. Padded it to 8 digits.
3. Reversed pairs of digits.
4. Converted pairs of hexadecimal digits to bytes of binary data.
5. Wrote the 4 bytes of binary data to the Hidden Sectors field.
There is no need for all this string manipulation to convert to a 32-bit
little endian value. Just do this:
1. Truncate (signed 64-bit) partition start sector to 32-bit.
2. Convert from host native to little endian.
3. Write as 4 bytes of binary data to the Hidden Sectors field.
The code also ignores write errors. ofstream.write() only copies the
data into an in process buffer [1] and the data is not passed to the OS
to write to the open file handle until ofstream.close() [2] is called.
However the status of close() was not checked so a failure of the OS to
perform the write would go unreported.
In the case of a failure providing the user with a command line to set
the Hidden Sectors field is excessive. Updating the Hidden Sectors is
no more or less likely to fail than for any other storage manipulation
action. For example GParted doesn't provide command line instructions
to update a partition size if a libparted call fails. Therefore remove.
Rewrite the code to resolve the above issues and lay it out using
if-operation-fails-return-early pattern.
[1] std::ostream::write()
"... it inserts characters into associated stream_buffer object as
if calling its member function sputc until n characters have been
written or until an insertion fails ..."
https://cplusplus.com/reference/ostream/ostream/write/
[2] std::ofstream::close()
"Any pending output sequence is written to the file."
https://cplusplus.com/reference/fstream/ofstream/close/Closes#164 - GParted crashes copying NTFS partition to starting beyond
2TiB
Create test setup using a 4 TiB loop device:
# truncate -s 4T /tmp/disk.img
# losetup -f --show /tmp/disk.img
/dev/loop0
Create 2 x 1 TiB partitions. First at offset 1 MiB, second at offset
2 TiB:
# sgdisk --new 1:2048:2147485696 --typecode 1:0700 /dev/loop0
# sgdisk --new 2:4294967296:6442450944 --typecode 2:0700 /dev/loop0
# partprobe /dev/loop0
Create NTFS file system in the first partition:
# mkntfs -Q /dev/loop0p1
Then use GParted to copy the first NTFS partition into the second
partition. GParted crashes:
# gpartedbin /dev/loop0
...
(gpartedbin:14660): glibmm-ERROR **: 20:39:01.191:
unhandled exception (type std::exception) in signal handler:
what: basic_string::_M_replace_aux
Trace/breakpoint trap
# echo $?
133
Overview of what is happening is that GParted_Core::update_bootsector()
is attempting to set the Hidden Sectors [1] field in the NTFS Partition
Boot Sector (PBS) to the start sector of the newly copied /dev/loop0p2
partition. But the sector number is greater than will fit in a 32-bit
unsigned integer, which the code doesn't handle.
Specifically the code prints the sector number as a hexadecimal number
into string 'hex'. As the target partition starts at exactly 2 TiB then
hex="100000000" (9 hexadecimal digits long). Next:
hex.insert(0, 8 - hex.length(), '0');
is meant to pad the beginning of the 'hex' string with '0's to make the
string 8 character long. But the string is already 9 character long so
8 - 9 is -1 which as unsigned integral type size_t [2] is 2^64-1. So
insert() is trying to insert 18446744073709551615 '0's at the start of
the 'hex' string! Hence the crash.
mkntfs refuses to accept an explicit partition start sector of 2^32 or
larger:
# mkntfs -Q --partition-start 4294967296 /dev/loop0p2
Invalid partition start sector. Maximum is 4294967295 (2^32-1).
# echo $?
1
When mkntfs can't determine the drive geometry and partition offset, as
is the case on loop devices or the partition start sector is 2^32 or
larger, then mkntfs writes zero into Hidden Sectors:
# mkntfs -Q /dev/loop0p1
The partition start sector was not specified for /dev/loop0p1 and it could not be obtained automatically. It has been set to 0.
...
To boot from a device, Windows needs the 'partition start sector', ...
Windows will not be able to boot from this device.
Creating NTFS volume structures.
mkntfs completed successfully. Have a nice day.
# echo $?
0
# hexdump -C /dev/loop0p1 | head -2
00000000 eb 52 90 4e 54 46 53 20 20 20 20 00 02 08 00 00 |.R.NTFS .....|
00000010 00 00 00 00 00 f8 00 00 00 00 00 00 00 00 00 00 |................|
^^ ^^ ^^ ^^
Hidden Sectors value at offset 0x1C in the NTFS Partition Boot Sector.
So mkntfs is warning, writing the Hidden Sectors as zero and reporting
success. Fix GParted in an equivalent way when it is updating the
Hidden Sectors for a moved or copied NTFS which starts at sector 2^32
and beyond.
After this fix the operational details for the same copy operation are:
Copy /dev/loop0p1 to /dev/loop0p2
* calibrate /dev/loop0p1 (SUCCESS)
* calibrate /dev/loop0p2 (SUCCESS)
* set partition type on /dev/loop0p2 (SUCCESS)
* copy file system from /dev/loop0p1 to /dev/loop0p2 (SUCCESS)
* update boot sector of ntfs file system on /dev/loop0p2 (WARNING)
Partition start (4294967296) is beyond sector 4294967295 (2^32-1).
Windows will not be able to boot from this file system.
* check file system on /dev/loop0p2 for errors and if p... (SUCCESS)
[1] NTFS, Partition Boot Sector (PBS)
"
Byte Field Field name Purpose
offset length
0x1C 5 bytes Hidden Sectors The number of sectors preceding
the partition.
"
https://en.wikipedia.org/wiki/NTFS#Partition_Boot_Sector_(PBS)
[2] std::string::insert
"fill (5) string& insert(size_t pos, size_t n, char c);
Insert into string
Inserts additional characters into the string right before the
character indicated by pos (or p):
(5) fill
Insert n consecutive copies of character c.
"
https://cplusplus.com/reference/string/string/insert/Closes#164 - GParted crashes copying NTFS partition to starting beyond
2TiB
GParted has been using 'btrfs filesystem show' to report file system
usage but that doesn't work on a file system image so doesn't work in a
GitLab CI test job, as discussed earlier in this patchset.
There is 'btrfs inspect-internal min-dev-size' but:
1. That only works on a mounted file system and GParted isn't going to
mount an unmounted file system just to query it's used space, so by
extension won't work on image files.
2. It reports a figure which is almost the same as the chunk usage of
the device within the btrfs file system. However if some files have
been deleted leaving chunks partially used, then 'btrfs filesystem
resize' will successfully shrink a btrfs smaller than the reported
minimum device size.
And there is also 'btrfs filesystem usage' but that also only works on a
mounted file system.
So instead use 'btrfs inspect-internal dump-super' to report some of the
figures previously obtained from 'btrfs filesystem show'. For example
for a single device btrfs in an image file:
$ truncate -s 256M /tmp/test.img
$ mkfs.btrfs /tmp/test.img
$ btrfs inspect-internal dump-super /tmp/test.img | egrep 'total_bytes|bytes_used|sectorsize|devid'
total_bytes 268435456
bytes_used 114688
sectorsize 4096
dev_item.total_bytes 268435456
dev_item.bytes_used 92274688
dev_item.devid 1
Comparing with results from 'btrfs filesystem show' for the same file
system, after adding a loop device to allow 'btrfs filesystem show' to
succeed:
$ su -
# losetup --find --show /tmp/test.img
# btrfs filesystem show --raw /dev/loop0
Label: none uuid: 32a1eb31-4691-41ae-9ede-c45d723655a3
Total devices 1 FS bytes used 114688
devid 1 size 268435456 used 92274688 path /dev/loop0
This does bring a forced change in the calculation which affects multi-
device btrfs file systems. 'btrfs filesystem show' provided chunk
allocation information per device ("used" figure for each "devid"). The
file system wide used bytes ("FS bytes used") was apportioned according
to the fraction of the chunk allocation each device contained. However
'btrfs inspect-internal dump-super' doesn't provide chunk allocation
information for all devices, only for the current device
("dev_item.bytes_used"). Instead the calculation now has to apportion
the file system wide used bytes ("bytes_used") according to the fraction
of the size of the current device ("dev_item.total_bytes") within the
total size ("total_bytes").
This can't make any difference to a single device btrfs file system as
both fractions will be 1. It only affects how the file system wide used
bytes is distributed among multiple devices.
As an example to see the difference between calculation methods, create
a 2 GiB btrfs taking the defaults so getting duplicated metadata and
single data. Add another 2 GiB partition and populate with some files.
# mkfs.btrfs /dev/sdb1
btrfs-progs v4.15.1
See http://btrfs.wiki.kernel.org for more information.
Label: (null)
UUID: 68195e7e-c13f-4095-945f-675af4b1a451
Node size: 16384
Sector size: 4096
Filesystem size: 2.00GiB
Block group profiles:
Data: single 8.00MiB
Metadata: DUP 102.38MiB
System: DUP 8.00MiB
SSD detected: no
Incompat features: extref, skinny-metadata
Number of devices: 1
Devices:
ID SIZE PATH
1 2.00GiB /dev/sdb1
# mount /dev/sdb1 /mnt/1
# btrfs device add /dev/sdc1 /mnt/1
# cp -a /home/$USER/programming/c/gparted/ /mnt/1/
Usage figures using the old calculation apportioning file system wide
usage according to chunk allocation per device:
# btrfs filesystem show --raw /dev/sdb1
Label: none uuid: 68195e7e-c13f-4095-945f-675af4b1a451
Total devices 2 FS bytes used 178749440
devid 1 size 2147483648 used 239861760 path /dev/sdb1
devid 2 size 2147483648 used 436207616 path /dev/sdc1
sum_devid_used = 239861760 + 436207616
= 676069376
sdb1 usage = 178749440 * 239861760 / 676069376
= 63418277
sdc1 usage = 178749440 * 436207616 / 676069376
= 115331163
Usage figures using the new calculation apportioning file system wide
usage according to device sizes:
# btrfs inspect-internal dump-super /dev/sdb1 | egrep 'total_bytes|^bytes_used'
total_bytes 4294967296
bytes_used 178749440
dev_item.total_bytes 2147483648
# btrfs inspect-internal dump-super /dev/sdc1 | egrep 'total_bytes|^bytes_used'
total_bytes 4294967296
bytes_used 178749440
dev_item.total_bytes 2147483648
sdb1 usage = 178749440 * 2147483648 / 4294967296
= 89374720
sdc1 usage = 178749440 * 2147483648 / 4294967296
= 89374720
Both calculation methods ignore that btrfs allocates chunks at the
volume manager level. So when fully compacted the last chunk for
metadata and data for each storage profile (RAID level) will be
partially filled and this is not accounted for.
Also for multi-device btrfs file systems the new calculation provides
different results. However given that shrinking a device in a multi-
device btrfs file system can and does relocate extents to other devices
(redundancy requirements of chunks permitting) it's minimum size is
virtually impossible to calculate and may not restrict how small the
btrfs device can be shrunk anyway. If it turns out that this new
calculation causes problems it's been made a separate commit from the
previous commit for easier reverting.
Closes!105 - Update used btrfs file system commands, new minimum is
btrfs-progs 4.5
'btrfs filesystem show' only used to report rounded human readable size
figures. Therefore the actual figure could have been anywhere within
the rounding limit. GParted also applied a heuristic to snap the file
system size figure to the partition size if the partition size was
within the rounding limit of the reported file system size [1].
btrfs-progs v4.1 added the --raw option to print the figures in bytes
[2][3][4].
# btrfs filesystem show --raw /dev/sdb1
Label: none uuid: 003a619e-856f-4b9c-bd29-4d0ae0296d66
Total devices 2 FS bytes used 178765824
devid 1 size 2147483648 used 239861760 path /dev/sdb1
devid 2 size 2147483648 used 436207616 path /dev/sdc1
Since the oldest supported distributions now use btrfs-progs v4.5.3 and
later (see the distribution End-of-Life table in the previous commit
message), unconditionally use this to get accurate figures.
[1] 7fc16a1b69
Handle btrfs tools rounding of figures (#499202)
[2] btrfs-progs: Allow "filesystem show" command to handle different units
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git/commit/?id=15379fa2257bf937cf7830c0b1b79f2daf5df72c
[3] btrfs-progs: docs: new size options for fi show
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git/commit/?id=81225f11d9ea58590476612e69211113ddb9b943
[4] Btrfs progs release 4.1
https://lore.kernel.org/linux-btrfs/20150622150023.GX6761@twin.jikos.cz/Closes!105 - Update used btrfs file system commands, new minimum is
btrfs-progs 4.5
GParted so far uses 'btrfs filesystem show' to read the file system
UUID. But this doesn't work on a file system image so doesn't work in
the GitLab CI test jobs, as discussed in the earlier commit "Use btrfs
filesystem label to read the FS label (!105)".
$ truncate -s 256M /tmp/test.img
$ mkfs.btrfs /tmp/test.img
...
UUID: 5ea62f88-fef3-4ece-a726-b88f3d81fe1c
...
$ btrfs filesystem show /tmp/test.img
ERROR: not a valid btrfs filesystem: /tmp/test.img
Instead use 'btrfs inspect-internal dump-super' which works on image
files too.
$ btrfs inspect-internal dump-super /tmp/test.img
...
fsid 5ea62f88-fef3-4ece-a726-b88f3d81fe1c
...
'btrfs inspect-internal dump-super' was added in btrfs-progs 4.5 [1][2]
so is available in the oldest supported distributions and can be used
unconditionally.
Distro EOL btrfs --version
Debian 10 2022-Jun v4.20
RHEL / CentOS 7 2024-Jun v4.9.1
Ubuntu 18.04 LTS 2023-Apr v4.15.1
SLES 12 SP5 2024-Oct v4.5.3 [3][4]
Unfortunately it returns 0 status on failure so use non-empty stderr to
identify failure.
$ rm /tmp/test.img
$ truncate -s 256M /tmp/test.img
$ btrfs inspect-internal dump-super /tmp/test.img 1> /dev/null
ERROR: bad magic on superblock on /tmp/test.img at 65536
$ echo $?
0
[1] btrfs-progs: introduce inspect-internal dump-super
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git/commit/?id=eaa93e3b0295fc94c774ec73056559a6b8c78b42
[2] Btrfs progs release 4.5
https://lore.kernel.org/linux-btrfs/20160320235330.GG21722@suse.cz/
"* new/moved commands
* btrfs-show-super -> btrfs inspect-internal dump-super
"
[3] SUSE Long Term Service Pack Support
https://links.imagerelay.com/cdn/3404/ql/f3a083e9bcd34c76addd096d7f60ec00/long_term_service_pack_support_flyer.pdf
[4] SUSE package search
https://scc.suse.com/packages?name=SUSE%20Linux%20Enterprise%20Server&version=12.5&arch=x86_64&query=btrfsprogs&module=Closes!105 - Update used btrfs file system commands, new minimum is
btrfs-progs 4.5
Create function to replace repeated code which optionally removes
trailing new line character from a string.
Closes!105 - Update used btrfs file system commands, new minimum is
btrfs-progs 4.5
Until now GParted has run 'btrfs filesystem show' to read the file
system label. This has a number of issues and limitations:
1. Doesn't work on a file system image so can't be unit tested in the
GitLab CI test job where a loop device can't be created [1] or as a
non-root user.
2. Reported failed exit status 1 when successfully showing a mounted
btrfs, but only when using btrfs-progs v3.14 and 3.14.1 [2][3].
3. Failed to distinguish between label set to "none" and no label
reported as none, but only when mounted and with btrfs-progs v3.12
[3].
As non-root user run btrfs read label unit test:
$ tests/test_SupportedFileSystems --gtest_filter='*CreateAndReadLabel/btrfs'
...
[ RUN ] My/SupportedFileSystemsTest.CreateAndReadLabel/btrfs
test_SupportedFileSystem.cc:539: Skip test. Not root to be able to create required loop device
[ OK ] My/SupportedFileSystemsTest.CreateAndReadLabel/btrfs (0 ms)
Even as root, 'btrfs filesystem show' fails to work on an image file:
# truncate -s 256M /tmp/test.img
# mkfs.btrfs /tmp/test.img
# btrfs filesystem show /tmp/test.img
ERROR: not a valid btrfs filesystem: /tmp/test.img
# echo $?
1
# rm /tmp/test.img
Instead use 'btrfs filesystem label' to read the label. It also works
on an image file, the exit status is informative and output is just the
label followed by a new line character [4] so very simple to parse.
Error case:
$ truncate -s 256M /tmp/test.img
$ btrfs filesystem label /tmp/test.img
No valid Btrfs found on /tmp/test.img
$ echo $?
255
No label case:
$ mkfs.btrfs /tmp/test.img
$ btrfs filesystem label /tmp/test.img
$ echo $?
0
$ btrfs filesystem label /tmp/test.img | hexdump -C
00000000 0a |.|
00000001
Label case:
$ mkfs.btrfs -L 'label with
> new line and trailing space ' /tmp/test.img
$ btrfs filesystem label /tmp/test.img
label with
new line and trailing space
$ echo $?
0
$ btrfs filesystem label /tmp/test.img | hexdump -C
00000000 6c 61 62 65 6c 20 77 69 74 68 0a 6e 65 77 20 6c |label with.new l|
00000010 69 6e 65 20 61 6e 64 20 74 72 61 69 6c 69 6e 67 |ine and trailing|
00000020 20 73 70 61 63 65 20 0a | space .|
00000028
Run 'btrfs filesystem label' always passing the block device as this
works for both mounted and unmounted file systems. This is in
contrast to writing the label for a mounted btrfs where the mount
mount must be used [5].
# mkfs.btrfs -L 'test label' /dev/sdb1
# btrfs filesystem label /dev/sdb1
test label
# mount /dev/sdb1 /mnt/1
# btrfs filesystem label /dev/sdb1
test label
[1] 07ad43a107
Create loop devices for BTRFS read file system interface tests (!49)
[2] 82c6265fa5
Update parsing of btrfs filesystem show for the UUID (#733601)
[3] eca732fb0c
Update parsing of btrfs filesystem show for the label (#733601)
[4] btrfs-progs v3.14, cmd_label() function
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git/tree/cmds-filesystem.c?h=v3.14#n984
[5] eb034b1759
Add labelling of mounted btrfs (#163)
Closes!105 - Update used btrfs file system commands, new minimum is
btrfs-progs 4.5
Now fix the error with GParted clearing the label when setting a new
UUID on a FAT16/32 file system. Reproduce the issue on the command
line:
# mkfs.fat -F 16 -v -I -n TEST_LABEL /dev/sdb1
# mdir -f -i /dev/sdb1 ::/
Volume in drive : is TEST_LABEL
Volume Serial Number is 5D4C-6E6E
...
# mlabel -n -i /dev/sdb1 ::
# mdir -f -i /dev/sdb1 ::/
Volume in drive : has no label
Volume Serial Number is 77BB-A883
...
This was broken by commit "Fix writing FAT16/32 FS UUID on Alpine Linux
(!104)" earlier in this patchset, which included this comment:
"... Also drop the '-s' option
as showing the current label is unrelated to writing a new UUID."
It is not mentioned in the mlabel[1] manual page that option -s is
needed in order to avoid clearing the label when assigning a new UUID.
Anyway add the option back.
[1] mlabel(1)
https://linux.die.net/man/1/mlabel
"s Shows the existing label, without prompting the user.
n Assigns a new (random) serial number to the disk
"
Closes!104 - Add Alpine Linux CI jobs and resolve label and UUID issues
with FAT16/32
Unit test writing FAT16/32 file system UUIDs fails on Alpine Linux like
this:
$ ./test_SupportedFileSystems --gtest_filter='*CreateAndWriteUUID/fat16'
...
[ RUN ] My/SupportedFileSystemsTest.CreateAndWriteUUID/fat16
test_SupportedFileSystems.cc:616: Failure
Value of: m_fs_object->write_uuid(m_partition, m_operation_detail)
Actual: false
Expected: true
Operation details:
mkfs.fat -F16 -v -I '/home/alpine/programming/c/gparted/tests/test_SupportedFileSystems.img' 00:00:00 (SUCCESS)
...
mlabel -s -n :: -i '/home/alpine/programming/c/gparted/tests/test_SupportedFileSystems.img' 00:00:00 (ERROR)
Mtools version 4.0.39, dated April 10th, 2022
Usage: mlabel [-vscVn] [-N serial] drive:
[ FAILED ] My/SupportedFileSystemsTest.CreateAndWriteUUID/fat16, where GetParam() = 13 (38 ms)
Using GParted on Alpine Linux to perform the same action produces the
same error in the operation results. Reproduce this on the command
line:
# mkfs.fat -F 16 -v -I /dev/sdb1
# mlabel -s -n :: -i /dev/sdb1
Mtools version 4.0.39, dated April 10th, 2022
Usage: mlabel [-vscVn] [-N serial] drive:
# echo $?
1
Again fix the same way, by moving the non-option '::' drive
specification to the end of the command line. Also drop the '-s' option
as showing the current label is unrelated to writing a new UUID.
# mdir -f -i /dev/sdb1 ::/ | grep 'Volume Serial Number is'
Volume Serial Number is B97E-59A3
# mlabel -n -i /dev/sdb1 ::
# echo $?
0
# mdir -f -i /dev/sdb1 ::/ | grep 'Volume Serial Number is'
Volume Serial Number is 1552-96A6
Closes!104 - Add Alpine Linux CI jobs and resolve label and UUID issues
with FAT16/32
Unit test reading FAT16/32 file system UUIDs fails on Alpine Linux like
this:
$ ./test_SupportedFileSystems --gtest_filter='*CreateAndReadUUID/fat16'
....
[ RUN ] My/SupportedFileSystemsTest.CreateAndReadUUID/fat16
test_SupportedFileSystems.cc:581: Failure
Expected: (m_partition.uuid.size()) >= (9U), actual: 0 vs 9
test_SupportedFileSystems.cc:584: Failure
Value of: m_partition.get_messages().empty()
Actual: false
Expected: true
Partition messages:
Drive '::' not supported
Cannot initialize '::'
Drive 'A:' not supported
Cannot initialize 'A:'
Drive 'A:' not supported
Cannot initialize 'A:'
[ FAILED ] My/SupportedFileSystemsTest.CreateAndReadUUID/fat16, where GetParam() = 13 (28 ms)
This doesn't normally affect GParted because it uses blkid as first
choice to read file system UUIDs, only using file system specific
commands when blkid isn't available. Reproduce this on the command
line:
# mkfs.fat -F 16 -v -I /dev/sdb1
# mdir -f :: -i /dev/sdb1
Drive '::' not supported
Cannot initialize '::'
Drive 'A:' not supported
Cannot initialize 'A:'
Drive 'A:' not supported
Cannot initialize 'A:'
Again, this is caused by having non-option '::' drive specification
before all the options on the mdir command line, which isn't supported
by the POSIX strict getopt(3) on Alpine Linux. Apply the same fix of
moving the non-option argument to the end.
# mdir -f -i /dev/sdb1 ::/
Volume is drive : has no label
Volume Serial Number is 7DC9-BCD9
Director for ::/
No files
# echo $?
0
Closes!104 - Add Alpine Linux CI jobs and resolve label and UUID issues
with FAT16/32
Mike Fleetwood