From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mDgk1-0002ol-7E for mharc-grub-devel@gnu.org; Wed, 11 Aug 2021 01:25:25 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57688) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mDgjz-0002nw-US for grub-devel@gnu.org; Wed, 11 Aug 2021 01:25:23 -0400 Received: from de-smtp-delivery-102.mimecast.com ([194.104.111.102]:60298) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mDgjv-0008L1-41 for grub-devel@gnu.org; Wed, 11 Aug 2021 01:25:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=mimecast20200619; t=1628659515; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ok0DMLO8gIBRYm8F9xnguM3XFlpyzcrvFghb1ohmqK0=; b=Aiysv4M21Dq5FPYvR4662jx4uuvI3cEAfuc6uzGKyQWby13j/AgTpVGkD1bI9+UjvHDY/P gC2YaXbZBg7VuZ4IJGqn30CJoMKUUuyuKcQFNqv9O0EQssO8Dz+TyRpZH4nXwmG0biQUb7 yEw2hAnNs8wBzS2s+z9W2s7k7dCkg6o= Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-ve1eur02lp2050.outbound.protection.outlook.com [104.47.6.50]) (Using TLS) by relay.mimecast.com with ESMTP id de-mta-8-Oy-Djjr4PWWlCATq7egUNw-1; Wed, 11 Aug 2021 07:25:14 +0200 X-MC-Unique: Oy-Djjr4PWWlCATq7egUNw-1 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iJhTsEf1Gvce2e1IK0092yLxGkXazjG4vKfz8U5ErxE1HwqjAaGH9pt+GTJUSOQFblfxHPTLb+UDT8TCDfCcM/jfjF1mD/np/NnqCP2qHP9ke5m5XJHiUM6Bq94K8ALAZZXT0vSQdUwb5/4m+pkKD3eG9tXhbclj14FQA5ZDjfnFXqNLylG11LGNfTUUGOSKOzcseP9aLzILFVpxI1u3WF8Wqk+BoJz95upsqEkUigVKBRuwTnm/nVgt6RyUgLNWPYYosnyl5U/uyV6L+JleR+YyMa85EPvMo+6om/R9XoUGqSRkh05FTevopJGz8hIu5JJeeSDSXfRozp1kxxlq2w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=7L7yryJ/Sb14o0QtAvkvYCA1QQsYxLe0ABt38rro3jU=; b=ZpijrPmg6EqGtSucpC0mhay5hfFbcKBxomA4RD6btcxo27HsN8d+XgwIqL49sd9ISG6f6S2xViVMGeT5KUUHJQTnw9rj0BYfSogXLdWdVVM5Ab8uZcrZZflEgQOn9mfv6CyGcPhy+/V3HotrWOcfRDjsmCneS4YFjYrjTHD/iBCfwcLVuK5Uk37OhKK89+u1OwsgSf9RosrtMcjmcA6eXVsLtHdBymS0R9MXW6liwY3ByThkcxFt+UVjpLqECVs0Wo/+vajmVuzXF0Dtbp4TAJbWoQ0iRyRUP/h0YDZPKDN0gWZcl6vCzMKOvNJ+yuUZHxikDiFCXv7ul/VoAl6byA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none Authentication-Results: gnu.org; dkim=none (message not signed) header.d=none;gnu.org; dmarc=none action=none header.from=suse.com; Received: from DU2PR04MB8648.eurprd04.prod.outlook.com (2603:10a6:10:2df::21) by DU2PR04MB8597.eurprd04.prod.outlook.com (2603:10a6:10:2d8::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.19; Wed, 11 Aug 2021 05:25:12 +0000 Received: from DU2PR04MB8648.eurprd04.prod.outlook.com ([fe80::5586:d75d:b656:b85]) by DU2PR04MB8648.eurprd04.prod.outlook.com ([fe80::5586:d75d:b656:b85%5]) with mapi id 15.20.4394.023; Wed, 11 Aug 2021 05:25:12 +0000 Date: Wed, 11 Aug 2021 13:25:06 +0800 From: Michael Chang To: The development of GNU GRUB CC: Michael Chang , Olav Reinert Subject: Re: [PATCH] diskfilter: use nodes in logical volume's segment as member device Message-ID: <20210811052506.GA7941@mercury> References: <20210802094020.19258-1-mchang@suse.com> <20210809153441.sqtvqktjkedeld27@tomti.i.net-space.pl> Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20210809153441.sqtvqktjkedeld27@tomti.i.net-space.pl> User-Agent: Mutt/1.10.1 (2018-07-13) X-ClientProxiedBy: HKAPR03CA0016.apcprd03.prod.outlook.com (2603:1096:203:c8::21) To DU2PR04MB8648.eurprd04.prod.outlook.com (2603:10a6:10:2df::21) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from localhost (114.36.80.193) by HKAPR03CA0016.apcprd03.prod.outlook.com (2603:1096:203:c8::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4415.5 via Frontend Transport; Wed, 11 Aug 2021 05:25:11 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 216bd009-7fd8-4fb7-387a-08d95c88646b X-MS-TrafficTypeDiagnostic: DU2PR04MB8597: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:7219; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: FhcJB+aMPw+ALkhtR4QWVE6UiHli4T7+8fEUVSR7JZiM2mCx6MqyNrrA3adJnFxNyBUGlnbCQyuwfQHdFWIFSSjQkUU0pmjLZBG0+30ZOLMTFsqfyi2a5E8nTnkK9GDu/kgglP4zgZvZL4M/+ze66uDMv1+3NjP3anPPIESeglieZkaPFCoW7CGxXkuBoctlyeZujUsh3cg2mj+WzScowaz6OU0sQ3cQ6RGQ5tysJmUa65DyOZkZZPqYyO4io6qBlVyIIzauVx4YFL6H8XbJBi5ZBwFvhWb0LVNmE9r1zrkmBYQrWPnmUdVtAwZKs1Qq237fFhwIBkAjyAptmQeyGLji1bMxCrSJkjPgvRApCoJMwFsv/WmUzdZ7kVJ/uVJjNJiwfNmr9cqRGHU673366ddOTmI1SwLXdcgAs4TfqEUo6IfrOoFAwgcbuwiqiPdl2lmSloIH6LLw/y3ezdYzjP0YbxmVl762xxh9sEiH9/O3A1nzCgbjaKpVupji6/SkpIhCZC9zHeSYOWZuYxDKT1KPkDqZV+BSyYGhcw31HpE8VRO4WKsb9sK0pB2wNCc/5DDkKrAi/xaj2/Y850QumIh2tN4T70W9EAY+RoPq42lpMqYnNe+tZvBqi+zSJoLN9Iju0UBEHSI6biXPotkwnfZxZt8a4rDZi9/USYP/risuRFIRMZRIsckC3enSgEnGrLdPUbXFdaqUHVCRUBYB8tN4D3Y9FPvbys5TTuaeBtETYaWt+rbXXK8KS5vT2jL94Fhg5K2+EkDNR6Bt6mwAVA== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DU2PR04MB8648.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(7916004)(366004)(39850400004)(136003)(376002)(346002)(396003)(1076003)(8676002)(26005)(54906003)(956004)(8936002)(5660300002)(4326008)(38100700002)(2906002)(6916009)(6496006)(86362001)(186003)(316002)(478600001)(83380400001)(33656002)(33716001)(66476007)(9686003)(6666004)(66946007)(6486002)(66556008)(966005)(11716005); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?1w8u/tdBiJ+Rk6RFpYh1ARwNEO5kD5sg1eOgZXPegeYpzXqwqeauZGaV6UQg?= =?us-ascii?Q?X8jWXOw0C0Hpc1lwvkPuG/hR0Ahv7unWMg4g8iyzwfWmXJ6xP34V2NtVXfU1?= =?us-ascii?Q?CkAY6ckBb4pK/nBi1CvvrrPkheXaH5eyIeyNvTJQyJYS45vjRRi9IFhSKBzo?= =?us-ascii?Q?9J39vQhI6zNmjAOfREbGFHbW7QDSIDk4hdQj4mLjD+QaRfUGAf4DvH737n8h?= =?us-ascii?Q?ZAUZV2S5YDALY7t+oYV5oeojVcRHovp1RTiLX0MIcbvp2gvozXVJclG52OH3?= =?us-ascii?Q?4aKzc5/GDdWGhtyGiLB9DnQ4xbwE5qOlLFfXXuk2qItvuugHx+H3AuAIjSvh?= =?us-ascii?Q?3caalbfs4wkPaHMCBF9KMofxx+wS/w4/16z8bnGG1l2GxusKIQNJ4JUapb5v?= =?us-ascii?Q?NZu4N5QLvL+wcCOISYRgZAOJCJazHyCQ6E4e9rxt3f4FK2MZqrmaG8Y9gigq?= =?us-ascii?Q?RDLWM+NSppKu3pKTl2vPCsYutgwVxsri5Xgxud/bcMsVofnb6y/SrgUNz1UQ?= =?us-ascii?Q?7K23LuGpQc8XazagUXwJ9bbAr0TwDi2hC8UODj7V93ubIye4l9eBYw/zuaum?= =?us-ascii?Q?S3T0+GlrPwTG4UFzTufdqN1iVFeOAYy+ns9n7bwxphDBdG6gW56a/mHQ1/0u?= =?us-ascii?Q?t52t1JWtlKh2NbcWBtXYLADx1hh5qvjnLFdI/WWa38pQn8DqhHAbya3oA/XG?= =?us-ascii?Q?TqedrlpC9FZLvuN9v00xif5kqa94dERcyllSBoXsyVjuR2NTOsB5KfJc+upC?= =?us-ascii?Q?Cn+r73b9j/mFJ7D9XMeKVj7PjTrOeEoGkytVixqa0gUj+B7FN93xtheUz8Ed?= =?us-ascii?Q?plbVAsjCz9FVX6dGAkaC8thJdueN+Mz34B3RQCQkX3EMu37XcGPPR4IKGbD0?= =?us-ascii?Q?4yfxMpyovzh9xsNpBHlFkkPN4Ubml1YoJrOIFjoHqUYE8ACrwiozbZS9MBKT?= =?us-ascii?Q?gbVhTIsRxQxxWTQP/udw7RsUmz+PZiBgzvaIo4fZD6jhhbaRjt61z41doNwV?= =?us-ascii?Q?vn4Q4JmP5l9Jh85vERzUbwD23kaFtjvfellfDoSBVAWHNgg4bc0lV2jtkwOh?= =?us-ascii?Q?COPNbU6j+oDi6phOFlETnMCwyAS0xG5yhXxGh+oN+LKSVuw/9ipXkRv8YTIY?= =?us-ascii?Q?g6RFdUJdIvxgZJW4rxkhr4ZuhLoks+Z1FFiI4hI3Vpxm18/JfnuG1qsKGLGN?= =?us-ascii?Q?iczhTOYGrvc4UKITCCKGDl/zZrHQGhkdKlrD6svyI0LgMJVPsfuWKELaWrTz?= =?us-ascii?Q?yz81B0N1Y2rY4/JfeSRywDwoEl7p0uxT1muRx3Cm8nGXaUIF0w9HVuNEFogW?= =?us-ascii?Q?oU2D4XwBH3voKYzLc8HjeqGn?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 216bd009-7fd8-4fb7-387a-08d95c88646b X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8648.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Aug 2021 05:25:12.0836 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 8DdC5mR+1L+DqPLf1dGuEYEGeJb1L6eK4cVrIwq73XX0b2IzBsJY0nCI1Yruq/on X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2PR04MB8597 Received-SPF: pass client-ip=194.104.111.102; envelope-from=mchang@suse.com; helo=de-smtp-delivery-102.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, MSGID_FROM_MTA_HEADER=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Aug 2021 05:25:24 -0000 On Mon, Aug 09, 2021 at 05:34:41PM +0200, Daniel Kiper wrote: > On Mon, Aug 02, 2021 at 05:40:20PM +0800, Michael Chang via Grub-devel wr= ote: > > Currently the grub_diskfilter_memberlist function returns all physical > > volumes added to a volume group to which a logical volume (LV) belongs. > > However this is suboptimal as it doesn't fit the intended behavior of > > returning underlying devices that make up the LV. To give a clear > > picture, the result should be identical to running commands below to > > display the logical volumes with underlying physical volumes in use. > > > > lvs -o +devices /dev/system/root > > lvdisplay --maps /dev/system/root >=20 > May I ask you to provide output from these two commands too? Additionally= , > I think it would be useful to have output from "pvs" and "lvs" commands > in the commit message. I know "lsblk" shows you most information but > I think both "pvs" and "lvs" are giving more readable output. It sounds good as that helps better understanding. I'll add these information to next version. >=20 > > This change is required if any part of the PV not used by root LV is > > encrypted. Using this lvm setup as an example: > > > > localhost:~ # lsblk > > NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT > > vda 253:0 0 20G 0 disk > > =E2=94=9C=E2=94=80vda1 253:1 0 8M 0 part > > =E2=94=94=E2=94=80vda2 253:2 0 20G 0 part > > =E2=94=9C=E2=94=80system-swap 254:0 0 1.4G 0 lvm [SWAP] > > =E2=94=94=E2=94=80system-root 254:1 0 18.6G 0 lvm / > > vdb 253:16 0 10M 0 disk > > =E2=94=94=E2=94=80data 254:2 0 8M 0 crypt > > =E2=94=94=E2=94=80system-data 254:3 0 4M 0 lvm /data > > > > Running grub-install would end up with error because "system" VG > > contains /dev/vdb which is encrypted. > > > > error: attempt to install to encrypted disk without cryptodisk > > enabled. Set `GRUB_ENABLE_CRYPTODISK=3Dy' in file `/etc/default/grub'= . > > > > Certainly we can enable GRUB_ENABLE_CRYPTODISK=3Dy and move on, but tha= t > > is not always acceptable since the server may need to boot unattended, >=20 > s/,/./ OK. >=20 > > in addition typing passphase for every system startup can be a big >=20 > s/in addition/Additionally,/ OK. >=20 > > hassle of which most users would like to avoid. > > > > This patch solves the problem by returning physical volumes, /dev/vda2, > > rightly used by system-root in the example above, thus changing how > > grub-install perceives the underlying block device to boot and avoids > > the error from happening. > > > > Signed-off-by: Michael Chang > > Tested-by: Olav Reinert > > --- > > grub-core/disk/diskfilter.c | 44 +++++++++++++++++++++++-------------- > > 1 file changed, 27 insertions(+), 17 deletions(-) > > > > diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c > > index 6eb2349a6..595f5f70f 100644 > > --- a/grub-core/disk/diskfilter.c > > +++ b/grub-core/disk/diskfilter.c > > @@ -300,6 +300,8 @@ grub_diskfilter_memberlist (grub_disk_t disk) > > grub_disk_dev_t p; > > struct grub_diskfilter_vg *vg; > > struct grub_diskfilter_lv *lv2 =3D NULL; > > + struct grub_diskfilter_segment *seg; > > + unsigned int i, j; > > > > if (!lv->vg->pvs) > > return NULL; > > @@ -331,25 +333,33 @@ grub_diskfilter_memberlist (grub_disk_t disk) > > } > > } > > > > - for (pv =3D lv->vg->pvs; pv; pv =3D pv->next) > > - { > > - if (!pv->disk) > > + for (i =3D 0, seg =3D lv->segments; i < lv->segment_count; i++, seg+= +) > > + for (j =3D0; j < seg->node_count; ++j) >=20 > s/=3D0/=3D 0/ OK. >=20 > > + if ((pv =3D seg->nodes[j].pv)) >=20 > if (seg->nodes[j].pv !=3D NULL) >=20 > ...and then later... >=20 > pv =3D seg->nodes[j].pv OK. >=20 > > { > > - /* TRANSLATORS: This message kicks in during the detection of > > - which modules needs to be included in core image. This happens > > - in the case of degraded RAID and means that autodetection may > > - fail to include some of modules. It's an installation time > > - message, not runtime message. */ > > - grub_util_warn (_("Couldn't find physical volume `%s'." > > - " Some modules may be missing from core image."), > > - pv->name); > > - continue; > > + > > + if (!pv->disk) > > + { > > + /* TRANSLATORS: This message kicks in during the detection of > > + which modules needs to be included in core image. This happens > > + in the case of degraded RAID and means that autodetection may > > + fail to include some of modules. It's an installation time > > + message, not runtime message. */ >=20 > Please fix this comment formatting if you move it. May I ask what is wrong as it looks good to the indention that has been adjusted to the new place ? >=20 > > + grub_util_warn (_("Couldn't find physical volume `%s'." > > + " Some modules may be missing from core image."), > > + pv->name); > > + continue; > > + } > > + > > + for (tmp =3D list; tmp; tmp =3D tmp->next) >=20 > tmp !=3D NULL please... OK. >=20 > > + if (grub_strcmp (tmp->disk->name, pv->disk->name) =3D=3D 0) >=20 > !grub_strcmp() instead of grub_strcmp() =3D=3D 0 OK. Well. It appears "grub_strcmp() =3D=3D 0" more popular as it has more grep hits . But I am fine in doing either way. :) grep -R '!grub_strcmp' | wc -l 26 grep -R 'grub_strcmp.*=3D=3D 0' | wc -l 385 >=20 > > + continue; >=20 > I know what you mean but this "continue" does not make much sense here... Indeed this looks bogus. I will fix this in next version. >=20 > > + > > + tmp =3D grub_malloc (sizeof (*tmp)); >=20 > Please do not ignore grub_malloc() failures. OK. Thanks a lot for review. Michael >=20 > Daniel >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel