[PATCH] change grub_print_error to use stderr for the utils

[PATCH] change grub_print_error to use stderr for the utils

[Felix Zielcke]

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

Here's now the patch Robert requested.
grub_print_error shouldn't use stdout for the utils.

Why?

# grub-install /dev/sda
Unknown extra argument `Unsupported'.

This is with Bean's or mine lastest RAID patch.
/dev/sda isn't on a raid at all.
There are 4 disks attached to the VM which have a RAID 6 on it.
But the RAID 6 isn't even assembled, i.e. /proc/mdstat says nothing.

As always, the changelog isn't probable that perfect for you, so please comment :)

2008-08-12  Felix Zielcke <fzielcke@z-51.de>

        * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
          (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
          grub_printf.


[-- Attachment #2: kern_err.diff --]
[-- Type: text/x-patch, Size: 559 bytes --]

Index: kern/err.c
===================================================================
--- kern/err.c	(revision 1800)
+++ kern/err.c	(working copy)
@@ -21,6 +21,9 @@
 #include <grub/misc.h>
 #include <stdarg.h>
 
+#ifdef GRUB_UTIL
+#include <stdio.h>
+#endif
 #define GRUB_MAX_ERRMSG		256
 #define GRUB_ERROR_STACK_SIZE	10
 
@@ -121,7 +124,11 @@
   do
     {
       if (grub_errno != GRUB_ERR_NONE)
+#ifdef GRUB_UTIL
+	fprintf (stderr, "error: %s\n");
+#else
         grub_printf ("error: %s\n", grub_errmsg);
+#endif
     } 
   while (grub_error_pop ());
   

Re: [PATCH] change grub_print_error to use stderr for the utils

[Felix Zielcke]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Am Dienstag, den 12.08.2008, 17:30 +0200 schrieb Felix Zielcke:
> Here's now the patch Robert requested.
> grub_print_error shouldn't use stdout for the utils.
> 
Bah and I attached the broken one not the working one.
So please use that one.

[-- Attachment #2: kern_err.diff --]
[-- Type: text/x-patch, Size: 572 bytes --]

Index: kern/err.c
===================================================================
--- kern/err.c	(revision 1800)
+++ kern/err.c	(working copy)
@@ -21,6 +21,9 @@
 #include <grub/misc.h>
 #include <stdarg.h>
 
+#ifdef GRUB_UTIL
+#include <stdio.h>
+#endif
 #define GRUB_MAX_ERRMSG		256
 #define GRUB_ERROR_STACK_SIZE	10
 
@@ -121,7 +124,11 @@
   do
     {
       if (grub_errno != GRUB_ERR_NONE)
+#ifdef GRUB_UTIL
+	fprintf (stderr, "error: %s\n", grub_errmsg);
+#else
         grub_printf ("error: %s\n", grub_errmsg);
+#endif
     } 
   while (grub_error_pop ());
   

Re: [PATCH] change grub_print_error to use stderr for the utils

[Vesa Jääskeläinen]

Felix Zielcke wrote:
> Here's now the patch Robert requested.
> grub_print_error shouldn't use stdout for the utils.

> +#ifdef GRUB_UTIL
> +	fprintf (stderr, "error: %s\n");
> +#else
>          grub_printf ("error: %s\n", grub_errmsg);
> +#endif

There is at least bug here....

Re: [PATCH] change grub_print_error to use stderr for the utils

[Robert Millan]

On Tue, Aug 12, 2008 at 05:30:15PM +0200, Felix Zielcke wrote:
> 
> 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
> 
>         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
>           (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
>           grub_printf.

IIRC we usually put the [FOO] before the (bar).

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Dienstag, den 12.08.2008, 18:36 +0300 schrieb Vesa Jääskeläinen:
> Felix Zielcke wrote:
> > Here's now the patch Robert requested.
> > grub_print_error shouldn't use stdout for the utils.
> 
> > +#ifdef GRUB_UTIL
> > +	fprintf (stderr, "error: %s\n");
> > +#else
> >          grub_printf ("error: %s\n", grub_errmsg);
> > +#endif
> 
> There is at least bug here....

Just see my 2nd mail, I noticed this even before I saw yours.

Re: [PATCH] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Dienstag, den 12.08.2008, 17:41 +0200 schrieb Robert Millan:
> On Tue, Aug 12, 2008 at 05:30:15PM +0200, Felix Zielcke wrote:
> > 
> > 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
> > 
> >         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
> >           (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
> >           grub_printf.
> 
> IIRC we usually put the [FOO] before the (bar).
> 

2008-07-30  Robert Millan  <rmh@aybabtu.com>

        * include /grub/powerpc/ieee1275/kernel.h [!ASM_FILE] (grub_prefix):
        New variable reference.

2008-05-05  Christian Franke  <franke@computer.org>

        * util/grub-mkdevicemap.c (get_floppy_disk_name) [__CYGWIN__]:
        Add Cygwin device names.

But because I have more to do with you Robert then with Christian ;)

2008-08-12  Felix Zielcke <fzielcke@z-51.de>
  
        * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
          [GRUB_UTIL] (grub_print_error): Use fprintf (stderr, ...) instead of   
          grub_printf.

Re: [PATCH] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Dienstag, den 12.08.2008, 17:30 +0200 schrieb Felix Zielcke:
> Here's now the patch Robert requested.
> grub_print_error shouldn't use stdout for the utils.
> 
> Why?
> 
> # grub-install /dev/sda
> Unknown extra argument `Unsupported'.
> 

Ok this is only clear for Bean, Robert and me :)
So here's a longer explaination for this.

The Unsupported comes from here

fz@fz:~/grub/grub2.svn$ grep -r "Unsupported" *
disk/raid.c:		  "Unsupported RAID version: %d.%d",
disk/raid.c:		  "Unsupported RAID level: %d",
disk/raid.c:		  "Unsupported RAID5 layout: %d",

In my case it's Unsupported RAID level: 6

The problem is grub_print_error () uses stdout even for grub-probe which
is used by the grub-install script
So instead of ext2 or whatever grub-probe would display for
grub-install, grub-install get's Unsupported.

It isn't right to use stdout for errors in the utils.
real grub of course doestn't care about stdout or stderr.

Re: [PATCH] change grub_print_error to use stderr for the utils

[Marco Gerards]

Felix Zielcke <fzielcke@z-51.de> writes:

> Am Dienstag, den 12.08.2008, 17:41 +0200 schrieb Robert Millan:
>> On Tue, Aug 12, 2008 at 05:30:15PM +0200, Felix Zielcke wrote:
>> > 
>> > 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
>> > 
>> >         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
>> >           (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
>> >           grub_printf.
>> 
>> IIRC we usually put the [FOO] before the (bar).
>> 
>
> 2008-07-30  Robert Millan  <rmh@aybabtu.com>
>
>         * include /grub/powerpc/ieee1275/kernel.h [!ASM_FILE] (grub_prefix):
>         New variable reference.
>
> 2008-05-05  Christian Franke  <franke@computer.org>
>
>         * util/grub-mkdevicemap.c (get_floppy_disk_name) [__CYGWIN__]:
>         Add Cygwin device names.
>
> But because I have more to do with you Robert then with Christian ;)
>
> 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
>   
>         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
>           [GRUB_UTIL] (grub_print_error): Use fprintf (stderr, ...) instead of   
>           grub_printf.

Actually, both syntaxes are used.  See:

http://www.gnu.org/prep/standards/html_node/Conditional-Changes.html#Conditional-Changes

Please do not use the current changelog as a manual, only for
examples.  The GCS is the manual :-)

--
Marco

Re: [PATCH] change grub_print_error to use stderr for the utils

[Marco Gerards]

Felix Zielcke <fzielcke@z-51.de> writes:

> Here's now the patch Robert requested.
> grub_print_error shouldn't use stdout for the utils.
>
> Why?
>
> # grub-install /dev/sda
> Unknown extra argument `Unsupported'.
>
> This is with Bean's or mine lastest RAID patch.
> /dev/sda isn't on a raid at all.
> There are 4 disks attached to the VM which have a RAID 6 on it.
> But the RAID 6 isn't even assembled, i.e. /proc/mdstat says nothing.
>
> As always, the changelog isn't probable that perfect for you, so please comment :)
>
> 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
>
>         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
>           (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
>           grub_printf.

If there is another way to do this, I would prefer this.

--
Marco

[RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Hello,

Am Mittwoch, den 13.08.2008, 11:38 +0200 schrieb Marco Gerards:
> > 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
> >
> >         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
> >           (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
> >           grub_printf.
> 
> If there is another way to do this, I would prefer this.

Now with remaking that grub_print_error () patch I began to doubt if
that `GRUB fears raid 6 so much that it goes to rescue mode if it just
sees one which it doestn't need to access at all' should be changed for
lenny.
Robert confirmed my doubts on IRC i.e. he said, it's better to discuss
the whole thing first.

This is my first mail about this:
Message-Id: <1218454936.4035.19.camel@fz.local>

just using grub_dprintf isn't that great, I'd like to have at least
`grub-probe -vv' show the unknown RAID level message.

The other patch, which changes grub_print_error () to use stderr and let
the raid.mod itself print the error instead of giving it to the upper
layer isn't that great either.

Real GRUB shows for every RAID 6 disk once `unknown RAID level 6' e.g.
in my VMware testcase with 4 disks 4 times.

grub-install /dev/sda with raid 6 not even assembled i.e.
empty /proc/mdstat shows it 6 times, I think once for each time
grub-probe gets called.

By the way my last patch isn't even compiling because I forgot to make
some other stuff from kern/err.c avaible to it i.e.
grub_error_stack_assert is declared as static  but used in
grub_print_error ()
and there's another problem grub-mkimage includes util/misc.c but not
kern/err.c so the other functions and variables needed by it aren't
avaible.

So please comment this.

Re: [RFC] change grub_print_error to use stderr for the utils

[Marco Gerards]

Felix Zielcke <fzielcke@z-51.de> writes:

> Hello,
>
> Am Mittwoch, den 13.08.2008, 11:38 +0200 schrieb Marco Gerards:
>> > 2008-08-12  Felix Zielcke <fzielcke@z-51.de>
>> >
>> >         * kern/err.c [GRUB_UTIL]: Include <stdio.h>.
>> >           (grub_print_error) [GRUB_UTIL]: Use fprintf (stderr, ...) instead of 
>> >           grub_printf.
>> 
>> If there is another way to do this, I would prefer this.
>
> Now with remaking that grub_print_error () patch I began to doubt if
> that `GRUB fears raid 6 so much that it goes to rescue mode if it just
> sees one which it doestn't need to access at all' should be changed for
> lenny.
> Robert confirmed my doubts on IRC i.e. he said, it's better to discuss
> the whole thing first.
>
> This is my first mail about this:
> Message-Id: <1218454936.4035.19.camel@fz.local>
>
> just using grub_dprintf isn't that great, I'd like to have at least
> `grub-probe -vv' show the unknown RAID level message.
>
> The other patch, which changes grub_print_error () to use stderr and let
> the raid.mod itself print the error instead of giving it to the upper
> layer isn't that great either.
>
> Real GRUB shows for every RAID 6 disk once `unknown RAID level 6' e.g.
> in my VMware testcase with 4 disks 4 times.
>
> grub-install /dev/sda with raid 6 not even assembled i.e.
> empty /proc/mdstat shows it 6 times, I think once for each time
> grub-probe gets called.
>
> By the way my last patch isn't even compiling because I forgot to make
> some other stuff from kern/err.c avaible to it i.e.
> grub_error_stack_assert is declared as static  but used in
> grub_print_error ()
> and there's another problem grub-mkimage includes util/misc.c but not
> kern/err.c so the other functions and variables needed by it aren't
> avaible.
>
> So please comment this.

So what do you propose? :-)  You lost me midway since you change
subjects a bit often ;(

--
Marco

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Mittwoch, den 13.08.2008, 20:41 +0200 schrieb Marco Gerards:
> So what do you propose? :-)  You lost me midway since you change
> subjects a bit often ;(

Oh sorry for that this time I only changed [PATCH] to [RFC]
but yeah this whole topic isn't in one subject :(

But the important things I already said in the [RFC] mail, or in the
mail with the msgid I said, e.g. the link to our bug report.

I wanted a small and simple fix for this problem which could go into
Lenny.
I think more a mix of grub_dprint for real GRUB and showing an error in
grub-probe IF a device which GRUB needs to access contains an
unsupported RAID level or unsupported version of superblock.
By the way that `unsupported version' is pretty useless to check for
because each version (0.90, 1.0, 1.1 , 1.2) stores it at different
position. Maybe 0.90 and one 1.x has the same one but then still 2 which
hasn't.

With Bean's raid_6.diff (i.e. the one before he just sent now) this
`unknown RAID level 6' message is shown even more then with mine.
I think 15-20 times maybe even more on a grub-install /dev/sda1
I haven't tried it on real GRUB.

As Robert already (i think more indirectly) said we shouldn't fix things
on different codebases.
So either we just wait for Bean to commit his changes and address the
problems there or we try to address this problem just with the current
one and Bean has to adjust his patch then.

Maybe it's better to have that bug report not fixed in lenny, but
probable that's more a discussion with just Robert.

I haven't tried yet but Bean said he implemented Raid 6 now so the whole
problem isn't that important then anymore.
But this won't be for sure in lenny :(

Re: [RFC] change grub_print_error to use stderr for the utils

[Marco Gerards]

Felix Zielcke <fzielcke@z-51.de> writes:

> Am Mittwoch, den 13.08.2008, 20:41 +0200 schrieb Marco Gerards:
>> So what do you propose? :-)  You lost me midway since you change
>> subjects a bit often ;(
>
> Oh sorry for that this time I only changed [PATCH] to [RFC]
> but yeah this whole topic isn't in one subject :(
>
> But the important things I already said in the [RFC] mail, or in the
> mail with the msgid I said, e.g. the link to our bug report.
>
> I wanted a small and simple fix for this problem which could go into
> Lenny.
> I think more a mix of grub_dprint for real GRUB and showing an error in
> grub-probe IF a device which GRUB needs to access contains an
> unsupported RAID level or unsupported version of superblock.
> By the way that `unsupported version' is pretty useless to check for
> because each version (0.90, 1.0, 1.1 , 1.2) stores it at different
> position. Maybe 0.90 and one 1.x has the same one but then still 2 which
> hasn't.
>
> With Bean's raid_6.diff (i.e. the one before he just sent now) this
> `unknown RAID level 6' message is shown even more then with mine.
> I think 15-20 times maybe even more on a grub-install /dev/sda1
> I haven't tried it on real GRUB.
>
> As Robert already (i think more indirectly) said we shouldn't fix things
> on different codebases.
> So either we just wait for Bean to commit his changes and address the
> problems there or we try to address this problem just with the current
> one and Bean has to adjust his patch then.
>
> Maybe it's better to have that bug report not fixed in lenny, but
> probable that's more a discussion with just Robert.
>
> I haven't tried yet but Bean said he implemented Raid 6 now so the whole
> problem isn't that important then anymore.
> But this won't be for sure in lenny :(

Well, ok.  So thus we are indeed fixing a problem in Lenny inside
GRUB?  On IRC someone said this was *not* the case.  I am very
confused...

--
Marco

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Donnerstag, den 14.08.2008, 09:07 +0200 schrieb Marco Gerards:

Honourly I want that whole topic discussed with everyone who is
subscribed here and not just with you Marco, but it's very kind that at
least you reply :)

> 
> Well, ok.  So thus we are indeed fixing a problem in Lenny inside
> GRUB?  On IRC someone said this was *not* the case.  I am very
> confused...

Yeah sorry it is very confusing.
I want (wanted ?) to have that problem _for_ lenny fixed.


The problem (in current SVN without Bean's RAID rewrite, this won't be
in lenny anway) is:

As soon as real GRUB _sees_ one disk wich has a mdraid raid level 6 (or
e.g. 10, it's not supported either) super block then it goes to rescue
mode.

First patch from me: <1218454936.4035.19.camel@fz.local>

Problem: The messages get totally lost for the average user, i.e. even
grub-probe --vv says nothing, that RAID 6 isn't supported.

Robert suggested (20080811142856.GB15950@thorin) to handle it with
grub_print_error () and grub_errno = GRUB_ERR_NONE in the raid.mod
itself.

Problem: With that way + my fprintf stderr change, it's displayed in
real GRUB once for every disk.
In grub-install /dev/sda1, where that isn't on RAID at all it's shown 6
times, I _think_ once for every grub-probe call.

Bean's raid_6.diff (ca0f59980808120449s61abd11iefb197cd3b1672d5@mail.gmail.com)
and my fprintf change

Problem: The message on grub-install is now shown much more then just 6
times as with mine, I think more then 20 times.


I haven't tried Bean's lastest patch with an unsupported RAID level like
10 now that he has RAID 6 implemented.
But the problem is still there.

I'd like to have that unknown RAID level message shown only once per
array not for every disk.

The `grub_print_error fprintf (stderr ..)' is probable the right first
step, but it isn't a 100% perfect fix for this.

Seems like I just have to try lastest patch from Bean, if he changed
something and then maybe complain at his topic about this.


For lenny Robert and me need a simple small patch to fix this.
Maybe it's just better to have it for lenny unfixed.

Re: [RFC] change grub_print_error to use stderr for the utils

[Marco Gerards]

Felix Zielcke <fzielcke@z-51.de> writes:

> Am Donnerstag, den 14.08.2008, 09:07 +0200 schrieb Marco Gerards:
>
> Honourly I want that whole topic discussed with everyone who is
> subscribed here and not just with you Marco, but it's very kind that at
> least you reply :)

Hey, at least you get a reply.  I can't say the same about many topics
I start :P

>> Well, ok.  So thus we are indeed fixing a problem in Lenny inside
>> GRUB?  On IRC someone said this was *not* the case.  I am very
>> confused...
>
> Yeah sorry it is very confusing.
> I want (wanted ?) to have that problem _for_ lenny fixed.

And I do not want to compromise the code quality because of a release
schedule ;-)

> The problem (in current SVN without Bean's RAID rewrite, this won't be
> in lenny anway) is:
>
> As soon as real GRUB _sees_ one disk wich has a mdraid raid level 6 (or
> e.g. 10, it's not supported either) super block then it goes to rescue
> mode.
>
> First patch from me: <1218454936.4035.19.camel@fz.local>

[...]

Ah! ;-)

--
Marco

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Donnerstag, den 14.08.2008, 10:03 +0200 schrieb Marco Gerards:
> Felix Zielcke <fzielcke@z-51.de> writes:
> 
> > Am Donnerstag, den 14.08.2008, 09:07 +0200 schrieb Marco Gerards:
> >
> > Honourly I want that whole topic discussed with everyone who is
> > subscribed here and not just with you Marco, but it's very kind that at
> > least you reply :)
> 
> Hey, at least you get a reply.  I can't say the same about many topics
> I start :P

Yep, that's why I said that this is very kind of you :)
But at least Robert and Bean should say something about that.
Bean is rewritting the whole RAID code anyway and Robert has at least to
decide if this could go into lenny or not

> > Yeah sorry it is very confusing.
> > I want (wanted ?) to have that problem _for_ lenny fixed.
> 
> And I do not want to compromise the code quality because of a release
> schedule ;-)
> 

Yep right Robert just wanted to have it reviewed by you which i can
totally understand.
Maybe I should say that we even have to stay with SVN 2008-07-24 i.e. no
new upstream versions during the freeze.
It doestn't get automatically to lenny if this is now commited
upstream :(

Re: [RFC] change grub_print_error to use stderr for the utils

[Robert Millan]

On Thu, Aug 14, 2008 at 10:03:27AM +0200, Marco Gerards wrote:
> >> Well, ok.  So thus we are indeed fixing a problem in Lenny inside
> >> GRUB?  On IRC someone said this was *not* the case.  I am very
> >> confused...
> >
> > Yeah sorry it is very confusing.
> > I want (wanted ?) to have that problem _for_ lenny fixed.
> 
> And I do not want to compromise the code quality because of a release
> schedule ;-)

Would all of you please stop talking about debian and lenny?  This is about
a bug in GRUB. :-/

The only thing I said, is that it is easier for us (debian) if the bug is
fixed before Bean's RAID refactoring.  But the problem will still be there
after that is merged.

Anyway, please let's not waste time in this discussion.  It's only a matter
on which patch goes first.  If Bean wants to checkin his code first, then
it just means we'll need to adjust the raid.c part a bit, and that's all.

--
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Donnerstag, den 14.08.2008, 19:54 +0200 schrieb Robert Millan:

> Would all of you please stop talking about debian and lenny?  This is about
> a bug in GRUB. :-/

Yep, I think so now too that this should be first fixed upstream and
then we can talk about lenny.


> Anyway, please let's not waste time in this discussion.  It's only a matter
> on which patch goes first.  If Bean wants to checkin his code first, then
> it just means we'll need to adjust the raid.c part a bit, and that's all.

The grub_print_error change is now commited.
The disk/raid.c change you proposed is in 

Date: Mon, 11 Aug 2008 23:34:28 +0200
Message-Id: <1218490468.31147.6.camel@fz.local>

I don't mind if that or Bean's get commited first.

I still haven't tried his lastest patch, now that it includes RAID 6
support the message is anyway for more users not at all shown.

But I still don't like it that the message is shown for every disk
instead of once for the whole array.
But that's a different problem and well for this we can just reuse that
[RFC] topic by me to discuss this then :)

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Donnerstag, den 14.08.2008, 19:54 +0200 schrieb Robert Millan:

> Anyway, please let's not waste time in this discussion.  It's only a matter
> on which patch goes first.  If Bean wants to checkin his code first, then
> it just means we'll need to adjust the raid.c part a bit, and that's all.
> 
This is with the current SVN without any patches except Marco's atap
scsi one.
This is on my real system, i.e. with my 2 SATA RAID 0 disks.

Does it get now clear why I want a discussion about this whole topic?

fz:~# update-grub
        error: out of disk
        error: out of disk
        Updating /boot/grub/grub.cfg ...
        error: out of disk
        error: out of disk
        error: out of disk
        Found Debian background: debian-blueish-wallpaper-640x480.png
        error: out of disk
        error: out of disk
        error: out of disk
        error: out of disk
        Found linux image: /boot/vmlinuz-2.6.26-1-amd64
        Found initrd image: /boot/initrd.img-2.6.26-1-amd64
        error: out of disk
        error: out of disk
        error: out of disk
        error: out of disk
        done
        fz:~# grub-install /dev/sda
        error: out of disk
        error: out of disk
        error: out of disk
        error: out of disk
        error: out of disk
        error: out of disk
        error: out of disk
        Installation finished. No error reported.
        This is the contents of the device map /boot/grub/device.map.
        Check if this is correct or not. If any of the lines is incorrect,
        fix it and re-run the script `grub-install'.
        (hd0)	/dev/hda
        (hd1)	/dev/sda
        (hd2)	/dev/sdb

Re: [RFC] change grub_print_error to use stderr for the utils

[Robert Millan]

On Sat, Aug 16, 2008 at 05:06:18PM +0200, Felix Zielcke wrote:
> Am Donnerstag, den 14.08.2008, 19:54 +0200 schrieb Robert Millan:
> 
> > Anyway, please let's not waste time in this discussion.  It's only a matter
> > on which patch goes first.  If Bean wants to checkin his code first, then
> > it just means we'll need to adjust the raid.c part a bit, and that's all.
> > 
> This is with the current SVN without any patches except Marco's atap
> scsi one.
> This is on my real system, i.e. with my 2 SATA RAID 0 disks.
> 
> Does it get now clear why I want a discussion about this whole topic?
> 
> fz:~# update-grub
>         error: out of disk
>         error: out of disk
>         Updating /boot/grub/grub.cfg ...

This just means someone is reporting this error and shouldn't.  It's a bug.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Sonntag, den 17.08.2008, 17:07 +0200 schrieb Robert Millan:
> > fz:~# update-grub
> >         error: out of disk
> >         error: out of disk
> >         Updating /boot/grub/grub.cfg ...
> 
> This just means someone is reporting this error and shouldn't.  It's a bug.
> 
Hi Robert,

Marco already said on IRC before I even send the mail that the error
shouldn't be displayed if a dmraid is detected on it.
Maybe too complicated and the error shouldn't be shown at all.
The error won't be shown on real GRUB anyway.

Please don't get me wrong, I don't want to complain about the already
commited code.

Now with Bean's lastest patch with even RAID 10 support there is never
for mdraid's now shown "unknown RAID level".
And my original intention was, to have it only shown once per array
instead of once per disk on array.

But I don't know what error messages can currently appear or will in the
future.
I only want a good solution for the long term :)

Maybe just grub-install and update-grub should be a little changed to
supress repeating the errors that much, i.e. not on every grub-probe
run?

Re: [RFC] change grub_print_error to use stderr for the utils

[Robert Millan]

On Sun, Aug 17, 2008 at 05:24:07PM +0200, Felix Zielcke wrote:
> Am Sonntag, den 17.08.2008, 17:07 +0200 schrieb Robert Millan:
> > > fz:~# update-grub
> > >         error: out of disk
> > >         error: out of disk
> > >         Updating /boot/grub/grub.cfg ...
> > 
> > This just means someone is reporting this error and shouldn't.  It's a bug.
> > 
> Hi Robert,
> 
> Marco already said on IRC before I even send the mail that the error
> shouldn't be displayed if a dmraid is detected on it.
> Maybe too complicated and the error shouldn't be shown at all.

Hiding errors is a very bad practice.  And hiding them because the output
is ugly or because something is reporting false positives is even worse
IMHO.

This "out of disk" sounds like a false positive indeed.  It's a bug,
somewhere.  Something is handling errors in a place where this particular
error shouldn't be reported.  Maybe it should handle them somewhere else,
or maybe they should be handled selectively.

> The error won't be shown on real GRUB anyway.

Why not?

> Maybe just grub-install and update-grub should be a little changed to
> supress repeating the errors that much, i.e. not on every grub-probe
> run?

That's difficult to archieve due to current design, but sounds to me as
a minor problem in comparison with what we had before.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Sonntag, den 17.08.2008, 17:34 +0200 schrieb Robert Millan:
> Hiding errors is a very bad practice.  And hiding them because the output
> is ugly or because something is reporting false positives is even worse
> IMHO.
> 
> This "out of disk" sounds like a false positive indeed.  It's a bug,
> somewhere.  Something is handling errors in a place where this particular
> error shouldn't be reported.  Maybe it should handle them somewhere else,
> or maybe they should be handled selectively.

This is because of my nForce 4 Hardware RAID 0 (i.e. dmraid).
Linux sees 2 disks (sda,sdb) and the partition table of the first is for
the 2 disks combined.
Both disk have only 80 GB but both combined have 160 GB.

> > The error won't be shown on real GRUB anyway.
> 
> Why not?

Oh I should have said in my case.
The BIOS presents GRUB only one (hd0) for the combined 2 disks.
Whereas the ata.mod from Marco sees both (well it fails to detect one
but in theory)
The BIOS does handle the RAID 0 for booting, but just for that.
As soon as the OS takes over it has to handle it.

> > Maybe just grub-install and update-grub should be a little changed to
> > supress repeating the errors that much, i.e. not on every grub-probe
> > run?
> 
> That's difficult to archieve due to current design, but sounds to me as
> a minor problem in comparison with what we had before.

Yes it isn't that important,
as I already said I only want to find a clean solution for the long
term.
The current way is better then before, but at least I don't like it to
have a error message shown more then needed.

This doestn't look that good either ;)

[...]
error: out of disk
Installation finished. No error reported.

Re: [RFC] change grub_print_error to use stderr for the utils

[Robert Millan]

On Sun, Aug 17, 2008 at 05:47:03PM +0200, Felix Zielcke wrote:
> Am Sonntag, den 17.08.2008, 17:34 +0200 schrieb Robert Millan:
> > Hiding errors is a very bad practice.  And hiding them because the output
> > is ugly or because something is reporting false positives is even worse
> > IMHO.
> > 
> > This "out of disk" sounds like a false positive indeed.  It's a bug,
> > somewhere.  Something is handling errors in a place where this particular
> > error shouldn't be reported.  Maybe it should handle them somewhere else,
> > or maybe they should be handled selectively.
> 
> This is because of my nForce 4 Hardware RAID 0 (i.e. dmraid).
> Linux sees 2 disks (sda,sdb) and the partition table of the first is for
> the 2 disks combined.
> Both disk have only 80 GB but both combined have 160 GB.

If I understood correctly, nvidia's is a _software_ RAID solution which is
implemented in Linux, in BIOS and in Windows (via non-native drivers), and
is marketed as if it were hardware RAID.

Anyway, this doesn't answer the question on where is this "out of disk"
error issued, and why.

> Oh I should have said in my case.
> The BIOS presents GRUB only one (hd0) for the combined 2 disks.

Ah, I see.  Sounds like a very nasty situation.  We'll probably need special
logic at install time to sort this out?  At least, just to skip raid.mod
inclussion and use UUIDs for the rest.

Anyway, how is this related to the "out of disk" error?

> This doestn't look that good either ;)
> 
> [...]
> error: out of disk
> Installation finished. No error reported.

This "No error reported" seems to assume non-critical errors aren't
possible.  How about we remove that part of the message?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [RFC] change grub_print_error to use stderr for the utils

[Felix Zielcke]

Am Sonntag, den 17.08.2008, 18:37 +0200 schrieb Robert Millan:
> 
> If I understood correctly, nvidia's is a _software_ RAID solution which is
> implemented in Linux, in BIOS and in Windows (via non-native drivers), and
> is marketed as if it were hardware RAID.

Bah normally I even say myself to this `fake hardware RAID'.
But yes, this is neither a real hardware RAID controller nor like real
software RAID like mdraid on Linux.

> Anyway, this doesn't answer the question on where is this "out of disk"
> error issued, and why.

This is just from grub-probe -vv / where / is even on my IDE disk.

/home/fz/grub/grub2-1.96+20080816/kern/disk.c:368: Reading `hd1,2'...
/home/fz/grub/grub2-1.96+20080816/kern/disk.c:375: Read out of range:
sector 0x100f9b45 (out of disk).
/home/fz/grub/grub2-1.96+20080816/kern/disk.c:312: Closing `hd1,2'.
/home/fz/grub/grub2-1.96+20080816/partmap/pc.c:143: partition 2: flag
0x0, type 0x0, start 0x0, len 0x0
/home/fz/grub/grub2-1.96+20080816/partmap/pc.c:143: partition 3: flag
0x0, type 0x0, start 0x0, len 0x0
/home/fz/grub/grub2-1.96+20080816/kern/disk.c:312: Closing `hd1'.
error: out of disk

> > Oh I should have said in my case.
> > The BIOS presents GRUB only one (hd0) for the combined 2 disks.
> 
> Ah, I see.  Sounds like a very nasty situation.  We'll probably need special
> logic at install time to sort this out?  At least, just to skip raid.mod
> inclussion and use UUIDs for the rest.

Yes not very easy.

> Anyway, how is this related to the "out of disk" error?

The partiton table is for both disks.
It says there are 160 GB, but grub-probe sees only 2 80 GB one.

> > error: out of disk
> > Installation finished. No error reported.
> 
> This "No error reported" seems to assume non-critical errors aren't
> possible.  How about we remove that part of the message?

Yes, would be at least better and I don't have yet a different idea.

Re: [RFC] change grub_print_error to use stderr for the utils

[Bean]

On Mon, Aug 18, 2008 at 12:58 AM, Felix Zielcke <fzielcke@z-51.de> wrote:
> Am Sonntag, den 17.08.2008, 18:37 +0200 schrieb Robert Millan:
>>
>> If I understood correctly, nvidia's is a _software_ RAID solution which is
>> implemented in Linux, in BIOS and in Windows (via non-native drivers), and
>> is marketed as if it were hardware RAID.
>
> Bah normally I even say myself to this `fake hardware RAID'.
> But yes, this is neither a real hardware RAID controller nor like real
> software RAID like mdraid on Linux.
>
>> Anyway, this doesn't answer the question on where is this "out of disk"
>> error issued, and why.
>
> This is just from grub-probe -vv / where / is even on my IDE disk.
>
> /home/fz/grub/grub2-1.96+20080816/kern/disk.c:368: Reading `hd1,2'...
> /home/fz/grub/grub2-1.96+20080816/kern/disk.c:375: Read out of range:
> sector 0x100f9b45 (out of disk).
> /home/fz/grub/grub2-1.96+20080816/kern/disk.c:312: Closing `hd1,2'.
> /home/fz/grub/grub2-1.96+20080816/partmap/pc.c:143: partition 2: flag
> 0x0, type 0x0, start 0x0, len 0x0
> /home/fz/grub/grub2-1.96+20080816/partmap/pc.c:143: partition 3: flag
> 0x0, type 0x0, start 0x0, len 0x0
> /home/fz/grub/grub2-1.96+20080816/kern/disk.c:312: Closing `hd1'.
> error: out of disk
>
>> > Oh I should have said in my case.
>> > The BIOS presents GRUB only one (hd0) for the combined 2 disks.
>>
>> Ah, I see.  Sounds like a very nasty situation.  We'll probably need special
>> logic at install time to sort this out?  At least, just to skip raid.mod
>> inclussion and use UUIDs for the rest.
>
> Yes not very easy.
>
>> Anyway, how is this related to the "out of disk" error?
>
> The partiton table is for both disks.
> It says there are 160 GB, but grub-probe sees only 2 80 GB one.
>
>> > error: out of disk
>> > Installation finished. No error reported.
>>
>> This "No error reported" seems to assume non-critical errors aren't
>> possible.  How about we remove that part of the message?
>
> Yes, would be at least better and I don't have yet a different idea.

Hi,

Actually, "out of disk" error is normal. raid needs to scan all
devices, some of them is virtual, like (host), it will cause problem
when trying to read from them.

In my raid patch, I just ignore error code GRUB_ERR_OUT_OF_RANGE.

-- 
Bean