Re: [PATCH v3] Mandatory install device check for PowerPC

[Avnish Chouhan <avnish@linux.ibm.com>]

On 2025-10-13 15:37, Michal Suchánek wrote:
> On Mon, Oct 13, 2025 at 03:09:59PM +0530, Avnish Chouhan wrote:
>> This patch adds a check on install_device while installing grub for 
>> PowerPC.
>> If install_device is not mentioned in grub2-install and machine is 
>> detected
>> as PowerPC, the error will be thrown and it will terminates the 
>> grub2-install
>> operation. Running grub2-install on PowerPC without the install_device 
>> may
>> result in bootlist corruption. When no install device is specified, it 
>> attempts
>> to load images from the filesystem, which leads to nvram bootlist 
>> corruption.
>> The idea is to fail the operation and avoid creating the invalid boot 
>> entry.
> 

Hi Michal,
Hope you're doing wonderful!
Thank you so much for your response!

Surely the changes can be done the way you mentioned. But in my honest 
opinion, keeping this check in "else" make the code looks lot more 
understandable (if we see the complete if else condition). Keeping this 
in as added  "else" part clearly stats that this check is only specific 
to PowerPC. And it has no impact on PowerMac.
Thank you!

Regards,
Avnish Chouhan

> Hello,
> 
> there is the part
> if (install_device)
>   is_prep = 1;
> 
> The check added by this patch now assumes PReP.
> 
> For consistency this should be set unconditionally now.
> 
> 
>> Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
>> ---
>>  util/grub-install.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/util/grub-install.c b/util/grub-install.c
>> index 0602465..f7389b3 100644
>> --- a/util/grub-install.c
>> +++ b/util/grub-install.c
>> @@ -1289,6 +1289,19 @@ main (int argc, char *argv[])
>>  	      is_prep = 0;
>>  	    }
>>  	}
>> +#if defined(__powerpc__)
>> +     else
>> +       {
>> +         /*
>> +	   * As the machine has been detected as PowerPC and not the 
>> PowerMac. We need to check
>> +	   * whether the install_device has been mentioned while installing. 
>> If no device has been
>> +	   * mentioned, we need to exit and mark it as an error as the 
>> install_device is required for
>> +	   * PowerPC installation. An installation with no device mentioned 
>> may lead to corruptions.
>> +	   */
>> +          if (!install_device)
> Then instead of nesting inside else you could change the condition to
> 
> if (is_prep && !install_device)
> 
> That clarifies what it is checking for, and will cover the cases when
> there is a partition that is checked for macppc content but none is
> found.
> 
> Thanks
> 
> Michal
>> +            grub_util_error ("%s", _("install device isn't specified 
>> required for PowerPC"));
> 
> I think this can be worded better.
> 
> Something like "No install device specified and no PowerPC Mac 
> partition found."
> 
> Thanks
> 
> Michal
> 
>> +       }
>> +#endif /* __powerpc__ */
>>      }
>> 
>>    size_t ndev = 0;
>> --
>> 2.47.3
>> 

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel