Re: [RFC PATCH 0/2] grub-probe: improve message when disk cannot be opened

[Antonio Ospite]

On Mon, 5 Sep 2016 21:40:30 +0200
Antonio Ospite <address@hidden> wrote:

> Ping.
> 
> two weeks ago I sent the patches referenced in the message below and
> I haven't received a reply yet.
> 

Hi,

another month has passed, has anyone had a chance to take a look?

As said in the original message quoted below the issue is also described
in https://savannah.gnu.org/bugs/?42409

Thanks,
   Antonio

> 
> On Tue, 23 Aug 2016 12:17:26 +0200
> Antonio Ospite <address@hidden> wrote:
> 
> > Hi,
> > 
> > right now when running grub-probe and the disk cannot be opened the
> > error message can be misleading, it always says "disk not found" but in
> > some case the disk may exist but the user has no permissions to open
> > it, see https://savannah.gnu.org/bugs/?42409
> > 
> > The message can be improved by propagating informations about the cause
> > of the failure from the bottom layers.
> > 
> > Please take a look at the patches in this series, I marked them RFC
> > because I am new to grub code and I am not sure that all the error paths
> > set a meaningful message, but I wanted some code to base the discussion
> > on.
> > 
> > Patch 1/2 aims to prevent overriding the error messages from the
> > disk->open() methods called in grub_disk_open().
> > 
> > Patch 2/2 exposes to the user the —now more informative— message from
> > grub_disk_open().
> > 
> > Before these changes I was getting this output when running grub-probe
> > with insufficient permissions:
> > 
> >   $ LANG=C ./grub-probe /
> >   ./grub-probe: warning: disk does not exist, so falling back to partition 
> > device /dev/sda1.
> >   ./grub-probe: warning: disk does not exist, so falling back to partition 
> > device /dev/sda1.
> >   ./grub-probe: warning: disk does not exist, so falling back to partition 
> > device /dev/sda1.
> >   ./grub-probe: error: disk `hostdisk//dev/sda1' not found.
> > 
> > 
> > After the patches the reason of the failure is clearer:
> > 
> >   $ LANG=C ./grub-probe /
> >   ./grub-probe: warning: cannot open `/dev/sda': Permission denied, so 
> > falling back to partition device /dev/sda1.
> >   ./grub-probe: warning: cannot open `/dev/sda': Permission denied, so 
> > falling back to partition device /dev/sda1.
> >   ./grub-probe: warning: cannot open `/dev/sda': Permission denied, so 
> > falling back to partition device /dev/sda1.
> >   ./grub-probe: error: cannot open `/dev/sda1': Permission denied.
> > 
> > 
> > As I said, this is the final effect I am aiming for but I am not 100%
> > sure my solution is complete.
> > 
> > Thanks,
> >    Antonio
> > 
> > 
> > Antonio Ospite (2):
> >   * grub-core/kern/disk.c: don't override grub_errmsg when
> >     grub_disk_open fails
> >   * util/getroot.c: improve error message when a disk cannot be opened
> > 
> >  grub-core/kern/disk.c |  3 +--
> >  util/getroot.c        | 15 +++++++++------
> >  2 files changed, 10 insertions(+), 8 deletions(-)
> > 
> 

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?