[gparted] Switch to POSIX open() in detect_filesystem_internal() (!46)(#16)
- From: Curtis Gedak <gedakc src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gparted] Switch to POSIX open() in detect_filesystem_internal() (!46)(#16)
- Date: Thu, 18 Jul 2019 16:05:23 +0000 (UTC)
commit 228374fe50d99ff826e5629af3160c59742b7a3c
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date: Thu Jul 11 19:16:09 2019 +0100
Switch to POSIX open() in detect_filesystem_internal() (!46)(#16)
For every partitioned device that GParted scans it triggers a set of
udev change events from it's internal file system signature detection
function. This is the sequence of events:
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 | open("/dev/sdb", O_RDONLY|O_NONBLOCK) = 8
gparted | read(8, ...)
gparted | close(8)
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 | settle_device()
gparted | Utils::execute_command("udevadm settle --timeout=1")
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")
Note that the udev block device change events triggered in
detect_filesystem_internal() occur before the waited for ones in the
second commit "Wait for udev to recreate /dev/PTN entries when querying
partition FSs (!46)" in get_disk() so these were already waited for.
The call chain is:
set_devices_thread()
set_device_from_disk()
detect_filesystem()
detect_filesystem_internal()
ped_device_open()
ped_device_read()
ped_device_close()
This occurs because file system detection is performed on whole disk
devices, but the device contains a partition table, so blkid won't
report any content GParted accepts as a file system, so it tries it's
own internal detection.
Fix this by changing detect_filesystem_internal() to use POSIX open(),
read() and close() so the file can be opened read-only and udev change
events aren't triggered.
Note that when using detect_filesystem_internal() on a partition this
also changes the device it reads from. Before it used libparted which
always reads from the whole disk device, now it reads from the partition
device. This doesn't matter because GParted ensures the data is
consistent between them [2] and blkid reads from each partition device
which GParted already uses.
As the new code avoids using the libparted API, and just skips to the
next signature on lseek() and read() errors, it therefore won't generate
libparted exceptions such as this when scanning very small drives:
Invalid argument during seek for read on /dev/PTN
[1] f8faee637787329c07771e495c9b26abc9ac1603
Avoid whole disk FAT being detected as MSDOS partition table
(#743181)
[2] 3bea067596e3e2d6513cda2a66df1b3e4fa432fb
Flush devices when scanning to prevent reading stale signatures
(#723842)
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
include/GParted_Core.h | 2 +-
src/GParted_Core.cc | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
---
diff --git a/include/GParted_Core.h b/include/GParted_Core.h
index 736e315a..b9b12ad8 100644
--- a/include/GParted_Core.h
+++ b/include/GParted_Core.h
@@ -89,7 +89,7 @@ private:
std::vector<Glib::ustring> & messages );
void set_luks_partition( PartitionLUKS & partition );
void set_partition_label_and_uuid( Partition & partition );
- static FSType detect_filesystem_internal( PedDevice * lp_device, PedPartition * lp_partition );
+ static FSType detect_filesystem_internal(const Glib::ustring& path, const PedDevice* lp_device);
static FSType detect_filesystem( PedDevice * lp_device, PedPartition * lp_partition,
std::vector<Glib::ustring> & messages );
void read_label( Partition & partition ) ;
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index 5c0d223a..d5ada63f 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -1116,9 +1116,10 @@ void GParted_Core::set_partition_label_and_uuid( Partition & partition )
}
}
+
// GParted simple internal file system signature detection. Use sparingly. Only when
// (old versions of) blkid and libparted don't recognise a signature.
-FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedPartition * lp_partition )
+FSType GParted_Core::detect_filesystem_internal(const Glib::ustring& path, const PedDevice* lp_device)
{
char magic1[16]; // Big enough for largest signatures[].sig1 or sig2
char magic2[16];
@@ -1128,7 +1129,8 @@ FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedParti
if ( ! buf )
return FS_UNKNOWN;
- if ( ! ped_device_open( lp_device ) )
+ int fd = open(path.c_str(), O_RDONLY|O_NONBLOCK);
+ if (fd == -1)
{
free( buf );
return FS_UNKNOWN;
@@ -1189,13 +1191,11 @@ FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedParti
if ( len1 == 0UL || ( signatures[i].sig2 != NULL && len2 == 0UL ) )
continue; // Don't allow 0 length signatures to match
- Sector start = 0LL;
- if ( lp_partition )
- start = lp_partition->geom.start;
- start += signatures[i].offset1 / lp_device->sector_size;
+ Byte_Value read_offset = signatures[i].offset1 / lp_device->sector_size *
lp_device->sector_size;
memset( buf, 0, lp_device->sector_size );
- if ( ped_device_read( lp_device, buf, start, 1 ) != 0 )
+ if (lseek(fd, read_offset, SEEK_SET) == read_offset &&
+ read(fd, buf, lp_device->sector_size) == lp_device->sector_size )
{
memcpy( magic1, buf + signatures[i].offset1 % lp_device->sector_size, len1 );
@@ -1215,7 +1215,7 @@ FSType GParted_Core::detect_filesystem_internal( PedDevice * lp_device, PedParti
}
}
- ped_device_close( lp_device );
+ close(fd);
free( buf );
return fstype;
@@ -1321,7 +1321,7 @@ FSType GParted_Core::detect_filesystem( PedDevice * lp_device, PedPartition * lp
}
// (Q4) Fallback to GParted simple internal file system detection
- FSType fstype = detect_filesystem_internal( lp_device, lp_partition );
+ FSType fstype = detect_filesystem_internal(path, lp_device);
if ( fstype != FS_UNKNOWN )
return fstype;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
