Re: [PATCH 2/4] Accept environment variables on the command line for Xen.

["Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4179 bytes --]

I was about to post similar remarks. I'm on phone so will tell just the key
points.
- Most of GRUB platforms have command lines. I.a xen, efi, loongson,
ieee1275, arc.
- Many platforms add their own command line arguments even if user didn't
ask for it. Those shouldn't interfere. In your patch root=sda1 from xen
will either be lost or will clobber GRUB root.
- IEEE1275 code already looks into command line. If it does sth sane it
could be used as base, otherwise it should be replaced with sth common.
- Perhaps we should use some prefix to avoid unintended clobbering.
- There should be a way to emulate pvgrub1 behaviour which took config name
from commandline. Perhaps pvgrub2 should even do it by default (by using
legacy_configfile)
On Dec 12, 2013 4:48 PM, "Andrey Borzenkov" <arvidjaar@gmail.com> wrote:

> В Thu, 12 Dec 2013 15:37:22 +0000
> Colin Watson <cjwatson@ubuntu.com> пишет:
>
> > * grub-core/kern/xen/init.c (fetch_command_line_word): New function.
> > (parse_command_line): Likewise.
> > (grub_machine_init): Call parse_command_line.
> > ---
> >  ChangeLog                 |  8 ++++++++
> >  grub-core/kern/xen/init.c | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 52 insertions(+)
> >
> > diff --git a/ChangeLog b/ChangeLog
> > index 766fe4b..fc86601 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,5 +1,13 @@
> >  2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
> >
> > +     Accept environment variables on the command line for Xen.
> > +
>
> Thank you!
>
> It may make sense to generalize it to other archs. At least EFI
> definitely comes in mind. In this case platform specific part is only
> to fetch parameter string(s). What about
>
> - move fetch_command_line_word and parse_command_line to separate file
>   that is included for specific platforms only
> - make each platform that can accept fetch parameters in platform
>   specific way and call common code (parse_command_line)
>
> Does it make sense?
>
> I will add EFI part then later.
>
> > +     * grub-core/kern/xen/init.c (fetch_command_line_word): New
> function.
> > +     (parse_command_line): Likewise.
> > +     (grub_machine_init): Call parse_command_line.
> > +
> > +2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
> > +
> >       Add an option to exclude devices from search results.
> >
> >       * grub-core/commands/search.c (struct search_ctx): Add excludes and
> > diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> > index 1d8eaec..eb9b8b3 100644
> > --- a/grub-core/kern/xen/init.c
> > +++ b/grub-core/kern/xen/init.c
> > @@ -525,6 +525,48 @@ map_all_pages (void)
> >    grub_mm_init_region ((void *) heap_start, heap_end - heap_start);
> >  }
> >
> > +static char *
> > + (char *pos, char **word)
> > +{
> > +  while (grub_isspace (*pos))
> > +    pos++;
> > +
> > +  if (!*pos)
> > +    return NULL;
> > +
> > +  *word = pos;
> > +  while (*pos && !grub_isspace (*pos))
> > +    pos++;
> > +  if (*pos)
> > +    *pos++ = '\0';
> > +  return pos;
> > +}
> > +
> > +static void
> > +parse_command_line (void)
> > +{
> > +  char *cmd_line;
> > +  char *pos, *word;
> > +
> > +  cmd_line = grub_malloc (MAX_GUEST_CMDLINE + 1);
> > +  grub_memcpy (cmd_line, grub_xen_start_page_addr->cmd_line,
> > +            MAX_GUEST_CMDLINE);
> > +  cmd_line[MAX_GUEST_CMDLINE] = '\0';
> > +  pos = cmd_line;
> > +  while ((pos = fetch_command_line_word (pos, &word)) != NULL)
> > +    {
> > +      char *equals;
> > +
> > +      equals = grub_strchr (word, '=');
> > +      if (!equals)
> > +     continue;
> > +
> > +      *equals = '\0';
> > +      grub_env_set (word, equals + 1);
> > +    }
> > +  grub_free (cmd_line);
> > +}
> > +
> >  extern char _end[];
> >
> >  void
> > @@ -547,6 +589,8 @@ grub_machine_init (void)
> >    grub_xendisk_init ();
> >
> >    grub_boot_init ();
> > +
> > +  parse_command_line ();
> >  }
> >
> >  void
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #1.2: Type: text/html, Size: 5329 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel