Skip to contentSkip to conversation

pvgrub2 xen cmdline xenstore var to grubenv

PATCH In review Mark Pryor 4 participants 4 messages Oct 24, 2015 – Nov 4, 2015

lore
Share & tools
lore

Mark Pryor

author
#0 lore raw

PATCH 1/1 pvgrub2 xen cmdline xenstore var to grubenv

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.

---
 grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
grub-core/kern/xen/init.c +25 -0
index 0559c03..2a3112d 100644
@@ -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 ();
Signature · 1 line
2.1.4
#1 lore raw
Quoted 2 lines · Mark Pryor

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.

Quoted 61 lines

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.

---
 grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
grub-core/kern/xen/init.c +25 -0
index 0559c03..2a3112d 100644
@@ -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 ();
Signature · 7 lines
2.1.4


_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
#2 lore raw
In reply to Mark Pryor
Quoted 3 lines · Mark Pryor

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.

Quoted 14 lines

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

---
 grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
grub-core/kern/xen/init.c +0 -0
index 0559c03..2a3112d 100644

grub-core/kern/xen/init.c map_all_pages (void) · line 529

Quoted 6 lines
grub-core/kern/xen/init.c +3 -0
@@ -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.

Quoted 8 lines
grub-core/kern/xen/init.c +8 -0
+ */
+static void
+export_cmdline (void)
+{
+  char *p;
+  const char *name="xen_cmdline";
+
+  p = grub_malloc (MAX_GUEST_CMDLINE + 1);

grub-core/kern/xen/init.c map_all_pages (void) · line 547

Quoted 10 lines
grub-core/kern/xen/init.c +10 -0
+  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.

#3 lore raw

.

Quoted 4 lines

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