From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1ObCTM-0001Bz-BC for mharc-grub-devel@gnu.org; Tue, 20 Jul 2010 09:03:00 -0400 Received: from [140.186.70.92] (port=55921 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ObCTI-0001Bj-KJ for grub-devel@gnu.org; Tue, 20 Jul 2010 09:02:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1ObCTE-00076C-RE for grub-devel@gnu.org; Tue, 20 Jul 2010 09:02:56 -0400 Received: from mail-bw0-f41.google.com ([209.85.214.41]:45661) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1ObCTE-00075b-J9 for grub-devel@gnu.org; Tue, 20 Jul 2010 09:02:52 -0400 Received: by bwz9 with SMTP id 9so3634530bwz.0 for ; Tue, 20 Jul 2010 06:02:51 -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:cc:subject:references:in-reply-to :x-enigmail-version:content-type; bh=twhk01bNn1GJIRGoUSX5NR4NlrCq7rM4PEmLk4qgyHQ=; b=rWmiiZgkB+XF5PZaQnIKdZXv+4C7JzQZvGjxsN4DrgMxbWQj/MoV+fKYgRJBFJBpvw E6XCLurjGdW6Jmgx93kXWX73KH/C0HLXDp3/eeZSqv17GDnX65l+dYxT/GSIqLFJyjjt LNda8VehVUBpTwPYg0qz7FRrtrw5/SacSfoxM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:x-enigmail-version:content-type; b=I/zDWAicSUv5Gh39oDKyJ5yyHUkTy3s8jTkPhTE9aXDR4b2Q+88PxwWKaXnabS9SJS b/8sRzBBt1JEwT2S1u33QJKvRGDEeO3rKuDTH2mfw4/4Izgo2U0FNXPo6lS3BI8lEyMi YZfunSQ+BUeTAz19a81ZA+RXP0Zz7DFjeE1Uk= Received: by 10.204.119.80 with SMTP id y16mr5097957bkq.113.1279630970842; Tue, 20 Jul 2010 06:02:50 -0700 (PDT) Received: from debian.bg45.phnet ([81.62.128.73]) by mx.google.com with ESMTPS id s17sm28060858bkx.6.2010.07.20.06.02.48 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 20 Jul 2010 06:02:48 -0700 (PDT) Message-ID: <4C459E77.3090908@gmail.com> Date: Tue, 20 Jul 2010 15:02:47 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100620 Icedove/3.0.5 MIME-Version: 1.0 To: Colin Watson , 554790@bugs.debian.org References: <201007090801.12986.vadic052@gmail.com> <20100711232621.GQ12396@riva.ucam.org> <20100712103815.GA10414@riva.ucam.org> In-Reply-To: <20100712103815.GA10414@riva.ucam.org> X-Enigmail-Version: 1.0.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig65E3DF4E97EBF031F1E7620A" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Cc: grub-devel@gnu.org, Vadim Solomin Subject: Re: Bug#554790: This breaks device.map on upgrade 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: Tue, 20 Jul 2010 13:02:58 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig65E3DF4E97EBF031F1E7620A Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/12/2010 12:38 PM, Colin Watson wrote: > [Re-sending with full quoting and from my @ubuntu.com account so that i= t > doesn't get stuck in the grub-devel moderation queue.] > > On Mon, Jul 12, 2010 at 12:26:21AM +0100, Colin Watson wrote: > =20 >> On Fri, Jul 09, 2010 at 08:01:12AM +0400, Vadim Solomin wrote: >> =20 >>> This fix, at least in its current form, has a potential to break grub= for=20 >>> users having more than one drive, unless they are careful enough to c= heck and=20 >>> fix their device.map after upgrade. >>> >>> Old mkdevicemaps assigned grub device numbers in the sort order of ke= rnel=20 >>> device names, which was right more often than not. On the other hand,= the sort=20 >>> order of (pretty much random) stable names, used by new version, is e= xtremely=20 >>> unlikely to have any correlation to grub device order. >>> >>> Included is a rough patch which preserves the kernel-name order for g= rub=20 >>> devices when generating device.map. >>> =20 >> I like this idea; it seems that it ought to minimise the likelihood of= >> upheaval due to the change in device.map generation. I agree that >> nothing is particularly guaranteed here but it would be nice to try to= >> minimise the chances of problems, if only to try to reduce the number = of >> people who find they need to ask for help on #grub ... >> >> Vladimir, would this patch need a copyright assignment? Most of it >> seems pretty straightforward; in fact I doubt that it would come out >> very much different if I'd written it from scratch. >> =20 > =20 No need of copyright assignment for this patch. > I've made a number of changes to this patch to fix up formatting and to= > behave a bit closer to the way I expect; in particular it's necessary t= o > compare by ->stable if comparing by ->kernel returns zero, for the > reasons previously explained in a comment. > > Vadim's original patch is quoted here, followed by my amended version > with a ChangeLog entry. > > =20 > > 2010-07-12 Vadim Solomin > 2010-07-12 Colin Watson > > Generate device.map in something closer to the old ordering. > > * util/deviceiter.c (struct device): New declaration. > (compare_file_names): Rename to ... > (compare_devices): ... this. Sort by kernel name in preference > to the stable by-id name, but keep the latter as a fallback > comparison. Update header comment. > (grub_util_iterate_devices) [__linux__]: Construct and sort an > array of `struct device' rather than of plain file names. > > =20 Go ahead. > =3D=3D=3D modified file 'util/deviceiter.c' > --- util/deviceiter.c 2010-07-06 14:10:36 +0000 > +++ util/deviceiter.c 2010-07-12 10:34:16 +0000 > @@ -467,13 +467,30 @@ clear_seen_devices (void) > } > =20 > #ifdef __linux__ > -/* Like strcmp, but doesn't require a cast for use as a qsort comparat= or. */ > +struct device > +{ > + char *stable; > + char *kernel; > +}; > + > +/* Sort by the kernel name for preference since that most closely matc= hes > + older device.map files, but sort by stable by-id names as a fallbac= k. > + This is because /dev/disk/by-id/ usually has a few alternative > + identifications of devices (e.g. ATA vs. SATA). > + check_device_readable_unique will ensure that we only get one for a= ny > + given disk, but sort the list so that the choice of which one we ge= t is > + stable. */ > static int > -compare_file_names (const void *a, const void *b) > +compare_devices (const void *a, const void *b) > { > - const char *left =3D *(const char **) a; > - const char *right =3D *(const char **) b; > - return strcmp (left, right); > + const struct device *left =3D (const struct device *) a; > + const struct device *right =3D (const struct device *) b; > + int ret; > + ret =3D strcmp (left->kernel, right->kernel); > + if (ret) > + return ret; > + else > + return strcmp (left->stable, right->stable); > } > #endif /* __linux__ */ > =20 > @@ -507,10 +524,10 @@ grub_util_iterate_devices (int NESTED_FU > if (dir) > { > struct dirent *entry; > - char **names; > - size_t names_len =3D 0, names_max =3D 1024, i; > + struct device *devs; > + size_t devs_len =3D 0, devs_max =3D 1024, i; > =20 > - names =3D xmalloc (names_max * sizeof (*names)); > + devs =3D xmalloc (devs_max * sizeof (*devs)); > =20 > /* Dump all the directory entries into names, resizing if > necessary. */ > @@ -526,35 +543,34 @@ grub_util_iterate_devices (int NESTED_FU > /* Skip RAID entries; they are handled by upper layers. */ > if (strncmp (entry->d_name, "md-", sizeof ("md-") - 1) =3D=3D 0) > continue; > - if (names_len >=3D names_max) > + if (devs_len >=3D devs_max) > { > - names_max *=3D 2; > - names =3D xrealloc (names, names_max * sizeof (*names)); > + devs_max *=3D 2; > + devs =3D xrealloc (devs, devs_max * sizeof (*devs)); > } > - names[names_len++] =3D xasprintf (entry->d_name); > + devs[devs_len].stable =3D > + xasprintf ("/dev/disk/by-id/%s", entry->d_name); > + devs[devs_len].kernel =3D > + canonicalize_file_name (devs[devs_len].stable); > + devs_len++; > } > =20 > - /* /dev/disk/by-id/ usually has a few alternative identifications of > - devices (e.g. ATA vs. SATA). check_device_readable_unique will > - ensure that we only get one for any given disk, but sort the list > - so that the choice of which one we get is stable. */ > - qsort (names, names_len, sizeof (*names), &compare_file_names); > + qsort (devs, devs_len, sizeof (*devs), &compare_devices); > =20 > closedir (dir); > =20 > /* Now add all the devices in sorted order. */ > - for (i =3D 0; i < names_len; ++i) > + for (i =3D 0; i < devs_len; ++i) > { > - char *path =3D xasprintf ("/dev/disk/by-id/%s", names[i]); > - if (check_device_readable_unique (path)) > + if (check_device_readable_unique (devs[i].stable)) > { > - if (hook (path, 0)) > + if (hook (devs[i].stable, 0)) > goto out; > } > - free (path); > - free (names[i]); > + free (devs[i].stable); > + free (devs[i].kernel); > } > - free (names); > + free (devs); > } > } > =20 > > Thanks, > > =20 --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------enig65E3DF4E97EBF031F1E7620A Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAkxFnncACgkQNak7dOguQgnmRwEAtVF0KUBKIWVN/y0kQpvcqVPZ vHIkoeQy5HpTsoUr6GkBAJBWijML1F+tzlQXoPlrFnxnGRUpXtZB0jL253dq49ET =qK3u -----END PGP SIGNATURE----- --------------enig65E3DF4E97EBF031F1E7620A--