From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1KE84K-0005np-Ql for mharc-grub-devel@gnu.org; Wed, 02 Jul 2008 15:32:44 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KE84I-0005nN-GV for grub-devel@gnu.org; Wed, 02 Jul 2008 15:32:42 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KE84H-0005nB-Ou for grub-devel@gnu.org; Wed, 02 Jul 2008 15:32:42 -0400 Received: from [199.232.76.173] (port=34738 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KE84H-0005n7-E7 for grub-devel@gnu.org; Wed, 02 Jul 2008 15:32:41 -0400 Received: from ug-out-1314.google.com ([66.249.92.173]:6781) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KE84G-00021N-Me for grub-devel@gnu.org; Wed, 02 Jul 2008 15:32:41 -0400 Received: by ug-out-1314.google.com with SMTP id l31so775689ugc.48 for ; Wed, 02 Jul 2008 12:32:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:in-reply-to :references:content-type:date:message-id:mime-version:x-mailer; bh=BBf7T9U3+E+1lUS0QwntZIFs4Pvpg6fha1bSWkv+F4Y=; b=Liv7QGZ0XU8ZWfnwqgbH23WONS54uYvcHyQrAn9y6EDA0IR9CsyfMgeZttw+uADGRK OVruJpIcQ3YaETVEzeJSdq5cWU2Z6wYlSMAL/5OyFJGLQBat2SiIdJfH2pQZfWj4jOHd cxSbF8E1WThxZRJCbvFW/tcF+qkGE3l8vnfgA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:in-reply-to:references:content-type:date:message-id :mime-version:x-mailer; b=T4QzNs8SSqt3fA6l/k24hPswuISN+hCBnmrYM/SaB0wyO8xg9h9K8oeZl1t+wCcnBS 70ACsvKbYtQpWq/8rBqXsjDTpoJqxXpdTlzeMrJ9xsWaAdT5JcHhYh2sLveH//OS+dWj ApaRGbIPB94oXqR9NWsPvDYVHCpo9CLxbqmRI= Received: by 10.67.102.6 with SMTP id e6mr1198281ugm.82.1215027158613; Wed, 02 Jul 2008 12:32:38 -0700 (PDT) Received: from ?192.168.1.100? ( [213.37.137.93]) by mx.google.com with ESMTPS id 30sm2141640ugf.63.2008.07.02.12.32.36 (version=SSLv3 cipher=RC4-MD5); Wed, 02 Jul 2008 12:32:37 -0700 (PDT) From: Javier =?ISO-8859-1?Q?Mart=EDn?= To: The development of GRUB 2 In-Reply-To: <20080702142245.GA21064@thorin> References: <20080629211957.GD24784@thorin> <1214794971.9353.32.camel@localhost> <4868C017.8040004@isaac.cedarswampstudios.org> <1214827937.9353.43.camel@localhost> <20080701160827.GF6985@thorin> <1214929545.13432.19.camel@dv> <1214937759.9353.64.camel@localhost> <20080701204816.GA31206@thorin> <1214954927.9353.91.camel@localhost> <20080702142245.GA21064@thorin> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-lXf0NkUbPaCfQPc+M6zl" Date: Wed, 02 Jul 2008 21:32:40 +0200 Message-Id: <1215027160.9353.125.camel@localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.22.2 X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 2) Subject: Re: grub-probe detects ext4 wronly as ext2 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, 02 Jul 2008 19:32:42 -0000 --=-lXf0NkUbPaCfQPc+M6zl Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable El mi=C3=A9, 02-07-2008 a las 16:22 +0200, Robert Millan escribi=C3=B3: > On Wed, Jul 02, 2008 at 01:28:47AM +0200, Javier Mart=C3=ADn wrote: > > > A --ignore-incompatible flag doesn't sound like a nice thing to do; = it means > > > we're passing our own problem to the user instead of solving it. > > We don't have any "problem" to pass to users: ext4 is not supported >=20 > We don't have an urge to support ext4, but that doesn't mean we shouldn't > consider it a problem. >=20 > I think adding an interface for the user to choose in which way to deal w= ith > our limitations is a nasty thing. I strongly object to that. May I ask why? Is it thus better to impose our own way of dealing with our limitations, unchangeable and possibly wrong in some instances (see below for a possible scenario)? Sensible defaults are good, but choice is one of the bases of freedom. > > However, given Pavel's and > > others' objections, I suggested the addition of an user override to it. > > Thus, the user will have to knowingly force the system to interpret the > > filesystem with its current code, and accept any failures he might get, > > instead of the current behaviour of having the FS mounted automatically > > without checking incompatibilities (and then getting the errors anyway)= . >=20 > I don't think this is necessary. First, let's take for granted that our = code > is in every situation smart enough not to crash when a filesystem isn't > readable (this should always be the case, since we might occasionaly be a= sked > to read corrupt filesystems). Then, what do flags mean? >=20 > If a flag means "GRUB won't be able to access this filesystem at all", we= could > explicitly refuse to probe it, but then again our code must be graceful e= nough > to cope with it without crashing anyway (see above), so maybe it's not wo= rth to > (depends on the time/size trade-off). >=20 > If a flag means "access to the filesystem isn't deterministic, and grub-p= robe > might be able to do things that real GRUB won't", then we're in a situati= on in > which we'd like grub-probe to be conservative _but_ real GRUB to be > best-effort. I think this means an internal switch to tell fs probes whe= ther > to be conservative or not. We could even use #ifdef GRUB_UTIL so the fla= g > checking stuff doesn't make real GRUB fatter. >=20 The problem with INCOMPAT_* flags is that we cannot always know what they mean, and thus, a "best-effort" reader risks not just bumping into metadata it does not understand (and thus crashing or spitting thousands of errors if it's not robust enough), but also ignoring it and reading wrong data to memory. In some circumstances, this can lead to nasty bugs, and this is the reason I want the "override-incompatible-flags" loophole in the driver to require explicit user activation. We can decide what to do with flags we currently know, like ext4 flags (extents and such), and that's the purpose of the IGNORE_INCOMPAT macro in the patch; but with future flags we have no clue. Consider a hypothetical ext5 file system with a new INCOMPAT_BLOCKCOMP feature flag set; and without any other "unimplemented" flags like extents and such. This new flag will mean that _some_ blocks of a file are LZMA-compressed and some are not (maybe to the criterion of the writing driver, like more than 5% space savings or such). The info on which blocks are compressed and which are not is saved on a bitmap linked to from an extended attribute in the inode (I'm making this out right now, so this might be impossible as described). If our current driver found this filesystem and tried to read a multiboot kernel from it, it might read the whole file as it if were uncompressed, thus putting a "corrupt" image into memory, possibly even reading past the end of the file in the disk, or less data than the stated file size. If the first blocks (with the multiboot headers) were among the uncompressed ones, GRUB would happily boot from it, thus leaving it to the criterion of the booted "kernel" what to do. The end result might be just a triple fault when the CPU runs into compressed code; or may come to the "kernel" doing something nasty to the computer due to a corrupt HD driver commands structure, for example. This scenario would be completely "transparent" to the user because GRUB would mount the FS without even warning the user. Thus, I think that "best-effort" is not always preferable when we're dealing with unknown INCOMPAT_* flags. Those flags mean, by default, "you can't read this filesystem if you don't know how to interpret this". Thus, I think GRUB should initially reject to mount any such filesystem, but provide an override for two cases: 1) The user explicitly requests it. Maybe he's trying to boot an older kernel which was not compressed in a filesystem that just got accidentally converted, or just check the source of the error. 2) GRUB is trying to boot from such a partition This can be just as risky as the scenario I depicted before, but in this case we might really not have another way out, since the user override would be unworkable here as there is no prompt. Of course, there are INCOMPAT_* flags that we know are not really such (like needs_recovery). We can add those flags to the ignore list and just forget about them, or implement some handling of them, like replaying the journal in memory; but we cannot transparently ignore all INCOMPAT_* flags because we don't know the future! Thus, I agree with you that grub-probe should adopt the most conservative stance possible, but reject the idea that the bootloader proper should automatically activate "best-effort" reading, since this can lead to very strange and untraceable bugs in the future (I'm just imagining the puzzled look on the osdev's face when his perfect multiboot kernel on the ext5 filesystem triple-faults without apparent motive). Thus, the default should be conservative reading, keeping best-effort reading to be enabled through an user override. --=-lXf0NkUbPaCfQPc+M6zl Content-Type: application/pgp-signature; name=signature.asc Content-Description: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada digitalmente -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iQIVAwUASGvX2KSl+Fbdeo72AQKl9A/8CHzjIcfTFVFgpwwma5m195aQfIaSSxdi jQecIh8R+RaR6FBEozHlSPNgBl3I3bzamHuFPDcD28tp//MDk2Z0/IpRhYGuxVmR VwSSbeuqT9BOxUN18HaoI+8qMXXXcCyheNRLsAmRF4X8c5sYlY7L0J7cB8Ceq2tN vHHEAH/lDqMlj19WiaQFLVjKXpFES2/Q8iaNvZgZMHHBr8+LXiYGpVqprKaHsjXl s0PXz+Aj+RQs+gF9m9ZwZZ+0fKJJFxaOtzXtSn4ufQj2p5+3I0hDlLT+f+qEYG1C YOkM3t9hCfggRMXjBHr5VSsOAEX5tV8JVNqpm4YzYfF2aCSf48brGHuROcbWSNW2 7GyZ+bfJiQ73s5ZpaNm5Fw9ILrltguXSVdtr/0joUPjq/yLoeDH6uqy2Z48cQBpZ 5KNaE/H6VgrYNFvOplLAEAzZCOH5MnVNEkCWCDs58Mk5kNTjg8dilmloyETSm2Yj zbpMktSqQmkgzaA2h1t4TILtxSG/tJjjISX79yHrGG5H7bL1xr/aS2Xa8+WkRKwY l8Jl1tJAf860PL7NnZIX6RK5Q4x6U1E2JcCw/XBpCeIxv/3YdxUbNFoLWiXjad+d mzYZ6vVBaQKJdd3b/sTgUXFSHFXiDpQrUiZ7xG4aMaYIIF+7iSQPEMf9J9dciY/E Q38Rg+XuT5w= =xkWp -----END PGP SIGNATURE----- --=-lXf0NkUbPaCfQPc+M6zl--