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

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

[Toomas Soome]

---
 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",
   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");
-- 
1.7.9.2

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

[Andrei Borzenkov]

В Thu, 16 Apr 2015 08:24:38 +0300
Toomas Soome <tsoome@me.com> пишет:

> 

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?

> ---
>  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",
>    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");

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

[Toomas Soome]

> On 19.04.2015, at 22:40, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
> В Thu, 16 Apr 2015 08:24:38 +0300
> Toomas Soome <tsoome@me.com> пишет:
> 
>> 
> 
> 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).

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",
>>   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");
> 

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

[Andrei Borzenkov]

В Sun, 19 Apr 2015 22:53:35 +0300
Toomas Soome <tsoome@me.com> пишет:

> 
> > On 19.04.2015, at 22:40, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> > 
> > В Thu, 16 Apr 2015 08:24:38 +0300
> > Toomas Soome <tsoome@me.com> пишет:
> > 
> >> 
> > 
> > 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?

> >>   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");
> > 
> 

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

[Toomas Soome]

> On 20.04.2015, at 6:40, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
> В Sun, 19 Apr 2015 22:53:35 +0300
> Toomas Soome <tsoome@me.com> пишет:
> 
>> 
>>> On 19.04.2015, at 22:40, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>> 
>>> В Thu, 16 Apr 2015 08:24:38 +0300
>>> Toomas Soome <tsoome@me.com> пишет:
>>> 
>>>> 
>>> 
>>> 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


> 
>>>>  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");

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

[Andrei Borzenkov]

В Mon, 20 Apr 2015 08:41:41 +0300
Toomas Soome <tsoome@me.com> пишет:

> 
> > On 20.04.2015, at 6:40, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> > 
> > В Sun, 19 Apr 2015 22:53:35 +0300
> > Toomas Soome <tsoome@me.com> пишет:
> > 
> >> 
> >>> On 19.04.2015, at 22:40, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> >>> 
> >>> В Thu, 16 Apr 2015 08:24:38 +0300
> >>> Toomas Soome <tsoome@me.com> пишет:
> >>> 
> >>>> 
> >>> 
> >>> 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");
> 

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

[Toomas Soome]

> On 03.05.2015, at 22:23, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
>>> 
>> 
>> 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?


they *should* be ok. at least browsing and loading files from datasets haven’t revealed other instances over the test time. but problem is, the cases of zapify (when “normal” structure is wrapped as zap object) I have seen, have been related to datasets using >128k recordsize, and its possible some cases may have been missed - the grub zfs code and real zfs code are not exactly easy to compare. at least so far I haven’t seen bugs related to this nor received any reports. the basic symptom can be either message about wrong type or “missing” directory entry in listing.

rgds,
toomas

> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Andrei Borzenkov]

В Sun, 03 May 2015 22:47:40 +0300
Toomas Soome <tsoome@me.com> пишет:

> 
> > On 03.05.2015, at 22:23, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> > 
> >>> 
> >> 
> >> 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?
> 
> 
> they *should* be ok. at least browsing and loading files from datasets haven’t revealed other instances over the test time. but problem is, the cases of zapify (when “normal” structure is wrapped as zap object) I have seen, have been related to datasets using >128k recordsize, and its possible some cases may have been missed - the grub zfs code and real zfs code are not exactly easy to compare. at least so far I haven’t seen bugs related to this nor received any reports. the basic symptom can be either message about wrong type or “missing” directory entry in listing.
> 


Can https://savannah.gnu.org/bugs/?44448 be somehow related?

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

[Toomas Soome]

> On 04.05.2015, at 7:32, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
> В Sun, 03 May 2015 22:47:40 +0300
> Toomas Soome <tsoome@me.com> пишет:
> 
>> 
>>> On 03.05.2015, at 22:23, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>> 
>>>>> 
>>>> 
>>>> 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?
>> 
>> 
>> they *should* be ok. at least browsing and loading files from datasets haven’t revealed other instances over the test time. but problem is, the cases of zapify (when “normal” structure is wrapped as zap object) I have seen, have been related to datasets using >128k recordsize, and its possible some cases may have been missed - the grub zfs code and real zfs code are not exactly easy to compare. at least so far I haven’t seen bugs related to this nor received any reports. the basic symptom can be either message about wrong type or “missing” directory entry in listing.
>> 
> 
> 
> Can https://savannah.gnu.org/bugs/?44448 be somehow related?

without grub debug=zfs its impossible to say - guessing there is not really helpful:) altho in general, feature check for label should generate error message if pool has enabled features not supported by version of grub used there.

basically, my first suggestion to cases like that is to set debug=zfs in grub and ask user to provide that debug output.

rgds,
toomas

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

[Toomas Soome]

hi!
so what is the status of this last feature pair?  its the final bundle to get full support of modern openzfs to grub and since   zfs on all major players (illumos/zol/freebsd) support this feature, its only question of time when someone will step on this missing piece;)

rgds,
toomas

> On 04.05.2015, at 9:49, Toomas Soome <tsoome@me.com> wrote:
> 
> 
>> On 04.05.2015, at 7:32, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>> 
>> В Sun, 03 May 2015 22:47:40 +0300
>> Toomas Soome <tsoome@me.com> пишет:
>> 
>>> 
>>>> On 03.05.2015, at 22:23, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>>> 
>>>>>> 
>>>>> 
>>>>> 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?
>>> 
>>> 
>>> they *should* be ok. at least browsing and loading files from datasets haven’t revealed other instances over the test time. but problem is, the cases of zapify (when “normal” structure is wrapped as zap object) I have seen, have been related to datasets using >128k recordsize, and its possible some cases may have been missed - the grub zfs code and real zfs code are not exactly easy to compare. at least so far I haven’t seen bugs related to this nor received any reports. the basic symptom can be either message about wrong type or “missing” directory entry in listing.
>>> 
>> 
>> 
>> Can https://savannah.gnu.org/bugs/?44448 be somehow related?
> 
> without grub debug=zfs its impossible to say - guessing there is not really helpful:) altho in general, feature check for label should generate error message if pool has enabled features not supported by version of grub used there.
> 
> basically, my first suggestion to cases like that is to set debug=zfs in grub and ask user to provide that debug output.
> 
> rgds,
> toomas
> 
> 

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

[Andrei Borzenkov]

В Fri, 15 May 2015 10:50:59 +0300
Toomas Soome <tsoome@me.com> пишет:

> 
> hi!
> so what is the status of this last feature pair?  its the final bundle to get full support of modern openzfs to grub and since   zfs on all major players (illumos/zol/freebsd) support this feature, its only question of time when someone will step on this missing piece;)

Committed with additional explanatory commit text scraped from this
discussion.

Thanks!

> 
> rgds,
> toomas
> 
> > On 04.05.2015, at 9:49, Toomas Soome <tsoome@me.com> wrote:
> > 
> > 
> >> On 04.05.2015, at 7:32, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> >> 
> >> В Sun, 03 May 2015 22:47:40 +0300
> >> Toomas Soome <tsoome@me.com> пишет:
> >> 
> >>> 
> >>>> On 03.05.2015, at 22:23, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> >>>> 
> >>>>>> 
> >>>>> 
> >>>>> 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?
> >>> 
> >>> 
> >>> they *should* be ok. at least browsing and loading files from datasets haven’t revealed other instances over the test time. but problem is, the cases of zapify (when “normal” structure is wrapped as zap object) I have seen, have been related to datasets using >128k recordsize, and its possible some cases may have been missed - the grub zfs code and real zfs code are not exactly easy to compare. at least so far I haven’t seen bugs related to this nor received any reports. the basic symptom can be either message about wrong type or “missing” directory entry in listing.
> >>> 
> >> 
> >> 
> >> Can https://savannah.gnu.org/bugs/?44448 be somehow related?
> > 
> > without grub debug=zfs its impossible to say - guessing there is not really helpful:) altho in general, feature check for label should generate error message if pool has enabled features not supported by version of grub used there.
> > 
> > basically, my first suggestion to cases like that is to set debug=zfs in grub and ask user to provide that debug output.
> > 
> > rgds,
> > toomas
> > 
> > 
>