From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1NK0zX-0002UO-PN for mharc-grub-devel@gnu.org; Sun, 13 Dec 2009 21:48:55 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NK0zT-0002Mk-Qa for grub-devel@gnu.org; Sun, 13 Dec 2009 21:48:51 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NK0zP-0002CJ-7p for grub-devel@gnu.org; Sun, 13 Dec 2009 21:48:51 -0500 Received: from [199.232.76.173] (port=36445 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NK0zO-0002By-QG for grub-devel@gnu.org; Sun, 13 Dec 2009 21:48:46 -0500 Received: from mga09.intel.com ([134.134.136.24]:14174) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NK0zO-00020f-FK for grub-devel@gnu.org; Sun, 13 Dec 2009 21:48:46 -0500 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 13 Dec 2009 18:47:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.47,392,1257148800"; d="scan'208";a="578452799" Received: from debian.sh.intel.com (HELO [10.239.13.180]) ([10.239.13.180]) by orsmga001.jf.intel.com with ESMTP; 13 Dec 2009 18:48:25 -0800 From: Zhu Yi To: The development of GNU GRUB In-Reply-To: <1260524367.2832.10.camel@fz.local> References: <1260523561-19086-1-git-send-email-yi.zhu@intel.com> <1260524367.2832.10.camel@fz.local> Content-Type: text/plain; charset="UTF-8" Date: Mon, 14 Dec 2009 10:48:42 +0800 Message-ID: <1260758922.12157.281.camel@debian> Mime-Version: 1.0 X-Mailer: Evolution 2.28.1 Content-Transfer-Encoding: 7bit X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Subject: Re: [PATCH] Backup old boot sectors before installation X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Dec 2009 02:48:52 -0000 On Fri, 2009-12-11 at 17:39 +0800, Felix Zielcke wrote: > > Someone already made a patch for this inside grub-setup > See here and also for the discussion of it: > http://lists.gnu.org/archive/html/grub-devel/2009-09/msg00242.html Thanks. If I knew this patch, I won't try to write one my own. I did do some basic search and asked in #grub2 IRC before writing it though. Anyway, I did a quick review for Garimella's patch. Both the ideas of the two patches are the same: backup the mbr and boot sectors will be overwritten by core.img to a file before grub2 overwrites them. The implementations slightly differ in: 1. The format of the backup file. The old boot sectors and the start of embed_region position is required for both patches. Garimella also added some redundant fields (i.e. image size and 0xff) for integrity checking. I think it's a good idea. But something like a md5/sha1 checksum should be even better. 2. The image backup and recovery. Both of the patches choose to backup the old boot sectors in grub-setup.c. On the recovery, Garimella selected to do it in grub-setup.c and I used grub-install script with dd(1). I don't think this is a very big deal and either way is OK. The reason I choose grub-install is I want to keep the grub-setup.c as clean as possible. Because given the backup file format, one can recover it with dd(1) even without any help with grub-setup. 3. The backup policy. Garimella's patch backups every time before grub-setup overwrote the boot sectors. There might be a problem when someone run grub-setup/grub-install twice. So the backup image ends up with the grub2 boot sectors. This makes the recovery for the "real" boot sectors impossible. My patch won't overwrite if a previous backup image already existed. grub-install will remove the backup image after a successful recovery. 4. Blocklists installation. This is not very important because blocklists is not recommended. To be completeness, my patch backups the mbr if blocklists are used due to embedding is not possible. This will be detected by grub-install as well. In conclusion, I believe this backup feature is useful and either implementation should do the work. If we can merge the good parts of them, we can get a better one. I just curious why the thread is stuck since September. Any blocking issues with it? Can we keep the topic moving forward? Your comments are highly welcome. Thanks, -yi