From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1LlLEW-0000tV-75 for mharc-grub-devel@gnu.org; Sun, 22 Mar 2009 06:48:48 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LlLES-0000sW-T8 for grub-devel@gnu.org; Sun, 22 Mar 2009 06:48:44 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LlLEO-0000rv-6n for grub-devel@gnu.org; Sun, 22 Mar 2009 06:48:44 -0400 Received: from [199.232.76.173] (port=56277 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LlLEO-0000rs-11 for grub-devel@gnu.org; Sun, 22 Mar 2009 06:48:40 -0400 Received: from mail-fx0-f166.google.com ([209.85.220.166]:52103) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LlLEN-00043D-Eh for grub-devel@gnu.org; Sun, 22 Mar 2009 06:48:39 -0400 Received: by fxm10 with SMTP id 10so1306149fxm.42 for ; Sun, 22 Mar 2009 03:48:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=IQBYFn8OiLRmfwRuXXILi/oMglDlXZQYfJl7P7Bah8g=; b=Hzz0xUvMQ8uAxoI8BsbPMQHyc9cfoB4DEMO+VcitL1eXdalbvbzfPsYalmSn2TjJ/B 5da0jcaX8V976zGUJ4IuxBSfTLVgUeQWJEPFF65AnKpe5r7yEeFjXk7JhaYWdxEnfwHr aU7lJS83RbrqLE/2+E+gBxJ7jz3BuC84Svytg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type:content-transfer-encoding; b=JtAd521dMVSTbA1zN/2HmYdzjY6bKi4rR35qee/OzZjihdnlQrHwYBQpgqQwEWrV1O udceAh9zgK8IcO1HwYWcIXZEt+XYXAAS4pchT/KjUOkq5+9eJUof+MHH3Kj0kJxEtfXl TWEQKaDv+AlvKeIKdrtJ/KeeC1eszYc9OoOi4= Received: by 10.86.86.12 with SMTP id j12mr2752956fgb.69.1237718918226; Sun, 22 Mar 2009 03:48:38 -0700 (PDT) Received: from ?192.168.1.25? (252.80.3.213.cust.bluewin.ch [213.3.80.252]) by mx.google.com with ESMTPS id 4sm1580624fgg.0.2009.03.22.03.48.36 (version=SSLv3 cipher=RC4-MD5); Sun, 22 Mar 2009 03:48:36 -0700 (PDT) Message-ID: <49C61784.6020602@gmail.com> Date: Sun, 22 Mar 2009 11:48:36 +0100 From: phcoder User-Agent: Thunderbird 2.0.0.21 (X11/20090318) MIME-Version: 1.0 To: The development of GRUB 2 References: <49A50DA2.20604@netsyncro.com> <200903150610.40823.okuji@enbug.org> <200903221601.36503.okuji@enbug.org> In-Reply-To: <200903221601.36503.okuji@enbug.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: Migrating to GRUB 2 in Debian (Re: Interesting GSoC project ideas for 09) 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: Sun, 22 Mar 2009 10:48:45 -0000 Hello, I agree that non-sector aligned writes should be handled correctly. However I disagree with removing of the magic number. I personally would prefer if this file would have magic number and checksum. AFAIK currently grub2 doesn't write to FS except in load_env/save_env so a bug in code calling the hook could easily be present. And I don't want grub2 to corrupt the filesystem because of any such mistakes Yoshinori K. Okuji wrote: > On Sunday 15 March 2009 14:52:05 Bean wrote: >> On Sun, Mar 15, 2009 at 5:10 AM, Yoshinori K. Okuji wrote: >>> On Friday 13 March 2009 21:23:19 phcoder wrote: >>>> Look at load_env/save_env commands and grub-editenv util >>> Thanks. Now I really regret that I didn't find those additions earlier. >>> >>> I do not like this implementation for the following reasons: >>> >>> - The saved file is not plain text, unlike GRUB Legacy. This is a bad >>> choice. Please let me know the reason why it must be binary, if any. >> Hi, >> >> As the command need to write to disk using blocklist information, >> which is not always correct (such as tail packing, sparse block), I >> use a magic header for verification. The length field is used to >> indicate the length of the block. because the command can't expand >> file, otherwise it would need to update fs information, which is too >> much for grub. > > I have read your code deeply, and have found the following: > > - in reality, you don't deal with tail packing, but just refuse it, because of > this check in the hook function: > > if ((offset != 0) || (length != GRUB_DISK_SECTOR_SIZE)) > return; > > - grub-editenv and save_env always write the magic at the beginning of a file, > thus the magic does not make sense (besides an extremely conservative sanity > check). > > I would say that this is a regression from GRUB Legacy, which takes care of > partial sector allocations gracefully. > > So, assuming that every filesystem driver calls a read hook correctly, we > should change it for: > > - eliminating the magic header (although it could be kept for a safety guard > for accidental writes) > > - refusing to write, only if any sparse blocks are in use (as GRUB may not > allocate new sectors physically) > > - dealing with partial sectors - possibly due to tail packing - with some > complicated code > > I will work towards this direction. I will first fix up the sector handling > and change the format to plain text. Naming changes are quite trivial, so > they can be done later. > > Regards, > Okuji > >>> - The command names are ugly. Why didn't anybody follow Pavel's advise >>> using "env"? >>> >>> - The utility name is also ugly. I like Pavel's suggestion "grub-env". >>> >>> If nobody stops me, I will rewrite it in one week, without caring about >>> backward compatibility. >> I have no objection for the rename, although there should be two env >> commands, one to load and one to save. > > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko