Date: Fri, 22 Feb 2008 01:27:24 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andriy Gapon <avg@icyb.net.ua> Cc: freebsd-fs@freebsd.org, freebsd-current@freebsd.org, freebsd-geom@freebsd.org Subject: Re: newfs_msdos and dvd-ram (fwsectors, fwheads) Message-ID: <20080222005213.W5655@besplex.bde.org> In-Reply-To: <47BD6F39.7080105@icyb.net.ua> References: <47B81D07.7090208@icyb.net.ua> <47BD6F39.7080105@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 21 Feb 2008, Andriy Gapon wrote: > on 17/02/2008 13:39 Andriy Gapon said the following: >> Should newfs_msdos be able to work on "whole" cdX/acdX device ? >> [ufs/ffs] newfs can do it. >> But with newfs_msdos I had to run disklabel first and then I could >> create a filesystem on cdXa, but I couldn't do it on the whole disk. > > It seems that the reason for this newfs_msdos behavior is in the > following lines: > if (ioctl(fd, DIOCGSECTORSIZE, &dlp.d_secsize) == -1) > errx(1, "Cannot get sector size, %s", strerror(errno)); > if (ioctl(fd, DIOCGFWSECTORS, &dlp.d_nsectors) == -1) > errx(1, "Cannot get number of sectors, %s", strerror(errno)); > if (ioctl(fd, DIOCGFWHEADS, &dlp.d_ntracks)== -1) > errx(1, "Cannot get number of heads, %s", strerror(errno)); > > While a failure to get sector size is a serious situation indeed, number > of sectors per track and number of heads are just relics of the past and > are not applicable to all types of should-be-supported media. > What's even more funny is that those values can be set via command line > options and in that case values from ioctl are not used at all. Also, it needs the BIOS geometry, but has been broken to ask for, and normally use, the firmware geometry. Setting the values on the command line is a fairly easy workaround. It's actually better for the ioctls and make you give the correct parameters on the command line instead of succeeding and giving wrong values. The "hidden sectors" calculation has also been broken. This can be fixed by setting the value on the command line too. The correct setting is normally or always the offset of the partition in sectors. > Because those numbers are used by msdosfs on-disk structures we have to > provide some reasonable values for them even if they are meaningless > with respect to physical media organization. > An example of simple calculations can be found in bsdlabel.c. Those don't give the BIOS geometry, which isn't a problem in disklabel since disklabel doesn't need any geometry, but newfs_msdos, fsck_msdosfs fdisk and sysinstall need more. newfs_msdos, fdisk and sysinstall used to read values from the label (including the label for the whole disk), and these parameters used to be set to the kernel's best guess at the BIOS geometry, so that everything saw a consistent geometry. fsck_msdosfs has never missed not being able to determine the correct values, since it has never checked or fixed the values. WinXP silently fixes at least "hidden sectors" at boot time. > I see two approaches: > 1) fake those properties in device drivers, primarily cd(4) and acd(4); > benefit: this can help other utilities besides newfs_msdos > 2) fake those properties in newfs_msdof; > benefit: this would help with other physical devices that can host > FAT; > > Or maybe do both. This won't help for other utilities that need the BIOS geometry. I use the following fixes in newfs_msdos. They reduce mainly to complaints about the breakage and trying alternative ioctls so that the same binary has some chance of working on new and old versions of FreeBSD. I didn't restore the DIOCGSLICEINFO ioctl or the code that uses it to determine "hidden sectors". Another possible fix for this is to use the NetBSD version, which last time I looked was just the FreeBSD version with all the unportable ioctls and code ifdefed out. The 2004/09/24 version of it reduces to assuming that the correct (BIOS) values are in the label. It attempts to calculate "hidden sectors" where FreeBSD now sets it to 0. I think this works iff the offsets in the label are absolute. FreeBSD used to virtualize these offsets to well to be 0-based, and I think these offsets can be physically 0-based now. It is necessary to know the base, which my unimplemented DICOMEDIAOFFSET ioctl returns. % Index: newfs_msdos.c % =================================================================== % RCS file: /home/ncvs/src/sbin/newfs_msdos/newfs_msdos.c,v % retrieving revision 1.20 % diff -u -2 -r1.20 newfs_msdos.c % --- newfs_msdos.c 17 Feb 2004 02:02:18 -0000 1.20 % +++ newfs_msdos.c 22 Apr 2004 10:36:28 -0000 % @@ -711,56 +711,165 @@ % struct bpb *bpb) % { % - struct disklabel *lp, dlp; % - struct fd_type type; % - off_t ms, hs; % - % - lp = NULL; % - % - /* If the user specified a disk type, try to use that */ % + struct fd_type fdtype; % + struct disklabel label, *lp; % + off_t ms, mo; % + u_int heads, secpertrack, secsize; % + u_int cruftmap, fwheads, fwsectors; % + % +#define NO_MEDIASIZE (1 << 0) % +#define NO_MEDIAOFFSET (1 << 1) % +#define NO_SECTORSIZE (1 << 2) % +#define NO_HEADS (1 << 3) % +#define NO_SECPERTRACK (1 << 4) % +#define NO_FWHEADS (1 << 5) % +#define NO_FWSECTORS (1 << 6) % +#define NO_FDTYPE (1 << 7) % +#define NO_LABEL (1 << 8) % + cruftmap = 0; % + % + /* First, try to use only the "right" primitive ioctls. */ % + if (ioctl(fd, DIOCGMEDIASIZE, &ms) == -1) % + cruftmap |= NO_MEDIASIZE; % +#ifdef DIOCGMEDIAOFFSET % + if (ioctl(fd, DIOCGMEDIAOFFSET, &mo) == -1) % +#endif % + cruftmap |= NO_MEDIAOFFSET; % + if (ioctl(fd, DIOCGSECTORSIZE, &secsize) == -1) % + cruftmap |= NO_SECTORSIZE; % +#ifdef DIOCGHEADS % + if (ioctl(fd, DIOCGHEADS, &nheads)== -1) % +#endif % + cruftmap |= NO_HEADS; % +#ifdef DIOCGSECPERTRACK % + if (ioctl(fd, DIOCGSECPERTRACK, &secpertrack) == -1) % +#endif % + cruftmap |= NO_SECPERTRACK; % + % + /* Prepare fallbacks. */ % + if (ioctl(fd, DIOCGFWHEADS, &fwheads) == -1) % + cruftmap |= NO_FWHEADS; % + if (ioctl(fd, DIOCGFWSECTORS, &fwsectors) == -1) % + cruftmap |= NO_FWSECTORS; % + if (ioctl(fd, FD_GTYPE, &fdtype) == -1) % + cruftmap |= NO_FDTYPE; % if (dtype != NULL) { % lp = getdiskbyname(dtype); % - hs = 0; % + if (lp == NULL) % + errx(1, "%s: unknown disk type", dtype); % + label = *lp; % + } else if (ioctl(fd, DIOCGDINFO, &label) == -1) % + cruftmap |= NO_LABEL; % + % + /* % + * Preliminary fallback for firmware heads and sectors: use floppy ones. % + * Non-GEOM drivers don't have DIOCGFWHEADS or DIOCGFWSECTORS, and % + * we know how to recover for the fd driver only. We could get the % + * media size and sector size from fdtype too, but shouldn't need them. % + */ % + if ((cruftmap & (NO_FWHEADS | NO_FDTYPE)) == NO_FWHEADS) { % + fwheads = fdtype.heads; % + cruftmap &= ~NO_FWHEADS; % } % - % - /* Maybe it's a floppy drive */ % - if (lp == NULL) { % - if (ioctl(fd, DIOCGMEDIASIZE, &ms) == -1) % - errx(1, "Cannot get disk size, %s", strerror(errno)); % - if (ioctl(fd, FD_GTYPE, &type) != -1) { % - dlp.d_secsize = 128 << type.secsize; % - dlp.d_nsectors = type.sectrac; % - dlp.d_ntracks = type.heads; % - dlp.d_secperunit = ms / dlp.d_secsize; % - lp = &dlp; % - hs = 0; % - } % + if ((cruftmap & (NO_FWSECTORS | NO_FDTYPE)) == NO_FWSECTORS) { % + fwsectors = fdtype.sectrac; % + cruftmap &= ~NO_FWSECTORS; % } % % - /* Maybe it's a fixed drive */ % - if (lp == NULL) { % - if (ioctl(fd, DIOCGDINFO, &dlp) == -1) { % - if (ioctl(fd, DIOCGSECTORSIZE, &dlp.d_secsize) == -1) % - errx(1, "Cannot get sector size, %s", strerror(errno)); % - if (ioctl(fd, DIOCGFWSECTORS, &dlp.d_nsectors) == -1) % - errx(1, "Cannot get number of sectors, %s", strerror(errno)); % - if (ioctl(fd, DIOCGFWHEADS, &dlp.d_ntracks)== -1) % - errx(1, "Cannot get number of heads, %s", strerror(errno)); % - dlp.d_secperunit = ms / dlp.d_secsize; % + /* Prefer to fall back to label info. */ % + if ((cruftmap & (NO_MEDIASIZE | NO_LABEL)) == NO_MEDIASIZE) { % + /* This shouldn't be needed. */ % + ms = label.d_secperunit * (off_t)label.d_secsize; % + cruftmap &= ~NO_MEDIASIZE; % + } % + if (cruftmap & NO_MEDIAOFFSET) { % +#ifdef NO_GEOM_LOSSAGE % + if (!(cruftmap | NO_LABEL)) { % + /* % + * XXX: lost non-GEOM code that handled this. The label info or % + * currently implemented primitive ioctls suffice for most file % + * systems, but msdosfs wants the absolute disk offset for one % + * thing (the number of hidden sectors). We can still use the old % + * code plus yet more compatibility cruft. In the GEOM case, GEOM % + * sysctl output must be parsed as in libdisk, or perhaps libdisk % + * can be used. libdisk takes a measly 300-400 lines and doesn't % + * seem to support more than the 2 layers (slice/partition) needed % + * for i386's, and demonstrates that xml is too hard to use by not % + * using it. % + * % + * Strangely, GEOM's breakage of compatibility for labels makes it % + * easy to determine the absolute disk offset here in some cases: % + * GEOM returns raw disk offsets in label.d_partitions[part].p_offset; % + * these already include the slice offset if the label is backwards % + * compatible, and we can determine if it is backwards compatible % + * by looking at the offset of the raw partition (until GEOM breaks % + * compatibility of that too). We would still need messy parsing % + * of the disk name to determine its partition number, if any. % + */ % + mo = ds.dss_slices[slice].ds_offset * (off_t)label.d_secsize; % + mo += label.d_partitions[part].p_offset * (off_t)label.d_secsize; % + cruftmap &= ~NO_MEDIAOFFSET; % } % +#else % + mo = 0; % + cruftmap &= ~NO_MEDIAOFFSET; % +#endif % + } % + if ((cruftmap & (NO_SECTORSIZE | NO_LABEL)) == NO_SECTORSIZE) { % + /* This shouldn't be needed. */ % + secsize = label.d_secsize; % + cruftmap &= ~NO_SECTORSIZE; % + } % + if ((cruftmap & (NO_HEADS | NO_LABEL)) == NO_HEADS) { % + heads = label.d_ntracks; % + cruftmap &= ~NO_HEADS; % + } % + if ((cruftmap & (NO_SECPERTRACK | NO_LABEL)) == NO_SECPERTRACK) { % + secpertrack = label.d_nsectors; % + cruftmap &= ~NO_SECPERTRACK; % + } % % - hs = (ms / dlp.d_secsize) - dlp.d_secperunit; % - lp = &dlp; % + /* % + * Fall back to "firmware" heads and sectors. % + * % + * XXX: this is quite broken, since we want the BIOS numbers, not the % + * firmware ones. GEOM has broken support for determining the BIOS % + * numbers in fdisk(8) and sysinstall(8) even more by changing things % + * to supply and use only the firmware numbers. Here we have a % + * preferred fallback to the numbers in the label, which can at least % + * be edited to give the BIOS numbers if they don't have them already. % + * Old labels defaulted to having BIOS numbers if possible, but GEOM % + * has got at new labels. % + */ % + if ((cruftmap & (NO_HEADS | NO_FWHEADS)) == NO_HEADS) { % + heads = fwheads; % + cruftmap &= ~NO_HEADS; % } % + if ((cruftmap & (NO_SECPERTRACK | NO_FWSECTORS)) == NO_SECPERTRACK) { % + secpertrack = fwsectors; % + cruftmap &= ~NO_SECPERTRACK; % + } % + % + /* % + * XXX this is too strict. We don't necessarily need the full disk % + * type, since we may have specified parts of it on the command line. % + * % + * Also, we don't support specifying the disk type in the normal way % + * (-T option in newfs). % + */ % + if ((cruftmap & (NO_MEDIASIZE | NO_MEDIAOFFSET | NO_SECTORSIZE | % + NO_HEADS | NO_SECPERTRACK)) != 0) % + warnx("%s: can't determine disk info; disk type must be specified", % + fname); % % if (bpb->bps == 0) % - bpb->bps = ckgeom(fname, lp->d_secsize, "bytes/sector"); % + bpb->bps = ckgeom(fname, secsize, "bytes/sector"); % if (bpb->spt == 0) % - bpb->spt = ckgeom(fname, lp->d_nsectors, "sectors/track"); % + bpb->spt = ckgeom(fname, secpertrack, "sectors/track"); % if (bpb->hds == 0) % - bpb->hds = ckgeom(fname, lp->d_ntracks, "drive heads"); % + bpb->hds = ckgeom(fname, heads, "drive heads"); % if (bpb->bsec == 0) % - bpb->bsec = lp->d_secperunit; % + bpb->bsec = ms / secsize; % if (bpb->hid == 0) % - bpb->hid = hs; % + bpb->hid = mo / secsize; % } % % @@ -802,5 +911,5 @@ % errx(1, "%s: no default %s", fname, msg); % if (val > MAXU16) % - errx(1, "%s: illegal %s %d", fname, msg, val); % + errx(1, "%s: illegal %s value %u", fname, msg, val); % return val; % } Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080222005213.W5655>