grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk
  2025-10-09  7:18 [PATCH v4 00/12] Add support for external environment block on Btrfs Michael Chang via Grub-devel
@ 2025-10-09  7:18 ` Michael Chang via Grub-devel
  0 siblings, 0 replies; 3+ messages in thread
From: Michael Chang via Grub-devel @ 2025-10-09  7:18 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Michael Chang, Neal Gompa, Marta Lewandowska

This patch changes set_variables() so that it can use an external
environment block when one is present. The variable next_entry is
written into the external block, env_block is treated as read only, and
all other variables are written into the normal file based envblk.

A cleanup step is added to handle cases where GRUB at runtime writes
variables into the external block because file based updates are not
safe on a copy on write filesystem such as Btrfs. For example, the
savedefault command can update saved_entry, and on Btrfs GRUB will place
that update in the external block instead of the file envblk. If an
older copy remains in the external block, it would override the newer
value from the file envblk when GRUB first loads the file and then
applies the external block on top of it. To avoid this, whenever a
variable is updated in the file envblk, any same named key in the
external block is deleted.

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
---
 util/grub-editenv.c | 58 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index 42a0c63f8..beb2dd4fc 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -402,12 +402,35 @@ fs_envblk_write (grub_envblk_t envblk)
   fclose (fp);
 }
 
+struct var_lookup_ctx {
+  const char *varname;
+  bool found;
+};
+typedef struct var_lookup_ctx var_lookup_ctx_t;
+
+static int
+var_lookup_iter (const char *varname, const char *value __attribute__ ((unused)), void *hook_data)
+{
+  var_lookup_ctx_t *ctx = (var_lookup_ctx_t *) hook_data;
+
+  if (grub_strcmp (ctx->varname, varname) == 0)
+    {
+      ctx->found = true;
+      return 1;
+    }
+  return 0;
+}
+
 static void
 set_variables (const char *name, int argc, char *argv[])
 {
   grub_envblk_t envblk;
+  grub_envblk_t envblk_on_block = NULL;
 
   envblk = open_envblk_file (name);
+  if (fs_envblk != NULL)
+    envblk_on_block = fs_envblk->ops->open (envblk);
+
   while (argc)
     {
       char *p;
@@ -418,15 +441,46 @@ set_variables (const char *name, int argc, char *argv[])
 
       *(p++) = 0;
 
-      if (! grub_envblk_set (envblk, argv[0], p))
-        grub_util_error ("%s", _("environment block too small"));
+      if ((strcmp (argv[0], "next_entry") == 0) && envblk_on_block != NULL)
+	{
+	  if (grub_envblk_set (envblk_on_block, argv[0], p) == 0)
+	    grub_util_error ("%s", _("environment block too small"));
+	  goto next;
+	}
+
+      if (strcmp (argv[0], "env_block") == 0)
+	{
+	  grub_util_warn (_("can't set env_block as it's read-only"));
+	  goto next;
+	}
+
+      if (grub_envblk_set (envblk, argv[0], p) == 0)
+	grub_util_error ("%s", _("environment block too small"));
 
+      if (envblk_on_block != NULL)
+	{
+	  var_lookup_ctx_t ctx = {
+	    .varname = argv[0],
+	    .found = false
+	  };
+
+	  grub_envblk_iterate (envblk_on_block, &ctx, var_lookup_iter);
+	  if (ctx.found == true)
+	    grub_envblk_delete (envblk_on_block, argv[0]);
+	}
+ next:
       argc--;
       argv++;
     }
 
   write_envblk (name, envblk);
   grub_envblk_close (envblk);
+
+  if (envblk_on_block != NULL)
+    {
+      fs_envblk->ops->write (envblk_on_block);
+      grub_envblk_close (envblk_on_block);
+    }
 }
 
 static void
-- 
2.51.0


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

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

* Re: [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk
       [not found] <mailman.8809.1759994405.1112.grub-devel@gnu.org>
@ 2025-10-09  8:43 ` Avnish Chouhan
  2025-10-14  4:37   ` Michael Chang via Grub-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Avnish Chouhan @ 2025-10-09  8:43 UTC (permalink / raw)
  To: mchang; +Cc: grub-devel, ngompa13, mlewando, Daniel Kiper

On 2025-10-09 12:50, grub-devel-request@gnu.org wrote:

Reviewed-by: Avnish Chouhan <avnish@linux.ibm.com>

> Message: 3
> Date: Thu,  9 Oct 2025 15:18:40 +0800
> From: Michael Chang <mchang@suse.com>
> To: The development of GNU GRUB <grub-devel@gnu.org>
> Cc: Neal Gompa <ngompa13@gmail.com>,	Marta Lewandowska
> 	<mlewando@redhat.com>
> Subject: [PATCH v4 07/12] util/grub-editenv: wire set_variables to
> 	optional fs_envblk
> Message-ID: <20251009071845.318828-8-mchang@suse.com>
> 
> This patch changes set_variables() so that it can use an external
> environment block when one is present. The variable next_entry is
> written into the external block, env_block is treated as read only, and
> all other variables are written into the normal file based envblk.
> 
> A cleanup step is added to handle cases where GRUB at runtime writes
> variables into the external block because file based updates are not
> safe on a copy on write filesystem such as Btrfs. For example, the
> savedefault command can update saved_entry, and on Btrfs GRUB will 
> place
> that update in the external block instead of the file envblk. If an
> older copy remains in the external block, it would override the newer
> value from the file envblk when GRUB first loads the file and then
> applies the external block on top of it. To avoid this, whenever a
> variable is updated in the file envblk, any same named key in the
> external block is deleted.
> 
> Signed-off-by: Michael Chang <mchang@suse.com>
> Reviewed-by: Neal Gompa <ngompa13@gmail.com>
> ---
>  util/grub-editenv.c | 58 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/util/grub-editenv.c b/util/grub-editenv.c
> index 42a0c63f8..beb2dd4fc 100644
> --- a/util/grub-editenv.c
> +++ b/util/grub-editenv.c
> @@ -402,12 +402,35 @@ fs_envblk_write (grub_envblk_t envblk)
>    fclose (fp);
>  }
> 
> +struct var_lookup_ctx {
> +  const char *varname;
> +  bool found;
> +};
> +typedef struct var_lookup_ctx var_lookup_ctx_t;
> +
> +static int
> +var_lookup_iter (const char *varname, const char *value __attribute__
> ((unused)), void *hook_data)
> +{
> +  var_lookup_ctx_t *ctx = (var_lookup_ctx_t *) hook_data;
> +
> +  if (grub_strcmp (ctx->varname, varname) == 0)
> +    {
> +      ctx->found = true;
> +      return 1;
> +    }
> +  return 0;
> +}
> +
>  static void
>  set_variables (const char *name, int argc, char *argv[])
>  {
>    grub_envblk_t envblk;
> +  grub_envblk_t envblk_on_block = NULL;
> 
>    envblk = open_envblk_file (name);
> +  if (fs_envblk != NULL)
> +    envblk_on_block = fs_envblk->ops->open (envblk);
> +
>    while (argc)
>      {
>        char *p;
> @@ -418,15 +441,46 @@ set_variables (const char *name, int argc, char 
> *argv[])
> 
>        *(p++) = 0;
> 
> -      if (! grub_envblk_set (envblk, argv[0], p))
> -        grub_util_error ("%s", _("environment block too small"));
> +      if ((strcmp (argv[0], "next_entry") == 0) && envblk_on_block != 
> NULL)
> +	{
> +	  if (grub_envblk_set (envblk_on_block, argv[0], p) == 0)
> +	    grub_util_error ("%s", _("environment block too small"));
> +	  goto next;
> +	}
> +
> +      if (strcmp (argv[0], "env_block") == 0)
> +	{
> +	  grub_util_warn (_("can't set env_block as it's read-only"));
> +	  goto next;
> +	}
> +
> +      if (grub_envblk_set (envblk, argv[0], p) == 0)
> +	grub_util_error ("%s", _("environment block too small"));
> 
> +      if (envblk_on_block != NULL)
> +	{
> +	  var_lookup_ctx_t ctx = {
> +	    .varname = argv[0],
> +	    .found = false
> +	  };
> +
> +	  grub_envblk_iterate (envblk_on_block, &ctx, var_lookup_iter);
> +	  if (ctx.found == true)
> +	    grub_envblk_delete (envblk_on_block, argv[0]);
> +	}
> + next:
>        argc--;
>        argv++;
>      }
> 
>    write_envblk (name, envblk);
>    grub_envblk_close (envblk);
> +
> +  if (envblk_on_block != NULL)
> +    {
> +      fs_envblk->ops->write (envblk_on_block);
> +      grub_envblk_close (envblk_on_block);
> +    }
>  }
> 
>  static void
> --
> 2.51.0

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

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

* Re: [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk
  2025-10-09  8:43 ` [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk Avnish Chouhan
@ 2025-10-14  4:37   ` Michael Chang via Grub-devel
  0 siblings, 0 replies; 3+ messages in thread
From: Michael Chang via Grub-devel @ 2025-10-14  4:37 UTC (permalink / raw)
  To: Avnish Chouhan
  Cc: Michael Chang, grub-devel, ngompa13, mlewando, Daniel Kiper

Hi Avnish,

On Thu, Oct 09, 2025 at 02:13:00PM +0530, Avnish Chouhan wrote:
> On 2025-10-09 12:50, grub-devel-request@gnu.org wrote:
> 
> Reviewed-by: Avnish Chouhan <avnish@linux.ibm.com>

Many thanks for your help in reviewing the patches. I'll add your
Reviewed-by tag in next version.

Regards,
Michael

> 
> > Message: 3
> > Date: Thu,  9 Oct 2025 15:18:40 +0800
> > From: Michael Chang <mchang@suse.com>
> > To: The development of GNU GRUB <grub-devel@gnu.org>
> > Cc: Neal Gompa <ngompa13@gmail.com>,	Marta Lewandowska
> > 	<mlewando@redhat.com>
> > Subject: [PATCH v4 07/12] util/grub-editenv: wire set_variables to
> > 	optional fs_envblk
> > Message-ID: <20251009071845.318828-8-mchang@suse.com>
> > 
> > This patch changes set_variables() so that it can use an external
> > environment block when one is present. The variable next_entry is
> > written into the external block, env_block is treated as read only, and
> > all other variables are written into the normal file based envblk.
> > 
> > A cleanup step is added to handle cases where GRUB at runtime writes
> > variables into the external block because file based updates are not
> > safe on a copy on write filesystem such as Btrfs. For example, the
> > savedefault command can update saved_entry, and on Btrfs GRUB will place
> > that update in the external block instead of the file envblk. If an
> > older copy remains in the external block, it would override the newer
> > value from the file envblk when GRUB first loads the file and then
> > applies the external block on top of it. To avoid this, whenever a
> > variable is updated in the file envblk, any same named key in the
> > external block is deleted.
> > 
> > Signed-off-by: Michael Chang <mchang@suse.com>
> > Reviewed-by: Neal Gompa <ngompa13@gmail.com>
> > ---
> >  util/grub-editenv.c | 58 +++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 56 insertions(+), 2 deletions(-)
> > 
> > diff --git a/util/grub-editenv.c b/util/grub-editenv.c
> > index 42a0c63f8..beb2dd4fc 100644
> > --- a/util/grub-editenv.c
> > +++ b/util/grub-editenv.c
> > @@ -402,12 +402,35 @@ fs_envblk_write (grub_envblk_t envblk)
> >    fclose (fp);
> >  }
> > 
> > +struct var_lookup_ctx {
> > +  const char *varname;
> > +  bool found;
> > +};
> > +typedef struct var_lookup_ctx var_lookup_ctx_t;
> > +
> > +static int
> > +var_lookup_iter (const char *varname, const char *value __attribute__
> > ((unused)), void *hook_data)
> > +{
> > +  var_lookup_ctx_t *ctx = (var_lookup_ctx_t *) hook_data;
> > +
> > +  if (grub_strcmp (ctx->varname, varname) == 0)
> > +    {
> > +      ctx->found = true;
> > +      return 1;
> > +    }
> > +  return 0;
> > +}
> > +
> >  static void
> >  set_variables (const char *name, int argc, char *argv[])
> >  {
> >    grub_envblk_t envblk;
> > +  grub_envblk_t envblk_on_block = NULL;
> > 
> >    envblk = open_envblk_file (name);
> > +  if (fs_envblk != NULL)
> > +    envblk_on_block = fs_envblk->ops->open (envblk);
> > +
> >    while (argc)
> >      {
> >        char *p;
> > @@ -418,15 +441,46 @@ set_variables (const char *name, int argc, char
> > *argv[])
> > 
> >        *(p++) = 0;
> > 
> > -      if (! grub_envblk_set (envblk, argv[0], p))
> > -        grub_util_error ("%s", _("environment block too small"));
> > +      if ((strcmp (argv[0], "next_entry") == 0) && envblk_on_block !=
> > NULL)
> > +	{
> > +	  if (grub_envblk_set (envblk_on_block, argv[0], p) == 0)
> > +	    grub_util_error ("%s", _("environment block too small"));
> > +	  goto next;
> > +	}
> > +
> > +      if (strcmp (argv[0], "env_block") == 0)
> > +	{
> > +	  grub_util_warn (_("can't set env_block as it's read-only"));
> > +	  goto next;
> > +	}
> > +
> > +      if (grub_envblk_set (envblk, argv[0], p) == 0)
> > +	grub_util_error ("%s", _("environment block too small"));
> > 
> > +      if (envblk_on_block != NULL)
> > +	{
> > +	  var_lookup_ctx_t ctx = {
> > +	    .varname = argv[0],
> > +	    .found = false
> > +	  };
> > +
> > +	  grub_envblk_iterate (envblk_on_block, &ctx, var_lookup_iter);
> > +	  if (ctx.found == true)
> > +	    grub_envblk_delete (envblk_on_block, argv[0]);
> > +	}
> > + next:
> >        argc--;
> >        argv++;
> >      }
> > 
> >    write_envblk (name, envblk);
> >    grub_envblk_close (envblk);
> > +
> > +  if (envblk_on_block != NULL)
> > +    {
> > +      fs_envblk->ops->write (envblk_on_block);
> > +      grub_envblk_close (envblk_on_block);
> > +    }
> >  }
> > 
> >  static void
> > --
> > 2.51.0

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

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

end of thread, other threads:[~2025-10-14  4:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <mailman.8809.1759994405.1112.grub-devel@gnu.org>
2025-10-09  8:43 ` [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk Avnish Chouhan
2025-10-14  4:37   ` Michael Chang via Grub-devel
2025-10-09  7:18 [PATCH v4 00/12] Add support for external environment block on Btrfs Michael Chang via Grub-devel
2025-10-09  7:18 ` [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk Michael Chang via Grub-devel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).