Date: Sun, 16 Mar 2008 12:50:55 -0600 From: "Rick C. Petty" <rick-freebsd@kiwi-computer.com> To: freebsd-geom@freebsd.org Subject: Re: [patch] geom_vinum platform fixes Message-ID: <20080316185055.GA64920@keira.kiwi-computer.com> In-Reply-To: <20080316090554.GA1230@carrot.studby.ntnu.no> References: <20080310052711.GA49676@keira.kiwi-computer.com> <20080313153551.82wlu8iio4088c44@webmail.ntnu.no> <20080313182257.GB14969@keira.kiwi-computer.com> <20080316090554.GA1230@carrot.studby.ntnu.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Mar 16, 2008 at 10:05:55AM +0100, Ulf Lilleengen wrote: > > I've reviewed the patch and done some modifications to it. I'll need some > testing first though (I don't have a testbed right now since I'm travelling). I've reviewed your patch but haven't had time to test it yet (hopefully this week..) > Here are some notes: > - Removed GV_VERSION. It is not used anywhere. Well, the point of this macro is for future changes to vinum on-disk structure, since I added version as part of the new magic. Strictly speaking, the macro isn't necessary (yet) and perhaps no future changes will happen to the structures. > - Avoid moving header includes around. Not sure I agree. Although style(9) doesn't specify that the sys/* headers should be alphabetized, many other places in kernel code have them sorted as such (with certain exceptions like sys/param). It was a trivial "fix" I had hoped to sneak in since that file was being modified anyway. > - Declare functions static when used inside one file. > - Style fixes. *nod* > - In gv_legacy_header_type, you can't be guaranteed that the hdr fields is > not always null (although I agree in practise its highly unlikely), but > it's hard to check this any other way, so I think it's alright :) I can't think of any _better_ way to check. In the struct timeval fields on amd64, I can't think of how these "zeros" would not appear. The bytes past the end of the i386 header will always be zero because of the malloc with M_ZERO. > - Use macro values instead of hard-coded values when testing legacy type. > - Pass data variable as argument rather than returning a header. > - Avoid memory allocation where possible. All good! > Please let me know how it fares, and thanks for your help btw :) I will do some testing using your patch. And thank _you_ for reviewing this. I really look forward to your upcoming merge from p4! -- Rick C. Petty
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080316185055.GA64920>