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

[Petr Uzel]

On Thu, Dec 01, 2011 at 04:41:14PM -0500, Phillip Susi wrote:
> 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. 

Nice, I hope this ioctl will get to the kernel. HDIO_GETGEO is really
horrible and broken on 32 bit kernels.

> This allows for disks > 2TB and partitioned loop
> devices, which don't support HDIO_GETGEO.
> 
> This only removes one instance of the HDIO_GETGEO call in the
> partition table sync path that handles in use partitions.  The
> other users of this ioctl still need updated.
> 
> Signed-off-by: Phillip Susi <psusi at cfl.rr.com>
> ---
>  libparted/arch/linux.c |   41 ++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 3799b9d..6152906 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 (PedDisk* disk, int n, long long *start, long long *length)
> +{
> +        struct blkpg_partition  linux_part;
> +	int ret;
> +
> +        memset (&linux_part, 0, sizeof (linux_part));
> +        linux_part.pno = n;
> +        if (_blkpg_part_command (disk->dev, &linux_part,
> +				 BLKPG_GET_PARTITION))
> +	{
> +		*start = linux_part.start;
> +		*length = linux_part.length;
> +		return 1;
> +	} else return 0;
> +}
> +#endif

Looks good to me except mixing tabs and spaces indentation.

> +
>  /* 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. */
> @@ -2519,30 +2538,42 @@ _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;
> -                                /* get start and length of existing partition */
> +                                unsigned long long length;
> +				unsigned long long start;
>                                  char *dev_name = _device_get_part_path (disk->dev, i);
>                                  if (!dev_name)
>                                          goto cleanup;
> +                                /* get start and length of existing partition */
> +#ifdef BLKPG_GET_PARTITION
> +				if (!_blkpg_get_partition(disk, i, &start, &length)) {
> +#else
> +                                struct hd_geometry geom;
>                                  int fd = open (dev_name, O_RDONLY);
>                                  if (fd == -1
>  				    || ioctl (fd, HDIO_GETGEO, &geom)
>  				    || ioctl (fd, BLKGETSIZE64, &length)) {
> +#endif
>                                          ped_exception_throw (
>                                                               PED_EXCEPTION_BUG,
>                                                               PED_EXCEPTION_CANCEL,
>  			    _("Unable to determine the size and length of %s."),
>                                                               dev_name);
> +#ifndef BLKPG_GET_PARTITION
>                                          if (fd != -1)
>                                                  close (fd);
>                                          free (dev_name);
> +#endif
>                                          goto cleanup;
>                                  }
> +#ifdef BLKPG_GET_PARTITION
> +				start /= disk->dev->sector_size;
> +#else
>                                  free (dev_name);
> -                                length /= disk->dev->sector_size;
>                                  close (fd);
> -                                if (geom.start == part->geom.start
> +				start = geom.start;
> +#endif
> +                                length /= disk->dev->sector_size;
> +                                if (start == part->geom.start
>  				    && length == part->geom.length)
>                                          ok[i - 1] = 1;
>                                  /* If the new partition is unchanged and the


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



Petr

--
Petr Uzel
IRC: ptr_uzl @ freenode