grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper discovery


From: Michael Chang
Subject: Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper discovery
Date: Tue, 25 Jun 2024 12:31:08 +0800

On Thu, Jun 20, 2024 at 03:14:59PM GMT, avnish wrote:
> Hi Vladimir,
> 
> We have implemented this code to enable the ieee1275 hint for grub. We had
> scenarios (in SLES) like the disk had PReP partition followed by an LVM and,
> inside this LVM , the boot partition. So, we implemented this code to make
> grub able to generate the hint. Without this code, we were unable to find
> the hint for the boot process. And, in some scenarios, due the amount of
> disks in the machine, the process could take a lot to finish.

As Vladimir pointed out, the correct search hint should be lvmid/<UUID>,
so grub will try to assemble the root logical volume first in the
process of looking up the root file system. This way, time won't be
wasted on trying those OpenFirmware raw disks, as they are not the
target.

The problem here is that bringing up the logical volume itself can be
very slow, and grub lacks measures like an LVM filter, which would help
to restrict which devices are scanned and improve performance. In the
end, the patch doesn't help either way.

In my opinion, this patch is more about updating the system bootlist
than achieving the proclaimed boot time enhancement.

Thanks,
Michael

> Shall we limit this function call specific to PowerPC? Something like this
> below:
> 
> #ifdef __powerpc__
>   realname = get_slave_from_dm (name_buf);
>   if (realname)
>     {
>       free (name_buf);
>       name_buf = realname;
>     }
> #endif
> 
> Please suggest us.
> Thank you so much!
> 
> Regards,
> Avnish Chouhan
> 
> 
> On 2024-06-07 15:05, grub-devel-request@gnu.org wrote:
> > Send Grub-devel mailing list submissions to
> >     grub-devel@gnu.org
> > ------------------------------
> > 
> > Message: 4
> > Date: Fri, 7 Jun 2024 11:34:45 +0200
> > From: "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>
> > To: avnish <avnish@imap.linux.ibm.com>
> > Cc: The development of GNU GRUB <grub-devel@gnu.org>, Daniel Kiper
> >     <daniel.kiper@oracle.com>, brking@linux.ibm.com,
> >     meghanaprakash@in.ibm.com, Diego Domingos <diegodo@br.ibm.com>
> > Subject: Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper
> >     discovery
> > Message-ID:
> >     <CAEaD8JN1BuOYCxSEP0dQTv7VML6vyCNFg3tSUrLigbrN9uY7AQ@mail.gmail.com>
> > Content-Type: text/plain; charset="utf-8"
> > 
> > Le ven. 7 juin 2024, 10:34, avnish <avnish@imap.linux.ibm.com> a écrit :
> > 
> > > On 2024-06-06 21:04, Vladimir 'phcoder' Serbinenko wrote:
> > > > 2 problems: * How does dm device ends up on ofpathname? It should be
> > > > handled by grub internal logic in most cases and not end up in
> > > > of-specific paths * Why not use existing devmapper functions already
> > > > present in codebase? Le jeu. 6 juin 2024,
> > > > > 2 problems:
> > > > * How does dm device ends up on ofpathname? It should be handled by
> > > > grub internal logic in most cases and not end up in of-specific paths
> > > > * Why not use existing devmapper functions already present in
> > > > codebase?
> > > >
> > > 
> > > Hi Vladimir,
> > > Thank you so much for your response!
> > > 
> > > We have observed that whenever we are dealing with the devices like
> > > "/dev/dm-*", the ofpath returns null.
> > > 
> > It's expected behavior
> > 
> > > To resolve this, as no such required functions has been implemented to
> > > handle this kind of case. We have done changes based on the
> > > requirement
> > > that will look into /sys/block/dm-* devices and search slave
> > > devices recursively inside slaves directory to find the root disk.
> > 
> > Installing on e.g. LVM is invalid. Only 3 cases can your approach work:
> > 1) multipath
> > 2) maybe RAID1 as well in some cases
> > 3) if ofw assembles raid somehow
> > Which one do you have? We need more details.
> > This patch enables invalid configs like installing on LVM
> > 
> > > 
> > > Regards,
> > > Avnish Chouhan
> > > 
> > > > Le jeu. 6 juin 2024, 14:40, Avnish Chouhan <avnish@linux.ibm.com> a
> > > > écrit :
> > > >
> > > >> This patch enables the device mapper discovery on ofpath.c.
> > > >> Currently,
> > > >> when we are dealing with a device like /dev/dm-* the ofpath returns
> > > >> null
> > > >> since there is no function implemented to handle this case.
> > > >>
> > > >> This patch implements a function that will look into /sys/block/dm-*
> > > >> devices and search recursively inside slaves directory to find the
> > > >> root
> > > >> disk.
> > > >>
> > > >> Signed-off-by: Diego Domingos <diegodo@br.ibm.com>
> > > >> Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
> > > >> ---
> > > >> grub-core/osdep/linux/ofpath.c | 64
> > > >> +++++++++++++++++++++++++++++++++++++++++-
> > > >> 1 file changed, 63 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/grub-core/osdep/linux/ofpath.c
> > > >> b/grub-core/osdep/linux/ofpath.c
> > > >> index 0f5d54e9f2d..cc849d9c94c 100644
> > > >> --- a/grub-core/osdep/linux/ofpath.c
> > > >> +++ b/grub-core/osdep/linux/ofpath.c
> > > >> @@ -37,6 +37,7 @@
> > > >> #include <fcntl.h>
> > > >> #include <errno.h>
> > > >> #include <ctype.h>
> > > >> +#include <dirent.h>
> > > >>
> > > >> #ifdef __sparc__
> > > >> typedef enum
> > > >> @@ -755,13 +756,74 @@ strip_trailing_digits (const char *p)
> > > >> return new;
> > > >> }
> > > >>
> > > >> +static char *
> > > >> +get_slave_from_dm (const char * device)
> > > >> +{
> > > >> +  char *curr_device, *tmp;
> > > >> +  char *directory;
> > > >> +  char *ret = NULL;
> > > >> +  directory = grub_strdup (device);
> > > >> +  tmp = get_basename (directory);
> > > >> +  curr_device = grub_strdup (tmp);
> > > >> +  *tmp = '\0';
> > > >> +
> > > >> +  /* Recursively check for slaves devices so we can find the root
> > > >> device */
> > > >> +  while ((curr_device[0] == 'd') && (curr_device[1] == 'm') &&
> > > >> (curr_device[2] == '-'))
> > > >> +    {
> > > >> +      DIR *dp;
> > > >> +      struct dirent *ep;
> > > >> +      char* device_path;
> > > >> +      device_path = grub_xasprintf ("/sys/block/%s/slaves",
> > > >> curr_device);
> > > >> +      dp = opendir (device_path);
> > > >> +
> > > >> +      if (dp != NULL)
> > > >> +        {
> > > >> +          ep = readdir (dp);
> > > >> +
> > > >> +          while (ep != NULL)
> > > >> +            {
> > > >> +              /* avoid some system directories */
> > > >> +              if (!strcmp(ep->d_name,"."))
> > > >> +                goto next_dir;
> > > >> +              if (!strcmp(ep->d_name,".."))
> > > >> +                goto next_dir;
> > > >> +
> > > >> +              free (curr_device);
> > > >> +              free (ret);
> > > >> +              curr_device = grub_strdup (ep->d_name);
> > > >> +              ret = grub_xasprintf ("%s%s", directory,
> > > >> curr_device);
> > > >> +              break;
> > > >> +
> > > >> +              next_dir:
> > > >> +              ep = readdir (dp);
> > > >> +              continue;
> > > >> +            }
> > > >> +          closedir (dp);
> > > >> +        }
> > > >> +      else
> > > >> +        grub_util_warn (_("cannot open directory `%s'"),
> > > >> device_path);
> > > >> +      free (device_path);
> > > >> +    }
> > > >> +  free (directory);
> > > >> +  free (curr_device);
> > > >> +
> > > >> +  return ret;
> > > >> +}
> > > >> +
> > > >> char *
> > > >> grub_util_devname_to_ofpath (const char *sys_devname)
> > > >> {
> > > >> -  char *name_buf, *device, *devnode, *devicenode, *ofpath;
> > > >> +  char *name_buf, *device, *devnode, *devicenode, *ofpath,
> > > >> *realname;
> > > >>
> > > >> name_buf = xrealpath (sys_devname);
> > > >>
> > > >> +  realname = get_slave_from_dm (name_buf);
> > > >> +  if (realname)
> > > >> +    {
> > > >> +      free (name_buf);
> > > >> +      name_buf = realname;
> > > >> +    }
> > > >> +
> > > >> device = get_basename (name_buf);
> > > >> devnode = strip_trailing_digits (name_buf);
> > > >> devicenode = strip_trailing_digits (device);
> > > >>
> > > >> _______________________________________________
> > > >> Grub-devel mailing list
> > > >> Grub-devel@gnu.org
> > > >> https://lists.gnu.org/mailman/listinfo/grub-devel
> > > >
> > > >
> > > > Links:
> > > > ------
> > > > [1]
> > > >
> > > https://us-phishalarm-ewt.proofpoint.com/EWT/v1/AdhS1Rd-!-XFVHHiYfI50ul9XDgoDMf5HTGn83xsPPR4XT9wPzmu8earnN0GgLmGjgJ1wrY0OEeVCDtYQvZuy51BEuwbJ2Q36rjgqhgETNXQL18Hn2QOfpE6f2H9XOn8Mici5$
> > > 
> > -------------- next part --------------
> > An HTML attachment was scrubbed...
> > URL:
> > <https://lists.gnu.org/archive/html/grub-devel/attachments/20240607/d6db1c71/attachment.htm>
> > 
> > ------------------------------
> > 
> > Subject: Digest Footer
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> > 
> > ------------------------------
> > 
> > End of Grub-devel Digest, Vol 244, Issue 12
> > *******************************************
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]