[parted-devel] [PATCH 3/5] libparted: avoid the HDIO_GETGEO ioctl when possible
Jim Meyering
jim at meyering.net
Thu Jan 5 18:00:50 UTC 2012
Phillip Susi wrote:
> We were using the long depreciated HDIO_GETGEO ioctl on the partition
> to get its start sector. Get the partition start and length from sysfs
> instead. If this fails, fall back to using BLKGETSIZE64 and HDIO_GETGEO
> as a last resort.
>
> This allows the blkpg code introduced last year to manipulate
> other partitions on a disk where one ( or more ) are in use to work
> correctly on disks > 2TB and partitioned loop devices, which
> don't support HDIO_GETGEO. It also fixes the blkpg code to work on disks
> with non 512 byte sectors ( this has been causing test suite failures ).
Thank you for this patch, and for your patience.
I've amended it with the following changes.
Most are stylistic or for consistency, but there were three
semantic changes:
- remove an always-true if (fd == -1) test
- adjust a misleadingly formatted "else" statement
- correct a diagnostic: s/size/start/
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 57bb0f7..e27accc 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2406,10 +2406,11 @@ _blkpg_remove_partition (PedDisk* disk, int n)
BLKPG_DEL_PARTITION);
}
-#ifdef BLKPG_GET_PARTITION
static int
-_blkpg_get_partition (PedPartition *part, long long *start, long long *length)
+_blkpg_get_partition (PedPartition const *part, long long *start,
+ long long *length)
{
+#ifdef BLKPG_GET_PARTITION
struct blkpg_partition linux_part;
memset (&linux_part, 0, sizeof (linux_part));
@@ -2421,8 +2422,10 @@ _blkpg_get_partition (PedPartition *part, long long *start, long long *length)
return 1;
} else
return 0;
-}
+#else
+ return 0;
#endif
+}
/* Read the integer from /sys/block/DEV_BASE/ENTRY and set *VAL
to that value, where DEV_BASE is the last component of DEV->path.
@@ -2452,7 +2455,7 @@ _sysfs_int_entry_from_dev(PedDevice const* dev, const char *entry, int *val)
Upon success, return true. Otherwise, return false. */
static bool
_sysfs_ull_entry_from_part(PedPartition const* part, const char *entry,
- unsigned long long *val)
+ unsigned long long *val)
{
char path[128];
char *part_name = linux_partition_get_path(part);
@@ -2483,23 +2486,19 @@ _sysfs_ull_entry_from_part(PedPartition const* part, const char *entry,
return false. */
static bool
_kernel_get_partition_start_and_length(PedPartition const *part,
- unsigned long long *start,
- unsigned long long *length)
+ unsigned long long *start,
+ unsigned long long *length)
{
- int fd = -1;
- int ok;
PED_ASSERT(part);
PED_ASSERT(start);
PED_ASSERT(length);
-#ifdef BLKPG_GET_PARTITION
- ok = _blkpg_get_partition (part, start, length);
+ int ok = _blkpg_get_partition (part, start, length);
if (ok) {
*length = (*length * 512) / part->disk->dev->sector_size;
*start = (*start * 512) / part->disk->dev->sector_size;
return ok;
}
-#endif
char *dev_name = linux_partition_get_path (part);
if (!dev_name)
return false;
@@ -2520,12 +2519,16 @@ _kernel_get_partition_start_and_length(PedPartition const *part,
}
*start = (*start * 512) / part->disk->dev->sector_size;
ok = _sysfs_ull_entry_from_part (part, "size", length);
+
+ int fd;
if (!ok) {
- if (fd == -1)
- fd = open (dev_name, O_RDONLY);
+ fd = open (dev_name, O_RDONLY);
if (fd != -1 && ioctl (fd, BLKGETSIZE64, length))
ok = true;
- } else *length *= 512;
+ } else {
+ fd = -1;
+ *length *= 512;
+ }
*length /= part->disk->dev->sector_size;
if (fd != -1)
close (fd);
@@ -2534,7 +2537,7 @@ _kernel_get_partition_start_and_length(PedPartition const *part,
ped_exception_throw (
PED_EXCEPTION_BUG,
PED_EXCEPTION_CANCEL,
- _("Unable to determine the size and length of %s."),
+ _("Unable to determine the start and length of %s."),
dev_name);
free (dev_name);
return ok;
Here's the complete patch:
>From 71a626295307e6bd2d6ee0dcc418eadf71b5020b Mon Sep 17 00:00:00 2001
From: Phillip Susi <psusi at cfl.rr.com>
Date: Fri, 16 Dec 2011 23:05:40 -0500
Subject: [PATCH] libparted: avoid the HDIO_GETGEO ioctl when possible
We were using the long depreciated HDIO_GETGEO ioctl on the
partition to get its start sector. Use the new BLKPG_GET_PARTITION
ioctl instead. This allows for disks > 2TB and partitioned loop
devices, which don't support HDIO_GETGEO. As a fallback when
BLKPG_GET_PARTITION fails or is not availible, try getting the
values from sysfs, and finally use BLKGETSIZE64 and HDIO_GETGEO
as a last resort.
---
libparted/arch/linux.c | 145 ++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 122 insertions(+), 23 deletions(-)
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 4cc72fd..e27accc 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2406,6 +2406,27 @@ _blkpg_remove_partition (PedDisk* disk, int n)
BLKPG_DEL_PARTITION);
}
+static int
+_blkpg_get_partition (PedPartition const *part, long long *start,
+ long long *length)
+{
+#ifdef BLKPG_GET_PARTITION
+ struct blkpg_partition linux_part;
+
+ memset (&linux_part, 0, sizeof (linux_part));
+ linux_part.pno = part->num;
+ if (_blkpg_part_command (part->disk->dev, &linux_part,
+ BLKPG_GET_PARTITION)) {
+ *start = linux_part.start;
+ *length = linux_part.length;
+ return 1;
+ } else
+ return 0;
+#else
+ return 0;
+#endif
+}
+
/* Read the integer from /sys/block/DEV_BASE/ENTRY and set *VAL
to that value, where DEV_BASE is the last component of DEV->path.
Upon success, return true. Otherwise, return false. */
@@ -2428,6 +2449,101 @@ _sysfs_int_entry_from_dev(PedDevice const* dev, const char *entry, int *val)
return ok;
}
+/* Read the unsigned long long from /sys/block/DEV_BASE/PART_BASE/ENTRY
+ and set *VAL to that value, where DEV_BASE is the last component of path to
+ block device corresponding to PART and PART_BASE is the sysfs name of PART.
+ Upon success, return true. Otherwise, return false. */
+static bool
+_sysfs_ull_entry_from_part(PedPartition const* part, const char *entry,
+ unsigned long long *val)
+{
+ char path[128];
+ char *part_name = linux_partition_get_path(part);
+ if (!part_name)
+ return false;
+
+ int r = snprintf(path, sizeof(path), "/sys/block/%s/%s/%s",
+ last_component(part->disk->dev->path),
+ last_component(part_name), entry);
+ free(part_name);
+ if (r < 0 || r >= sizeof(path))
+ return false;
+
+ FILE *fp = fopen(path, "r");
+ if (!fp)
+ return false;
+
+ bool ok = fscanf(fp, "%llu", val) == 1;
+ fclose(fp);
+
+ return ok;
+}
+
+
+/* Get the starting sector and length of a partition PART within a block device
+ Use blkpg if available, then check sysfs and then use HDIO_GETGEO and
+ BLKGETSIZE64 ioctls as fallback. Upon success, return true. Otherwise,
+ return false. */
+static bool
+_kernel_get_partition_start_and_length(PedPartition const *part,
+ unsigned long long *start,
+ unsigned long long *length)
+{
+ PED_ASSERT(part);
+ PED_ASSERT(start);
+ PED_ASSERT(length);
+
+ int ok = _blkpg_get_partition (part, start, length);
+ if (ok) {
+ *length = (*length * 512) / part->disk->dev->sector_size;
+ *start = (*start * 512) / part->disk->dev->sector_size;
+ return ok;
+ }
+ char *dev_name = linux_partition_get_path (part);
+ if (!dev_name)
+ return false;
+
+ ok = _sysfs_ull_entry_from_part (part, "start", start);
+ if (!ok) {
+ struct hd_geometry geom;
+ int dev_fd = open (dev_name, O_RDONLY);
+ if (dev_fd != -1 && ioctl (dev_fd, HDIO_GETGEO, &geom)) {
+ *start = geom.start;
+ ok = true;
+ } else {
+ if (dev_fd != -1)
+ close(dev_fd);
+ free (dev_name);
+ return false;
+ }
+ }
+ *start = (*start * 512) / part->disk->dev->sector_size;
+ ok = _sysfs_ull_entry_from_part (part, "size", length);
+
+ int fd;
+ if (!ok) {
+ fd = open (dev_name, O_RDONLY);
+ if (fd != -1 && ioctl (fd, BLKGETSIZE64, length))
+ ok = true;
+ } else {
+ fd = -1;
+ *length *= 512;
+ }
+ *length /= part->disk->dev->sector_size;
+ if (fd != -1)
+ close (fd);
+
+ if (!ok)
+ ped_exception_throw (
+ PED_EXCEPTION_BUG,
+ PED_EXCEPTION_CANCEL,
+ _("Unable to determine the start and length of %s."),
+ dev_name);
+ free (dev_name);
+ return ok;
+}
+
+
/*
* The number of partitions that a device can have depends on the kernel.
* If we don't find this value in /sys/block/DEV/ext_range, we will use our own
@@ -2512,33 +2628,16 @@ _disk_sync_part_table (PedDisk* disk)
}
for (i = 1; i <= lpn; i++) {
- const PedPartition *part = ped_disk_get_partition (disk, i);
+ PedPartition *part = ped_disk_get_partition (disk, i);
if (part) {
if (!ok[i - 1] && errnums[i - 1] == EBUSY) {
- struct hd_geometry geom;
- unsigned long long length = 0;
+ unsigned long long length;
+ unsigned long long start;
/* get start and length of existing partition */
- char *dev_name = _device_get_part_path (disk->dev, i);
- if (!dev_name)
- goto cleanup;
- int fd = open (dev_name, O_RDONLY);
- if (fd == -1
- || ioctl (fd, HDIO_GETGEO, &geom)
- || ioctl (fd, BLKGETSIZE64, &length)) {
- ped_exception_throw (
- PED_EXCEPTION_BUG,
- PED_EXCEPTION_CANCEL,
- _("Unable to determine the size and length of %s."),
- dev_name);
- if (fd != -1)
- close (fd);
- free (dev_name);
+ if (!_kernel_get_partition_start_and_length(part,
+ &start, &length))
goto cleanup;
- }
- free (dev_name);
- length /= disk->dev->sector_size;
- close (fd);
- if (geom.start == part->geom.start
+ if (start == part->geom.start
&& length == part->geom.length)
ok[i - 1] = 1;
/* If the new partition is unchanged and the
--
1.7.8.2.334.gd4e886
More information about the parted-devel
mailing list