On Tue, Feb 25, 2020 at 11:26:45AM -0800, Paul Dagnelie wrote:
> 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 <p...@delphix.com>
> AuthorDate: Mon Feb 24 14:29:35 2020 -0800
> Commit: Paul Dagnelie <p...@delphix.com>
> 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 <stddef.h>
> #include <grub/err.h>
> #include <grub/file.h>
> #include <grub/mm.h>
> @@ -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)
This change looks strange...
> 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;
Please add empty line here...
> + 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);
Ditto...
> + err = zfs_devs_read_zbt (data, offset, data->file_buf,
> + sizeof (vdev_boot_envblock_t));
Why does open read?
> + 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;
> + grub_memmove (buf, data->file_buf + offset, olen);
I think that read should happen here...
> + return olen;
> +}
> +
> static grub_ssize_t
> grub_zfs_read (grub_file_t file, char *buf, grub_size_t len)
> {
> @@ -3859,6 +3916,9 @@ grub_zfs_read (grub_file_t file, char *buf,
> grub_size_t len)
> grub_size_t read;
> grub_err_t err;
>
> + if (grub_file_envblk (file))
> + return grub_zfs_envblk_read (file, buf, len);
Is it regular ZFS read? If it is I think that it should return contents
of regular envblk file on the file system even if special region is used
for that.
> +
> /*
> * If offset is in memory, move it into the buffer provided and return.
> */
> @@ -3924,6 +3984,49 @@ grub_zfs_read (grub_file_t file, char *buf,
> grub_size_t len)
> return len;
> }
>
> +static grub_err_t
> +grub_zfs_envblk_write (struct grub_file *file, char *buf, grub_size_t len)
> +{
> + grub_uint64_t offset = file->offset + offsetof
> (vdev_boot_envblock_t, vbe_bootenv);
> + grub_err_t err = GRUB_ERR_NONE;
> + struct grub_zfs_data *data = file->data;
> + unsigned int i, l;
> + zio_cksum_t cksum;
> + vdev_boot_envblock_t *vbe = (vdev_boot_envblock_t *)data->file_buf;
> + if (len + file->offset > file->size)
> + return GRUB_ERR_OUT_OF_RANGE;
> +
> + grub_memmove (data->file_buf + offset, buf, len);
> +
> + for (l = 0; l < VDEV_LABELS / 2; l++)
> + {
> + offset = l * sizeof (vdev_label_t) + offsetof (vdev_label_t, vl_be);
> + vbe->vbe_zbt.zec_magic = ZEC_MAGIC;
> + ZIO_SET_CHECKSUM(&vbe->vbe_zbt.zec_cksum, offset, 0, 0, 0);
> + vbe->vbe_zbt.zec_magic = ZEC_MAGIC;
> + zio_checksum_SHA256(vbe, VDEV_PAD_SIZE, GRUB_ZFS_LITTLE_ENDIAN,
> &cksum);
> + vbe->vbe_zbt.zec_cksum = cksum;
> +
> + for (i = 0; i < data->n_devices_attached; i++)
> + {
> + struct grub_zfs_device_desc *desc = &data->devices_attached[i];
> + int num_sectors = VDEV_PAD_SIZE >> desc->ashift;
> + int label_seek = l * sizeof (vdev_label_t) >> desc->ashift;
> + int j;
Empty line here...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
