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

[Toomas Soome]

> 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).

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