Re: [PATCH] added support for whole disk zfs embedding

[Andrei Borzenkov <arvidjaar@gmail.com>]

В Wed, 15 Apr 2015 23:00:11 +0300
Toomas Soome <tsoome@me.com> пишет:

> hi!
> 
> well, scratch please the previous zfs related updates, sorry for confusion:)
> 
> anyhow, next small chunk adds support for "whole disk" (GPT partitioned) zpool embedding. if the disk already got BIOS boot partition, it will be used, if there is none, zfs partition is used and grub is embedded to zpool bootblock area.

No. Linux never paid any attention to partition types (neither MBR nor
GPT) and we have no idea how partition is used. That is why
grub-bios-setup actually checks that we *do* know what is on this
partition and ensures there is reserved space for embedding.

bios_grub is different - here we explicitly declare this partition type
as reserved for GRUB.

The right thing to do is to decouple boot code from core.img on
install similar to

grub-(install|buis-setup) --embed /dev/sda1 /dev/sda

which will embed into reserved area on /dev/sda1 and write boot code
into MBR pointing at /dev/sda1. 

I'm fine with making it --boot-code /dev/sda /dev/sda1 :)

> 
> rgds,
> toomas
> 
> ---
>  grub-core/fs/zfs/zfs.c       |    2 +-
>  grub-core/partmap/gpt.c      |    9 +++++++++
>  include/grub/gpt_partition.h |    7 +++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
> index 0cbb84b..2689986 100644
> --- a/grub-core/fs/zfs/zfs.c
> +++ b/grub-core/fs/zfs/zfs.c
> @@ -4291,7 +4291,7 @@ static struct grub_fs grub_zfs_fs = {
>  #ifdef GRUB_UTIL
>    .embed = grub_zfs_embed,
>    .reserved_first_sector = 1,
> -  .blocklist_install = 0,
> +  .blocklist_install = 1,

Why? Nothing in your patch enables blocklists support on ZFS nor is it
even remotely possible for all I can tell.

>  #endif
>    .next = 0
>  };
> diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
> index 83bcba7..cacc8e8 100644
> --- a/grub-core/partmap/gpt.c
> +++ b/grub-core/partmap/gpt.c
> @@ -24,6 +24,8 @@
>  #include <grub/dl.h>
>  #include <grub/msdos_partition.h>
>  #include <grub/gpt_partition.h>
> +#include <grub/zfs/zio.h>
> +#include <grub/zfs/vdev_impl.h>
>  #include <grub/i18n.h>
>  
>  GRUB_MOD_LICENSE ("GPLv3+");
> @@ -37,6 +39,7 @@ static const grub_gpt_part_type_t grub_gpt_partition_type_empty = GRUB_GPT_PARTI
>  
>  #ifdef GRUB_UTIL
>  static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot = GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
> +static const grub_gpt_part_type_t grub_gpt_partition_type_zfs = GRUB_GPT_PARTITION_TYPE_ZFS;
>  #endif
>  
>  /* 512 << 7 = 65536 byte sectors.  */
> @@ -162,6 +165,12 @@ find_usable_region (grub_disk_t disk __attribute__ ((unused)),
>        return 1;
>      }
>  
> +  if (! grub_memcmp (&gptdata.type, &grub_gpt_partition_type_zfs, 16))
> +    {
> +      ctx->start = p->start + (VDEV_BOOT_OFFSET >> GRUB_DISK_SECTOR_BITS);
> +      ctx->len = (VDEV_BOOT_SIZE >> GRUB_DISK_SECTOR_BITS);
> +      return 1;
> +    }
>    return 0;
>  }
>  
> diff --git a/include/grub/gpt_partition.h b/include/grub/gpt_partition.h
> index 1b32f67..04c9f97 100644
> --- a/include/grub/gpt_partition.h
> +++ b/include/grub/gpt_partition.h
> @@ -50,6 +50,13 @@ typedef struct grub_gpt_part_type grub_gpt_part_type_t;
>  	{ 0x85, 0xD2, 0xE1, 0xE9, 0x04, 0x34, 0xCF, 0xB3 }	\
>    }
>  
> +#define GRUB_GPT_PARTITION_TYPE_ZFS \
> +  { grub_cpu_to_le32_compile_time (0x6A898CC3U),\
> +      grub_cpu_to_le16_compile_time (0x1DD2), \
> +      grub_cpu_to_le16_compile_time (0x11B2),	       \
> +	{ 0x99, 0xA6, 0x08, 0x00, 0x20, 0x73, 0x66, 0x31 }	\
> +  }
> +
>  struct grub_gpt_header
>  {
>    grub_uint8_t magic[8];