Xen devel - Dec 2013 - [PATCH 0/4] Turn-key PV-GRUB2 installation

This patch set should make it easier to maintain PV-GRUB2 installations.
The general idea is based on discussions I had with Xen developers
(mainly Ian Jackson) at the Ubuntu Developer Summit in May 2011; though
I never did manage to get the core port done and Vladimir beat me to
that, I think the configuration approach we discussed there is still
valid and useful.

The idea here is that people maintaining dom0s, especially in big cloud
installations, will probably not want to update their PV-GRUB2 image
(which will be read from the dom0 filesystem but will run in the domU)
very often; but on the other hand people maintaining distributions will
want to be able to move on and make use of latest-and-greatest features.
To make this work, we build a dom0-filesystem-suitable grub.xen image
which just tries to chainload another grub.xen from a couple of
reasonably standard locations in the domU''s filesystem
(/boot/grub/grub.xen and /boot/grub2/grub.xen).

I''ve arranged that you can either use that very same grub.xen image in
the domU by way of a new mechanism that passes GRUB environment
variables via the Xen guest command line so that we can tell which
context we''re in, or you can run "grub-install
--target=x86_64-xen" in
the domU which will build and install a suitable one in more or less the
usual way.

Colin Watson (4):
  Add an option to exclude devices from search results.
  Accept environment variables on the command line for Xen.
  Build grub.xen.
  Improve installation on Xen.

 .gitignore                       |  1 +
 ChangeLog                        | 45 ++++++++++++++++++++++++++++++++++++++++
 Makefile.am                      | 14 +++++++++++++
 configure.ac                     |  1 +
 docs/grub.texi                   |  7 ++++++-
 grub-core/Makefile.core.def      |  7 +++++++
 grub-core/boot/xen/xen.cfg.in    | 31 +++++++++++++++++++++++++++
 grub-core/commands/search.c      | 15 ++++++++++++--
 grub-core/commands/search_wrap.c | 27 +++++++++++++++++++-----
 grub-core/kern/xen/init.c        | 44 +++++++++++++++++++++++++++++++++++++++
 include/grub/search.h            |  9 +++++---
 util/grub-install.c              | 11 ++++++++--
 12 files changed, 199 insertions(+), 13 deletions(-)
 create mode 100644 grub-core/boot/xen/xen.cfg.in

-- 
1.8.4.4

* grub-core/commands/search.c (struct search_ctx): Add excludes and
nexcludes.
(iterate_device): Ignore devices listed in ctx->excludes.
(FUNC_NAME): Accept excludes and nexcludes parameters.
(grub_cmd_do_search): Pass empty excludes.
* grub-core/commands/search_wrap.c (options): Add --exclude option.
(grub_cmd_search): Handle --exclude.
* include/grub/search.h (grub_search_fs_file): Update prototype.
(grub_search_fs_uuid): Likewise.
(grub_search_label): Likewise.
* docs/grub.texi (search): Document --exclude.
---
 ChangeLog                        | 16 ++++++++++++++++
 docs/grub.texi                   |  7 ++++++-
 grub-core/commands/search.c      | 15 +++++++++++++--
 grub-core/commands/search_wrap.c | 27 ++++++++++++++++++++++-----
 include/grub/search.h            |  9 ++++++---
 5 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9cec63f..766fe4b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+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
+	nexcludes.
+	(iterate_device): Ignore devices listed in ctx->excludes.
+	(FUNC_NAME): Accept excludes and nexcludes parameters.
+	(grub_cmd_do_search): Pass empty excludes.
+	* grub-core/commands/search_wrap.c (options): Add --exclude option.
+	(grub_cmd_search): Handle --exclude.
+	* include/grub/search.h (grub_search_fs_file): Update prototype.
+	(grub_search_fs_uuid): Likewise.
+	(grub_search_label): Likewise.
+	* docs/grub.texi (search): Document --exclude.
+
 2013-12-11  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	* grub-core/normal/charset.c: Fix premature line wrap and crash.
diff --git a/docs/grub.texi b/docs/grub.texi
index 91fa1de..4c53665 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4731,7 +4731,8 @@ unbootable. @xref{Using digital signatures}, for more
information.
 
 @deffn Command search @
  [@option{--file}|@option{--label}|@option{--fs-uuid}] @
- [@option{--set} [var]] [@option{--no-floppy}] name
+ [@option{--set} [var]] [@option{--no-floppy}] @
+ [@option{--exclude} @var{name} ...] name
 Search devices by file (@option{-f}, @option{--file}), filesystem label
 (@option{-l}, @option{--label}), or filesystem UUID (@option{-u},
 @option{--fs-uuid}).
@@ -4743,6 +4744,10 @@ value of environment variable @var{var}.  The default
variable is
 The @option{--no-floppy} option prevents searching floppy devices, which can
 be slow.
 
+The @option{--exclude} option excludes any matches of the given GRUB device
+name.  For example, this may be used to search for any file named
+@file{/boot/grub/grub.cfg} other than the one in a memdisk.
+
 The @samp{search.file}, @samp{search.fs_label}, and @samp{search.fs_uuid}
 commands are aliases for @samp{search --file}, @samp{search --label}, and
 @samp{search --fs-uuid} respectively.
diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
index 16143a3..03cfea1 100644
--- a/grub-core/commands/search.c
+++ b/grub-core/commands/search.c
@@ -50,6 +50,8 @@ struct search_ctx
   int no_floppy;
   char **hints;
   unsigned nhints;
+  char **excludes;
+  unsigned nexcludes;
   int count;
   int is_cache;
 };
@@ -59,8 +61,15 @@ static int
 iterate_device (const char *name, void *data)
 {
   struct search_ctx *ctx = data;
+  unsigned i;
   int found = 0;
 
+  for (i = 0; i < ctx->nexcludes; i++)
+    {
+      if (grub_strcmp (name, ctx->excludes[i]) == 0)
+	return 0;
+    }
+
   /* Skip floppy drives when requested.  */
   if (ctx->no_floppy &&
       name[0] == ''f'' && name[1] ==
''d'' && name[2] >= ''0'' &&
name[2] <= ''9'')
@@ -262,7 +271,7 @@ try (struct search_ctx *ctx)
 
 void
 FUNC_NAME (const char *key, const char *var, int no_floppy,
-	   char **hints, unsigned nhints)
+	   char **hints, unsigned nhints, char **excludes, unsigned nexcludes)
 {
   struct search_ctx ctx = {
     .key = key,
@@ -270,6 +279,8 @@ FUNC_NAME (const char *key, const char *var, int no_floppy,
     .no_floppy = no_floppy,
     .hints = hints,
     .nhints = nhints,
+    .excludes = excludes,
+    .nexcludes = nexcludes,
     .count = 0,
     .is_cache = 0
   };
@@ -304,7 +315,7 @@ grub_cmd_do_search (grub_command_t cmd __attribute__
((unused)), int argc,
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument
expected"));
 
   FUNC_NAME (args[0], argc == 1 ? 0 : args[1], 0, (args + 2),
-	     argc > 2 ? argc - 2 : 0);
+	     argc > 2 ? argc - 2 : 0, NULL, 0);
 
   return grub_errno;
 }
diff --git a/grub-core/commands/search_wrap.c b/grub-core/commands/search_wrap.c
index 3f75fec..7493df4 100644
--- a/grub-core/commands/search_wrap.c
+++ b/grub-core/commands/search_wrap.c
@@ -63,6 +63,9 @@ static const struct grub_arg_option options[]      
N_("First try the device HINT if currently running on ARC."
 	" If HINT ends in comma, also try subpartitions"),
      N_("HINT"), ARG_TYPE_STRING},
+    {"exclude",		0, GRUB_ARG_OPTION_REPEATABLE,
+     N_("Exclude the device whose GRUB device name is NAME."),
N_("NAME"),
+     ARG_TYPE_STRING},
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -79,6 +82,7 @@ enum options
     SEARCH_HINT_BAREMETAL,
     SEARCH_HINT_EFI,
     SEARCH_HINT_ARC,
+    SEARCH_EXCLUDE,
  };
 
 static grub_err_t
@@ -87,8 +91,8 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, char
**args)
   struct grub_arg_list *state = ctxt->state;
   const char *var = 0;
   const char *id = 0;
-  int i = 0, j = 0, nhints = 0;
-  char **hints = NULL;
+  int i = 0, j = 0, nhints = 0, nexcludes = 0;
+  char **hints = NULL, **excludes = NULL;
 
   if (state[SEARCH_HINT].set)
     for (i = 0; state[SEARCH_HINT].args[i]; i++)
@@ -164,6 +168,19 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, char
**args)
     if (grub_memcmp (args[j], "--hint-", sizeof ("--hint-")
- 1) != 0)
       break;
 
+  if (state[SEARCH_EXCLUDE].set)
+    {
+      for (i = 0; state[SEARCH_EXCLUDE].args[i]; i++)
+	nexcludes++;
+
+      excludes = grub_malloc (sizeof (excludes[0]) * nexcludes);
+      if (!excludes)
+	return grub_errno;
+
+      for (i = 0; state[SEARCH_EXCLUDE].args[i]; i++)
+	excludes[i] = state[SEARCH_EXCLUDE].args[i];
+    }
+
   if (state[SEARCH_SET].set)
     var = state[SEARCH_SET].arg ? state[SEARCH_SET].arg : "root";
 
@@ -179,13 +196,13 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc,
char **args)
 
   if (state[SEARCH_LABEL].set)
     grub_search_label (id, var, state[SEARCH_NO_FLOPPY].set, 
-		       hints, nhints);
+		       hints, nhints, excludes, nexcludes);
   else if (state[SEARCH_FS_UUID].set)
     grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
-			 hints, nhints);
+			 hints, nhints, excludes, nexcludes);
   else if (state[SEARCH_FILE].set)
     grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set, 
-			 hints, nhints);
+			 hints, nhints, excludes, nexcludes);
   else
     return grub_error (GRUB_ERR_INVALID_COMMAND, "unspecified search
type");
 
diff --git a/include/grub/search.h b/include/grub/search.h
index d80347d..e7bcd4b 100644
--- a/include/grub/search.h
+++ b/include/grub/search.h
@@ -20,10 +20,13 @@
 #define GRUB_SEARCH_HEADER 1
 
 void grub_search_fs_file (const char *key, const char *var, int no_floppy,
-			  char **hints, unsigned nhints);
+			  char **hints, unsigned nhints,
+			  char **excludes, unsigned nexcludes);
 void grub_search_fs_uuid (const char *key, const char *var, int no_floppy,
-			  char **hints, unsigned nhints);
+			  char **hints, unsigned nhints,
+			  char **excludes, unsigned nexcludes);
 void grub_search_label (const char *key, const char *var, int no_floppy,
-			char **hints, unsigned nhints);
+			char **hints, unsigned nhints,
+			char **excludes, unsigned nexcludes);
 
 #endif
-- 
1.8.4.4

* 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.
+
+	* 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 *
+fetch_command_line_word (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
-- 
1.8.4.4

* .gitignore: Add xen.cfg.
* Makefile.am (grub-core/xen.cfg): New rule.
(grub.xen): Likewise.  Add to platform_DATA.
* configure.ac (COND_xen): New conditional.
* grub-core/Makefile.core.def (xen.cfg): New definition.
* grub-core/boot/xen/xen.cfg.in: New file.  This is suitable for
installation both in the dom0 filesystem (where it will find and
chainload a secondary grub.xen) and in the domU filesystem (where it
will load a matching grub.cfg).
---
 .gitignore                    |  1 +
 ChangeLog                     | 14 ++++++++++++++
 Makefile.am                   | 14 ++++++++++++++
 configure.ac                  |  1 +
 grub-core/Makefile.core.def   |  7 +++++++
 grub-core/boot/xen/xen.cfg.in | 31 +++++++++++++++++++++++++++++++
 6 files changed, 68 insertions(+)
 create mode 100644 grub-core/boot/xen/xen.cfg.in

diff --git a/.gitignore b/.gitignore
index 2292cc9..51aa8b4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -153,6 +153,7 @@ trigtables.c
 uhci_test
 update-grub_lib
 unidata.c
+xen.cfg
 xzcompress_test
 Makefile.in
 GPATH
diff --git a/ChangeLog b/ChangeLog
index fc86601..58304f7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
 
+	Build grub.xen.
+
+	* .gitignore: Add xen.cfg.
+	* Makefile.am (grub-core/xen.cfg): New rule.
+	(grub.xen): Likewise.  Add to platform_DATA.
+	* configure.ac (COND_xen): New conditional.
+	* grub-core/Makefile.core.def (xen.cfg): New definition.
+	* grub-core/boot/xen/xen.cfg.in: New file.  This is suitable for
+	installation both in the dom0 filesystem (where it will find and
+	chainload a secondary grub.xen) and in the domU filesystem (where it
+	will load a matching grub.cfg).
+
+2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
+
 	Accept environment variables on the command line for Xen.
 
 	* grub-core/kern/xen/init.c (fetch_command_line_word): New function.
diff --git a/Makefile.am b/Makefile.am
index 0a2c099..e9792f6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -402,6 +402,20 @@ default_payload.elf: grub-mkstandalone grub-mkimage
 	pkgdatadir=. ./grub-mkstandalone --grub-mkimage=./grub-mkimage -O
i386-coreboot -o $@ --modules=''ahci pata ehci uhci ohci usb_keyboard
usbms part_msdos xfs ext2 fat at_keyboard part_gpt usbserial_usbdebug
cbfs'' --install-modules=''ls linux search configfile normal
cbtime cbls memrw iorw minicmd lsmmap lspci halt reboot hexdump pcidump regexp
setpci lsacpi chain test'' --fonts= --themes= --locales= -d grub-core/
/boot/grub/grub.cfg=$(srcdir)/coreboot.cfg
 endif
 
+if COND_xen
+# The grub-core/util split means that we have to duplicate rules a little
+# bit here.
+grub-core/xen.cfg: grub-core/boot/xen/xen.cfg.in
+	$(MAKE) -C grub-core xen.cfg
+
+grub.xen: grub-mkstandalone grub-mkimage grub-core/xen.cfg
+	pkgdatadir=. ./grub-mkstandalone --grub=mkimage=./grub-mkimage \
+		   -o $@ -O $(target_cpu)-$(platform) -d grub-core/ \
+		   /boot/grub/grub.cfg=grub-core/xen.cfg
+
+platform_DATA += grub.xen
+endif
+
 windowsdir=$(top_builddir)/$(PACKAGE)-$(VERSION)-for-windows
 windowsdir: $(PROGRAMS) $(starfield_DATA) $(platform_DATA)
 	test -d $(windowsdir) && rm -rf $(windowsdir) || true
diff --git a/configure.ac b/configure.ac
index 0abbb99..08aa751 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1590,6 +1590,7 @@ AM_CONDITIONAL([COND_i386_ieee1275], [test x$target_cpu =
xi386 -a x$platform  AM_CONDITIONAL([COND_i386_coreboot], [test x$target_cpu =
xi386 -a x$platform = xcoreboot])
 AM_CONDITIONAL([COND_i386_multiboot], [test x$target_cpu = xi386 -a x$platform
= xmultiboot])
 AM_CONDITIONAL([COND_x86_64_efi], [test x$target_cpu = xx86_64 -a x$platform =
xefi])
+AM_CONDITIONAL([COND_xen], [test x$platform = xxen])
 AM_CONDITIONAL([COND_i386_xen], [test x$target_cpu = xi386 -a x$platform =
xxen])
 AM_CONDITIONAL([COND_x86_64_xen], [test x$target_cpu = xx86_64 -a x$platform =
xxen])
 AM_CONDITIONAL([COND_mips_loongson], [test x$target_cpu = xmipsel -a x$platform
= xloongson])
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 060de44..63a8351 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -44,6 +44,13 @@ script = {
   enable = powerpc_ieee1275;
 };
 
+script = {
+  installdir = platform;
+  name = xen.cfg;
+  common = boot/xen/xen.cfg.in;
+  enable = xen;
+};
+
 kernel = {
   name = kernel;
 
diff --git a/grub-core/boot/xen/xen.cfg.in b/grub-core/boot/xen/xen.cfg.in
new file mode 100644
index 0000000..e2e590c
--- /dev/null
+++ b/grub-core/boot/xen/xen.cfg.in
@@ -0,0 +1,31 @@
+if [ -z "$grub_xen_guest" ]; then
+	# This is the copy of grub.xen installed in the dom0''s filesystem.
+	# Look for a copy in the domU''s filesystem and chainload that.  This
+	# allows us to guarantee that GRUB will be in sync with the
+	# configuration file in the domU.  The file locations here must not
+	# have any configure-generated substitutions applied, as the intent
+	# is that a single grub.xen should be able to cope with a variety of
+	# domU systems.
+	if search --set=root --file /boot/grub/grub.xen; then
+		linux /boot/grub/grub.xen grub_xen_guest=1
+		boot
+	elif search --set=root --file=/boot/grub2/grub.xen; then
+		linux /boot/grub2/grub.xen grub_xen_guest=1
+		boot
+	else
+		echo "No grub.xen found in guest filesystem.  Tried:"
+		echo "  /boot/grub/grub.xen"
+		echo "  /boot/grub2/grub.xen"
+		halt
+	fi
+else
+	# This is the copy of grub.xen installed in the domU''s filesystem.
+	# Read its configuration file.
+	if search --set=root --exclude=memdisk --file \
+			/@bootdirname@/@grubdirname@/grub.cfg; then
+		configfile /@bootdirname@/@grubdirname@/grub.cfg
+	else
+		echo "No /@bootdirname@/@grubdirname@/grub.cfg found in guest
filesystem."
+		halt
+	fi
+fi
-- 
1.8.4.4

* util/grub-install.c: Copy core image to GRUBDIR/grub.xen on Xen
platforms.
---
 ChangeLog           |  7 +++++++
 util/grub-install.c | 11 +++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 58304f7..f280b59 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
 
+	Improve installation on Xen.
+
+	* util/grub-install.c: Copy core image to GRUBDIR/grub.xen on Xen
+	platforms.
+
+2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
+
 	Build grub.xen.
 
 	* .gitignore: Add xen.cfg.
diff --git a/util/grub-install.c b/util/grub-install.c
index 5d22f90..2da05c2 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1576,6 +1576,15 @@ main (int argc, char *argv[])
 	}
       break;
 
+    case GRUB_INSTALL_PLATFORM_I386_XEN:
+    case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+      {
+	char *dst = grub_util_path_concat (2, grubdir, "grub.xen");
+	grub_install_copy_file (imgfile, dst, 1);
+	free (dst);
+      }
+      break;
+
     case GRUB_INSTALL_PLATFORM_MIPSEL_LOONGSON:
     case GRUB_INSTALL_PLATFORM_MIPSEL_QEMU_MIPS:
     case GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS:
@@ -1584,8 +1593,6 @@ main (int argc, char *argv[])
     case GRUB_INSTALL_PLATFORM_MIPSEL_ARC:
     case GRUB_INSTALL_PLATFORM_ARM_UBOOT:
     case GRUB_INSTALL_PLATFORM_I386_QEMU:
-    case GRUB_INSTALL_PLATFORM_I386_XEN:
-    case GRUB_INSTALL_PLATFORM_X86_64_XEN:
       grub_util_warn ("%s",
 		      _("WARNING: no platform-specific install was performed"));
       break;
-- 
1.8.4.4

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
>

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

This patch would make 32-bit and 64-bit GRUB conflict. This is a valid
usecase as it''s possible to make biarch image by having both kernels
and
32-bit userspace
On Dec 12, 2013 4:38 PM, "Colin Watson" <cjwatson@ubuntu.com>
wrote:
> * util/grub-install.c: Copy core image to GRUBDIR/grub.xen on Xen
> platforms.
> ---
>  ChangeLog           |  7 +++++++
>  util/grub-install.c | 11 +++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 58304f7..f280b59 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,12 @@
>  2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
>
> +       Improve installation on Xen.
> +
> +       * util/grub-install.c: Copy core image to GRUBDIR/grub.xen on Xen
> +       platforms.
> +
> +2013-12-12  Colin Watson  <cjwatson@ubuntu.com>
> +
>         Build grub.xen.
>
>         * .gitignore: Add xen.cfg.
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 5d22f90..2da05c2 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1576,6 +1576,15 @@ main (int argc, char *argv[])
>         }
>        break;
>
> +    case GRUB_INSTALL_PLATFORM_I386_XEN:
> +    case GRUB_INSTALL_PLATFORM_X86_64_XEN:
> +      {
> +       char *dst = grub_util_path_concat (2, grubdir,
"grub.xen");
> +       grub_install_copy_file (imgfile, dst, 1);
> +       free (dst);
> +      }
> +      break;
> +
>      case GRUB_INSTALL_PLATFORM_MIPSEL_LOONGSON:
>      case GRUB_INSTALL_PLATFORM_MIPSEL_QEMU_MIPS:
>      case GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS:
> @@ -1584,8 +1593,6 @@ main (int argc, char *argv[])
>      case GRUB_INSTALL_PLATFORM_MIPSEL_ARC:
>      case GRUB_INSTALL_PLATFORM_ARM_UBOOT:
>      case GRUB_INSTALL_PLATFORM_I386_QEMU:
> -    case GRUB_INSTALL_PLATFORM_I386_XEN:
> -    case GRUB_INSTALL_PLATFORM_X86_64_XEN:
>        grub_util_warn ("%s",
>                       _("WARNING: no platform-specific install was
> performed"));
>        break;
> --
> 1.8.4.4
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

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

---------- Forwarded message ----------
From: "Vladimir ''phcoder'' Serbinenko"
<phcoder@gmail.com>
Date: Dec 12, 2013 5:45 PM
Subject: Re: [PATCH 3/4] Build grub.xen.
To: "The development of GNU GRUB" <grub-devel@gnu.org>

No I meant full access to just obe of guest partitions. E.g. FTP may be
using separate partition and non-admins may have control over it. If system
has some kind of automatic user creation and /home is separate someone may
register as boot or grub and put grub.xen in his directory. If /tmp is on
separate partition and not in RAM then everybody can put grub.xen to
/tmp/grub/grub.xen
On Dec 12, 2013 5:39 PM, "Colin Watson" <cjwatson@ubuntu.com>
wrote:
> On Thu, Dec 12, 2013 at 05:24:50PM +0100, Vladimir
''phcoder'' Serbinenko
> wrote:
> > This config has a security problem. If a user has full acces to some
> > partition (e.g. fto server partition) he can put grub.xen there and
load
> > his own code
>
> Only in the domU context, though.  If a user has full access to a guest
> filesystem then of course they can run code in the domU.  This seems
> unsurprising and not a problem?
>
> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

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

On 12.12.2013 18:41, Andrey Borzenkov wrote:
> В Thu, 12 Dec 2013 17:36:43 +0000
> Colin Watson <cjwatson@ubuntu.com> пишет:
> 
>> On Thu, Dec 12, 2013 at 05:45:30PM +0100, Vladimir
''phcoder'' Serbinenko wrote:
>>> No I meant full access to just obe of guest partitions. E.g. FTP
may be
>>> using separate partition and non-admins may have control over it.
If system
>>> has some kind of automatic user creation and /home is separate
someone may
>>> register as boot or grub and put grub.xen in his directory. If /tmp
is on
>>> separate partition and not in RAM then everybody can put grub.xen
to
>>> /tmp/grub/grub.xen
>>
>> Oh, right.  Perhaps we could just look in a limited set of devices,
e.g.
>> (xen/xvda) or (xen/xvda1)?
> 
> Is passing it as argument acceptable?
That''s what pvgrub1 does. AFAIU it first tries configfile passed as
initrd, then configfile name passed on command line, failing both it
tries netboot.
Perhaps we need a parameter to make it sure that we won''t try to
interpret old menu.lst or to load it. E.g.
grub.xenfile=(xen/xvda,1)/boot/grub/grubx64.xen
or
grub.config=(xen/xvda,1)/boot/grub/grub.cfg
> I''m afraid that no automagic is
> going to work for all cases.
> 
What we do here is essentially define "firmware" interface that would
be
followed by installer. Ideally virtual nvram would be a solution but xen
has none.
A lazy possibility is to force creating of EFI system partition (in GPT
or MBR) and load /xen/boot/boot[x64|pae].xen but probably this is far
from ideal
>>                            I''m not very familiar with how
>> multi-partition Xen guests are typically set up.  How is the root
>> partition generally designated at the moment?
>>
> 
> As I understand it is passed as argument to kernel (i.e. in VM
> definition) or embedded in initrd as usual.
> 
There are also scenarios when guest admin has no right to modify machine
description but wants to install another OS. Idk if we''ll need special
support for this as guest admin can always arrange for grub.xen to land
in right place by using tiny partition for this.



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

On Thu, Dec 12, 2013 at 03:37:41PM +0000, Colin Watson
wrote:
> +if [ -z "$grub_xen_guest" ]; then
> +	# This is the copy of grub.xen installed in the dom0''s
filesystem.
> +	# Look for a copy in the domU''s filesystem and chainload that. 
This
> +	# allows us to guarantee that GRUB will be in sync with the
> +	# configuration file in the domU.  The file locations here must not
> +	# have any configure-generated substitutions applied, as the intent
> +	# is that a single grub.xen should be able to cope with a variety of
> +	# domU systems.
> +	if search --set=root --file /boot/grub/grub.xen; then
> +		linux /boot/grub/grub.xen grub_xen_guest=1
> +		boot
[...]

I talked about this with Ian Jackson in the pub last night.  We came to
the same conclusion more or less at the same time, that this is in fact
a new boot protocol; since it essentially just expects a bzImage here,
there''s nothing to stop somebody for example putting a bare kernel
there.  We''d like people to be able to set up PV-GRUB2 in their dom0s
even for domUs that aren''t new enough to have PV-GRUB2 inside them.
Furthermore we''d like to be able to arrange that PV-GRUB (Legacy) and
PyGRUB can at least in principle use the same boot protocol; in the case
of PV-GRUB that would presumably involve a stub menu.lst, but it
shouldn''t take much more than that.  As such the file name in the
domU''s
filesystem shouldn''t be GRUB-specific, although per Vladimir
it''d be
good for it to be distinct for each architecture.

I''ll put this patch series on hold for the time being (with the
possible
exception of "search --exclude", which I think has been
uncontroversial
so far and could perhaps be merged as a generally-useful gadget?) and
write this up as a proper protocol document for inclusion in xen.git.
Ian said he''d like to get this into Xen 4.4''s documentation. 
No changes
to Xen code are required as far as I know.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

On 13.12.2013 12:56, Colin Watson wrote:
> On Thu, Dec 12, 2013 at 03:37:41PM +0000, Colin Watson wrote:
>> +if [ -z "$grub_xen_guest" ]; then
>> +	# This is the copy of grub.xen installed in the dom0''s
filesystem.
>> +	# Look for a copy in the domU''s filesystem and chainload
that.  This
>> +	# allows us to guarantee that GRUB will be in sync with the
>> +	# configuration file in the domU.  The file locations here must not
>> +	# have any configure-generated substitutions applied, as the intent
>> +	# is that a single grub.xen should be able to cope with a variety of
>> +	# domU systems.
>> +	if search --set=root --file /boot/grub/grub.xen; then
>> +		linux /boot/grub/grub.xen grub_xen_guest=1
>> +		boot
> [...]
> 
> I talked about this with Ian Jackson in the pub last night.
>  We came to
> the same conclusion more or less at the same time, that this is in fact
> a new boot protocol; since it essentially just expects a bzImage here,
Not a bzimage but ELF, optionally compressed/wrapped in
bzimage.
> there''s nothing to stop somebody for example putting a bare kernel
> there.
Yes, this is a possibility. You''d have to comile the options in it
though.
> We''d like people to be able to set up PV-GRUB2 in their dom0s
> even for domUs that aren''t new enough to have PV-GRUB2 inside
them.
Yes, that''s why I spoke about compatibility. But it''s good
that this
subject is explicitly discussed.
The main problem with this is security, as discussed in unprivileged
partition subthread. The best way is to replicate pvgrub1 behaviour for
such systems.
> Furthermore we''d like to be able to arrange that PV-GRUB (Legacy)
and
> PyGRUB can at least in principle use the same boot protocol; in the case
> of PV-GRUB that would presumably involve a stub menu.lst, but it
> shouldn''t take much more than that.
The legacy_configfile would need adjustments for that. PVGRUB1 used hdX
notation for virtual disks which legacy_configfile just maps to hdX.
This is a mess because even on runtime it''s not obvious how disks are
mapped, it may even differ between pvgrub1 versions.
>  As such the file name in the domU''s
> filesystem shouldn''t be GRUB-specific, although per Vladimir
it''d be
> good for it to be distinct for each architecture.
> 
Agreed. But if we save it, it should contain full xen device name,
partition specification (beware that partition numbering is ambiguous
and if you use partition number you have to specify how it''s counted.
Otherwise you may opt to have some kind of static specification where to
locate the file. Using a special partition for this is an option and if
so, IMO, it''s better to share it with EFI system
partition.
> I''ll put this patch series on hold for the time being (with the
possible
> exception of "search --exclude", which I think has been
uncontroversial
> so far and could perhaps be merged as a generally-useful gadget?)
I didn''t review it for the reasons of clarifying intended usage. It has
problems, I''ll answer in its thread.
> and
> write this up as a proper protocol document for inclusion in xen.git.
> Ian said he''d like to get this into Xen 4.4''s
documentation.  No changes
> to Xen code are required as far as I know.
>
Ok, could we be kept in the loop on the drafts? I''d hate to have to
cope
with yet another ambiguous protocol specification.





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