From owner-freebsd-hackers@FreeBSD.ORG Mon May 7 14:05:30 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 84E46106564A; Mon, 7 May 2012 14:05:30 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 589578FC0C; Mon, 7 May 2012 14:05:30 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id C1E01B95B; Mon, 7 May 2012 10:05:29 -0400 (EDT) From: John Baldwin To: freebsd-fs@freebsd.org Date: Mon, 7 May 2012 09:53:03 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p13; KDE/4.5.5; amd64; ; ) References: <4F8999D2.1080902@FreeBSD.org> <4FA4F36A.6030903@FreeBSD.org> <4FA4F883.2060008@FreeBSD.org> In-Reply-To: <4FA4F883.2060008@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201205070953.04032.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 07 May 2012 10:05:29 -0400 (EDT) Cc: freebsd-hackers@freebsd.org, Andriy Gapon Subject: Re: [review request] zfsboot/zfsloader: support accessing filesystems within a pool X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 May 2012 14:05:30 -0000 On Saturday, May 05, 2012 5:53:07 am Andriy Gapon wrote: > on 05/05/2012 12:31 Andriy Gapon said the following: > > on 04/05/2012 18:25 John Baldwin said the following: > >> On Thursday, May 03, 2012 11:23:51 am Andriy Gapon wrote: > >>> on 03/05/2012 18:02 Andriy Gapon said the following: > >>>> > >>>> Here's the latest version of the patches: > >>>> http://people.freebsd.org/~avg/zfsboot.patches.4.diff > >>> > >>> I've found a couple of problems in the previous version, so here's another one: > >>> http://people.freebsd.org/~avg/zfsboot.patches.5.diff > >>> The important change is in the first patch (__exec args). > >> > >> A few comments/suggestions on the args bits: > > > > John, > > > > these are excellent suggestions! Thank you! > > The new patchset: http://people.freebsd.org/~avg/zfsboot.patches.7.diff Looks great, thanks! A few replies below: > >> - Add a CTASSERT() in loader/main.c that BI_SIZE == sizeof(struct bootinfo) > > > > I have added a definition of CTASSERT to boostrap.h as it was not available for > > sys/boot and there were two local definitions of the macro in individual files. > > > > However the assertion would fail right now. > > The backward-compatible value of BI_SIZE (72 == 0x48) covers only part of the > > fields in struct bootinfo, those up to the following comment: > > /* Items below only from advanced bootloader */ > > > > I am a little bit hesitant: should I increase BI_SIZE to cover the whole struct > > bootinfo or should I compare BI_SIZE to offsetof bi_kernend? Actually, we should probably be reading the 'bi_size' field and not using a BI_SIZE constant at all? Looks like only the non-functional EFI boot loader doesn't set bi_size (and it should just be fixed to do so since it needs to pass new fields in anyway). > > I've decided to define ARGADJ in the new common header, then I've had to rename > > btxcsu.s to .S, so that the preprocessing is executed for it. Ok. Maybe add one comment to the bootargs.h head to explain that the 'bootargs' struct starts at ARGOFF and can grow up, while struct bootinfo is copied such that it's end is at the top of the argument area and grows down. Also, at some point we could use a genassym.c file ala the kernel builds to generate some of the constants in bootargs.h instead (e.g. the offsets of fields within structures, and BA_SIZE, though we probably want to ensure that BA_SIZE never changes). -- John Baldwin