[parted-devel] [PATCH 3/3] Use new BLKPG_GET_PARTITION ioctl to get partition start sector

Petr Uzel petr.uzel at suse.cz
Tue Dec 6 11:31:26 UTC 2011


On Mon, Dec 05, 2011 at 06:09:40PM -0500, Phillip Susi wrote:
> On 12/05/2011 01:32 PM, Petr Uzel wrote:
> >> I merged your patch with mine.  
> > 
> >> How does this look?
> > 
> > You did not try to compile it, did you? :)
> 
> No, that was my first draft.  How's this look?

Overall, it looks good to me. I especially like the simplification of
_disk_sync_part_table(). I'm attaching an incremental patch fixing
some (mostly stylistic) issues which I spotted.

Do you have any feedback from the kernel developers regarding the
GET_PARTITION ioctl() ?


> 

> From 3d5047e5e78bc0b4957877d530d2392e7e268f5a Mon Sep 17 00:00:00 2001
> From: Phillip Susi <psusi at cfl.rr.com>
> Date: Mon, 5 Dec 2011 18:02:28 -0500
> Subject: [PATCH 3/3] 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 |  134 +++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 111 insertions(+), 23 deletions(-)
> 
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 3799b9d..460d0e8 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2410,6 +2410,26 @@ _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 +2452,92 @@ _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_length(PedPartition *part, 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);
> +        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)) {
> +                        *start = geom.start;
> +                        ok = true;
> +                }
> +        }
> +        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);
> +
> +        if (!ok)
> +                ped_exception_throw (
> +                        PED_EXCEPTION_BUG,
> +                        PED_EXCEPTION_CANCEL,
> +                        _("Unable to determine the size 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
> @@ -2516,33 +2622,15 @@ _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.5.4
> 




Petr

--
Petr Uzel
IRC: ptr_uzl @ freenode
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libparted-start-length-incremental.diff
Type: text/x-patch
Size: 2595 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/parted-devel/attachments/20111206/8b3533d8/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/parted-devel/attachments/20111206/8b3533d8/attachment.pgp>


More information about the parted-devel mailing list