svn commit: r274721 - head/sys/geom/part

[Bruce Evans]

On Wed, 19 Nov 2014, Warner Losh wrote:

> Log:
>  The number of BSD partitions is variable. Return the proper number
>  (which is in basetable->gpt_entries).
>
>  Submitted by: ae@
>
> Modified:
>  head/sys/geom/part/g_part_bsd.c
>
> Modified: head/sys/geom/part/g_part_bsd.c
> ==============================================================================
> --- head/sys/geom/part/g_part_bsd.c	Wed Nov 19 18:19:21 2014	(r274720)
> +++ head/sys/geom/part/g_part_bsd.c	Wed Nov 19 18:55:27 2014	(r274721)
> @@ -521,7 +521,7 @@ g_part_bsd_ioctl(struct g_part_table *ba
>
> 		table = (struct g_part_bsd_table *)basetable;
> 		p = table->bbarea + pp->sectorsize;
> -		return (bsd_disklabel_le_dec(p, data, MAXPARTITIONS));
> +		return (bsd_disklabel_le_dec(p, data, basetable->gpt_entries));
> 	}
> 	default:
> 		return (ENOIOCTL);

How can this work?  I think you are implementing DIOCGDINFO.  This returns
a struct disklabel, which is limited by binary compatibility to the
historical number of partitions (8).  So this ioctl just cannot support
more than 8 partitions.  The best that could happen is for
bsd_disklabel_le_dec() to return an error in this case.  But it actually
does essentially the reverse -- you pass it the maximum supported by the
ioctl, and it returns an error if the actual number is larger.  So
MAXPARTITIONS was correct if this is to implement DIOCGDINFO.

I thought that binary compatibility was already broken by expanding
MAXPARTITIONS, but actually it is still 8 (modulo garbage ifdefs and
comments), but actually there is a new struct disklabel64
with MAXPARTITIONS64 = 16 to expand by a bit.  I don't understand
the plumbing for this.  There don't seem to be any related ioctls.
I think all it could be used for is to start with a disk with 16
partitions and translate to another format which can handle that many,
and use only ioctls for the other format.  The reverse translation to
formats that can't handle that many is impossible.

<sys/disklabel.h> still contains the following garbage related to this.
The garbage hasn't changed since at least FreeBSD-1; it was presumably
in Net/2:

% #ifndef MAXPARTITIONS
% #define	MAXPARTITIONS	8
% #endif

Actually changing this would break binary compatibility.

MAXPARTITIONS isn't even a bogus option (in /sys/conf/options), so this
is not easy to change.  Editing the file and recompiling the world is
the only safe way to change it.

% struct disklabel {
% ...
% 	u_int16_t d_npartitions;	/* number of partitions in following */
% 	u_int32_t d_bbsize;		/* size of boot area at sn0, bytes */
% 	u_int32_t d_sbsize;		/* max size of fs superblock, bytes */
% 	struct partition {		/* the partition table */
% 		u_int32_t p_size;	/* number of sectors in partition */
% 		u_int32_t p_offset;	/* starting sector */
% 		u_int32_t p_fsize;	/* filesystem basic fragment size */
% 		u_int8_t p_fstype;	/* filesystem type, see below */
% 		u_int8_t p_frag;	/* filesystem fragments per block */
% 		u_int16_t p_cpg;	/* filesystem cylinders per group */
% 	} d_partitions[MAXPARTITIONS];	/* actually may be more */

Actually, it MUST not be more.  It may be more on disk (d_npartitions gives
the number), but then none of the ioctls can support it.

% };

Bruce