All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv
@ 2015-10-24  0:11 Mark Pryor
  2015-10-25  0:46 ` Konrad Rzeszutek Wilk
  2015-11-02 12:12 ` Ian Campbell
  0 siblings, 2 replies; 4+ messages in thread
From: Mark Pryor @ 2015-10-24  0:11 UTC (permalink / raw)
  To: grub-devel; +Cc: ian.campbell

When entering the grub2 shell during a pvgrub2 boot, there is no info about the current
domU in the grubenv (set). Starting with a patch submitted by Olaf Herring I exported
the xenstore cmdline only.

The env var, xen_cmdline, can then be used in the top level script used to make
the pvgrub2 kernel blob.

Signed-off-by: Mark Pryor <pryorm09@gmail.com>
---
 grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
index 0559c03..2a3112d 100644
--- a/grub-core/kern/xen/init.c
+++ b/grub-core/kern/xen/init.c
@@ -524,6 +524,29 @@ map_all_pages (void)
   grub_mm_init_region ((void *) heap_start, heap_end - heap_start);
 }
 
+/*
+ * Find all name=val pairs in the provided cmd_line and export them
+ * so that scripts can evaluate the variables for their own purpose.
+ */
+static void
+export_cmdline (void)
+{
+  char *p;
+  const char *name="xen_cmdline";
+
+  p = grub_malloc (MAX_GUEST_CMDLINE + 1);
+  if (!p)
+    return;
+
+  grub_memcpy (p, grub_xen_start_page_addr->cmd_line, MAX_GUEST_CMDLINE);
+  p[MAX_GUEST_CMDLINE] = '\0';
+
+  grub_env_set (name, p);
+  grub_env_export (name);
+
+   grub_free (p);
+}
+
 extern char _end[];
 
 void
@@ -539,6 +562,8 @@ grub_machine_init (void)
 
   map_all_pages ();
 
+  export_cmdline ();
+
   grub_console_init ();
 
   grub_tsc_init ();
-- 
2.1.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv
  2015-10-24  0:11 [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv Mark Pryor
@ 2015-10-25  0:46 ` Konrad Rzeszutek Wilk
  2015-11-02 12:12 ` Ian Campbell
  1 sibling, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-10-25  0:46 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: ian.campbell

On Fri, Oct 23, 2015 at 05:11:33PM -0700, Mark Pryor wrote:
> When entering the grub2 shell during a pvgrub2 boot, there is no info about the current
> domU in the grubenv (set). Starting with a patch submitted by Olaf Herring I exported

Which patch? Is there a name for it so it can be part of the
git commit history?

Thanks.
> the xenstore cmdline only.
> 
> The env var, xen_cmdline, can then be used in the top level script used to make
> the pvgrub2 kernel blob.
> 
> Signed-off-by: Mark Pryor <pryorm09@gmail.com>
> ---
>  grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> index 0559c03..2a3112d 100644
> --- a/grub-core/kern/xen/init.c
> +++ b/grub-core/kern/xen/init.c
> @@ -524,6 +524,29 @@ map_all_pages (void)
>    grub_mm_init_region ((void *) heap_start, heap_end - heap_start);
>  }
>  
> +/*
> + * Find all name=val pairs in the provided cmd_line and export them
> + * so that scripts can evaluate the variables for their own purpose.
> + */
> +static void
> +export_cmdline (void)
> +{
> +  char *p;
> +  const char *name="xen_cmdline";
> +
> +  p = grub_malloc (MAX_GUEST_CMDLINE + 1);
> +  if (!p)
> +    return;
> +
> +  grub_memcpy (p, grub_xen_start_page_addr->cmd_line, MAX_GUEST_CMDLINE);
> +  p[MAX_GUEST_CMDLINE] = '\0';
> +
> +  grub_env_set (name, p);
> +  grub_env_export (name);
> +
> +   grub_free (p);
> +}
> +
>  extern char _end[];
>  
>  void
> @@ -539,6 +562,8 @@ grub_machine_init (void)
>  
>    map_all_pages ();
>  
> +  export_cmdline ();
> +
>    grub_console_init ();
>  
>    grub_tsc_init ();
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv
  2015-10-24  0:11 [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv Mark Pryor
  2015-10-25  0:46 ` Konrad Rzeszutek Wilk
@ 2015-11-02 12:12 ` Ian Campbell
  2015-11-04 11:20   ` Vladimir 'phcoder' Serbinenko
  1 sibling, 1 reply; 4+ messages in thread
From: Ian Campbell @ 2015-11-02 12:12 UTC (permalink / raw)
  To: Mark Pryor, grub-devel

On Fri, 2015-10-23 at 17:11 -0700, Mark Pryor wrote:
> When entering the grub2 shell during a pvgrub2 boot, there is no info about the current
> domU in the grubenv (set). Starting with a patch submitted by Olaf Herring I exported
> the xenstore cmdline only.

xenstore? The command line comes from the start info. I'd just drop the
mention of xenstore rather than trying to change it.

> 
> The env var, xen_cmdline, can then be used in the top level script used to make
> the pvgrub2 kernel blob.
> 
> Signed-off-by: Mark Pryor <pryorm09@gmail.com>
> ---
>  grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> index 0559c03..2a3112d 100644
> --- a/grub-core/kern/xen/init.c
> +++ b/grub-core/kern/xen/init.c
> @@ -524,6 +524,29 @@ map_all_pages (void)
>    grub_mm_init_region ((void *) heap_start, heap_end - heap_start);
>  }
>  
> +/*
> + * Find all name=val pairs in the provided cmd_line and export them
> + * so that scripts can evaluate the variables for their own purpose.

I don't think that is what this function does, it seems to simply provide a
single env var containing the complete command line. I think that is a fine
behaviour to be starting with.

> + */
> +static void
> +export_cmdline (void)
> +{
> +  char *p;
> +  const char *name="xen_cmdline";
> +
> +  p = grub_malloc (MAX_GUEST_CMDLINE + 1);
> +  if (!p)
> +    return;
> +
> +  grub_memcpy (p, grub_xen_start_page_addr->cmd_line, MAX_GUEST_CMDLINE);
> +  p[MAX_GUEST_CMDLINE] = '\0';
> +
> +  grub_env_set (name, p);
> +  grub_env_export (name);
> +
> +   grub_free (p);

I'm not sure what grub coding style is, but either this last line or all
the others seem to be incorrectly indented (judging from the
inconsistency).

Also, I might be wrong but I think grub_xen_start_page_addr->cmd_line is
guaranteed to be NULL terminated, if you assume that then you can get away
with just calling grub_env_set direct without the copying, I think.

Lastly, if you also cc xen-devel then you might get feedback from people
more knowledgable about such things as whether si->cmd_line is NULL
terminated than I am.

Ian.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv
  2015-11-02 12:12 ` Ian Campbell
@ 2015-11-04 11:20   ` Vladimir 'phcoder' Serbinenko
  0 siblings, 0 replies; 4+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2015-11-04 11:20 UTC (permalink / raw)
  To: The development of GRUB 2; +Cc: Mark Pryor

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

.
>
> I think that is a fine
> behaviour to be starting with.
>
Whatever we do now, we're stuck with it. It's interface and almost
impossible to retire. We need a sane interface from day 1

[-- Attachment #2: Type: text/html, Size: 246 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-11-04 11:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-24  0:11 [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv Mark Pryor
2015-10-25  0:46 ` Konrad Rzeszutek Wilk
2015-11-02 12:12 ` Ian Campbell
2015-11-04 11:20   ` Vladimir 'phcoder' Serbinenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.