Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Jun 2012 13:37:11 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        "Andrey V. Elsukov" <bu7cher@yandex.ru>
Cc:        freebsd-hackers <freebsd-hackers@freebsd.org>, Doug Rabson <dfr@freebsd.org>, freebsd-current <freebsd-current@freebsd.org>, Pawel Jakub Dawidek <pjd@freebsd.org>, Andriy Gapon <avg@freebsd.org>
Subject:   Re: [CFC/CFT] large changes in the loader(8) code
Message-ID:  <201206261337.11741.jhb@freebsd.org>
In-Reply-To: <4FE9B01C.30306@yandex.ru>
References:  <4FE9B01C.30306@yandex.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, June 26, 2012 8:50:36 am Andrey V. Elsukov wrote:
> Hi All,
> 
> Some time ago i have started reading the code in the sys/boot.
> Especially i'm interested in the partition tables handling.
> I found several problems:
> 1. There are several copies of the same code in the libi386/biosdisk.c
> and common/disk.c, and partially libpc98/biosdisk.c.
> 2. ZFS probing is very slow, because the ZFS code doesn't know how many
> disks and partitions the system has:
> 	http://www.freebsd.org/cgi/query-pr.cgi?pr=148296
> 	http://www.freebsd.org/cgi/query-pr.cgi?pr=161897
> 3. The GPT support doesn't check CRC and even doesn't know anything
> about the secondary GPT header/table.
> 
> So, i have created the branch and committed the changes:
> 	http://svnweb.freebsd.org/base/user/ae/bootcode/
> The patch is here:
> 	http://people.freebsd.org/~ae/boot.diff
> 
> What i already did:
> 1. The partition tables handling now is machine independent,
> and it is compatible with the kernel's GEOM_PART implementation.
> There is new API for disk drivers in the loader to get information
> about partitions and tables:
>         common/Makefile.inc
>  	common/part.c
> 	common/part.h
> 
> 2. The similar and general code from the disk drivers merged in the
> disk.c:
>         common/disk.c
>         common/disk.h
>         i386/libi386/libi386.h
>         i386/libi386/biosdisk.c
>         userboot/test/test.c
>         userboot/userboot/userboot_disk.c
>         userboot/userboot.h
> 3. ZFS code now uses new API and probing on the systems with many disks
> should be greatly increased:
>         zfs/zfs.c
>         i386/loader/main.c
> 4. The gptboot now searches the backup GPT header in the previous sectors,
> when it finds the "GEOM::" signature in the last sector. PMBR code also
> tries to do the same:
>         common/gpt.c
>         i386/pmbr/pmbr.s

GPT really wants the backup header at the last LBA.  I know you can set it, 
but I've interpreted that as a way to see if the primary header is correct or 
not.  It seems to me that GPT tables created in this fashion (inside a GEOM 
provider) will not work properly with partition editors for other OS's.  I'm 
hesitant to encourage the use of this as I do think putting GPT inside of a 
gmirror violates the GPT spec.

> 5. Also the pmbr image now contains one fake partition record.
> When several first sectors are damaged the kernel can't detect GPT
> (see RECOVERING section in the gpart(8)). We can restore PMBR with dd(1)
> command, but the old pmbr image has an empty partition table and
> loader doesn't able to boot from GPT, when there is no partition record
> in the PMBR. Now it will be able. When pmbr is installed via 'gpart 
bootcode'
> command, the kernel correctly modifies this partition record. So, this is 
only
> for the first rescue step.

As I said earlier, I do not think this is appropriate and that instead
gpart should have an appropriate 'recover' command to install just the pmbr on 
a disk and also create a correct entry in the MBR if needed while doing so.

> 6. I have changed userboot interface. I guess there is none consumers except
> the one test program. But if it isn't that, i can make it compatible.

One other consumer is in the bhyve branch.  I think the 'kload' patches also 
use it.  However, they can probably be adapted easily.

[ Note, I haven't done a detailed review of the patch at all yet. ]

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201206261337.11741.jhb>