[parted-devel] [PATCH 3/3] Use new BLKPG_GET_PARTITION ioctl to get partition start sector
Petr Uzel
petr.uzel at suse.cz
Mon Dec 5 18:32:19 UTC 2011
On Fri, Dec 02, 2011 at 11:34:54AM -0500, Phillip Susi wrote:
> On 12/2/2011 3:50 AM, Petr Uzel wrote:
> >Hm, this code was hardly readable before and now with the ifdefs and
> >ifndefs, it is even harder not to get lost in it. Perhaps this could
> >be factored out somehow to separate function? Eventually also using
> >/sys/block/DEV/PART/start as a fallback if the new ioctl() is not
> >available?
> >
> >http://lists.alioth.debian.org/pipermail/parted-devel/2011-November/003975.html
>
> I merged your patch with mine.
> How does this look?
You did not try to compile it, did you? :)
See comments below.
>
>
> From d45ed0d6cb2519464807f4185a5ad1644e802507 Mon Sep 17 00:00:00 2001
> From: Phillip Susi <psusi at cfl.rr.com>
> Date: Wed, 30 Nov 2011 21:24:45 -0500
> Subject: [PATCH] Avoid the HDIO_GETGEO 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.
>
> This modifies and superceeds Petr Uzel's patch:
> libparted: fix reading partition start sector from kernel
> ---
> libparted/arch/linux.c | 124 ++++++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 105 insertions(+), 19 deletions(-)
>
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 3799b9d..2d10b80 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2410,6 +2410,25 @@ _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)
> +{
> + struct blkpg_partition linux_part;
> + int ret;
> +
> + 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;
> +}
> +#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. */
> @@ -2432,6 +2451,85 @@ _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 availible, 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_legnth(PedPartition const *part, unsigned long long *start,
> + unsigned long long *length)
Typo: s/legnth/length
> +{
> + int fd = -1;
> + int ok;
> + PED_ASSERT(part);
> + PED_ASSERT(start);
> + PED_ASSERT(length);
> +
Trailing whitespace ^^
> +#ifdef BLKPG_GET_PARTITION
> + ok = _blkpg_get_partition (part, start, length);
> + if (ok)
> + return ok;
> +#endif
> + 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 fd = open (dev_name, O_RDONLY);
> + if (fd != -1 && ioctl (fd, HDIO_GETGEO, &geom)) {
> + *val = geom.start;
val is undeclared. s/val/start ?
> + ok = true;
Missing '}' ^^ here
> + }
> + ok = _sysfs_ull_entry_from_part (part, "size", length);
> + if (!ok)
> + {
> + if (fd == -1)
> + fd = open (dev_name, O_RDONLY);
> + if (fd != -1 && ioctl (fd, BLKGETSIZE64, length))
> + ok = true;
> + }
> + if (ok)
> + *length /= part->disk->dev->sector_size;
> + if (fd != -1)
> + close (fd);
> +
> + 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
> @@ -2519,30 +2617,18 @@ _disk_sync_part_table (PedDisk* disk)
> const 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)) {
> + if (!_kernel_get_partition_start_and_length(part, &start, &length)) {
_kernel_get_partition_start_and_length udeclared -> see the typo above
> ped_exception_throw (
> - PED_EXCEPTION_BUG,
> - PED_EXCEPTION_CANCEL,
> + PED_EXCEPTION_BUG,
> + PED_EXCEPTION_CANCEL,
> _("Unable to determine the size and length of %s."),
> - dev_name);
dev_name undeclared
> - if (fd != -1)
> - close (fd);
> - free (dev_name);
> + dev_name);
> 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.5.4
>
Petr
--
Petr Uzel
IRC: ptr_uzl @ freenode
More information about the parted-devel
mailing list