Re: [PATCH 5/5] zfs extensible_dataset and large_blocks feature support

[Andrei Borzenkov]

В Mon, 20 Apr 2015 08:41:41 +0300
Toomas Soome <address@hidden> пишет:

> 
> > On 20.04.2015, at 6:40, Andrei Borzenkov <address@hidden> wrote:
> > 
> > В Sun, 19 Apr 2015 22:53:35 +0300
> > Toomas Soome <address@hidden> пишет:
> > 
> >> 
> >>> On 19.04.2015, at 22:40, Andrei Borzenkov <address@hidden> wrote:
> >>> 
> >>> В Thu, 16 Apr 2015 08:24:38 +0300
> >>> Toomas Soome <address@hidden> пишет:
> >>> 
> >>>> 
> >>> 
> >>> Could you explain how these changes affect large block read? As far as
> >>> I understand, this feature is basically "for free" - changes were
> >>> needed to allow large block writes, but on disk format already
> >>> supported them so reading should have just worked?
> >>> 
> >> 
> >> 
> >> yes, large block is basically free, but, there are 2 conditions. 
> >> 
> >> 1. large blocks basically use extensible dataset feature, or to be exact, 
> >> setting recordsize above 128k will trigger large_block feature to be 
> >> enabled and storing such blocks is using feature extensible dataset. so 
> >> the extensible dataset is prerequisite.
> >> 
> >> 2. once large block is enabled, its listed in label as well, and therefore 
> >> it should be listed in spa_feature_names, which is list of supported 
> >> features.
> >> 
> >> but otherwise, reading large blocks does not require any other changes 
> >> (assuming there is enough memory to read those blocks).
> > 
> > Good. So the means that the only change needed was ...
> > 
> >> 
> >> rgds,
> >> toomas
> >> 
> >> 
> >> 
> >>>> ---
> >>>> grub-core/fs/zfs/zfs.c |   18 +++++++++++-------
> >>>> 1 file changed, 11 insertions(+), 7 deletions(-)
> >>>> 
> >>>> diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
> >>>> index da44131..4a542e0 100644
> >>>> --- a/grub-core/fs/zfs/zfs.c
> >>>> +++ b/grub-core/fs/zfs/zfs.c
> >>>> @@ -283,6 +283,8 @@ static const char *spa_feature_names[] = {
> >>>>  "org.illumos:lz4_compress",
> >>>>  "com.delphix:hole_birth",
> >>>>  "com.delphix:embedded_data",
> >>>> +  "com.delphix:extensible_dataset",
> >>>> +  "org.open-zfs:large_blocks",
> > 
> > ... this one. What all other changes below do and how they are related
> > to these two features?
> 
> they do implement read support extensible dataset… instead of fixed DMU types 
> they dont specify type, making it possible to use fat zap objects from bonus 
> area. 
> 
> reference commit:
> https://github.com/illumos/illumos-gate/commit/2acef22db7808606888f8f92715629ff3ba555b9
> 

OK; just to be sure - there are several remaining uses of
DMU_OT_DSL_DATASET and DMU_OT_DSL_DIR; are these intentional?

> 
> > 
> >>>>  NULL
> >>>> };
> >>>> 
> >>>> @@ -3080,7 +3082,7 @@ get_filesystem_dnode (dnode_end_t * mosmdn, char 
> >>>> *fsname,
> >>>> 
> >>>>  grub_dprintf ("zfs", "alive\n");
> >>>> 
> >>>> -  err = dnode_get (mosmdn, objnum, DMU_OT_DSL_DIR, mdn, data);
> >>>> +  err = dnode_get (mosmdn, objnum, 0, mdn, data);
> >>>>  if (err)
> >>>>    return err;
> >>>> 
> >>>> @@ -3113,7 +3115,7 @@ get_filesystem_dnode (dnode_end_t * mosmdn, char 
> >>>> *fsname,
> >>>>      if (err)
> >>>>  return err;
> >>>> 
> >>>> -      err = dnode_get (mosmdn, objnum, DMU_OT_DSL_DIR, mdn, data);
> >>>> +      err = dnode_get (mosmdn, objnum, 0, mdn, data);
> >>>>      if (err)
> >>>>  return err;
> >>>> 
> >>>> @@ -3268,8 +3270,7 @@ dnode_get_fullpath (const char *fullpath, struct 
> >>>> subvolume *subvol,
> >>>> 
> >>>>  grub_dprintf ("zfs", "endian = %d\n", subvol->mdn.endian);
> >>>> 
> >>>> -  err = dnode_get (&(data->mos), headobj, DMU_OT_DSL_DATASET, 
> >>>> &subvol->mdn,
> >>>> -                   data);
> >>>> +  err = dnode_get (&(data->mos), headobj, 0, &subvol->mdn, data);
> >>>>  if (err)
> >>>>    {
> >>>>      grub_free (fsname);
> >>>> @@ -3665,8 +3666,11 @@ zfs_mount (grub_device_t dev)
> >>>>  if (ub->ub_version >= SPA_VERSION_FEATURES &&
> >>>>      check_mos_features(&((objset_phys_t *) 
> >>>> osp)->os_meta_dnode,ub_endian,
> >>>>                   data) != 0)
> >>>> -    return NULL;
> >>>> -        
> >>>> +    {
> >>>> +      grub_error (GRUB_ERR_BAD_FS, "Unsupported features in pool");
> >>>> +      return NULL;
> >>>> +    }
> >>>> +
> >>>>  /* Got the MOS. Save it at the memory addr MOS. */
> >>>>  grub_memmove (&(data->mos.dn), &((objset_phys_t *) osp)->os_meta_dnode,
> >>>>          DNODE_SIZE);
> >>>> @@ -3963,7 +3967,7 @@ fill_fs_info (struct grub_dirhook_info *info,
> >>>>    {
> >>>>      headobj = grub_zfs_to_cpu64 (((dsl_dir_phys_t *) DN_BONUS 
> >>>> (&mdn.dn))->dd_head_dataset_obj, mdn.endian);
> >>>> 
> >>>> -      err = dnode_get (&(data->mos), headobj, DMU_OT_DSL_DATASET, &mdn, 
> >>>> data);
> >>>> +      err = dnode_get (&(data->mos), headobj, 0, &mdn, data);
> >>>>      if (err)
> >>>>  {
> >>>>    grub_dprintf ("zfs", "failed here\n");
>