Re: [PATCH v4 07/12] util/grub-editenv: wire set_variables to optional fs_envblk

[Avnish Chouhan <avnish@linux.ibm.com>]

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