On May 23, 2011, at 3:55 AM, Andrey V. Elsukov wrote:

> Hi,
> 
> Since after r221788 many people report about lost of access to their
> MBR partitions, i prepared new patch:
>       
>       http://people.freebsd.org/~ae/mbr_geometry.diff
> 
> 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.
> 
> 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.
> 
> 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 = MIN(pp->mediasize / pp->sectorsize, UINT32_MAX);
+       msize = 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 
:)_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-geom
To unsubscribe, send any mail to "[email protected]"

Reply via email to