All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avnish Chouhan <avnish@linux.ibm.com>
To: "Michal Suchánek" <msuchanek@suse.de>
Cc: "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>,
	The development of GRUB 2 <grub-devel@gnu.org>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Michael Chang <mchang@suse.com>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Subject: Re: [PATCH v4] Mandatory install device check for PowerPC
Date: Fri, 30 Jan 2026 16:08:11 +0530	[thread overview]
Message-ID: <ad8abf1d62cc7bb87a76c688e94465a2@linux.ibm.com> (raw)
In-Reply-To: <aXtBFzNZZHI9lQQV@kitsune.suse.cz>

On 2026-01-29 16:44, Michal Suchánek wrote:
> On Thu, Jan 29, 2026 at 12:52:30PM +0300, Vladimir 'phcoder' Serbinenko 
> wrote:
>> How does it handle PPC macs? They don't use install device
> 
> As already commented on v3 this is in the else branch of the PPC Mac
> detection which is not clear from how the patch is written.
> 
> It could be better in that regard but that part of feedback was
> rejected.

Hi Michal,

Please don't get me wrong. I didn't reject your feedback. Just that I 
felt it will be better if we do this way. But after my reply, I haven't 
heard from you. You replied to it after I sent v4. I have shared the 
whole code block where I have added an else condition in my earlier 
reply.

If "if (macppcdir)" evaluates true (where I have added an else). this 
means it is indeed a Mac machine. And in this "if (macppcdir)" block, we 
set "is_prep = 0;". So adding an if condition based on "is_prep" after 
this "if (macppcdir)" rather than adding new else condition. I found the 
latter better, having an else rather than if with "is_prep = 0;".

But I really don't mind changing the way you like :) Please let me know, 
I'll change it v5.
Thank you!


Regards,
Avnish Chouhan


> 
> Thanks
> 
> Michal
> 
>> 
>> Regards
>> Vladimir 'phcoder' Serbinenko
>> 
>> Le mar. 27 janv. 2026, 16:48, Avnish Chouhan <avnish@linux.ibm.com> a
>> écrit :
>> 
>> > 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.
>> >
>> > 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 a
>> > 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)
>> > +            grub_util_error ("%s", _("install device isn't specified,
>> > required for PowerPC"));
>> > +       }
>> > +#endif /* __powerpc__ */
>> >      }
>> >
>> >    size_t ndev = 0;
>> > --
>> > 2.50.1 (Apple Git-155)
>> >
>> >

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

  reply	other threads:[~2026-01-30 10:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-27 13:48 [PATCH v4] Mandatory install device check for PowerPC Avnish Chouhan
2026-01-27 16:13 ` Michal Suchánek
2026-01-29  9:48   ` Avnish Chouhan
2026-01-29  9:52 ` Vladimir 'phcoder' Serbinenko
2026-01-29 10:59   ` Avnish Chouhan
2026-01-29 11:14   ` Michal Suchánek
2026-01-30 10:38     ` Avnish Chouhan [this message]
2026-01-30 12:00       ` Michal Suchánek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad8abf1d62cc7bb87a76c688e94465a2@linux.ibm.com \
    --to=avnish@linux.ibm.com \
    --cc=daniel.kiper@oracle.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=grub-devel@gnu.org \
    --cc=mchang@suse.com \
    --cc=msuchanek@suse.de \
    --cc=phcoder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.