From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1azSJr-0000CY-NF for mharc-grub-devel@gnu.org; Sun, 08 May 2016 13:16:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52519) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1azSJp-00008e-8y for grub-devel@gnu.org; Sun, 08 May 2016 13:16:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1azSJk-0004HH-VX for grub-devel@gnu.org; Sun, 08 May 2016 13:16:36 -0400 Received: from smtpgw.dds.nl ([91.142.252.201]:45094 helo=smtp1.dds.nl) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1azSJk-0004H9-Kg for grub-devel@gnu.org; Sun, 08 May 2016 13:16:32 -0400 Received: from webmail.dds.nl (app1.dds.nl [81.21.136.61]) by smtp1.dds.nl (Postfix) with ESMTP id 7B13B7FCCA for ; Sun, 8 May 2016 19:16:29 +0200 (CEST) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Date: Sun, 08 May 2016 19:16:29 +0200 From: Dryden Personalis To: The development of GNU GRUB Subject: Re: [PATCH] Install to LVM PVs In-Reply-To: <572EFD18.5070009@gmail.com> References: <572ED720.30800@gmail.com> <572EFD18.5070009@gmail.com> Message-ID: X-Sender: list@xenhideout.nl User-Agent: Roundcube Webmail/1.1.5 X-Virus-Scanned: clamav-milter 0.99.1 at smtp1 X-Virus-Status: Clean Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 91.142.252.201 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 May 2016 17:16:38 -0000 Andrei Borzenkov schreef op 08-05-2016 10:47: > 08.05.2016 09:05, Andrei Borzenkov =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>=20 >>> +#ifdef GRUB_UTIL >>> +int >>> +grub_util_is_lvm(grub_disk_t disk) >>> +{ >>> + struct grub_diskfilter_pv_id id; >>> + struct grub_diskfilter_vg *vg; >>> + grub_disk_addr_t start_sector; >>> + vg =3D grub_lvm_detect(disk, &id, &start_sector); >>> + if (! vg) >>> + return 0; >>> + /* don't free the vg, it's held by grub_diskfilter_vg_register */ >>> + grub_free(id.uuid); >>> + return 1; >>> +} >>> + >>=20 >> This has side effect of adding duplicate VG definitions; this may=20 >> later >> confuse grub. What about just checking array->driver for LVM? Go=20 >> through >> registered arrays, find disk match and check array driver. See >> scan_disk_partition_iter () for example. >>=20 >=20 > Which is basically call grub_diskfilter_get_pv_from_disk() and check > vg_out->driver. This method also has the downside that is_lvm will fail without any=20 indication as to why, when no volume group has been created. lvm_detect=20 will return null because it can only return a VG. And, you cannot error=20 out on that if a non-existing LVM should be okay. So this call to lvm_detect is pretty annoying. If it had more info, I could at least give an error in setup.c. I am not sure whether installation is *possible* without a VG but it=20 would be odd if it couldn't? Anyway I would like a better error message on this because I just wasted=20 2 hours trying to get GRUB_UTIL to work and failing. That means as per=20 your method I can probably distinguish between PV present and VG present=20 (or not present). Then the only way I know how to report back is to give more values to=20 is_lvm, but do we need a VG? Why error out on no VG? I will test.