Re: [PATCH v4] Mandatory install device check for PowerPC
On Fri, Jan 30, 2026 at 04:08:11PM +0530, Avnish Chouhan wrote: > 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! Either way should work. The repeated questions about how to handle the mac case suggest that this way is not very clear. There is common practice of adding comments to preprocessor #else to make it clear what the other branch is about, and reducing nesting of the C if/else. Not sure it would do much for this particular case. Thanks Michal ___ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4] Mandatory install device check for PowerPC
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 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
> ---
> 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
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4] Mandatory install device check for PowerPC
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.
Thanks
Michal
>
> Regards
> Vladimir 'phcoder' Serbinenko
>
> Le mar. 27 janv. 2026, 16:48, Avnish Chouhan 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
> > ---
> > 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
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4] Mandatory install device check for PowerPC
On 2026-01-29 15:22, Vladimir 'phcoder' Serbinenko wrote:
How does it handle PPC macs? They don't use install device Regards
Vladimir 'phcoder' Serbinenko Le mar. 27 janv. 2026, 16: 48, Avnish
Chouhan a écrit : This patch adds a
check on install_device while
How does it handle PPC macs? They don't use install device
Regards
Vladimir 'phcoder' Serbinenko
Hi Vladimir,
Thank you so much for your review!
I have added this check in else condition, which is newly added to the
if condition where we decide whether machine is PowerMac or Non
PowerMac. So this check is not applicable to PowerMac. Sharing the whole
code block below where I have introduced this new else condition for
better understanding! Please let me know your suggestions on it. I have
tested this on IBM Power and it is working as expected. No issues
observed.
Regards,
Avnish Chouhan
Code:
if (platform == GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275)
{
int is_guess = 0;
if (!macppcdir)
{
char *d;
is_guess = 1;
d = grub_util_path_concat (2, bootdir, "macppc");
if (!grub_util_is_directory (d))
{
free (d);
d = grub_util_path_concat (2, bootdir, "efi");
}
/* Find the Mac HFS(+) System Partition. */
if (!grub_util_is_directory (d))
{
free (d);
d = grub_util_path_concat (2, bootdir, "EFI");
}
if (!grub_util_is_directory (d))
{
free (d);
d = 0;
}
if (d)
macppcdir = d;
}
if (macppcdir) <<< added else against this if condition
{
char **macppcdir_device_names = NULL;
grub_device_t macppcdir_grub_dev = NULL;
char *macppcdir_grub_devname;
grub_fs_t fs;
macppcdir_device_names = grub_guess_root_devices (macppcdir);
if (!macppcdir_device_names || !macppcdir_device_names[0])
grub_util_error (_("cannot find a device for %s (is /dev
mounted?)"),
macppcdir);
for (curdev = macppcdir_device_names; *curdev; curdev++)
grub_util_pull_device (*curdev);
macppcdir_grub_devname = grub_util_get_grub_dev
(macppcdir_device_names[0]);
if (!macppcdir_grub_devname)
grub_util_error (_("cannot find a GRUB drive for %s. Check
your device.map"),
macppcdir_device_names[0]);
macppcdir_grub_dev = grub_device_open
(macppcdir_grub_devname);
if (! macppcdir_grub_dev)
grub_util_error ("%s", grub_errmsg);
fs = grub_fs_probe (macppcdir_grub_dev);
if (! fs)
grub_util_error ("%s", grub_errmsg);
if (grub_strcmp (fs->name, "hfs") != 0
&& grub_strcmp (fs->name, "hfsplus") != 0
&& !is_guess)
grub_util_error (_("filesystem on %s is neither HFS nor
HFS+"),
macppcdir);
if (grub_strcmp (fs->name, "hfs") == 0
|| grub_strcmp (fs->name, "hfsplus") == 0)
{
install_device = macppcdir_device_names[0];
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__ */
}
*
Le mar. 27 janv. 2026, 16:48, Avnish Chouhan 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
---
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
PowerMa
Re: [PATCH v4] Mandatory install device check for PowerPC
How does it handle PPC macs? They don't use install device
Regards
Vladimir 'phcoder' Serbinenko
Le mar. 27 janv. 2026, 16:48, Avnish Chouhan 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
> ---
> 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
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4] Mandatory install device check for PowerPC
On 2026-01-27 21:43, Michal Suchánek wrote:
Hello,
how is this different from the previous revision?
Hi Michal,
Thank you so much for you views on the patch :)
There's a small change in the comment as addressed by John.
On Tue, Jan 27, 2026 at 07:18:14PM +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.
Signed-off-by: Avnish Chouhan
---
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__)
Is this ifdef needed?
This whole block is already in
if (platform == GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275)
and the same ifdef selects it as the default platform.
Presumably grub-install should be able to install for other platforms,
and as aresult such ifdefs are not welcome.
Sure. I'll drop this in next version!
+ 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.
Corruptions of what?
"nvram bootlist corruption". I'll add this too!
Regards,
Avnish Chouhan
+ */
+ 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)
Thanks
Michal
___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4] Mandatory install device check for PowerPC
Hello,
how is this different from the previous revision?
On Tue, Jan 27, 2026 at 07:18:14PM +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.
>
> Signed-off-by: Avnish Chouhan
> ---
> 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__)
Is this ifdef needed?
This whole block is already in
if (platform == GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275)
and the same ifdef selects it as the default platform.
Presumably grub-install should be able to install for other platforms,
and as aresult such ifdefs are not welcome.
> + 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.
Corruptions of what?
> +*/
> + 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)
Thanks
Michal
___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
