Re: [PATCH 0/7] ZFS/other CoW FS save_env support
Not a problem, I suspected as much. We don't have any strong need to get this work integrated ASAP, since we are going to have to build a custom version of GRUB for internal use until this makes its way into a release that lands in an Ubuntu LTS. We just want to make sure that we get some good reviews on the changes from people with experience in the codebase, and that one day we will be able to stop maintaining our fork. On Wed, Mar 25, 2020 at 10:06 AM Daniel Kiper wrote: > > Hi Paul, > > On Wed, Mar 11, 2020 at 10:37:08AM -0700, Paul Dagnelie wrote: > > Hey all, I previously discussed my concept for this patch in my email > > https://lists.gnu.org/archive/html/grub-devel/2020-01/msg4.html . > > I'm pleased to announce that I've gotten it into a working state and > > it is ready to review. There are a number of changes here, which I > > will break down below. > > > > First, the concept of "special files" is introduced into grub. These > > are files that are manipulated using different functions from the > > remainder of the filesystem. A filesystem advertises its support for > > these files by filling in new entries in the grub_fs struct. > > > > Second, the loadenv command was modified to use the special file > > functions of the root filesystem if they are detected. If that > > happens, these functions are used to open, read, and write to the > > loadenv file instead of the normal grub file functions. A variable > > was also added that allows the user to force a file to be used instead > > of the special files, or vice versa. > > > > Third, the grub-editenv command was modified to teach it to probe the > > root filesystem and then check in an array of structures that contain > > functions that will read and modify the filesystem's special file in > > userland. These functions are very similar to normal read and write > > functions, and so don't result in a very different code flow. > > > > Finally, the GRUB ZFS logic was modified to have new special_file > > functions. These functions manipulate the pad2 area of the ZFS label, > > which was previously unused. It now stores a version number, the raw > > contents of the grubenv file, and an embedded checksum for integrity > > purposes. GRUB was taught to read and verify these areas, and also to > > modify them, computing the embeddded checksum appropriately. In > > addition, if GRUB is build with libzfs support, an entry is added to > > the grub-editenv table that points GRUB to the appropriate libzfs > > functions, and init and fini functions are built into grub-editenv > > itself. > > > > Future work: > > * In the ZFS code could store a packed nvlist instead of a raw file, > > but this would involve further changes to the grub environment file > > code and was deferred for when it would be more useful and important. > > * For the special files code, there is only one special file allowed > > per filesystem, but a specification interface (using either an enum or > > a set of names) could be added in the future. > > * Other filesystem support was not built into this change, but > > extensibility was a primary goal, so adding support for btrfs or other > > filesystems should be relatively straightforward. > > * I did not implement the proposed UEFI variable support, but I did > > develop this with that future in mind, so adding it should be a > > relatively simple extension of these changes. > > > > This patchset has been tested on systems with and without ZFS as the boot > > filesystem, and built with and without ZFS libraries installed. It > > worked in each case I tried, including with manual corruption applied > > to the ZFS labels to force GRUB to read from the other label. This > > was tested in conjunction with > > https://github.com/zfsonlinux/zfs/pull/10009 , the ZoL patch that > > enables ZFS to read from and write to the padding areas we reserved > > for the data. > > First of all I would like to thank you for doing this work. This is very > interesting feature. I went quickly through the code and it looks quite > nice. However, I realized that it touches a lot of critical places in the > GRUB code. ...and we are in freeze right now. So, at this point I think > it is dangerous to merge that code. Simply we can make a lot of fallout > which we may not be able to catch and clear before the release. Hence, > I would like to suggest to defer this work until the release. Sorry about > that but I do not want to risk GRUB code breakage at this point. I hope > this is not a problem for you. > > Daniel -- Paul Dagnelie ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 5/7] Add ZFS envblock functions
This patch adds a ZFS implementation of the fs_envblk_* functions. These
functions will be used to load the grubenv block from a padding area in the
label of ZFS. This padding area is protected by an embedded checksum, and
multiple copies are stored on each disk. This should provide sufficient
reliability for most use cases. Since this area is not part of the block tree,
it can be easily modified at boot time with a simple recalculation of the
embedded checksum, enabling save_env to work for ZFS boot filesystems.
Note that the open() function is doing the read; this is because the ZFS
envblk logic doesn't store the file size, so the only way to retrieve it is to
determine the length of the file using strlen. If this is considered too poor
of an approach to be acceptable, format changes can still be made to the
structure of the padding area.
Signed-off-by: Paul Dagnelie
---
grub-core/fs/zfs/zfs.c | 134 ---
include/grub/zfs/vdev_impl.h | 33 +
2 files changed, 139 insertions(+), 28 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 2f72e42bf..d741426cd 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -30,6 +30,7 @@
*
*/
+#include
#include
#include
#include
@@ -1146,7 +1147,6 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
{
int label = 0;
uberblock_phys_t *ub_array, *ubbest = NULL;
- vdev_boot_header_t *bh;
grub_err_t err;
int vdevnum;
struct grub_zfs_device_desc desc;
@@ -1155,13 +1155,6 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
if (!ub_array)
return grub_errno;
- bh = grub_malloc (VDEV_BOOT_HEADER_SIZE);
- if (!bh)
-{
- grub_free (ub_array);
- return grub_errno;
-}
-
vdevnum = VDEV_LABELS;
desc.dev = dev;
@@ -1175,7 +1168,7 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
{
desc.vdev_phys_sector
= label * (sizeof (vdev_label_t) >> SPA_MINBLOCKSHIFT)
- + ((VDEV_SKIP_SIZE + VDEV_BOOT_HEADER_SIZE) >> SPA_MINBLOCKSHIFT)
+ + ((VDEV_PAD_SIZE * 2) >> SPA_MINBLOCKSHIFT)
+ (label < VDEV_LABELS / 2 ? 0 :
ALIGN_DOWN (grub_disk_get_size (dev->disk), sizeof (vdev_label_t))
- VDEV_LABELS * (sizeof (vdev_label_t) >> SPA_MINBLOCKSHIFT));
@@ -1184,6 +1177,7 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
err = grub_disk_read (dev->disk, desc.vdev_phys_sector
+ (VDEV_PHYS_SIZE >> SPA_MINBLOCKSHIFT),
0, VDEV_UBERBLOCK_RING, (char *) ub_array);
+
if (err)
{
grub_errno = GRUB_ERR_NONE;
@@ -1219,12 +1213,10 @@ scan_disk (grub_device_t dev, struct grub_zfs_data
*data,
continue;
#endif
grub_free (ub_array);
- grub_free (bh);
return GRUB_ERR_NONE;
}
grub_free (ub_array);
- grub_free (bh);
return grub_error (GRUB_ERR_BAD_FS, "couldn't find a valid label");
}
@@ -3765,6 +3757,60 @@ zfs_mtime (grub_device_t device, grub_int32_t *mt)
return GRUB_ERR_NONE;
}
+static grub_err_t
+zfs_devs_read_zbt (struct grub_zfs_data *data, grub_uint64_t offset, char
*buf, grub_size_t len)
+{
+ grub_err_t err = GRUB_ERR_NONE;
+ zio_cksum_t zc;
+ unsigned int i;
+ ZIO_SET_CHECKSUM(&zc, offset, 0, 0, 0);
+
+ for (i = 0; i < data->n_devices_attached; i++)
+{
+ err = grub_disk_read (data->devices_attached[i].dev->disk,
+ offset >> SPA_MINBLOCKSHIFT,
+ offset & ((1 << SPA_MINBLOCKSHIFT) - 1),
+ len, buf);
+ if (err)
+ continue;
+ ZIO_SET_CHECKSUM(&zc, offset, 0, 0, 0);
+ err = zio_checksum_verify (zc, ZIO_CHECKSUM_LABEL,
GRUB_ZFS_LITTLE_ENDIAN,
+buf, len);
+ if (!err)
+ return GRUB_ERR_NONE;
+}
+ return err;
+}
+
+static grub_err_t
+grub_zfs_envblk_open (struct grub_file *file)
+{
+ grub_err_t err;
+ struct grub_zfs_data *data;
+ vdev_boot_envblock_t *vbe;
+ int l;
+
+ file->offset = 0;
+ data = zfs_mount (file->device);
+ file->data = data;
+ data->file_buf = grub_malloc (sizeof (vdev_boot_envblock_t));
+ for (l = 0; l < VDEV_LABELS / 2; l++)
+{
+ grub_uint64_t offset = l * sizeof (vdev_label_t) + offsetof
(vdev_label_t, vl_be);
+
+ err = zfs_devs_read_zbt (data, offset, data->file_buf,
+ sizeof (vdev_boot_envblock_t));
+ if (err == GRUB_ERR_NONE)
+ {
+ vbe = (vdev_boot_envblock_t *)data->file_buf;
+ file->size = grub_strlen (vbe->vbe_bootenv);
+ return err;
+ }
+}
+ zfs_unmount (data);
+ return err;
+}
+
/*
* zfs_open() locates a file in the rootpool by following the
* MOS and places the dnode of the file in the memory address DNODE.
@@ -3850,6 +389
[PATCH 3/7] Factor out envblk buffer creation for reuse
This patch factors out the filling of the grubenv buffer into a separate
function for reuse.
Signed-off-by: Paul Dagnelie
---
include/grub/util/install.h | 3 +++
util/editenv.c | 26 +-
2 files changed, 20 insertions(+), 9 deletions(-)
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_SIZE1024
#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));
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 7/7] Update editenv to support editing zfs envblock, fix whitespace, and autodetect bootenv support in libzfs
This patch updates the grub-editenv command to support editing the zfs
envblock by using specific libzfs functions. In order to ensure that GRUB will
continue to build against both old and new versions of libzfs, logic was also
added to the configure script to detect if the bootenv functions are present
in the version of libzfs present at build time. There is also a small
whitespace fix in grub-editenv unrelated to the content of these changes.
Signed-off-by: Paul Dagnelie
---
configure.ac | 8 ++
include/grub/util/libzfs.h | 7 +
util/grub-editenv.c| 278 +++--
3 files changed, 284 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 7d74eba66..7107cc3b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1853,6 +1853,14 @@ if test x"$libzfs_excuse" = x ; then
LIBNVPAIR="-lnvpair"
AC_DEFINE([HAVE_LIBNVPAIR], [1],
[Define to 1 if you have the NVPAIR library.])
+
+ AC_CHECK_LIB([zfs], [zpool_get_bootenv],
+ [libzfs_have_bootenv="yes"],
+ [])
+ if test x"$libzfs_have_bootenv" = xyes ; then
+AC_DEFINE([HAVE_LIBZFS_BOOTENV], [1],
+ [Define to 1 if you have a version of the ZFS library with
bootenv support.])
+ fi
fi
AC_SUBST([LIBZFS])
diff --git a/include/grub/util/libzfs.h b/include/grub/util/libzfs.h
index a02caa335..5ebc085d8 100644
--- a/include/grub/util/libzfs.h
+++ b/include/grub/util/libzfs.h
@@ -40,6 +40,13 @@ extern int zpool_get_physpath (zpool_handle_t *, const char
*);
extern nvlist_t *zpool_get_config (zpool_handle_t *, nvlist_t **);
+extern libzfs_handle_t *zpool_get_handle (zpool_handle_t *);
+
+extern int zpool_set_bootenv (zpool_handle_t *, const char *);
+extern int zpool_get_bootenv (zpool_handle_t *, char *, size_t, off_t);
+
+extern int libzfs_errno (libzfs_handle_t *);
+
#endif /* ! HAVE_LIBZFS_H */
libzfs_handle_t *grub_get_libzfs_handle (void);
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index f3662c95b..a282f45ee 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -24,7 +24,12 @@
#include
#include
#include
+#include
#include
+#include
+#include
+#include
+#include
#include
#include
@@ -36,7 +41,6 @@
#pragma GCC diagnostic error "-Wmissing-prototypes"
#pragma GCC diagnostic error "-Wmissing-declarations"
-
#include "progname.h"
#define DEFAULT_ENVBLK_PATH DEFAULT_DIRECTORY "/" GRUB_ENVBLK_DEFCFG
@@ -120,6 +124,79 @@ block, use `rm %s'."),
NULL, help_filter, NULL
};
+struct fs_envblk_spec {
+ const char *fs_name;
+ int (*fs_read) (void *, char *, size_t, off_t);
+ int (*fs_write) (void *, const char *);
+ void *(*fs_init) (grub_device_t);
+ void (*fs_fini) (void *);
+};
+typedef struct fs_envblk_spec fs_envblk_spec_t;
+
+struct fs_envblk {
+ struct fs_envblk_spec *spec;
+ grub_fs_t fs;
+ void *data;
+};
+typedef struct fs_envblk fs_envblk_t;
+
+fs_envblk_t *fs_envblk = NULL;
+
+#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR) &&
defined(HAVE_LIBZFS_BOOTENV)
+static void *
+grub_zfs_init (grub_device_t dev)
+{
+ libzfs_handle_t *g_zfs = libzfs_init();
+ int err;
+ char *name;
+ zpool_handle_t *zhp;
+
+ if (g_zfs == NULL)
+return NULL;
+
+ err = fs_envblk->fs->fs_label(dev, &name);
+ if (err != GRUB_ERR_NONE) {
+libzfs_fini(g_zfs);
+return NULL;
+ }
+ zhp = zpool_open(g_zfs, name);
+ if (zhp == NULL)
+{
+ libzfs_fini(g_zfs);
+ return NULL;
+}
+ return zhp;
+}
+
+static void
+grub_zfs_fini (void *arg)
+{
+ zpool_handle_t *zhp = arg;
+ libzfs_handle_t *g_zfs = zpool_get_handle(zhp);
+ zpool_close(zhp);
+ libzfs_fini(g_zfs);
+}
+
+/* We need to convert ZFS's error returning pattern to the one we expect */
+static int
+grub_zfs_get_bootenv (void *arg, char *buf, size_t size, off_t offset)
+{
+ zpool_handle_t *zhp = arg;
+ int error = zpool_get_bootenv (zhp, buf, size, offset);
+ if (error != -1)
+return error;
+ error = libzfs_errno(zpool_get_handle(zhp));
+ return error;
+}
+#endif
+
+fs_envblk_spec_t fs_envblk_table[] = {
+#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR) &&
defined(HAVE_LIBZFS_BOOTENV)
+ { "zfs", grub_zfs_get_bootenv, zpool_set_bootenv, grub_zfs_init,
grub_zfs_fini},
+#endif
+ { NULL, NULL, NULL, NULL, NULL }
+};
+
static grub_envblk_t
open_envblk_file (const char *name)
{
@@ -164,6 +241,58 @@ open_envblk_file (const char *name)
return envblk;
}
+static grub_envblk_t
+open_envblk (const char *name)
+{
+ char *buf = NULL;
+ off_t off = 0;
+ size_t size = GRUB_ENVBLK_DEFAULT_SIZE;
+ grub_envblk_t envblk;
+
+ if (fs_envblk == NULL)
+return open_envblk_file(name);
+
+ /*
+ * For normal grubenv files, we can just use the size of the file to
+ * allocate our buffer, but envblks don't necessarily advertise their size
+
[PATCH 1/7] Factor out file filter function for reuse
This patch refactors out the logic that applies filters to files for reuse in
other code.
Signed-off-by: Paul Dagnelie
---
grub-core/kern/file.c | 46 +--
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 58454458c..75eb5e2fa 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -57,14 +57,37 @@ grub_file_get_device_name (const char *name)
return 0;
}
+static grub_file_t
+grub_apply_file_filters (grub_file_t file, enum grub_file_type type, const
char *name)
+{
+ grub_file_filter_id_t filter;
+ grub_file_t last_file = NULL;
+
+ for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters);
+ filter++)
+if (grub_file_filters[filter])
+ {
+ last_file = file;
+ file = grub_file_filters[filter] (file, type);
+ if (file && file != last_file)
+ {
+ file->name = grub_strdup (name);
+ grub_errno = GRUB_ERR_NONE;
+ }
+ }
+ if (!file)
+grub_file_close (last_file);
+
+ return file;
+}
+
grub_file_t
grub_file_open (const char *name, enum grub_file_type type)
{
- grub_device_t device = 0;
- grub_file_t file = 0, last_file = 0;
+ grub_device_t device = NULL;
+ grub_file_t file = NULL;
char *device_name;
const char *file_name;
- grub_file_filter_id_t filter;
device_name = grub_file_get_device_name (name);
if (grub_errno)
@@ -113,22 +136,7 @@ grub_file_open (const char *name, enum grub_file_type type)
file->name = grub_strdup (name);
grub_errno = GRUB_ERR_NONE;
- for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters);
- filter++)
-if (grub_file_filters[filter])
- {
- last_file = file;
- file = grub_file_filters[filter] (file, type);
- if (file && file != last_file)
- {
- file->name = grub_strdup (name);
- grub_errno = GRUB_ERR_NONE;
- }
- }
- if (!file)
-grub_file_close (last_file);
-
- return file;
+ return grub_apply_file_filters(file, type, name);
fail:
if (device)
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 4/7] Use envblk file and fs functions to implement reading the grubenv file from special FS regions
This patch modifies the loadenv command to take advantage of the new envblock
functions provided by a previous patch. These functions will be used instead
of the normal file-based functions if the boot filesystem is detected to
support them. There is also a config variable, grubenv_src, that can be used
to force the file or envblock logic to be used.
Signed-off-by: Paul Dagnelie
---
grub-core/commands/loadenv.c | 122 ++-
include/grub/lib/envblk.h| 2 +
2 files changed, 109 insertions(+), 15 deletions(-)
diff --git a/grub-core/commands/loadenv.c b/grub-core/commands/loadenv.c
index 3fd664aac..66f5345ec 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 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_SRC_BLK))
+{
+ char *device_name;
+ const char *prefix;
+ grub_dprintf ("loadenv", "checking for envblk\n");
+
+ 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 was closed in
grub_fs_probe. */
+ device = grub_device_open (device_name);
+ grub_free (device_name);
+ if (! device)
+ return NULL;
+
+}
+ if (! source)
+{
+ if (fs->fs_envblk_open)
+ {
+ source = GRUB_ENVBLK_SRC_BLK;
+ grub_dprintf ("loadenv", "envblk support detected\n");
+ }
+ else
+ {
+ source = GRUB_ENVBLK_SRC_FILE;
+ grub_dprintf ("loadenv", "envblk support not detected\n");
+ }
+}
+
+ if (! grub_strcmp (source, GRUB_ENVBLK_SRC_FILE))
+{
+ if (device)
+grub_device_close (device);
+ file = open_envblk_file (filename, type);
+
+ if (! file)
+ return NULL;
+}
+ else if (! grub_strcmp (source, GRUB_ENVBLK_SRC_BLK))
+{
+ 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_
[PATCH 2/7] Add support for special envblk functions in GRUB
This patch introducces the idea of envblk functions to GRUB. These functions
are used to interact with a grubenv file stored not in the usual grub tree,
but instead in a special filesystem-managed region that can more easily be
accessed at boot time. These functions interface between calling code and the
per-fs logic, an interface for which is also added as part of this patch.
Signed-off-by: Paul Dagnelie
---
grub-core/kern/file.c | 30 --
include/grub/file.h | 10 ++
include/grub/fs.h | 12
3 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 75eb5e2fa..f929f61a0 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -149,6 +149,27 @@ grub_file_open (const char *name, enum grub_file_type type)
return 0;
}
+grub_file_t
+grub_file_envblk_open (grub_fs_t fs, grub_device_t device, enum grub_file_type
type)
+{
+ grub_file_t file = NULL;
+ file = (grub_file_t) grub_zalloc (sizeof (*file));
+ if (! file)
+return NULL;
+ file->device = device;
+ file->special = 1;
+
+ if ((fs->fs_envblk_open) (file) != GRUB_ERR_NONE) {
+grub_free(file);
+return NULL;
+ }
+
+ file->fs = fs;
+ grub_errno = GRUB_ERR_NONE;
+
+ return grub_apply_file_filters(file, type, NULL);
+}
+
grub_disk_read_hook_t grub_file_progress_hook;
grub_ssize_t
@@ -185,7 +206,10 @@ grub_file_read (grub_file_t file, void *buf, grub_size_t
len)
file->read_hook_data = file;
file->progress_offset = file->offset;
}
- res = (file->fs->fs_read) (file, buf, len);
+ if (grub_file_envblk (file))
+res = (file->fs->fs_envblk_read) (file, buf, len);
+ else
+res = (file->fs->fs_read) (file, buf, len);
file->read_hook = read_hook;
file->read_hook_data = read_hook_data;
if (res > 0)
@@ -197,7 +221,9 @@ grub_file_read (grub_file_t file, void *buf, grub_size_t
len)
grub_err_t
grub_file_close (grub_file_t file)
{
- if (file->fs->fs_close)
+ if (grub_file_envblk (file) && file->fs->fs_envblk_close)
+(file->fs->fs_envblk_close) (file);
+ else if (file->fs->fs_close)
(file->fs->fs_close) (file);
if (file->device)
diff --git a/include/grub/file.h b/include/grub/file.h
index 31567483c..1cc688f55 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -170,6 +170,9 @@ struct grub_file
/* Caller-specific data passed to the read hook. */
void *read_hook_data;
+
+ /* If the file is an FS's special file, which uses separate functions for
interaction. */
+ int special;
};
typedef struct grub_file *grub_file_t;
@@ -211,6 +214,7 @@ grub_ssize_t EXPORT_FUNC(grub_file_read) (grub_file_t file,
void *buf,
grub_size_t len);
grub_off_t EXPORT_FUNC(grub_file_seek) (grub_file_t file, grub_off_t offset);
grub_err_t EXPORT_FUNC(grub_file_close) (grub_file_t file);
+grub_file_t EXPORT_FUNC(grub_file_envblk_open) (grub_fs_t fs, grub_device_t
device, enum grub_file_type type);
/* Return value of grub_file_size() in case file size is unknown. */
#define GRUB_FILE_SIZE_UNKNOWN 0xULL
@@ -233,6 +237,12 @@ grub_file_seekable (const grub_file_t file)
return !file->not_easily_seekable;
}
+static inline int
+grub_file_envblk (const grub_file_t file)
+{
+ return file->special;
+}
+
grub_file_t
grub_file_offset_open (grub_file_t parent, enum grub_file_type type,
grub_off_t start, grub_off_t size);
diff --git a/include/grub/fs.h b/include/grub/fs.h
index 302e48d4b..ec1a7a50e 100644
--- a/include/grub/fs.h
+++ b/include/grub/fs.h
@@ -96,6 +96,18 @@ struct grub_fs
/* Whether blocklist installs have a chance to work. */
int blocklist_install;
#endif
+
+ /*
+ * The envblk functions are defined on filesystems that need to handle
+ * grub-writable files in a special way. This is most commonly the case for
+ * CoW filesystems like btrfs and ZFS. The normal read and close functions
+ * should detect that they are being called on a special file and act
+ * appropriately.
+ */
+ grub_err_t (*fs_envblk_open) (struct grub_file *file);
+ grub_ssize_t (*fs_envblk_read) (struct grub_file *file, char *buf,
grub_size_t len);
+ grub_err_t (*fs_envblk_write) (struct grub_file *file, char *buf,
grub_size_t len);
+ grub_err_t (*fs_envblk_close) (struct grub_file *file);
};
typedef struct grub_fs *grub_fs_t;
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 6/7] Refactor default envblk size out for reuse
This patch refactors the DEFAULT_ENVBLK_SIZE definition into a header file for
reuse in other code, and renames it to match other definitions in that header.
Signed-off-by: Paul Dagnelie
---
include/grub/lib/envblk.h | 1 +
util/editenv.c| 9 -
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/grub/lib/envblk.h b/include/grub/lib/envblk.h
index 287c87105..b9067dc37 100644
--- a/include/grub/lib/envblk.h
+++ b/include/grub/lib/envblk.h
@@ -23,6 +23,7 @@
#define GRUB_ENVBLK_DEFCFG "grubenv"
#define GRUB_ENVBLK_SRC_BLK"block"
#define GRUB_ENVBLK_SRC_FILE "file"
+#define GRUB_ENVBLK_DEFAULT_SIZE 1024
#ifndef ASM_FILE
diff --git a/util/editenv.c b/util/editenv.c
index 45aeba259..ef1aba25b 100644
--- a/util/editenv.c
+++ b/util/editenv.c
@@ -29,13 +29,12 @@
#include
#include
-#define DEFAULT_ENVBLK_SIZE1024
#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)
+ if (size < GRUB_ENVBLK_DEFAULT_SIZE)
grub_util_error (_("Envblock buffer too small"));
char *pbuf;
pbuf = buf;
@@ -53,8 +52,8 @@ grub_util_create_envblk_file (const char *name)
FILE *fp;
char *buf, *namenew;
- buf = xmalloc (DEFAULT_ENVBLK_SIZE);
- grub_util_create_envblk_buffer(buf, DEFAULT_ENVBLK_SIZE);
+ buf = xmalloc (GRUB_ENVBLK_DEFAULT_SIZE);
+ grub_util_create_envblk_buffer(buf, GRUB_ENVBLK_DEFAULT_SIZE);
namenew = xasprintf ("%s.new", name);
fp = grub_util_fopen (namenew, "wb");
@@ -62,7 +61,7 @@ grub_util_create_envblk_file (const char *name)
grub_util_error (_("cannot open `%s': %s"), namenew,
strerror (errno));
- if (fwrite (buf, 1, DEFAULT_ENVBLK_SIZE, fp) != DEFAULT_ENVBLK_SIZE)
+ if (fwrite (buf, 1, GRUB_ENVBLK_DEFAULT_SIZE, fp) !=
GRUB_ENVBLK_DEFAULT_SIZE)
grub_util_error (_("cannot write to `%s': %s"), namenew,
strerror (errno));
--
2.19.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 0/7] ZFS/other CoW FS save_env support
Hey all, I previously discussed my concept for this patch in my email https://lists.gnu.org/archive/html/grub-devel/2020-01/msg4.html . I'm pleased to announce that I've gotten it into a working state and it is ready to review. There are a number of changes here, which I will break down below. First, the concept of "special files" is introduced into grub. These are files that are manipulated using different functions from the remainder of the filesystem. A filesystem advertises its support for these files by filling in new entries in the grub_fs struct. Second, the loadenv command was modified to use the special file functions of the root filesystem if they are detected. If that happens, these functions are used to open, read, and write to the loadenv file instead of the normal grub file functions. A variable was also added that allows the user to force a file to be used instead of the special files, or vice versa. Third, the grub-editenv command was modified to teach it to probe the root filesystem and then check in an array of structures that contain functions that will read and modify the filesystem's special file in userland. These functions are very similar to normal read and write functions, and so don't result in a very different code flow. Finally, the GRUB ZFS logic was modified to have new special_file functions. These functions manipulate the pad2 area of the ZFS label, which was previously unused. It now stores a version number, the raw contents of the grubenv file, and an embedded checksum for integrity purposes. GRUB was taught to read and verify these areas, and also to modify them, computing the embeddded checksum appropriately. In addition, if GRUB is build with libzfs support, an entry is added to the grub-editenv table that points GRUB to the appropriate libzfs functions, and init and fini functions are built into grub-editenv itself. Future work: * In the ZFS code could store a packed nvlist instead of a raw file, but this would involve further changes to the grub environment file code and was deferred for when it would be more useful and important. * For the special files code, there is only one special file allowed per filesystem, but a specification interface (using either an enum or a set of names) could be added in the future. * Other filesystem support was not built into this change, but extensibility was a primary goal, so adding support for btrfs or other filesystems should be relatively straightforward. * I did not implement the proposed UEFI variable support, but I did develop this with that future in mind, so adding it should be a relatively simple extension of these changes. This patchset has been tested on systems with and without ZFS as the boot filesystem, and built with and without ZFS libraries installed. It worked in each case I tried, including with manual corruption applied to the ZFS labels to force GRUB to read from the other label. This was tested in conjunction with https://github.com/zfsonlinux/zfs/pull/10009 , the ZoL patch that enables ZFS to read from and write to the padding areas we reserved for the data. Paul Dagnelie (7): Factor out file filter function for reuse Add support for special envblk functions in GRUB Factor out envblk buffer creation for reuse Use envblk file and fs functions to implement reading the grubenv file from special FS regions Add ZFS envblock functions Refactor default envblk size out for reuse Update editenv to support editing zfs envblock, fix whitespace, and autodetect bootenv support in libzfs configure.ac | 8 + grub-core/commands/loadenv.c | 122 +-- grub-core/fs/zfs/zfs.c | 134 +++-- grub-core/kern/file.c| 76 +++--- include/grub/file.h | 10 ++ include/grub/fs.h| 12 ++ include/grub/lib/envblk.h| 3 + include/grub/util/install.h | 3 + include/grub/util/libzfs.h | 7 + include/grub/zfs/vdev_impl.h | 33 ++--- util/editenv.c | 31 ++-- util/grub-editenv.c | 278 +-- 12 files changed, 632 insertions(+), 85 deletions(-) -- 2.19.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 4/4] Update grub editenv to support modifying envblk
This patch adds the capability for the editenv utility to modify the
envblk, and adds ZFS-specific handlers that will be built if GRUB is
built with libzfs support. It also adds logic that editenv uses to
detect GRUB's (root) filesystem.
One question I have related to this patch is if there is some existing
requirement on the minimum version of ZFS being used to build GRUB; if
not, this code probably needs some additional checks to verify that
the attached version of libzfs has the zpool_set_bootenv and
zpool_get_bootenv functions.
commit 98f854215f7e0488cbe2a639b8dde76015c26e55
Author: Paul Dagnelie
AuthorDate: Mon Feb 24 14:29:47 2020 -0800
Commit: Paul Dagnelie
CommitDate: Tue Feb 25 10:08:08 2020 -0800
Update editenv to support editing zfs envblock
---
include/grub/util/libzfs.h | 7 ++
util/grub-editenv.c| 279 +++--
2 files changed, 277 insertions(+), 9 deletions(-)
diff --git a/include/grub/util/libzfs.h b/include/grub/util/libzfs.h
index a02caa335..9990e8120 100644
--- a/include/grub/util/libzfs.h
+++ b/include/grub/util/libzfs.h
@@ -40,6 +40,13 @@ extern int zpool_get_physpath (zpool_handle_t *,
const char *);
extern nvlist_t *zpool_get_config (zpool_handle_t *, nvlist_t **);
+extern libzfs_handle_t *zpool_get_handle(zpool_handle_t *);
+
+extern int zpool_set_bootenv(zpool_handle_t *, const char *);
+extern int zpool_get_bootenv(zpool_handle_t *, char *, size_t, off_t);
+
+extern int libzfs_errno(libzfs_handle_t *);
+
#endif /* ! HAVE_LIBZFS_H */
libzfs_handle_t *grub_get_libzfs_handle (void);
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index f3662c95b..b636ed1b8 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -24,7 +24,12 @@
#include
#include
#include
+#include
#include
+#include
+#include
+#include
+#include
#include
#include
@@ -36,7 +41,6 @@
#pragma GCC diagnostic error "-Wmissing-prototypes"
#pragma GCC diagnostic error "-Wmissing-declarations"
-
#include "progname.h"
#define DEFAULT_ENVBLK_PATH DEFAULT_DIRECTORY "/" GRUB_ENVBLK_DEFCFG
@@ -120,6 +124,80 @@ block, use `rm %s'."),
NULL, help_filter, NULL
};
+struct fs_envblk_spec {
+ const char *fs_name;
+ int (*fs_read) (void *, char *, size_t, off_t);
+ int (*fs_write) (void *, const char *);
+ void *(*fs_init) (grub_device_t);
+ void (*fs_fini) (void *);
+};
+
+struct fs_envblk {
+ struct fs_envblk_spec *spec;
+ grub_fs_t fs;
+ void *data;
+};
+
+typedef struct fs_envblk_spec fs_envblk_spec_t;
+typedef struct fs_envblk fs_envblk_t;
+
+fs_envblk_t *fs_envblk = NULL;
+
+#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
+static void *
+grub_zfs_init (grub_device_t dev)
+{
+ libzfs_handle_t *g_zfs = libzfs_init();
+ int err;
+ char *name;
+ zpool_handle_t *zhp;
+
+ if (g_zfs == NULL)
+return NULL;
+
+ err = fs_envblk->fs->fs_label(dev, &name);
+ if (err != GRUB_ERR_NONE) {
+libzfs_fini(g_zfs);
+return NULL;
+ }
+ zhp = zpool_open(g_zfs, name);
+ if (zhp == NULL)
+{
+ libzfs_fini(g_zfs);
+ return NULL;
+}
+ return zhp;
+}
+
+static void
+grub_zfs_fini (void *arg)
+{
+ zpool_handle_t *zhp = arg;
+ libzfs_handle_t *g_zfs = zpool_get_handle(zhp);
+ zpool_close(zhp);
+ libzfs_fini(g_zfs);
+}
+
+/* We need to convert ZFS's error returning pattern to the one we expect */
+static int
+grub_zfs_get_bootenv (void *arg, char *buf, size_t size, off_t offset)
+{
+ zpool_handle_t *zhp = arg;
+ int error = zpool_get_bootenv (zhp, buf, size, offset);
+ if (error != -1)
+return error;
+ error = libzfs_errno(zpool_get_handle(zhp));
+ return error;
+}
+#endif
+
+fs_envblk_spec_t fs_envblk_table[] = {
+#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
+ { "zfs", grub_zfs_get_bootenv, zpool_set_bootenv, grub_zfs_init,
grub_zfs_fini},
+#endif
+ { NULL, NULL, NULL, NULL, NULL }
+};
+
static grub_envblk_t
open_envblk_file (const char *name)
{
@@ -164,6 +242,51 @@ open_envblk_file (const char *name)
return envblk;
}
+static grub_envblk_t
+open_envblk (const char *name)
+{
+ char *buf = NULL;
+ off_t off = 0;
+ size_t size = 1024;
+ grub_envblk_t envblk;
+
+ if (fs_envblk == NULL)
+return open_envblk_file(name);
+
+ while (1)
+{
+ int res;
+ buf = xrealloc(buf, size);
+ res = fs_envblk->spec->fs_read(fs_envblk->data, buf + off, size, off);
+ if (res < 0)
+{
+ grub_util_error (_("cannot read envblock: %s"), strerror (errno));
+ free(buf);
+ return NULL;
+}
+ if (res < size)
+{
+ envblk = grub_envblk_open (buf, res + off);
+ if (! envblk)
+grub_util_error ("%s", _("invalid environment block"));
+ return envblk;
+}
+ off += size;
+ size *= 2;
+}
+}
+
+static void
+close_envblk (grub_envblk_t envblk)
+{
+ grub_envblk_close (en
[PATCH 2/4] Add envblk reading/writing functionality to GRUB
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.
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
AuthorDate: Mon Feb 24 13:40:55 2020 -0800
Commit: Paul Dagnelie
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);
+}
+
+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;
+
+ 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. */
+ 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 *stat
[PATCH 3/4] Add ZFS envblock functions
This patch adds a ZFS implementation of the new envblock functions,
storing the data for the envblock in the second padding area of the
label. This data is protected by an embedded checksum and is stored
redundantly, so even though it is not part of the block tree it
provides reasonable reliability.
commit 0f108fee27917afd72d834620db8f8b1e7ca1699
Author: Paul Dagnelie
AuthorDate: Mon Feb 24 14:29:35 2020 -0800
Commit: Paul Dagnelie
CommitDate: Tue Feb 25 10:08:08 2020 -0800
Add ZFS envblock functions
---
grub-core/fs/zfs/zfs.c | 129 +++
include/grub/zfs/vdev_impl.h | 33 ++-
2 files changed, 134 insertions(+), 28 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index 2f72e42bf..1ee9dd56b 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -30,6 +30,7 @@
*
*/
+#include
#include
#include
#include
@@ -1146,7 +1147,6 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
{
int label = 0;
uberblock_phys_t *ub_array, *ubbest = NULL;
- vdev_boot_header_t *bh;
grub_err_t err;
int vdevnum;
struct grub_zfs_device_desc desc;
@@ -1155,13 +1155,6 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
if (!ub_array)
return grub_errno;
- bh = grub_malloc (VDEV_BOOT_HEADER_SIZE);
- if (!bh)
-{
- grub_free (ub_array);
- return grub_errno;
-}
-
vdevnum = VDEV_LABELS;
desc.dev = dev;
@@ -1175,7 +1168,7 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
{
desc.vdev_phys_sector
= label * (sizeof (vdev_label_t) >> SPA_MINBLOCKSHIFT)
-+ ((VDEV_SKIP_SIZE + VDEV_BOOT_HEADER_SIZE) >> SPA_MINBLOCKSHIFT)
++ ((VDEV_PAD_SIZE * 2) >> SPA_MINBLOCKSHIFT)
+ (label < VDEV_LABELS / 2 ? 0 :
ALIGN_DOWN (grub_disk_get_size (dev->disk), sizeof (vdev_label_t))
- VDEV_LABELS * (sizeof (vdev_label_t) >> SPA_MINBLOCKSHIFT));
@@ -1184,6 +1177,7 @@ scan_disk (grub_device_t dev, struct grub_zfs_data *data,
err = grub_disk_read (dev->disk, desc.vdev_phys_sector
+ (VDEV_PHYS_SIZE >> SPA_MINBLOCKSHIFT),
0, VDEV_UBERBLOCK_RING, (char *) ub_array);
+ if (label == 0)
if (err)
{
grub_errno = GRUB_ERR_NONE;
@@ -1219,12 +1213,10 @@ scan_disk (grub_device_t dev, struct
grub_zfs_data *data,
continue;
#endif
grub_free (ub_array);
- grub_free (bh);
return GRUB_ERR_NONE;
}
grub_free (ub_array);
- grub_free (bh);
return grub_error (GRUB_ERR_BAD_FS, "couldn't find a valid label");
}
@@ -3765,6 +3757,58 @@ zfs_mtime (grub_device_t device, grub_int32_t *mt)
return GRUB_ERR_NONE;
}
+static grub_err_t
+zfs_devs_read_zbt (struct grub_zfs_data *data, grub_uint64_t offset,
char *buf, grub_size_t len)
+{
+ grub_err_t err = GRUB_ERR_NONE;
+ zio_cksum_t zc;
+ unsigned int i;
+ ZIO_SET_CHECKSUM(&zc, offset, 0, 0, 0);
+
+ for (i = 0; i < data->n_devices_attached; i++)
+{
+ err = grub_disk_read (data->devices_attached[i].dev->disk,
+offset >> SPA_MINBLOCKSHIFT,
+offset & ((1 << SPA_MINBLOCKSHIFT) - 1),
+len, buf);
+ if (err)
+continue;
+ ZIO_SET_CHECKSUM(&zc, offset, 0, 0, 0);
+ err = zio_checksum_verify (zc, ZIO_CHECKSUM_LABEL,
GRUB_ZFS_LITTLE_ENDIAN,
+ buf, len);
+ if (!err)
+return GRUB_ERR_NONE;
+}
+ return err;
+}
+
+static grub_err_t
+grub_zfs_envblk_open (struct grub_file *file)
+{
+ grub_err_t err;
+ struct grub_zfs_data *data;
+ vdev_boot_envblock_t *vbe;
+ int l;
+ file->offset = 0;
+ data = zfs_mount (file->device);
+ file->data = data;
+ data->file_buf = grub_malloc (sizeof (vdev_boot_envblock_t));
+ for (l = 0; l < VDEV_LABELS / 2; l++)
+{
+ grub_uint64_t offset = l * sizeof (vdev_label_t) + offsetof
(vdev_label_t, vl_be);
+ err = zfs_devs_read_zbt (data, offset, data->file_buf,
+ sizeof (vdev_boot_envblock_t));
+ if (err == GRUB_ERR_NONE)
+{
+ vbe = (vdev_boot_envblock_t *)data->file_buf;
+ file->size = grub_strlen (vbe->vbe_bootenv);
+ return err;
+}
+}
+ zfs_unmount (data);
+ return err;
+}
+
/*
* zfs_open() locates a file in the rootpool by following the
* MOS and places the dnode of the file in the memory address DNODE.
@@ -3850,6 +3894,19 @@ grub_zfs_open (struct grub_file *file, const
char *fsfilename)
return GRUB_ERR_NONE;
}
+static grub_ssize_t
+grub_zfs_envblk_read (grub_file_t file, char *buf, grub_size_t len)
+{
+ struct grub_zfs_data *data = (struct grub_zfs_data *) file->data;
+ grub_ssize_t olen = len;
+ grub_uint64_t offset = file->offset + offsetof
(vdev_boot_envblock_t, vbe_bootenv);
+
+ if (len + file->offset > file->size)
+olen = file->size - file->offset;
+
[PATCH 1/4] Add envblk open functions to grub file interface
These functions are added to support the remainder of this patch set.
In order to add these special functions we have to refactor out the
logic that applies file filters so that it can be applied to both the
normal and envblk open functions.
commit 45ee383e11ecd15ac2820131d410a434eeea47a1
Author: Paul Dagnelie
AuthorDate: Mon Feb 24 11:19:40 2020 -0800
Commit: Paul Dagnelie
CommitDate: Tue Feb 25 10:08:07 2020 -0800
Add support for special envblk functions in GRUB
---
grub-core/kern/file.c | 76 +--
include/grub/file.h | 10 +++
include/grub/fs.h | 12
3 files changed, 77 insertions(+), 21 deletions(-)
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 58454458c..f929f61a0 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -57,14 +57,37 @@ grub_file_get_device_name (const char *name)
return 0;
}
+static grub_file_t
+grub_apply_file_filters (grub_file_t file, enum grub_file_type type,
const char *name)
+{
+ grub_file_filter_id_t filter;
+ grub_file_t last_file = NULL;
+
+ for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters);
+ filter++)
+if (grub_file_filters[filter])
+ {
+last_file = file;
+file = grub_file_filters[filter] (file, type);
+if (file && file != last_file)
+ {
+file->name = grub_strdup (name);
+grub_errno = GRUB_ERR_NONE;
+ }
+ }
+ if (!file)
+grub_file_close (last_file);
+
+ return file;
+}
+
grub_file_t
grub_file_open (const char *name, enum grub_file_type type)
{
- grub_device_t device = 0;
- grub_file_t file = 0, last_file = 0;
+ grub_device_t device = NULL;
+ grub_file_t file = NULL;
char *device_name;
const char *file_name;
- grub_file_filter_id_t filter;
device_name = grub_file_get_device_name (name);
if (grub_errno)
@@ -113,22 +136,7 @@ grub_file_open (const char *name, enum grub_file_type type)
file->name = grub_strdup (name);
grub_errno = GRUB_ERR_NONE;
- for (filter = 0; file && filter < ARRAY_SIZE (grub_file_filters);
- filter++)
-if (grub_file_filters[filter])
- {
-last_file = file;
-file = grub_file_filters[filter] (file, type);
-if (file && file != last_file)
- {
-file->name = grub_strdup (name);
-grub_errno = GRUB_ERR_NONE;
- }
- }
- if (!file)
-grub_file_close (last_file);
-
- return file;
+ return grub_apply_file_filters(file, type, name);
fail:
if (device)
@@ -141,6 +149,27 @@ grub_file_open (const char *name, enum grub_file_type type)
return 0;
}
+grub_file_t
+grub_file_envblk_open (grub_fs_t fs, grub_device_t device, enum
grub_file_type type)
+{
+ grub_file_t file = NULL;
+ file = (grub_file_t) grub_zalloc (sizeof (*file));
+ if (! file)
+return NULL;
+ file->device = device;
+ file->envblk = 1;
+
+ if ((fs->fs_envblk_open) (file) != GRUB_ERR_NONE) {
+grub_free(file);
+return NULL;
+ }
+
+ file->fs = fs;
+ grub_errno = GRUB_ERR_NONE;
+
+ return grub_apply_file_filters(file, type, NULL);
+}
+
grub_disk_read_hook_t grub_file_progress_hook;
grub_ssize_t
@@ -177,7 +206,10 @@ grub_file_read (grub_file_t file, void *buf,
grub_size_t len)
file->read_hook_data = file;
file->progress_offset = file->offset;
}
- res = (file->fs->fs_read) (file, buf, len);
+ if (grub_file_envblk (file))
+res = (file->fs->fs_envblk_read) (file, buf, len);
+ else
+res = (file->fs->fs_read) (file, buf, len);
file->read_hook = read_hook;
file->read_hook_data = read_hook_data;
if (res > 0)
@@ -189,7 +221,9 @@ grub_file_read (grub_file_t file, void *buf,
grub_size_t len)
grub_err_t
grub_file_close (grub_file_t file)
{
- if (file->fs->fs_close)
+ if (grub_file_envblk (file) && file->fs->fs_envblk_close)
+(file->fs->fs_envblk_close) (file);
+ else if (file->fs->fs_close)
(file->fs->fs_close) (file);
if (file->device)
diff --git a/include/grub/file.h b/include/grub/file.h
index 31567483c..1cc688f55 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -170,6 +170,9 @@ struct grub_file
/* Caller-specific data passed to the read hook. */
void *read_hook_data;
+
+ /* If the file is an FS's envblk, which uses separate functions for
interaction. */
+ int envblk;
};
typedef struct grub_file *grub_file_t;
@@ -211,6 +214,7 @@ grub_ssize_t EXPORT_FUNC(grub_file_read)
(grub_file_t file, void *buf,
grub_size_t len);
grub_off_t EXPORT_FUNC(grub_file_seek) (grub_file_t file, grub_off_t offset);
grub_err_t EXPORT_FUNC(grub_file_close) (grub_file_t file);
+grub_file_t EXPORT_FUNC(grub_file_envblk_open) (grub_fs_t fs,
grub_device_t device, enum grub_file_type type);
/* Return value of grub_file_size() in case file size is unknown. */
#define GRUB_FILE
Re: [PATCH] ZFS/other CoW FS save_env support
On Mon, Feb 24, 2020 at 3:03 AM Daniel Kiper wrote: > > > Why "root" not "boot"? That was a typo on my part; the code uses grub_guess_root_device to find the devices backing the default grub directory, but in most configurations, this should attempt to locate the boot filesystem instead of the root. I was uncertain of a better way to consistently determine the boot filesystem, and this portion of the code was copied from another GRUB patch (https://build.opensuse.org/package/view_file/openSUSE:Factory/grub2/grub2-grubenv-in-btrfs-header.patch?expand=1). > > Yes, please split the patch into smaller patches. Please do one logical > change per patch. > > I quickly went through the patch and pointed things which I spotted at > first sight. I will provide more comments when you split the patch into > separate patches. > > Next time please CC following people too: javi...@redhat.com, > maciej.pijanow...@3mdeb.com and piotr.k...@3mdeb.com. Understood! I will post an updated version hopefully today or tomorrow. > > I think that you can drop parenthesis here. And please use NULL instead of 0. Will do. In general, this was one of my questions about writing new code in this code base. There are several things where I decided to go with consistency with surrounding code instead of what would commonly be preferred in modern coding standards or by the style guide (see also, the block comment style you mentioned further down). Is there a preference in this codebase against consistency when other considerations are also relevant? -- Paul Dagnelie ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] ZFS/other CoW FS save_env support
grub_mdraid09_fini ();
+ grub_mdraid1x_fini ();
+ grub_diskfilter_fini ();
+ grub_diskfilter_init ();
+ grub_mdraid09_init ();
+ grub_mdraid1x_init ();
+ grub_lvm_init ();
+
+ grub_devices = grub_guess_root_devices (DEFAULT_DIRECTORY);
+
+ if (!grub_devices || !grub_devices[0])
+grub_util_error (_("cannot find a device for %s (is /dev
mounted?)"), DEFAULT_DIRECTORY);
+
+ fs_envblk_device = grub_devices[0];
+
+ for (curdev = grub_devices; *curdev; curdev++)
+{
+ grub_util_pull_device (*curdev);
+ ndev++;
+}
+
+ grub_drives = xmalloc (sizeof (grub_drives[0]) * (ndev + 1));
+
+ for (curdev = grub_devices, curdrive = grub_drives; *curdev; curdev++,
+ curdrive++)
+{
+ *curdrive = grub_util_get_grub_dev (*curdev);
+ if (! *curdrive)
+grub_util_error (_("cannot find a GRUB drive for %s. Check your
device.map"),
+ *curdev);
+}
+ *curdrive = 0;
+
+ grub_dev = grub_device_open (grub_drives[0]);
+
+ grub_fs = grub_fs_probe (grub_dev);
+
+ if (grub_dev->disk)
+{
+ probe_abstraction (grub_dev->disk);
+}
+ for (curdrive = grub_drives + 1; *curdrive; curdrive++)
+{
+ grub_device_t dev = grub_device_open (*curdrive);
+ if (!dev)
+continue;
+ if (dev->disk)
+probe_abstraction (dev->disk);
+ grub_device_close (dev);
+}
+
+ free (grub_drives);
+ grub_gcry_fini_all ();
+ grub_fini_all ();
+ grub_util_biosdisk_fini ();
+
+ fs_envblk_spec_t *p;
+
+ for (p = spec; p->fs_name; p++)
+{
+ if (strcmp (grub_fs->name, p->fs_name) == 0 && !have_abstraction)
+{
+ grub_util_info ("Detected envblock support in %s, leveraging",
grub_fs->name);
+ fs_envblk = xmalloc (sizeof (fs_envblk_t));
+ fs_envblk->spec = p;
+ fs_envblk->fs = grub_fs;
+ fs_envblk->data = p->fs_init (grub_dev);
+ grub_device_close (grub_dev);
+ return fs_envblk;
+}
+}
+
+ grub_device_close (grub_dev);
+ return NULL;
}
int
@@ -278,8 +524,11 @@ main (int argc, char *argv[])
command = argv[curindex++];
}
+ if (strcmp (filename, DEFAULT_ENVBLK_PATH) == 0)
+fs_envblk = probe_fs_envblk (fs_envblk_table);
+
if (strcmp (command, "create") == 0)
-grub_util_create_envblk_file (filename);
+create_envblk (filename);
else if (strcmp (command, "list") == 0)
list_variables (filename);
else if (strcmp (command, "set") == 0)
--
Paul Dagnelie
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
ZFS grubenv write support
Hey all, I'm interested in trying to enable support for boot-time modification of the grubenv file on ZFS and other non-standard filesystems that don't necessarily work today. I have a proposed design, and I was wondering if you all would be interested in it. I'd be implementing it and testing it myself, my main goal was to try to make sure that after doing so, the community would be interested in integrating it. This has the dual advantage of allowing everyone using ZFS and other filesystems to benefit from it, and also making it so I don't have to maintain a fork indefinitely. The proposal is that rather than always opening the grubenv file and reading it using the normal file API, we can add a function (or functions) to the fs struct that allows a filesystem to advertise that it needs to use special logic to handle writable files. Then the grubenv file can be stored in a special location (probably one of that padding areas in the label, which is where the FreeBSD loader stores some boot data in zfs) where it can be written safely by the bootloader without harming the checksum tree in ZFS. The GRUB code would need to change to invoke the special "Please let the fs handle this writable file" logic (which would have open, read, and write functions) for filesystems that have them available, but this should be extensible to other CoW or sparse filesystems like BTRFS. If a filesystem doesn't advertise that logic, GRUB would fall back to using the currently file-based logic. An alternative proposal that I found slightly less elegant but would involve fewer invasive changes is to just modify the zfs grub logic to check if it's opening the grubenv file, and if so divert to special read and open logic that would access this padding area. This would involve hardcoding the grubenv file name in the ZFS code, but I figured I would mention it as a possibility in case it was preferred for other reasons. Do people have thoughts or opinions about this concept and this design? I would greatly appreciate any feedback or thoughts on alternative approaches. Thank you for your time and attention! -- Paul Dagnelie ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
