From owner-svn-src-all@FreeBSD.ORG Thu Nov 20 05:06:21 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 4E134C54; Thu, 20 Nov 2014 05:06:21 +0000 (UTC) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id D5DA7C25; Thu, 20 Nov 2014 05:06:20 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 07CF81A1FB1; Thu, 20 Nov 2014 15:38:38 +1100 (AEDT) Date: Thu, 20 Nov 2014 15:38:37 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Warner Losh Subject: Re: svn commit: r274721 - head/sys/geom/part In-Reply-To: <201411191855.sAJItShA048381@svn.freebsd.org> Message-ID: <20141120150210.P976@besplex.bde.org> References: <201411191855.sAJItShA048381@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=dMCfxopb c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=Y1h1eLOWUWDwmyq3NvkA:9 a=NyhIRQUL_XerPQeh:21 a=Un_jFPcKyi9qFG51:21 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Nov 2014 05:06:21 -0000 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. 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