> On 18 Feb 2019, at 01:32, Ian Lepore <i...@freebsd.org> wrote:
> 
> Author: ian
> Date: Sun Feb 17 23:32:09 2019
> New Revision: 344238
> URL: https://svnweb.freebsd.org/changeset/base/344238
> 
> Log:
>  Restore loader(8)'s ability for lsdev to show partitions within a bsd slice.
> 
>  I'm pretty sure this used to work at one time, perhaps long ago.  It has
>  been failing recently because if you call disk_open() with dev->d_partition
>  set to -1 when d_slice refers to a bsd slice, it assumes you want it to
>  open the first partition within that slice.  When you then pass that open
>  dev instance to ptable_open(), it tries to read the start of the 'a'
>  partition and decides there is no recognizable partition type there.
> 
>  This restores the old functionality by resetting d_offset to the start
>  of the raw slice after disk_open() returns.  For good measure, d_partition
>  is also set back to -1, although that doesn't currently affect anything.
> 
>  I would have preferred to make disk_open() avoid such rude assumptions and
>  if you ask for partition -1 you get the raw slice.  But the commit history
>  shows that someone already did that once (r239058), and had to revert it
>  (r239232), so I didn't even try to go down that road.


What was the reason for the revert? I still do think the current disk_open() 
approach is not good because it does break the promise to get MBR partition 
(see common/disk.h). 

Of course I can see the appeal for something like “boot disk0:” but case like 
that should be solved by iterating partition table, and not by making API to do 
wrong thing - if I did ask to for disk0s0: I really would expect to get 
disk0s0: and not disk0s0a:

But anyhow, it would be good to understand the actual reasoning behind that 
decision.

rgds,
toomas

> 
> Modified:
>  head/stand/common/disk.c
> 
> Modified: head/stand/common/disk.c
> ==============================================================================
> --- head/stand/common/disk.c  Sun Feb 17 20:25:07 2019        (r344237)
> +++ head/stand/common/disk.c  Sun Feb 17 23:32:09 2019        (r344238)
> @@ -133,6 +133,13 @@ ptable_print(void *arg, const char *pname, const struc
>               dev.d_partition = -1;
>               if (disk_open(&dev, part->end - part->start + 1,
>                   od->sectorsize) == 0) {
> +                     /*
> +                      * disk_open() for partition -1 on a bsd slice assumes
> +                      * you want the first bsd partition.  Reset things so
> +                      * that we're looking at the start of the raw slice.
> +                      */
> +                     dev.d_partition = -1;
> +                     dev.d_offset = part->start;
>                       table = ptable_open(&dev, part->end - part->start + 1,
>                           od->sectorsize, ptblread);
>                       if (table != NULL) {
> 

_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to