[parted-devel] [PATCH] better fix for 'partprobe gives errors with dvh partition no. >16'

[Joel Granados]

----- "Petr Uzel" <petr.uzel at suse.cz> wrote:

> On Wed, Nov 05, 2008 at 09:40:27AM +0100, Petr Uzel wrote:
> > There's a bug in current development version of parted which causes
> that the
> > partition table, as seen by the kernel, is not updated correctly
> after deleting
> > a partition. This bug was introduced in commit
> > f6bd20573e3ecfb63f62d88c52a0870fb8851b59 (fix computation of largest
> partition
> > number). Yes, I know - this patch was proposed by me - sorry for
> that :)
> <SNIP>
> > Before the broken commit was checked in, parted used PED_MAX(16,
> last_partition_num())
> > to determine which partitions might be changed. But this also is not
> correct, because : 
> > 1. for <= 16 partitions on the disk, the last_partition_num() call
> is useless 
> > 2. if there's >16 partitions, the last_partition_num() might return
> inconsistent
> > result (in case the last partition was just deleted)
> > 
> > I know that there may be situations where there are more than 16
> partitions on
> > the disk, but does parted want to support this? If not, the
> PED_MAX(16, last_partition_num())
> > might be changed to be just 16 and everything should work fine.
> 
>                         ^^^^^^^^
> If you decide not to go this way, there will still be the DVH bug
> that the broken patch tried to fix. So better fix follows:
> 
> First revert the broken commit and one commit that depend on it:
> 
> From cca6449a30db22545a217ac779d1cf60ad6d3e1f Mon Sep 17 00:00:00
> 2001
> From: Petr Uzel <petr.uzel at suse.cz>
> Date: Tue, 4 Nov 2008 15:11:54 +0100
> Subject: [PATCH] reverted 1742b051d78493b90bcb801e68a2be0277bcf72f and
> f6bd20573e3ecfb63f62d88c52a0870fb8851b59
> 
> ---
>  libparted/arch/linux.c |   14 +++-----------
>  1 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 83e24c8..0107dd2 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2247,15 +2247,11 @@ _blkpg_remove_partition (PedDisk* disk, int
> n)
>  static int
>  _disk_sync_part_table (PedDisk* disk)
>  {
> -        int largest_partnum = ped_disk_get_last_partition_num
> (disk);
> -        if (largest_partnum <= 0)
> -          return 1;
> -
> -        int     last = PED_MIN (largest_partnum, 16);
> +        int     i;
> +        int     last = PED_MAX (ped_disk_get_last_partition_num
> (disk), 16);
>          int*    rets = ped_malloc(sizeof(int) * last);
>          int*    errnums = ped_malloc(sizeof(int) * last);
>          int     ret = 1;
> -        int     i;
>  
>          for (i = 1; i <= last; i++) {
>                  rets[i - 1] = _blkpg_remove_partition (disk, i);
> @@ -2473,12 +2469,8 @@ err:
>  static int
>  _dm_reread_part_table (PedDisk* disk)
>  {
> -        int largest_partnum = ped_disk_get_last_partition_num
> (disk);
> -        if (largest_partnum <= 0)
> -          return 1;
> -
>          int     rc = 1;
> -        int     last = PED_MIN (largest_partnum, 16);
> +        int     last = PED_MAX (ped_disk_get_last_partition_num
> (disk), 16);
>          int     i;
>  
>          sync();
> -- 
> 1.6.0.2
> 
> 
> The fix itself. I believe that the comment in the patch describes
> sufficiently what it does.
> 
> From cd27f73beb0c8d2aa7f28597ec4bf4f990f3b48f Mon Sep 17 00:00:00
> 2001
> From: Petr Uzel <petr.uzel at suse.cz>
> Date: Thu, 6 Nov 2008 16:26:59 +0100
> Subject: [PATCH] DVH: do not inform kernel/DM about directory entries
> 
> ---
>  libparted/arch/linux.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 0107dd2..041208a 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2248,7 +2248,15 @@ static int
>  _disk_sync_part_table (PedDisk* disk)
>  {
>          int     i;
> -        int     last = PED_MAX (ped_disk_get_last_partition_num
> (disk), 16);
> +        int     last;
> +
> +	/* parted treats DVH directory entries as logical partitions with
> number > 16;
> +	 * we don't want to inform kernel about directory entries
> +	 */
> +	if (strcmp (disk->type->name, "dvh") == 0)
> +		last = 16;
> +	else
> +	 	last = PED_MAX (ped_disk_get_last_partition_num (disk), 16);
>          int*    rets = ped_malloc(sizeof(int) * last);
>          int*    errnums = ped_malloc(sizeof(int) * last);
>          int     ret = 1;
> @@ -2470,8 +2478,16 @@ static int
>  _dm_reread_part_table (PedDisk* disk)
>  {
>          int     rc = 1;
> -        int     last = PED_MAX (ped_disk_get_last_partition_num
> (disk), 16);
>          int     i;
> +        int     last;
> +
> +	/* parted treats DVH directory entries as partitions logical with

 the comment here would read "logical partitions" instead of "partition logical".

> number > 16;
> +	 * we don't want to inform kernel about directory entries
> +	 */
> +	if (strcmp (disk->type->name, "dvh") == 0)
> +		last = 16;
> +	else
> +	 	last = PED_MAX (ped_disk_get_last_partition_num (disk), 16);

There is an additional space in this line that should not be there.

>  
>          sync();
>          if (!_dm_remove_parts(disk->dev))
> -- 

I ask this: Are we suppose to be treating the DVH directories as partitions?  If not, I think the patch should be somewhere else.  maybe in the dvh.* files.

Regards.


-- 
Joel Andres Granados
Red Hat / Brno Czech Republic