Date: Mon, 23 May 2011 10:14:45 -0600 From: Warner Losh <imp@bsdimp.com> To: "Andrey V. Elsukov" <bu7cher@yandex.ru> Cc: Marcel Moolenaar <marcel@freebsd.org>, Warner Losh <imp@freebsd.org>, Poul-Henning Kamp <phk@freebsd.org>, freebsd-geom@freebsd.org Subject: Re: [RFC] Remove requirement of alignment to track from MBR scheme Message-ID: <6DF62987-141B-4BB3-8E8D-9966EBAC828B@bsdimp.com> In-Reply-To: <4DDA2F0B.2040203@yandex.ru> References: <4DDA2F0B.2040203@yandex.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 23, 2011, at 3:55 AM, Andrey V. Elsukov wrote: > Hi, >=20 > Since after r221788 many people report about lost of access to their > MBR partitions, i prepared new patch: > =09 > http://people.freebsd.org/~ae/mbr_geometry.diff >=20 > It removes from GEOM_PART_MBR constraints to alignment to track. > Now it is possible to create MBR partitions with exactly specified > start offset and size, and they will not be recalculated by kernel. >=20 > Also the patch adds new option "-g" to the gpart(8) utility. This > option can be specified for "add" and "resize" subcommands. > gpart(8) uses information about provider's "geometry" and does > partition alignment how it did before for MBR. >=20 > With these changes we give to users the choice how align their > partitions and also we still able to use some "broken" partition > tables. This looks fairly good. I have one or two minor questions... Since we don't have a good review tool for the project, this may seem a = little fragmented, but here goes: + if (pp->sectorsize > 4096) + return (EOPNOTSUPP); What's the motivation for this? This seems unrelated to the problem at = hand. It may be good, but 4096-byte sector size is very new and I'm not = sure MS-DOS can cope... :) There has been a long tradition in the MBR, = however, of supporting different sector sizes to allow old-old-old = school floppies that had 128 or 256 byte sectors to work, as well as = allowing the supper-compressed 1.77MB floppies to work with their = 1024-byte (or was that 2048-byte?) sectors. Not sure this really = matters for reading, and for writing this case is so far beyond the edge = I'm not even sure why I bring it up[*] In any event, I'd be tempted to use a #define for 4096 like = MBR_MAX_SECTOR_SIZE. - msize =3D MIN(pp->mediasize / pp->sectorsize, UINT32_MAX); + msize =3D MIN(pp->mediasize / pp->sectorsize, 2 * UINT32_MAX); Why this change? I think that it is in two places. The rest looks good. Thanks for coming up with such a complete patch = after my grumpy email... Warner [*] I guess old farts like to bring up this stuff to show how old they = are :)=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6DF62987-141B-4BB3-8E8D-9966EBAC828B>