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

[Andrei Borzenkov]

В Wed, 15 Apr 2015 23:00:11 +0300
Toomas Soome <address@hidden> пишет:

> 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];