[PATCH v2 2/3] grub: Make grub_envblk_iterate() return an int

2019-01-16 Thread Prarit Bhargava 
grub_envblk_iterate() is a void.  Future functions will require the
ability to interpret return codes from the iteration, so
grub_envblk_iterate() should be an int.

The value of 0 returned from the hook functions is overloaded and cannot
be parsed by callers to grub_envblk_iterate() for error handling.  To fix
this add GRUB_ERR_ITERATE_STOP and GRUB_ERR_ITERATE_CONTINUE to the error
returns so the iteration can be stopped and continued respectively.

Make grub_envblk_iterate() return an int.

Signed-off-by: Prarit Bhargava 
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
Cc: Daniel Kiper 
---
 grub-core/commands/loadenv.c |  6 +++---
 grub-core/lib/envblk.c   | 17 -
 include/grub/err.h   |  4 +++-
 include/grub/lib/envblk.h|  6 +++---
 util/grub-editenv.c  |  2 +-
 5 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index 3fd664aac331..1bbae5f7f332 100644
--- a/grub-core/commands/loadenv.c
+++ b/grub-core/commands/loadenv.c
@@ -144,14 +144,14 @@ set_var (const char *name, const char *value, void 
*whitelist)
   if (! whitelist)
 {
   grub_env_set (name, value);
-  return 0;
+  return GRUB_ERR_ITERATE_CONTINUE;
 }
 
   if (test_whitelist_membership (name,
 (const grub_env_whitelist_t *) whitelist))
 grub_env_set (name, value);
 
-  return 0;
+  return GRUB_ERR_ITERATE_CONTINUE;
 }
 
 static grub_err_t
@@ -192,7 +192,7 @@ print_var (const char *name, const char *value,
void *hook_data __attribute__ ((unused)))
 {
   grub_printf ("%s=%s\n", name, value);
-  return 0;
+  return GRUB_ERR_ITERATE_CONTINUE;
 }
 
 static grub_err_t
diff --git a/grub-core/lib/envblk.c b/grub-core/lib/envblk.c
index 230e0e9d9abe..3e43843c4535 100644
--- a/grub-core/lib/envblk.c
+++ b/grub-core/lib/envblk.c
@@ -223,7 +223,7 @@ grub_envblk_delete (grub_envblk_t envblk, const char *name)
 }
 }
 
-void
+int
 grub_envblk_iterate (grub_envblk_t envblk,
  void *hook_data,
  int hook (const char *name, const char *value, void 
*hook_data))
@@ -247,8 +247,7 @@ grub_envblk_iterate (grub_envblk_t envblk,
   while (p < pend && *p != '=')
 p++;
   if (p == pend)
-/* Broken.  */
-return;
+return GRUB_ERR_OUT_OF_RANGE;
   name_end = p;
 
   p++;
@@ -264,13 +263,11 @@ grub_envblk_iterate (grub_envblk_t envblk,
 }
 
   if (p >= pend)
-/* Broken.  */
-return;
+return GRUB_ERR_OUT_OF_RANGE;
 
   name = grub_malloc (p - name_start + 1);
   if (! name)
-/* out of memory.  */
-return;
+return GRUB_ERR_OUT_OF_MEMORY;
 
   value = name + (value_start - name_start);
 
@@ -288,10 +285,12 @@ grub_envblk_iterate (grub_envblk_t envblk,
 
   ret = hook (name, value, hook_data);
   grub_free (name);
-  if (ret)
-return;
+  if (ret != GRUB_ERR_ITERATE_CONTINUE)
+return ret;
 }
 
   p = find_next_line (p, pend);
 }
+
+return GRUB_ERR_NONE;
 }
diff --git a/include/grub/err.h b/include/grub/err.h
index 1590c688e1d3..75ebb54201e8 100644
--- a/include/grub/err.h
+++ b/include/grub/err.h
@@ -71,7 +71,9 @@ typedef enum
 GRUB_ERR_NET_PACKET_TOO_BIG,
 GRUB_ERR_NET_NO_DOMAIN,
 GRUB_ERR_EOF,
-GRUB_ERR_BAD_SIGNATURE
+GRUB_ERR_BAD_SIGNATURE,
+GRUB_ERR_ITERATE_STOP,
+GRUB_ERR_ITERATE_CONTINUE
   }
 grub_err_t;
 
diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h
index c3e655921709..7637791f4228 100644
--- a/include/grub/lib/envblk.h
+++ b/include/grub/lib/envblk.h
@@ -34,9 +34,9 @@ typedef struct grub_envblk *grub_envblk_t;
 grub_envblk_t grub_envblk_open (char *buf, grub_size_t size);
 int grub_envblk_set (grub_envblk_t envblk, const char *name, const char 
*value);
 void grub_envblk_delete (grub_envblk_t envblk, const char *name);
-void grub_envblk_iterate (grub_envblk_t envblk,
-  void *hook_data,
-  int hook (const char *name, const char *value, void 
*hook_data));
+int grub_envblk_iterate (grub_envblk_t envblk,
+ void *hook_data,
+ int hook (const char *name, const char *value, void 
*hook_data));
 void grub_envblk_close (grub_envblk_t envblk);
 
 static inline char *
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 0b6c69b9688c..5f87b09d924a 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -169,7 +169,7 @@ print_var (const char *varname, const char *value,
void *hook_data __attribute__ ((unused)))
 {
   printf ("%s=%s\n", varname, value);
-  return 0;
+  return GRUB_ERR_ITERATE_CONT

[PATCH v2 0/3] grub: add grub variable extend functionality

2019-01-16 Thread Prarit Bhargava 
This patchset allows a user to update an existing grub variable on the
commandline.

v2:  
- Split into 3 patches.
- fix "=value" bug.  An additional consequence of this is that the
  value pointer p in set_variable() cannot underflow.
- make grub_envblk_iterate return an int and fix return values.
- add "+=" functionality
- Changes suggested by David Kiper: Change name to grub_envblk_extend(),
capitalize comments, use standard GRUB error enum, change goto from
"out" to "err", drop cast in grub_strlen() call, use grub_snprintf()
instead of strcpy & memcpy.
- Instead of parsing envblk separately, use grub_envblk_iterate()
- use a case statement in set_variable() error handling, and add a comment
about deleting the "=" sign.

Signed-off-by: Prarit Bhargava 
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
Cc: Daniel Kiper 

Prarit Bhargava (3):
  grub-editenv: Verify the variable size
  grub: Make grub_envblk_iterate() return an int
  grub: add extend variable functionality

 grub-core/commands/loadenv.c |  6 +--
 grub-core/lib/envblk.c   | 17 ---
 include/grub/err.h   |  4 +-
 include/grub/lib/envblk.h|  6 +--
 util/grub-editenv.c  | 86 ++--
 5 files changed, 100 insertions(+), 19 deletions(-)

-- 
2.17.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 3/3] grub: add extend variable functionality

2019-01-16 Thread Prarit Bhargava 
Customers and users of the kernel are commenting that there is no way to update
a grub variable without copy and pasting the existing data.

For example,

[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel newarg"
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel newarg

which is cumbersome.

Add functionality to add to an existing variable.  For example,

[10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel
[10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
kernelopts+="newarg"
[10:59 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel newarg

Signed-off-by: Prarit Bhargava 
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
---
 util/grub-editenv.c | 81 +++--
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 5f87b09d924a..bc997c5312d0 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -201,6 +201,49 @@ write_envblk (const char *name, grub_envblk_t envblk)
   fclose (fp);
 }
 
+struct extend_variable_data {
+  grub_envblk_t envblk;
+  const char *name;
+  const char *add;
+};
+
+static int
+extend_variable_hook (const char *varname, const char *value, void *hook_data)
+{
+  char *new = NULL;
+  int ret = GRUB_ERR_NONE;
+  struct extend_variable_data *data = hook_data;
+
+  if (grub_strcmp (data->name, varname))
+return GRUB_ERR_ITERATE_CONTINUE;
+
+  /* Create the new entry.*/
+  new = grub_zalloc (grub_strlen (value) + grub_strlen (data->add) + 2);
+  if (!new)
+{
+  return GRUB_ERR_OUT_OF_MEMORY;
+  goto err;
+}
+  grub_snprintf(new, grub_strlen (value) + grub_strlen (data->add) + 2,
+   "%s %s", value, data->add);
+
+  /* Erase the current entry. */
+  grub_envblk_delete (data->envblk, varname);
+  /* Add the updated entry. */
+  if (! grub_envblk_set (data->envblk, varname, new))
+{
+  /* Restore the original entry.  This should always work. */
+  grub_envblk_set (data->envblk, varname, value);
+  ret = GRUB_ERR_EOF;
+  goto err;
+}
+
+ err:
+grub_free(new);
+return GRUB_ERR_ITERATE_STOP;
+
+}
+
 static void
 set_variables (const char *name, int argc, char *argv[])
 {
@@ -210,18 +253,52 @@ set_variables (const char *name, int argc, char *argv[])
   while (argc)
 {
   char *p;
+  struct extend_variable_data data;
+  int err;
 
   p = strchr (argv[0], '=');
   if (! p)
 grub_util_error (_("invalid parameter %s"), argv[0]);
 
+  /* Delete "=" to separate variable name and value */
   *(p++) = 0;
 
   if (! grub_strlen(argv[0]))
 grub_util_error (_("No parameter specified"));
 
-  if (! grub_envblk_set (envblk, argv[0], p))
-grub_util_error ("%s", _("environment block too small"));
+  switch (*(p - 2))
+{
+  case '+': /* Extend a parameter */
+*(p - 2) = 0; /* Delete "+" from variable name */
+
+data.envblk = envblk;
+data.name = argv[0];
+data.add = p;
+err = grub_envblk_iterate (envblk, , extend_variable_hook);
+switch (err)
+  {
+   case GRUB_ERR_ITERATE_STOP: /* Success */
+  break;
+
+case GRUB_ERR_OUT_OF_RANGE:
+case GRUB

[PATCH v2 1/3] grub-editenv: Verify the variable size

2019-01-16 Thread Prarit Bhargava 
It is possible to execute 'grub-editenv - set ="some data"',
which results in an unremoveable entry

="some data"

Verify the variable has a size before setting a value.

Signed-off-by: Prarit Bhargava 
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
Cc: Daniel Kiper 
---
 util/grub-editenv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 118e89fe57fe..0b6c69b9688c 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -217,6 +217,9 @@ set_variables (const char *name, int argc, char *argv[])
 
   *(p++) = 0;
 
+  if (! grub_strlen(argv[0]))
+grub_util_error (_("No parameter specified"));
+
   if (! grub_envblk_set (envblk, argv[0], p))
 grub_util_error ("%s", _("environment block too small"));
 
-- 
2.17.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] grub: add grub variable update functionality

2019-01-15 Thread Prarit Bhargava 



On 1/14/19 10:49 AM, Daniel Kiper wrote:
> On Fri, Jan 04, 2019 at 07:53:42AM -0500, Prarit Bhargava wrote:
>> Please be aware I am NOT subscribed to grub-devel.
>>
>> P.
>>
>> ---8<---
>>
>> Customers and users of the kernel are commenting that there is no way to 
>> update
>> a grub variable without copy and pasting the existing data.
>>
>> For example,
>>
>> [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
>> saved_entry=0
>> next_entry=
>> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
>> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
>> ignore_loglevel
>> [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
>> kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
>> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
>> ignore_loglevel newarg"
>> [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
>> saved_entry=0
>> next_entry=
>> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
>> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
>> ignore_loglevel newarg
>>
>> which is cumbersome.
>>
>> Add functionality to add to an existing variable.  For example,
>>
>> [10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
>> saved_entry=0
>> next_entry=
>> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
>> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
>> ignore_loglevel
>> [10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
>> kernelopts+="newarg"
>> [10:59 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
>> saved_entry=0
>> next_entry=
>> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
>> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
>> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
>> ignore_loglevel newarg
>>
>> Signed-off-by: Prarit Bhargava 
>> Cc: mleit...@redhat.com
>> Cc: pjo...@redhat.com
>> Cc: javi...@redhat.com
>> Cc: ar...@redhat.com
>> ---
>>  grub-core/lib/envblk.c| 61 +++
>>  include/grub/lib/envblk.h |  1 +
>>  util/grub-editenv.c   | 25 +++-
>>  3 files changed, 86 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/lib/envblk.c b/grub-core/lib/envblk.c
>> index 230e0e9d9abe..8ddbe2e8267e 100644
>> --- a/grub-core/lib/envblk.c
>> +++ b/grub-core/lib/envblk.c
>> @@ -295,3 +295,64 @@ grub_envblk_iterate (grub_envblk_t envblk,
>>p = find_next_line (p, pend);
>>  }
>>  }
>> +
>> +int
>> +grub_envblk_add (grub_envblk_t envblk, const char *name, const char *add)
> 
> s/grub_envblk_add/grub_envblk_extend/?

Hi Daniel, thanks for the valuable feedback.

After reading your comments I saw that using grub_envblk_iterate() to modify the
entry was easier than attempting to parse everything myself.  I've added that to
my v2.  Unfortunately this means I'm skipping over some of your comments which
are no longer applicable to v2.

> 
>> +{
>> +  char *current, *new, *ostart, *pstart, *pend;
>> +  int newsize, ret = 1;
>> +
>> +  /* get a copy of the existing entry */
>> +  pstart = envblk->buf + sizeof (GRUB_ENVBLK_SIGNATURE) - 1;
>> +  pstart = grub_strstr (pstart, name);
> 
> grub_strcmp() instead of grub_strstr()?
> 
> Even if not what will happen if e.g. env file contains this line among others:
> 
> xyx=kernelopts
> 
>> +  if (!pstart)
>> +  {
>> + ret = -1; /* existing entry not found */
>> + goto out;
> 
> s/out/err/

Applied to v2.

> 
>> +  }
>> +  pend = grub_strchr (pstart, '\n');
> 
> If grub_strcmp() then you do not need this...
> 
>> +  if (!pend || pend > (envblk->buf + envblk->size))
>> +  {
>> + ret = -1; /* existing entry not found */
>> + goto out;
&g

Re: [PATCH] grub: add grub variable update functionality

2019-01-11 Thread Prarit Bhargava 


Just re-pinging on this ...

P.

On 1/4/19 7:53 AM, Prarit Bhargava wrote:
> Please be aware I am NOT subscribed to grub-devel.
> 
> P.
> 
> ---8<---
> 
> Customers and users of the kernel are commenting that there is no way to 
> update
> a grub variable without copy and pasting the existing data.
> 
> For example,
> 
> [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel
> [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
> kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel newarg"
> [10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel newarg
> 
> which is cumbersome.
> 
> Add functionality to add to an existing variable.  For example,
> 
> [10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel
> [10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
> kernelopts+="newarg"
> [10:59 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
> saved_entry=0
> next_entry=
> kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
> crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
> rd.lvm.lv=rhel_intel-wildcatpass-07/root 
> rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
> ignore_loglevel newarg
> 
> Signed-off-by: Prarit Bhargava 
> Cc: mleit...@redhat.com
> Cc: pjo...@redhat.com
> Cc: javi...@redhat.com
> Cc: ar...@redhat.com
> ---
>  grub-core/lib/envblk.c| 61 +++
>  include/grub/lib/envblk.h |  1 +
>  util/grub-editenv.c   | 25 +++-
>  3 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/lib/envblk.c b/grub-core/lib/envblk.c
> index 230e0e9d9abe..8ddbe2e8267e 100644
> --- a/grub-core/lib/envblk.c
> +++ b/grub-core/lib/envblk.c
> @@ -295,3 +295,64 @@ grub_envblk_iterate (grub_envblk_t envblk,
>p = find_next_line (p, pend);
>  }
>  }
> +
> +int
> +grub_envblk_add (grub_envblk_t envblk, const char *name, const char *add)
> +{
> +  char *current, *new, *ostart, *pstart, *pend;
> +  int newsize, ret = 1;
> +
> +  /* get a copy of the existing entry */
> +  pstart = envblk->buf + sizeof (GRUB_ENVBLK_SIGNATURE) - 1;
> +  pstart = grub_strstr (pstart, name);
> +  if (!pstart)
> +  {
> + ret = -1; /* existing entry not found */
> + goto out;
> +  }
> +  pend = grub_strchr (pstart, '\n');
> +  if (!pend || pend > (envblk->buf + envblk->size))
> +  {
> + ret = -1; /* existing entry not found */
> + goto out;
> +  }
> +
> +  current = grub_zalloc (pend - pstart + 1);
> +  if (!current)
> +  {
> + ret = -2; /* out of memory */
> + goto out;
> +  }
> +  grub_strncpy (current, pstart, (int)(pend - pstart));
> +
> +  ostart = grub_strchr (current, '=');
> +  ostart++;
> +
> +  /* create a buffer for the updated entry. */
> +  newsize = grub_strlen (ostart) + grub_strlen (add) + 2;
> +  new = grub_zalloc (newsize);
> +  if (!new)
> +{
> +  return -2; /* out of memory */
> +  goto out;
> +}
> +
> +  grub_strcpy (new, ostart);
> +  grub_memcpy (new + grub_strlen (new), " ", 1);
> +  grub_strcpy (new + grub_strlen (new), add);
> +
> +  /* erase the current entry */
> +  grub_envblk_delete (envblk, name);
> +  /* add the updated entry */
> +  if (!grub_envblk_set (envblk, name, new))
> +{
> +  /* restore the original entry.  This should always work */
> +  grub_envblk_set (envblk, name, ostart);
> +  ret = 0;
> +}
&g

[PATCH] grub: add grub variable update functionality

2019-01-04 Thread Prarit Bhargava 
Please be aware I am NOT subscribed to grub-devel.

P.

---8<---

Customers and users of the kernel are commenting that there is no way to update
a grub variable without copy and pasting the existing data.

For example,

[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
kernelopts="root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel newarg"
[10:57 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel newarg

which is cumbersome.

Add functionality to add to an existing variable.  For example,

[10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel
[10:58 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv - set 
kernelopts+="newarg"
[10:59 AM root@intel-wildcatpass-07 grub-2.02]# ./grub-editenv list
saved_entry=0
next_entry=
kernelopts=root=/dev/mapper/rhel_intel--wildcatpass--07-root ro 
crashkernel=auto resume=/dev/mapper/rhel_intel--wildcatpass--07-swap 
rd.lvm.lv=rhel_intel-wildcatpass-07/root 
rd.lvm.lv=rhel_intel-wildcatpass-07/swap console=ttyS0,115200n81  
ignore_loglevel newarg

Signed-off-by: Prarit Bhargava 
Cc: mleit...@redhat.com
Cc: pjo...@redhat.com
Cc: javi...@redhat.com
Cc: ar...@redhat.com
---
 grub-core/lib/envblk.c| 61 +++
 include/grub/lib/envblk.h |  1 +
 util/grub-editenv.c   | 25 +++-
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/grub-core/lib/envblk.c b/grub-core/lib/envblk.c
index 230e0e9d9abe..8ddbe2e8267e 100644
--- a/grub-core/lib/envblk.c
+++ b/grub-core/lib/envblk.c
@@ -295,3 +295,64 @@ grub_envblk_iterate (grub_envblk_t envblk,
   p = find_next_line (p, pend);
 }
 }
+
+int
+grub_envblk_add (grub_envblk_t envblk, const char *name, const char *add)
+{
+  char *current, *new, *ostart, *pstart, *pend;
+  int newsize, ret = 1;
+
+  /* get a copy of the existing entry */
+  pstart = envblk->buf + sizeof (GRUB_ENVBLK_SIGNATURE) - 1;
+  pstart = grub_strstr (pstart, name);
+  if (!pstart)
+  {
+ ret = -1; /* existing entry not found */
+ goto out;
+  }
+  pend = grub_strchr (pstart, '\n');
+  if (!pend || pend > (envblk->buf + envblk->size))
+  {
+ ret = -1; /* existing entry not found */
+ goto out;
+  }
+
+  current = grub_zalloc (pend - pstart + 1);
+  if (!current)
+  {
+ ret = -2; /* out of memory */
+ goto out;
+  }
+  grub_strncpy (current, pstart, (int)(pend - pstart));
+
+  ostart = grub_strchr (current, '=');
+  ostart++;
+
+  /* create a buffer for the updated entry. */
+  newsize = grub_strlen (ostart) + grub_strlen (add) + 2;
+  new = grub_zalloc (newsize);
+  if (!new)
+{
+  return -2; /* out of memory */
+  goto out;
+}
+
+  grub_strcpy (new, ostart);
+  grub_memcpy (new + grub_strlen (new), " ", 1);
+  grub_strcpy (new + grub_strlen (new), add);
+
+  /* erase the current entry */
+  grub_envblk_delete (envblk, name);
+  /* add the updated entry */
+  if (!grub_envblk_set (envblk, name, new))
+{
+  /* restore the original entry.  This should always work */
+  grub_envblk_set (envblk, name, ostart);
+  ret = 0;
+}
+
+out:
+  grub_free(new);
+  grub_free(current);
+  return ret;
+}
diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h
index c3e655921709..2a0f09e3435b 100644
--- a/include/grub/lib/envblk.h
+++ b/include/grub/lib/envblk.h
@@ -37,6 +37,7 @@ void grub_envblk_delete (grub_envblk_t envblk, const char 
*name);
 void grub_envblk_iterate (grub_envblk_t envblk,
   void *hook_data,
   int hook (const char *name, const char *value, void 
*hook_data));
+int grub_envblk_add(grub_envblk_t envblk, const char *name, const char *add);
 void grub_envblk_close (grub_envblk_t envblk);
 
 static inline char *
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 118e89fe57fe..

Re: [PATCH v2] Add a module for retrieving SMBIOS information

2015-02-09 Thread Prarit Bhargava 


On 02/09/2015 10:27 AM, Rajat Jain wrote:
 Hello,
 
 1) We have a board that boots Linux and this board itself can be plugged
 into one of different chassis types. We need to pass different parameters to
 the kernel based on the CHASSIS_TYPE information that is passed by the
 bios in the DMI / SMBIOS tables.
 
 Actually, I had simplified the problem statement, to avoid complicating the 
 discussion. Basically, we have enhanced the grub with a devicetree command 
 that supplies a flattened device tree to the kernel (just like u-boot or 
 others do in embedded world).
 
 http://www.denx.de/wiki/view/DULG/LinuxFDTBlob
 
 However, we need to pass different flattened device tree to the kernel based 
 on different CHASSIS_TYPE. (because different chassis have different hardware 
 components, slots etc that need to be handled differently).
 


 Whups ... somehow stripped all the cc's on my original reply.

 why isn't 1) already in the kernel itself?
 
 I hope my problem statement clarification above makes it clearer. However, I 
 think the problem is more generic - essentially depending on the current 
 machine type, we may have to do different things on different platforms, such 
 as:
 * Loading different kernels / initrd / device tree etc.
 * Passing different kernel parameters for:
   - different root fs mount points (root=/dev/sda[a/b/c/d]..)
   - enabling disabling different kernel features / drivers / subsystem 
 (pci=[nocrs/use_crs]...)
   - etc.
 

Well I understand the request for different kernels, but 1) implies that you're
adding additional kernel parameters based on the CHASSIS type?  So I'm wondering
why you're not making some boot time decision based on the CHASSIS type instead
of a bootloader time decision.

/me is not against the patchset.  I think the idea of booting different kernels
based on the HW sounds reasonable from a administration perspective.

P.
 Thanks,
 
 Rajat 
 
 
 
 -Original Message-
 From: Prarit Bhargava [mailto:pra...@redhat.com]
 Sent: Monday, February 09, 2015 4:44 AM
 To: David Michael
 Cc: grub-devel@gnu.org; Leif Lindholm; Andrei Borzenkov; Rajat Jain; Sanjay
 Jain; Raghuraman Thirumalairajan; Stu Grossman
 Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information

 On 02/08/2015 01:54 PM, David Michael wrote:
 The following are two use cases from Rajat Jain rajatj...@juniper.net:

 1) We have a board that boots Linux and this board itself can be plugged
 into one of different chassis types. We need to pass different parameters to
 the kernel based on the CHASSIS_TYPE information that is passed by the
 bios in the DMI / SMBIOS tables.


 Whups ... somehow stripped all the cc's on my original reply.

 why isn't 1) already in the kernel itself?

 P.
 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Add a module for retrieving SMBIOS information

2015-02-09 Thread Prarit Bhargava 


On 02/09/2015 12:38 PM, Rajat Jain wrote:
 Hello Prarit,
 
 -Original Message-
 From: Prarit Bhargava [mailto:pra...@redhat.com]
 Sent: Monday, February 09, 2015 8:29 AM
 To: Rajat Jain
 Cc: David Michael; grub-devel@gnu.org; Leif Lindholm; Andrei Borzenkov;
 Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
 Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS information



 On 02/09/2015 10:27 AM, Rajat Jain wrote:
 Hello,

 1) We have a board that boots Linux and this board itself can be
 plugged
 into one of different chassis types. We need to pass different
 parameters to the kernel based on the CHASSIS_TYPE information that
 is passed by the bios in the DMI / SMBIOS tables.

 Actually, I had simplified the problem statement, to avoid complicating the
 discussion. Basically, we have enhanced the grub with a devicetree
 command that supplies a flattened device tree to the kernel (just like u-boot
 or others do in embedded world).

 http://www.denx.de/wiki/view/DULG/LinuxFDTBlob

 However, we need to pass different flattened device tree to the kernel
 based on different CHASSIS_TYPE. (because different chassis have different
 hardware components, slots etc that need to be handled differently).



 Whups ... somehow stripped all the cc's on my original reply.

 why isn't 1) already in the kernel itself?

 I hope my problem statement clarification above makes it clearer.
 However, I think the problem is more generic - essentially depending on the
 current machine type, we may have to do different things on different
 platforms, such as:
 * Loading different kernels / initrd / device tree etc.
 * Passing different kernel parameters for:
 - different root fs mount points (root=/dev/sda[a/b/c/d]..)
 - enabling disabling different kernel features / drivers / subsystem
 (pci=[nocrs/use_crs]...)
 - etc.


 Well I understand the request for different kernels, but 1) implies that 
 you're
 adding additional kernel parameters based on the CHASSIS type?  So I'm
 wondering why you're not making some boot time decision based on the
 CHASSIS type instead of a bootloader time decision.
 
 Oh OK. I guess you are saying why not read the CHASSIS_TYPE in the kernel and 
 take different paths depending on the value? The reason is because we may 
 want to pass different kernel options that *exist today*. (E.g. different 
 root file systems, or hw tuning parametes etc). Ideally once deployed, we 
 neither want to change the bootloader, nor the kernel since that would 
 require a recompilation which is best avoided. If something changes, it is 
 much easier to change the grub config file which can be changed anytime, and 
 thus we are requesting the *capability* to do it in a grub script or config 
 file.
 

Ah, I get it.  That certainly makes sense.  I just thought there was some
specific HW command that you always wanted executed on these boards.


 /me is not against the patchset.  I think the idea of booting different 
 kernels
 based on the HW sounds reasonable from a administration perspective.
 
 Yup, understood. 
 
 Also from admin perspective, I'm suggesting that we may need tune the kernel 
 differently depending on the HW.

Thanks for the info :)

P.

 
 Thanks,
 
 Rajat
 

 P.
 Thanks,

 Rajat



 -Original Message-
 From: Prarit Bhargava [mailto:pra...@redhat.com]
 Sent: Monday, February 09, 2015 4:44 AM
 To: David Michael
 Cc: grub-devel@gnu.org; Leif Lindholm; Andrei Borzenkov; Rajat Jain;
 Sanjay Jain; Raghuraman Thirumalairajan; Stu Grossman
 Subject: Re: [PATCH v2] Add a module for retrieving SMBIOS
 information

 On 02/08/2015 01:54 PM, David Michael wrote:
 The following are two use cases from Rajat Jain rajatj...@juniper.net:

 1) We have a board that boots Linux and this board itself can be
 plugged
 into one of different chassis types. We need to pass different
 parameters to the kernel based on the CHASSIS_TYPE information that
 is passed by the bios in the DMI / SMBIOS tables.


 Whups ... somehow stripped all the cc's on my original reply.

 why isn't 1) already in the kernel itself?

 P.

 
 ___
 Grub-devel mailing list
 Grub-devel@gnu.org
 https://lists.gnu.org/mailman/listinfo/grub-devel
 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Add a module for retrieving SMBIOS information

2015-02-09 Thread Prarit Bhargava 
On 02/08/2015 01:54 PM, David Michael wrote:
 The following are two use cases from Rajat Jain rajatj...@juniper.net:
 
 1) We have a board that boots Linux and this board itself can be plugged into 
 one of different chassis types. We need to pass different parameters to the 
 kernel based on the CHASSIS_TYPE information that is passed by the bios in 
 the DMI / SMBIOS tables.
 

Whups ... somehow stripped all the cc's on my original reply.

why isn't 1) already in the kernel itself?

P.


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] Add a module for retrieving SMBIOS information

2015-02-08 Thread Prarit Bhargava 


On 02/08/2015 01:54 PM, David Michael wrote:
 The following are two use cases from Rajat Jain rajatj...@juniper.net:
 
 1) We have a board that boots Linux and this board itself can be plugged into 
 one of different chassis types. We need to pass different parameters to the 
 kernel based on the CHASSIS_TYPE information that is passed by the bios in 
 the DMI / SMBIOS tables.
 

why isn't 1) already in the kernel itself?

P.

 2) We may have a USB stick that can go into multiple boards, and the exact 
 kernel to be loaded depends on the machine information (PRODUCT_NAME etc) 
 passed via the DMI.
 
 * grub-core/commands/smbios.c: New file.
 * grub-core/Makefile.core.def (smbios): New module.
 * docs/grub.texi (smbios): New node.
 (Command-line and menu entry commands): Add a menu entry for smbios.
 ---
 
 Changes since v1:
 
 * Removed formfeeds and quoted use cases in the commit messages as
   suggested by Prarit Bhargava.
 
 * Enabled building the module on all EFI platforms as suggested by Leif
   Lindholm.
 
  docs/grub.texi  |  67 
  grub-core/Makefile.core.def |   7 +
  grub-core/commands/smbios.c | 361 
 
  3 files changed, 435 insertions(+)
  create mode 100644 grub-core/commands/smbios.c
 
 diff --git a/docs/grub.texi b/docs/grub.texi
 index 46b9e7f..f4e187f 100644
 --- a/docs/grub.texi
 +++ b/docs/grub.texi
 @@ -3830,6 +3830,7 @@ you forget a command, you can run the command 
 @command{help}
  * sha256sum::   Compute or check SHA256 hash
  * sha512sum::   Compute or check SHA512 hash
  * sleep::   Wait for a specified number of seconds
 +* smbios::  Retrieve SMBIOS information
  * source::  Read a configuration file in same context
  * test::Check file types and compare values
  * true::Do nothing, successfully
 @@ -4944,6 +4945,72 @@ if timeout was interrupted by @key{ESC}.
  @end deffn
  
  
 +@node smbios
 +@subsection smbios
 +
 +@deffn Command smbios @
 + [@option{-t} @var{type}] @
 + [@option{-h} @var{handle}] @
 + [@option{-m} @var{match}] @
 + [ (@option{-b} | @option{-w} | @option{-d} | @option{-q} | @option{-s}) @
 +   @var{offset} [@option{-v} @var{variable}] ]
 +Retrieve SMBIOS information.  This command is only available on x86 and EFI
 +systems.
 +
 +When run with no options, @command{smbios} will print the SMBIOS table 
 entries
 +to the console as the header values, a hex dump, and a string list.  The
 +following options can filter which table entries are printed.
 +
 +@itemize @bullet
 +@item
 +Specifying @option{-t} will only print entries with a matching @var{type}.  
 The
 +type can be any value from 0 to 255.  Specify a value outside this range to
 +match entries with any type.  The default is -1.
 +@item
 +Specifying @option{-h} will only print entries with a matching @var{handle}.
 +The handle can be any value from 0 to 65535.  Specify a value outside this
 +range to match entries with any handle.  The default is -1.
 +@item
 +Specifying @option{-m} will only print number @var{match} in the list of all
 +filtered entries; e.g. @code{smbios -m 3} will print the third entry.  The 
 list
 +is always ordered the same as the hardware's SMBIOS table.  The match number
 +can be any positive value.  Specify 0 to match all entries.  The default is 
 0.
 +@end itemize
 +
 +The remaining options print the value of a field in the matched SMBIOS table
 +entry.  Only one of these options may be specified.  When they are used, the
 +option @code{-m 0} is synonymous with @code{-m 1} to match the first entry.
 +
 +@itemize @bullet
 +@item
 +When given @option{-b}, print the value of the byte
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-w}, print the value of the word (two bytes)
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-d}, print the value of the dword (four bytes)
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-q}, print the value of the qword (eight bytes)
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-s}, print the string table entry with its index found
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@end itemize
 +
 +If any of the options in the above list were given, a variable name can be
 +specified with @option{-v} to store the value instead of printing it.
 +
 +For example, this will store and display the system manufacturer string.
 +
 +@example
 +smbios -t 1 -s 4 -v system_manufacturer
 +echo $system_manufacturer
 +@end example
 +@end deffn
 +
 +
  @node source
  @subsection source
  
 diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
 index 42443bc..f87de70 100644
 --- a/grub-core/Makefile.core.def
 +++ b/grub-core/Makefile.core.def
 @@ -1023,6 +1023,13

Re: [PATCH] Add a module for retrieving SMBIOS information

2015-02-04 Thread Prarit Bhargava 


On 02/03/2015 01:41 PM, David Michael wrote:
 On Tue, Feb 3, 2015 at 10:04 AM, Prarit Bhargava pra...@redhat.com wrote:
 On 02/02/2015 02:26 PM, Prarit Bhargava wrote:


 On 02/02/2015 12:09 PM, David Michael wrote:
 On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava pra...@redhat.com wrote:
 On 02/01/2015 09:05 PM, David Michael wrote:
 * grub-core/commands/i386/smbios.c: New file.
 * grub-core/Makefile.core.def (smbios): New module.
 * docs/grub.texi (smbios): New node.
 (Command-line and menu entry commands): Add a menu entry for smbios.
 ---

 Hi,

 There was some interest on help-grub about supporting SMBIOS access
 upstream.

 OOC, why?  Why would you need to do this?  I'm certainly not against 
 doing this
 but just wondering exactly why you want to do this.

 The thread on grub-help asked about booting particular kernel versions
 off a hot-pluggable drive based on the detected hardware, which this
 would allow.

 I originally wrote it to change what options are available based on
 whether a disk is being booted physically or virtually.  Since QEMU
 makes it easy to add SMBIOS entries on the command line, I've also
 been using it for random tweaks like showing a vga_text boot menu
 instead of gfxterm when running QEMU with -display curses.


 Ah interesting David -- and good job on getting the efi.smbios stuff in 
 there
 too as that's an easy thing to miss.  I'll take a closer look ...


 FWIW, I think it looks fine and it definitely has a valid use case.  I'd 
 suggest
 that you update the description with Rajat's comment.
 
 Okay, to be clear, by description here do you mean to put the use
 case in the commit message?

Yes, I find it useful to note the use case.  Later on if someone wants to make a
change to the module we'll know exactly why it was created.

^^^ The above is IMO and has become a sort of standard for other projects.  The
maintainer here may not like it ... but a few extra sentences in the commit
message can't hurt anything ;)

 
 One odd thing in the patch (and it may be something weird on my end that I've
 never seen before).  When I saved and applied your patch via 'git am', the 
 patch
 contained a few ^L lines.
 
 Yes, I used formfeeds to follow the GNU coding standards document when
 I first wrote the module.  I'll take them out of the updated patch.

Thanks!

P.

 
 Thanks.
 
 David
 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add a module for retrieving SMBIOS information

2015-02-03 Thread Prarit Bhargava 
On 02/02/2015 02:26 PM, Prarit Bhargava wrote:
 
 
 On 02/02/2015 12:09 PM, David Michael wrote:
 On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava pra...@redhat.com wrote:
 On 02/01/2015 09:05 PM, David Michael wrote:
 * grub-core/commands/i386/smbios.c: New file.
 * grub-core/Makefile.core.def (smbios): New module.
 * docs/grub.texi (smbios): New node.
 (Command-line and menu entry commands): Add a menu entry for smbios.
 ---

 Hi,

 There was some interest on help-grub about supporting SMBIOS access
 upstream.

 OOC, why?  Why would you need to do this?  I'm certainly not against doing 
 this
 but just wondering exactly why you want to do this.

 The thread on grub-help asked about booting particular kernel versions
 off a hot-pluggable drive based on the detected hardware, which this
 would allow.

 I originally wrote it to change what options are available based on
 whether a disk is being booted physically or virtually.  Since QEMU
 makes it easy to add SMBIOS entries on the command line, I've also
 been using it for random tweaks like showing a vga_text boot menu
 instead of gfxterm when running QEMU with -display curses.

 
 Ah interesting David -- and good job on getting the efi.smbios stuff in there
 too as that's an easy thing to miss.  I'll take a closer look ...
 

FWIW, I think it looks fine and it definitely has a valid use case.  I'd suggest
that you update the description with Rajat's comment.

One odd thing in the patch (and it may be something weird on my end that I've
never seen before).  When I saved and applied your patch via 'git am', the patch
contained a few ^L lines.  For example

^L
/* Reference: DMTF Standard DSP0134 2.7.1 Section 6.1.3 */


Not sure why that happened, but I do see them in the patch as well as the email
you sent to the list.

P.


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add a module for retrieving SMBIOS information

2015-02-02 Thread Prarit Bhargava 
On 02/01/2015 09:05 PM, David Michael wrote:
 * grub-core/commands/i386/smbios.c: New file.
 * grub-core/Makefile.core.def (smbios): New module.
 * docs/grub.texi (smbios): New node.
 (Command-line and menu entry commands): Add a menu entry for smbios.
 ---
 
 Hi,
 
 There was some interest on help-grub about supporting SMBIOS access
 upstream.  

OOC, why?  Why would you need to do this?  I'm certainly not against doing this
but just wondering exactly why you want to do this.


P.


I've updated the module I wrote a while ago to work with
 current GRUB and added documentation.  Is this acceptable to be applied?
 Are there any comments or suggestions for improvement?
 
 I have not yet gone through any copyright assignment procedure for
 GRUB.  If this patch is acceptable, can anyone advise me on what is
 required on that front?
 
 Thanks.
 
 David
 
  docs/grub.texi   |  66 +++
  grub-core/Makefile.core.def  |   6 +
  grub-core/commands/i386/smbios.c | 368 
 +++
  3 files changed, 440 insertions(+)
  create mode 100644 grub-core/commands/i386/smbios.c
 
 diff --git a/docs/grub.texi b/docs/grub.texi
 index 46b9e7f..ff1028f 100644
 --- a/docs/grub.texi
 +++ b/docs/grub.texi
 @@ -3830,6 +3830,7 @@ you forget a command, you can run the command 
 @command{help}
  * sha256sum::   Compute or check SHA256 hash
  * sha512sum::   Compute or check SHA512 hash
  * sleep::   Wait for a specified number of seconds
 +* smbios::  Retrieve SMBIOS information
  * source::  Read a configuration file in same context
  * test::Check file types and compare values
  * true::Do nothing, successfully
 @@ -4944,6 +4945,71 @@ if timeout was interrupted by @key{ESC}.
  @end deffn
  
  
 +@node smbios
 +@subsection smbios
 +
 +@deffn Command smbios @
 + [@option{-t} @var{type}] @
 + [@option{-h} @var{handle}] @
 + [@option{-m} @var{match}] @
 + [ (@option{-b} | @option{-w} | @option{-d} | @option{-q} | @option{-s}) @
 +   @var{offset} [@option{-v} @var{variable}] ]
 +Retrieve SMBIOS information.  This command is only available on x86 systems.
 +
 +When run with no options, @command{smbios} will print the SMBIOS table 
 entries
 +to the console as the header values, a hex dump, and a string list.  The
 +following options can filter which table entries are printed.
 +
 +@itemize @bullet
 +@item
 +Specifying @option{-t} will only print entries with a matching @var{type}.  
 The
 +type can be any value from 0 to 255.  Specify a value outside this range to
 +match entries with any type.  The default is -1.
 +@item
 +Specifying @option{-h} will only print entries with a matching @var{handle}.
 +The handle can be any value from 0 to 65535.  Specify a value outside this
 +range to match entries with any handle.  The default is -1.
 +@item
 +Specifying @option{-m} will only print number @var{match} in the list of all
 +filtered entries; e.g. @code{smbios -m 3} will print the third entry.  The 
 list
 +is always ordered the same as the hardware's SMBIOS table.  The match number
 +can be any positive value.  Specify 0 to match all entries.  The default is 
 0.
 +@end itemize
 +
 +The remaining options print the value of a field in the matched SMBIOS table
 +entry.  Only one of these options may be specified.  When they are used, the
 +option @code{-m 0} is synonymous with @code{-m 1} to match the first entry.
 +
 +@itemize @bullet
 +@item
 +When given @option{-b}, print the value of the byte
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-w}, print the value of the word (two bytes)
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-d}, print the value of the dword (four bytes)
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-q}, print the value of the qword (eight bytes)
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@item
 +When given @option{-s}, print the string table entry with its index found
 +at @var{offset} bytes into the matched SMBIOS table entry.
 +@end itemize
 +
 +If any of the options in the above list were given, a variable name can be
 +specified with @option{-v} to store the value instead of printing it.
 +
 +For example, this will store and display the system manufacturer string.
 +
 +@example
 +smbios -t 1 -s 4 -v system_manufacturer
 +echo $system_manufacturer
 +@end example
 +@end deffn
 +
 +
  @node source
  @subsection source
  
 diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
 index 42443bc..2e1e075 100644
 --- a/grub-core/Makefile.core.def
 +++ b/grub-core/Makefile.core.def
 @@ -1023,6 +1023,12 @@ module = {
  };
  
  module = {
 +  name = smbios;
 +  common = commands/i386/smbios.c;
 +  enable = x86;
 +};
 +
 +module = {
name = suspend;
ieee1275 =

Re: [PATCH] Add a module for retrieving SMBIOS information

2015-02-02 Thread Prarit Bhargava 


On 02/02/2015 01:01 PM, Rajat Jain wrote:
 Hello,
 
 -Original Message-
 From: David Michael [mailto:fedora@gmail.com]
 Sent: Monday, February 02, 2015 9:10 AM
 To: Prarit Bhargava
 Cc: The development of GNU GRUB; Andrei Borzenkov; Rajat Jain; Stu
 Grossman; Raghuraman Thirumalairajan; Sanjay Jain
 Subject: Re: [PATCH] Add a module for retrieving SMBIOS information

 On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava pra...@redhat.com
 wrote:
 On 02/01/2015 09:05 PM, David Michael wrote:
 * grub-core/commands/i386/smbios.c: New file.
 * grub-core/Makefile.core.def (smbios): New module.
 * docs/grub.texi (smbios): New node.
 (Command-line and menu entry commands): Add a menu entry for
 smbios.
 ---

 Hi,

 There was some interest on help-grub about supporting SMBIOS access
 upstream.
 
 
 I think having the capability to parse the SMBIOS data is certainly very 
 interesting to us, and currently we have the following use cases to make use 
 of it:
 
 1) We have a board that boots Linux and this board itself can be plugged into 
 one of different chassis types. We need to pass different parameters to the 
 kernel based on the CHASSIS_TYPE information that is passed by the bios in 
 the DMI / SMBIOS tables.
 
 2) We may have a USB stick that can go into multiple boards, and the exact 
 kernel to be loaded depends on the machine information (PRODUCT_NAME etc) 
 passed via the DMI. 
 
 I hope that clarifies our usecase and needs where this functionality will be 
 immensely helpful.
 
 Thanks,

Thanks Rajat, that does really explain things.  I'll take a closer look in the
next few days ...

P.

 
 Rajat
 
 
 
 

 OOC, why?  Why would you need to do this?  I'm certainly not against
 doing this but just wondering exactly why you want to do this.

 The thread on grub-help asked about booting particular kernel versions off a
 hot-pluggable drive based on the detected hardware, which this would
 allow.

 I originally wrote it to change what options are available based on whether a
 disk is being booted physically or virtually.  Since QEMU makes it easy to 
 add
 SMBIOS entries on the command line, I've also been using it for random
 tweaks like showing a vga_text boot menu instead of gfxterm when running
 QEMU with -display curses.

 Thanks.

 David

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add a module for retrieving SMBIOS information

2015-02-02 Thread Prarit Bhargava 


On 02/02/2015 12:09 PM, David Michael wrote:
 On Mon, Feb 2, 2015 at 6:20 AM, Prarit Bhargava pra...@redhat.com wrote:
 On 02/01/2015 09:05 PM, David Michael wrote:
 * grub-core/commands/i386/smbios.c: New file.
 * grub-core/Makefile.core.def (smbios): New module.
 * docs/grub.texi (smbios): New node.
 (Command-line and menu entry commands): Add a menu entry for smbios.
 ---

 Hi,

 There was some interest on help-grub about supporting SMBIOS access
 upstream.

 OOC, why?  Why would you need to do this?  I'm certainly not against doing 
 this
 but just wondering exactly why you want to do this.
 
 The thread on grub-help asked about booting particular kernel versions
 off a hot-pluggable drive based on the detected hardware, which this
 would allow.
 
 I originally wrote it to change what options are available based on
 whether a disk is being booted physically or virtually.  Since QEMU
 makes it easy to add SMBIOS entries on the command line, I've also
 been using it for random tweaks like showing a vga_text boot menu
 instead of gfxterm when running QEMU with -display curses.
 

Ah interesting David -- and good job on getting the efi.smbios stuff in there
too as that's an easy thing to miss.  I'll take a closer look ...

P.

 Thanks.
 
 David
 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Clarify Press any key to continue... message

2014-04-03 Thread Prarit Bhargava 
On 04/03/2014 10:23 AM, Colin Watson wrote:
 When grub_wait_after_message says Press any key to continue..., it
 really means that it will continue anyway after a short delay, but that
 you can press a key to skip the delay.  Unfortunately, the delay is just
 long enough that in practice a number of users have time to see it,
 press a key, and then report a bug saying that their system won't boot
 without manual intervention, even though it would have booted just fine
 if they'd left it alone.  Rephrase this message to make it clearer
 what's really happening.

I have mixed feelings about changing this message since it is such a well known
message.  OTOH, I've never really liked it myself.

Maybe a better idea is to do a timeout message

Press any key for menu (boot will continue in 5 seconds) ...
Press any key for menu (boot will continue in 4 seconds) ...
...
etc.

?

 
 * grub-core/normal/menu.c (grub_wait_after_message): Rephrase
 message to make it clear that GRUB will continue even if the user
 does not press a key.
 ---
  ChangeLog   | 6 ++
  grub-core/normal/menu.c | 4 +++-
  2 files changed, 9 insertions(+), 1 deletion(-)
 
 diff --git a/ChangeLog b/ChangeLog
 index 6eca73e..53c2aff 100644
 --- a/ChangeLog
 +++ b/ChangeLog
 @@ -1,3 +1,9 @@
 +2014-04-03  Colin Watson  cjwat...@ubuntu.com
 +
 + * grub-core/normal/menu.c (grub_wait_after_message): Rephrase
 + message to make it clear that GRUB will continue even if the user
 + does not press a key.
 +
  2014-03-31  Thomas Falcon tlfal...@linux.vnet.ibm.com
  
   btrfs: fix get_root key comparison failures due to endianness
 diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
 index b47991a..a6fa93c 100644
 --- a/grub-core/normal/menu.c
 +++ b/grub-core/normal/menu.c
 @@ -63,7 +63,9 @@ grub_wait_after_message (void)
  {
grub_uint64_t endtime;
grub_xputs (\n);
 -  grub_printf_ (N_(Press any key to continue...));
 +  grub_printf_ (N_(
 +Waiting a moment so that you can read previous messages.\n
 +Press any key to skip this delay...));
grub_refresh ();
  
endtime = grub_get_time_ms () + 1;
 


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Clarify Press any key to continue... message

2014-04-03 Thread Prarit Bhargava 


On 04/03/2014 12:52 PM, Colin Watson wrote:
 On Thu, Apr 03, 2014 at 12:49:50PM -0400, Prarit Bhargava wrote:
 On 04/03/2014 10:23 AM, Colin Watson wrote:
 When grub_wait_after_message says Press any key to continue..., it
 really means that it will continue anyway after a short delay, but that
 you can press a key to skip the delay.  Unfortunately, the delay is just
 long enough that in practice a number of users have time to see it,
 press a key, and then report a bug saying that their system won't boot
 without manual intervention, even though it would have booted just fine
 if they'd left it alone.  Rephrase this message to make it clearer
 what's really happening.

 I have mixed feelings about changing this message since it is such a
 well known message.  OTOH, I've never really liked it myself.
 
 Surely nobody can really be depending on it; I don't see a problem with
 changing that kind of text, in and of itself.

Yeah ... but it is one of those things like the NMI dazed and confused message.
 Everyone knows about it, everyone agrees it could be better, but no one wants
to change it because so much open documentation references it.

Try googling linux + Press any key to continue and you'll see what I mean.

But again, I've never really cared for the message so I'm favour of changing it 
;)

 
 Maybe a better idea is to do a timeout message

 Press any key for menu (boot will continue in 5 seconds) ...
 Press any key for menu (boot will continue in 4 seconds) ...
 ...
 etc.

 ?
 
 It's not necessarily for menu - you get this for errors after a menu
 item has been selected too, and then it's just a delay before booting.

Hmm ... really?  After a menu item has been selected?  I've never done that 
before.
 
 Seems like a lot of effort.  The actual delay is 2.5 seconds so a
 countdown would look jumpy at the end ...
 

Just do a countdown and change the delay to an even 3 seconds.  Who is really
going to care about a 1/2 second?

P.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: How to pick a kernel to boot ?

2013-10-24 Thread Prarit Bhargava 
On 10/24/2013 12:44 PM, Lennart Sorensen wrote:
 On Thu, Oct 24, 2013 at 04:46:19PM +0200, Lukáš Czerner wrote:
 Hi,

 I have a simple question about grub2 which I can not find satisfying
 answer to.

 What is a proper way to pick a kernel to boot from in case I have
 multiple kernels installed ? Note that picking the right kernel from
 the menu is not an option I do not always have access to the
 console.

 With grub legacy, this was easy. Simply edit

  /boot/grub/menu.lst

 and pick the right number. All the information I need are in that
 file.

 So what is a recommended way to do this with grub2 ?
 
 Same thing in /boot/grub/grub.cfg.  There is default with a number.
 

Sorry ... some how left the list off my initial reply ...

To set a default to boot, use

grub2-set-default  [kernel entry number]

To boot a kernel once and keep the default the same (ie, boot a kernel one time)

grub2-reboot [kernel entry number]

I've noticed a lot of people messing with /boot/grub2/grub.conf ... you
shouldn't have to do that to select a kernel to boot.

P.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: at boot-time echoed kernel is older that started kernel

2013-02-21 Thread Prarit Bhargava 
On 02/19/2013 03:31 PM, poma wrote:
 On 02/18/13 21:39, Reindl Harald wrote:
 […]
 
 i would be thankful if even grub2-mkconfig would not create
 this advanced submenu at all

 
 
 Actually there is a patch proposal at 'grub-devel' by Prarit Bhargava,
 for such a case - disable submenu[1][2].

Reindl and poma,

FYI the latest fedora (it might be in rawhide by now) built grub2 has
GRUB_DISABLE_SUBMENU functionality.  It would be good to get testing on this...

http://koji.fedoraproject.org/koji/buildinfo?buildID=386531

* Thu Feb 14 2013 Peter Jones pjo...@redhat.com - 2.00-16
- Allow the user to disable submenu generation

You can simply add 'GRUB_DISABLE_SUBMENU=true' to /etc/default/grub and it
magically works :)

P.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH]: Add GRUB_DISABLE_SUBMENU option

2013-01-30 Thread Prarit Bhargava 


 /usr/sbin/grub2-reboot 'Advanced options for openSUSE 12.2openSUSE 12.2, 
 with
 Linux 3.4.6-2.10-desktop (recovery mode)'

 
 That's what good command line completion is for. Really :)

Heh :)  Fair enough :)

 
 How is user going to know which number to type? Open grub.cfg and count?

This is what I used to do in grub1 ... I wrote a nice little script that dumped
the contents of grub config and enumerated the entries so that I could select
which one I wanted.

FWIW, I could do *exactly* the same thing with grub2, but I'm wondering if there
is a better way so that the kernel debugging users of grub2 could all benefit.

 Actually I have patch to disable submenu somewhere around as well. So I
 can well understand your feelings, I just think this is unrelated to
 grub-reboot.

You're right, and I'm not saying it is :)  I like to provide a decent use-case
scenario with a patch so people who read it later understand _why_ I've made a
change.

 
 Alternatively, would you object to me to adding some additional 
 functionality to
 grub2-reboot that would grep grub.cfg and display a menu of entries so that
 'grub2-reboot #number' would work with submenus?

 
 I think something like grub-reboot --select would be interesting,
 yes. But it is not for me to decide :)

I've talked with a few other kernel engineers, and they like the idea of just
adding GRUB_DISABLE_SUBMENU=true to /etc/default/grub ... IMO it still seems
like the easiest approach.

Beyond the grub-reboot stuff, do you have any objections to the patch going in?

Thanks for your input (and the info!) Andrey.  That was really helpful :)

P.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH]: Add GRUB_DISABLE_SUBMENU option

2013-01-29 Thread Prarit Bhargava 
I'm not sure what the proper patch posting protocol is on this list and
I've never used bzr before.  If this is incorrect, please email me and let
me know what I've done wrong.

Thanks,

P.




revno: 4686
committer: Prarit Bhargava pra...@redhat.com
branch nick: grub
timestamp: Tue 2013-01-29 14:14:01 -0500
message:
  Add GRUB_DISABLE_MENU option
  
  When doing kernel development it is often advantageous to do the following:
  
  1.  Select a kernel to boot with grub2-reboot
  2.  Reboot, capture output from bad kernel.
  3.  On subsequent reboot, boot known good or default kernel.
  
  The problem is that the submenus get in the way of doing this.  grub2-reboot
  does not allow one to set a reboot to a specific kernel.
  
  I *could* write a rule to be executed on the existing grub.cfg to remove
  the submenu entries, however, given that a google search for grub2 remove
  submenus leads to many many hits, I think a better approach is to make
  GRUB_DISABLE_SUBMENU a generally available option.
  
  Please note I have only tested this on Linux (specifically Fedora 17 and
  Fedora 18).
  
  Signed-off-by: Prarit Bhargava pra...@redhat.com


diff:
=== modified file 'util/grub-mkconfig.in'
--- util/grub-mkconfig.in   2012-12-28 07:21:17 +
+++ util/grub-mkconfig.in   2013-01-29 19:14:01 +
@@ -215,7 +215,8 @@
   GRUB_INIT_TUNE \
   GRUB_SAVEDEFAULT \
   GRUB_ENABLE_CRYPTODISK \
-  GRUB_BADRAM
+  GRUB_BADRAM \
+  GRUB_DISABLE_SUBMENU
 
 if test x${grub_cfg} != x; then
   rm -f ${grub_cfg}.new

=== modified file 'util/grub.d/10_hurd.in'
--- util/grub.d/10_hurd.in  2012-09-18 11:04:06 +
+++ util/grub.d/10_hurd.in  2013-01-29 19:14:01 +
@@ -156,14 +156,15 @@
 for kernel in ${kernels}
 do
 
-  if [ x$is_first_entry = xtrue ]; then
+  if [ x${GRUB_DISABLE_SUBMENU} = x ]; then 
+if [ x$is_first_entry = xtrue ]; then
   hurd_entry $kernel simple
   submenu_indentation=$grub_tab
 
   # TRANSLATORS: %s is replaced with an OS name
   echo submenu '$(gettext_printf Advanced options for %s ${OS} | 
grub_quote)' \$menuentry_id_option 'gnuhurd-advanced-$(grub_get_device_id 
${GRUB_DEVICE_BOOT})' {
+fi
   fi
-
   hurd_entry $kernel advanced
   hurd_entry $kernel recovery
   is_first_entry=false
@@ -171,8 +172,10 @@
 
 # If at least one kernel was found, then we need to
 # add a closing '}' for the submenu command.
-if [ x$is_first_entry != xtrue ]; then
-  echo '}'
+if [ x${GRUB_DISABLE_SUBMENU} = x ]; then 
+  if [ x$is_first_entry != xtrue ]; then
+echo '}'
+  fi
 fi
 
 echo $title_correction_code

=== modified file 'util/grub.d/10_kfreebsd.in'
--- util/grub.d/10_kfreebsd.in  2013-01-03 22:19:19 +
+++ util/grub.d/10_kfreebsd.in  2013-01-29 19:14:01 +
@@ -211,15 +211,17 @@
 module_dir_rel=$(make_system_path_relative_to_its_root $module_dir)
   fi
 
-  if [ x$is_first_entry = xtrue ]; then
-  kfreebsd_entry ${OS} ${version} simple
-  submenu_indentation=$grub_tab
-
-  if [ -z $boot_device_id ]; then
+  if [ x${GRUB_DISABLE_SUBMENU} = x ]; then 
+if [ x$is_first_entry = xtrue ]; then
+kfreebsd_entry ${OS} ${version} simple
+submenu_indentation=$grub_tab
+
+if [ -z $boot_device_id ]; then
  boot_device_id=$(grub_get_device_id ${GRUB_DEVICE})
-  fi
-  # TRANSLATORS: %s is replaced with an OS name
-  echo submenu '$(gettext_printf Advanced options for %s ${OS} | 
grub_quote)' \$menuentry_id_option 'kfreebsd-advanced-$boot_device_id' {
+fi
+# TRANSLATORS: %s is replaced with an OS name
+echo submenu '$(gettext_printf Advanced options for %s ${OS} | 
grub_quote)' \$menuentry_id_option 'kfreebsd-advanced-$boot_device_id' {
+fi
   fi
 
   kfreebsd_entry ${OS} ${version} advanced
@@ -233,8 +235,10 @@
 
 # If at least one kernel was found, then we need to
 # add a closing '}' for the submenu command.
-if [ x$is_first_entry != xtrue ]; then
-  echo '}'
+if [ x${GRUB_DISABLE_SUBMENU} = x ]; then 
+  if [ x$is_first_entry != xtrue ]; then
+echo '}'
+  fi
 fi
 
 echo $title_correction_code

=== modified file 'util/grub.d/10_linux.in'
--- util/grub.d/10_linux.in 2012-09-18 11:04:06 +
+++ util/grub.d/10_linux.in 2013-01-29 19:14:01 +
@@ -226,18 +226,20 @@
 linux_root_device_thisversion=${GRUB_DEVICE}
   fi
 
-  if [ x$is_first_entry = xtrue ]; then
-linux_entry ${OS} ${version} simple \
-${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}
+  if [ x${GRUB_DISABLE_SUBMENU} = x ]; then 
+if [ x$is_first_entry = xtrue ]; then
+  linux_entry ${OS} ${version} simple \
+  ${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}
 
-submenu_indentation=$grub_tab
+  submenu_indentation=$grub_tab
 
-if [ -z $boot_device_id ]; then
-   boot_device_id=$(grub_get_device_id ${GRUB_DEVICE})
+  if [ -z $boot_device_id ]; then
+ boot_device_id=$(grub_get_device_id

Re: [PATCH]: Add GRUB_DISABLE_SUBMENU option

2013-01-29 Thread Prarit Bhargava 


On 01/29/2013 03:09 PM, Andrey Borzenkov wrote:
   The problem is that the submenus get in the way of doing this.  
 grub2-reboot
   does not allow one to set a reboot to a specific kernel.
   
 
 It does.
 
 sudo /usr/sbin/grub2-reboot 'Advanced options for openSUSE 12.2openSUSE 
 12.2, with Linux 3.4.6-2.10-desktop (recovery mode)'
 
 better is to use menuentry id, but this works too.

Hi Andrey,

Interesting :), and I'm not going to deny I could code a script to do that.

However, given the large number of requests for disabling the submenus (google
grub2 disable submenus) it would still be preferable to myself and others
(from just a general debug point of view) to be able to have the entries in a
single list so that I can use the numeric feature of grub2-reboot.

ie) /usr/sbin/grub2-reboot 2

is always easier than having to type

/usr/sbin/grub2-reboot 'Advanced options for openSUSE 12.2openSUSE 12.2, with
Linux 3.4.6-2.10-desktop (recovery mode)'

which requires me to grep through grub.cfg, figure out what submenu I'm under,
and figure out what the entry is.

Again, I could script that out, but there seems to be enough momentum behind
disabling the submenus that I think the patch may still have some merit.

Alternatively, would you object to me to adding some additional functionality to
grub2-reboot that would grep grub.cfg and display a menu of entries so that
'grub2-reboot #number' would work with submenus?

P.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel