From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1KazhA-0004g5-Us for mharc-grub-devel@gnu.org; Wed, 03 Sep 2008 17:15:20 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kazh8-0004fp-G6 for grub-devel@gnu.org; Wed, 03 Sep 2008 17:15:18 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kazh6-0004fE-LK for grub-devel@gnu.org; Wed, 03 Sep 2008 17:15:17 -0400 Received: from [199.232.76.173] (port=43309 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kazh6-0004f1-FF for grub-devel@gnu.org; Wed, 03 Sep 2008 17:15:16 -0400 Received: from moutng.kundenserver.de ([212.227.126.183]:59566) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Kazh6-0003PO-1n for grub-devel@gnu.org; Wed, 03 Sep 2008 17:15:16 -0400 Received: from [85.180.19.197] (e180019197.adsl.alicedsl.de [85.180.19.197]) by mrelayeu.kundenserver.de (node=mrelayeu6) with ESMTP (Nemesis) id 0ML29c-1Kazh43gHC-0001td; Wed, 03 Sep 2008 23:15:15 +0200 From: Felix Zielcke To: The development of GRUB 2 In-Reply-To: <87wshzhkh7.fsf@xs4all.nl> References: <20080828111405.GD8224@gagh.ehv.newtec.eu> <1219922711.4598.49.camel@fz.local> <20080829090019.GA6721@gagh.ehv.newtec.eu> <87wshzhkh7.fsf@xs4all.nl> Content-Type: text/plain Date: Wed, 03 Sep 2008 23:15:13 +0200 Message-Id: <1220476513.21922.7.camel@fz.local> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 7bit X-Provags-ID: V01U2FsdGVkX18NqnfixsfmcvZa0nfqBXU8VwQx89SYfTCHhyn Q/+A+kQTvRshHGGEwBVYZcVafmkEBmTbMa0ndB0JN1iI0OeZLC ddofpp1uVtMp8+W4Vdep9zm3z1DI+VC X-detected-kernel: by monty-python.gnu.org: Linux 2.6? (barebone, rare!) Subject: Re: grub 1.96 svn 20080813 and circular lvm2 metadata X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Sep 2008 21:15:18 -0000 Hans, could you please address Marco's issues and send a new patch so the topic is brought up again? Am Freitag, den 29.08.2008, 18:08 +0200 schrieb Marco Gerards: > > > diff -uwr grub-1.96_svn20080813-org/ChangeLog grub-1.96_svn20080813-new/ChangeLog > > --- grub-1.96_svn20080813-org/ChangeLog 2008-08-13 17:24:36.000000000 +0200 > > +++ grub-1.96_svn20080813-new/ChangeLog 2008-08-29 10:33:03.000000000 +0200 > > @@ -1,3 +1,8 @@ > > +2008-08-28 Hans Lambermont (tiny change) > > + Jan Derk Gerlings (tiny change) > > + > > + * disk/lvm.c: Add capability to read circular metadata > > Please describe changes in the changelog entry, not the effect. > > For example: > > * disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the > worst case scenario. > (grub_lvm_scan_device): ... > > Where ... has to be filled in, I have no idea what this code actually > does or what you changed ;-) > > The tiny change syntax does not seem familiar to me, AFAIK it is not > from the GCS. Can you please change that? Furthermore, if you both > worked on there two parts of the patch, please send in separate > patches. It will make my life a lot easier... :-) > > If only one person worked on this, for example Jan Derk, which Hans > only forwards this patch, please only list Jan Derk as the > contributor. > > > 2008-08-12 Robert Millan > > > > * loader/i386/pc/multiboot.c (grub_multiboot_load_elf32): Move part > > diff -uwr grub-1.96_svn20080813-org/disk/lvm.c grub-1.96_svn20080813-new/disk/lvm.c > > --- grub-1.96_svn20080813-org/disk/lvm.c 2008-08-28 14:32:53.000000000 +0200 > > +++ grub-1.96_svn20080813-new/disk/lvm.c 2008-08-28 18:31:19.000000000 +0200 > > @@ -281,7 +281,8 @@ > > goto fail; > > } > > > > - metadatabuf = grub_malloc (mda_size); > > + /* alloc for circular worst-case scenario */ > > Nitpick: Please start a sentence with a capital letter and end with a > `.'. So this will become: > > /* Assume circular buffer in a worst case scenario. */ > > > > + metadatabuf = grub_malloc (2*mda_size); > > Please use spaces around operators: > > 2 * md_size > > > if (! metadatabuf) > > goto fail; > > > > @@ -300,6 +301,12 @@ > > } > > > > rlocn = mdah->raw_locns; > > + if (rlocn->offset + rlocn->size > mdah->size) > > Here rlcon->offset seems to be little endian (64 bits), so please use > grub_le_to_cpu64. Same for rlocn->size, please check the size of this > member before you use a macro (I couldn't find it immediately...). > > > + { > > + /* metadata is circular */ > > Same as above. > > > + grub_memcpy(metadatabuf + mda_size, metadatabuf + mdah->start, > > + ((rlocn->offset + rlocn->size) - mdah->size)); > > + } > > Please check and correct the endianess. you use a lot of > parenthesises. Actually, I think none are requires. -- Felix Zielcke