Leo Weppelman <leo@wau.mis.ah.nl> writes:
> So what I would like to do is:
>     - move getmaxpartitions.[c3] to libc through cvscopy
>     - apply the attached diff which does:
>         - fix getdiskbyname()
>         - extend the d_partitions field of the disklabel to MAXMAXPARTITIONS
>           entries.
>         - bump the shlib version of libutil to 6.0
>         - move the getmaxpartitions() prototype from util.h to disktab.h

This, especially extending d_partitions, will likely cause problems
for a lot of ports that use the disklabel struct to describe their
on-disk labels.

e.g. from the i386 port:

        for (dlp = (struct disklabel *)bp->b_data;
            dlp <= (struct disklabel *)(bp->b_data + lp->d_secsize - sizeof(*dlp));
            dlp = (struct disklabel *)((char *)dlp + sizeof(long))) {
                if (dlp->d_magic == DISKMAGIC && dlp->d_magic2 == DISKMAGIC &&
                    dkcksum(dlp) == 0) {
                        *dlp = *lp;
                        bp->b_flags = B_BUSY | B_WRITE;
                        (*strat)(bp);
                        error = biowait(bp);
                        goto done;
                }
        }

Note the "*dlp = *lp" -- that's going to clobber a lot more than it
used to -- including memory outside the sector that's been read -- if
you expand the size of the disklabel.

Similarly, the readdisklabel code isn't going to initialize a whole
disklabel from the bits it reads off disk (or will read memory it
doesn't have access to).  This may cause leakage of somewhat-sensitive
information from the kernel, or may cause the system to crash, or...

Also, ports have to be adjusted to sanity check the number of
partitions claimed in the disklabel vs. the actual value of
MAXPARTITIONS.

This is made more complex because of the issues around in-core and
on-disk disklabels.  e.g. you want 'mbrlabel' to be able to use all
MAXMAXPARTITIONS for its faked-up in-core labels, but you don't want
more than MAXPARTITIONS partitions to be written out to an on-disk
label.

Finally, since we're truly decoupling the size of the disklabel struct
from MAXPARTITIONS, and saying that 'struct disklabel' no longer
fits in the places that it used to be stuck, why not go further and
bump MAXMAXPARTITIONS up to something larger, e.g. 64?  ("who'd want
more than 64 partitions on a disk?"  8-)


> Besides an opinion on the above, there are a few other related things that
> I have not yet decided upon:
> 
>    - Since the above changes affect sizeof(struct disklabel), the following
>      ioctls will stop working: DIOCGDINFO,DIOCSDINFO,DIOCWDINFO,DIOCGDEFLABEL.
>      Do we want to provide backward compatibility (through COMPAT_14) and
>      provide ODIOCGDINFO etc? The problem is that the handling of these are
>      scattered all over the kernel. Is there a better method than fixing all
>      these locations?

Uh, no, we don't want to, as far as i'm concerned we have to.  8-)



I think you're going in the right direction, but you're still closer
to the start of the journey than the end.  8-)


cgd
-- 
Chris Demetriou - cgd@netbsd.org - http://www.netbsd.org/People/Pages/cgd.html
Disclaimer: Not speaking for NetBSD, just expressing my own opinion.