On Tue, Feb 25, 2020 at 11:26:41AM -0800, Paul Dagnelie wrote: > We leverage the grub envblk file and fs functions to read from and > write to the envblk. We also tweak the editenv code by factoring out > the logic that creates the buffer with the envblk contents so it can > be reused.
The commit message suggests that we need at least two patches here... > We also add the grubenv_src variable, which can be used to force grub > to load the grubenv file from a file or from the envblk even if it > would not automatically choose to do so. > > commit d9e66e27592ad83f4a90d0e231a9ffc679617cae > Author: Paul Dagnelie <p...@delphix.com> > AuthorDate: Mon Feb 24 13:40:55 2020 -0800 > Commit: Paul Dagnelie <p...@delphix.com> > CommitDate: Tue Feb 25 10:08:07 2020 -0800 > > Use envblk file and fs functions to implement reading the grubenv > file from special FS regions > --- > grub-core/commands/loadenv.c | 122 > +++++++++++++++++++++++++++++++++++++------ > include/grub/lib/envblk.h | 2 + > include/grub/util/install.h | 3 ++ > util/editenv.c | 26 +++++---- > 4 files changed, 129 insertions(+), 24 deletions(-) > > diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c > index 3fd664aac..0f20543a3 100644 > --- a/grub-core/commands/loadenv.c > +++ b/grub-core/commands/loadenv.c > @@ -79,6 +79,94 @@ open_envblk_file (char *filename, > return file; > } > > +static grub_file_t > +open_envblk_block (grub_fs_t fs, grub_device_t dev, enum grub_file_type type) > +{ > + if (!fs->fs_envblk_open) > + return NULL; > + return grub_file_envblk_open(fs, dev, type); Nit but... if (fs->fs_envblk_open) return grub_file_envblk_open (fs, dev, type); return NULL; > +} > + > +static grub_file_t > +open_envblk (grub_extcmd_context_t ctxt, enum grub_file_type type) > +{ > + struct grub_arg_list *state = ctxt->state; > + grub_file_t file = NULL; > + grub_device_t device = NULL; > + const char *source; > + grub_fs_t fs = NULL; > + char *filename = state[0].set ? state[0].arg : NULL; > + > + source = grub_env_get ("grubenv_src"); > + if (! source || ! grub_strcmp (source, GRUB_ENVBLK_SRCBLK)) > + { > + grub_dprintf ("loadenv", "checking for envblk\n"); > + char *device_name; > + const char *prefix; Please do not mix variable declarations with the code. The former should precede the latter. > + prefix = grub_env_get ("prefix"); > + if (! prefix) > + { > + grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' > isn't set"), "prefix"); > + return NULL; > + } > + > + device_name = grub_file_get_device_name (prefix); > + if (grub_errno) > + return NULL; > + > + device = grub_device_open (device_name); > + if (! device) > + return NULL; > + > + fs = grub_fs_probe (device); > + if (! fs) { > + grub_device_close (device); > + return NULL; > + } > + > + /* We have to reopen the device here because it will be closed > in grub_fs_probe. */ s/it will be/it was/? > + device = grub_device_open (device_name); > + grub_free (device_name); > + if (! device) > + return NULL; > + > + } > + if (! source) > + { > + if (fs->fs_envblk_open) > + { > + source = GRUB_ENVBLK_SRCBLK; > + grub_dprintf ("loadenv", "envblk support detected\n"); > + } > + else > + { > + source = GRUB_ENVBLK_SRCFILE; > + grub_dprintf ("loadenv", "envblk support not detected\n"); > + } > + } > + > + if (! grub_strcmp (source, GRUB_ENVBLK_SRCFILE)) > + { > + if (device) > + grub_device_close (device); > + file = open_envblk_file (filename, type); > + > + if (! file) > + return NULL; > + } > + else if (! grub_strcmp (source, GRUB_ENVBLK_SRCBLK)) > + { > + file = open_envblk_block (fs, device, type); > + if (! file) > + { > + grub_device_close (device); > + return NULL; > + } > + } > + return file; > +} > + > static grub_envblk_t > read_envblk_file (grub_file_t file) > { > @@ -165,11 +253,10 @@ grub_cmd_load_env (grub_extcmd_context_t ctxt, > int argc, char **args) > whitelist.len = argc; > whitelist.list = args; > > - /* state[0] is the -f flag; state[1] is the --skip-sig flag */ > - file = open_envblk_file ((state[0].set) ? state[0].arg : 0, > - GRUB_FILE_TYPE_LOADENV > - | (state[1].set > - ? GRUB_FILE_TYPE_SKIP_SIGNATURE : GRUB_FILE_TYPE_NONE)); > + file = open_envblk (ctxt, GRUB_FILE_TYPE_LOADENV > + | (state[1].set > + ? GRUB_FILE_TYPE_SKIP_SIGNATURE : > + GRUB_FILE_TYPE_NONE)); > if (! file) > return grub_errno; > > @@ -204,10 +291,9 @@ grub_cmd_list_env (grub_extcmd_context_t ctxt, > grub_file_t file; > grub_envblk_t envblk; > > - file = open_envblk_file ((state[0].set) ? state[0].arg : 0, > - GRUB_FILE_TYPE_LOADENV > - | (state[1].set > - ? GRUB_FILE_TYPE_SKIP_SIGNATURE : GRUB_FILE_TYPE_NONE)); > + file = open_envblk (ctxt, GRUB_FILE_TYPE_LOADENV > + | (state[1].set > + ? GRUB_FILE_TYPE_SKIP_SIGNATURE : GRUB_FILE_TYPE_NONE)); > if (! file) > return grub_errno; > > @@ -379,7 +465,6 @@ save_env_read_hook (grub_disk_addr_t sector, > unsigned offset, unsigned length, > static grub_err_t > grub_cmd_save_env (grub_extcmd_context_t ctxt, int argc, char **args) > { > - struct grub_arg_list *state = ctxt->state; > grub_file_t file; > grub_envblk_t envblk; > struct grub_cmd_save_env_ctx ctx = { > @@ -390,9 +475,8 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int > argc, char **args) > if (! argc) > return grub_error (GRUB_ERR_BAD_ARGUMENT, "no variable is specified"); > > - file = open_envblk_file ((state[0].set) ? state[0].arg : 0, > - GRUB_FILE_TYPE_SAVEENV > - | GRUB_FILE_TYPE_SKIP_SIGNATURE); > + file = open_envblk (ctxt, GRUB_FILE_TYPE_SAVEENV > + | GRUB_FILE_TYPE_SKIP_SIGNATURE); > if (! file) > return grub_errno; > > @@ -409,7 +493,7 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, int > argc, char **args) > if (! envblk) > goto fail; > > - if (check_blocklists (envblk, ctx.head, file)) > + if (! grub_file_envblk (file) && check_blocklists (envblk, ctx.head, file)) > goto fail; > > while (argc) > @@ -432,7 +516,15 @@ grub_cmd_save_env (grub_extcmd_context_t ctxt, > int argc, char **args) > args++; > } > > - write_blocklists (envblk, ctx.head, file); > + if (grub_file_envblk (file)) > + { > + grub_file_seek (file, 0); > + file->fs->fs_envblk_write (file, grub_envblk_buffer (envblk), > grub_envblk_size (envblk)); > + } > + else > + { > + write_blocklists (envblk, ctx.head, file); > + } > > fail: > if (envblk) > diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h > index c3e655921..7bef0c601 100644 > --- a/include/grub/lib/envblk.h > +++ b/include/grub/lib/envblk.h > @@ -21,6 +21,8 @@ > > #define GRUB_ENVBLK_SIGNATURE "# GRUB Environment Block\n" > #define GRUB_ENVBLK_DEFCFG "grubenv" > +#define GRUB_ENVBLK_SRCBLK "block" s/GRUB_ENVBLK_SRCBLK/GRUB_ENVBLK_SRC_BLK/g > +#define GRUB_ENVBLK_SRCFILE "file" s/GRUB_ENVBLK_SRCFILE/GRUB_ENVBLK_SRC_FILE/g > #ifndef ASM_FILE > > diff --git a/include/grub/util/install.h b/include/grub/util/install.h > index 2631b1074..2ca349503 100644 > --- a/include/grub/util/install.h > +++ b/include/grub/util/install.h > @@ -246,6 +246,9 @@ grub_install_get_blocklist (grub_device_t root_dev, > void *data), > void *hook_data); > > +void > +grub_util_create_envblk_buffer (char *, size_t); > + > void > grub_util_create_envblk_file (const char *name); > > diff --git a/util/editenv.c b/util/editenv.c > index 81f68bd10..45aeba259 100644 > --- a/util/editenv.c > +++ b/util/editenv.c > @@ -32,13 +32,29 @@ > #define DEFAULT_ENVBLK_SIZE 1024 > #define GRUB_ENVBLK_MESSAGE "# WARNING: Do not edit this file by > tools other than "PACKAGE"-editenv!!!\n" > > +void > +grub_util_create_envblk_buffer (char *buf, size_t size) > +{ > + if (size < DEFAULT_ENVBLK_SIZE) > + grub_util_error (_("Envblock buffer too small")); > + char *pbuf; > + pbuf = buf; > + memcpy (pbuf, GRUB_ENVBLK_SIGNATURE, sizeof (GRUB_ENVBLK_SIGNATURE) - 1); > + pbuf += sizeof (GRUB_ENVBLK_SIGNATURE) - 1; > + memcpy (pbuf, GRUB_ENVBLK_MESSAGE, sizeof (GRUB_ENVBLK_MESSAGE) - 1); > + pbuf += sizeof (GRUB_ENVBLK_MESSAGE) - 1; > + memset (pbuf , '#', > + size - sizeof (GRUB_ENVBLK_SIGNATURE) - sizeof > (GRUB_ENVBLK_MESSAGE) + 2); > +} > + > void > grub_util_create_envblk_file (const char *name) > { > FILE *fp; > - char *buf, *pbuf, *namenew; > + char *buf, *namenew; > > buf = xmalloc (DEFAULT_ENVBLK_SIZE); > + grub_util_create_envblk_buffer(buf, DEFAULT_ENVBLK_SIZE); > > namenew = xasprintf ("%s.new", name); > fp = grub_util_fopen (namenew, "wb"); > @@ -46,14 +62,6 @@ grub_util_create_envblk_file (const char *name) > grub_util_error (_("cannot open `%s': %s"), namenew, > strerror (errno)); > > - pbuf = buf; > - memcpy (pbuf, GRUB_ENVBLK_SIGNATURE, sizeof (GRUB_ENVBLK_SIGNATURE) - 1); > - pbuf += sizeof (GRUB_ENVBLK_SIGNATURE) - 1; > - memcpy (pbuf, GRUB_ENVBLK_MESSAGE, sizeof (GRUB_ENVBLK_MESSAGE) - 1); > - pbuf += sizeof (GRUB_ENVBLK_MESSAGE) - 1; > - memset (pbuf , '#', > - DEFAULT_ENVBLK_SIZE - sizeof (GRUB_ENVBLK_SIGNATURE) - > sizeof (GRUB_ENVBLK_MESSAGE) + 2); > - > if (fwrite (buf, 1, DEFAULT_ENVBLK_SIZE, fp) != DEFAULT_ENVBLK_SIZE) > grub_util_error (_("cannot write to `%s': %s"), namenew, > strerror (errno)); Please provide util changes in separate patches. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel