Darren J Moffat wrote:
> eric kustarz wrote:
>> Darren J Moffat wrote:
>>> eric kustarz wrote:
>>>
>>>> Not sure if you've already posted this or not, but do you have a 
>>>> webrev?
>>>
>>> Yes it was in the original message:
>>>
>>> http://cr.grommit.com/~darrenm/zfs-crypto/
>>>
>>
>> I didn't see anything obvious in the webrev, but have you tried to 
>> temporarily make zio_decrypt_data() and zio_encrypt_data() no-ops to 
>> rule out that new code and focus on the existing framework?
> 
> I haven't recently but I can try that again.
> 
>> If it isn't that then i'd ping Bill to make sure the zio pipeline 
>> changes are corrrect and Matt to make sure the DMU changes are right 
>> (if you haven't already).
> 
> Bill ping
> Matt ping
> 
> I know they both hang out here :-)

I just looked at your code and didn't see anything that would be causing 
your bugs.  I can't speak for the SPA stuff, but the rest looks good. 
Here are some general comments:

in dmu_object_info_t, you need to decrease the size of doi_pad by 1 
byte.  This is so that all structure members will have the same offset 
on 32 and 64 bits, so that it can be passed between a 64-bit kernel and 
a 32-bit app.

in dbuf_sync(), the comment says "ZFS metadata is in the clear but ZPL 
metadata should be encrypted".  I assume that "ZFS metadata" means 
"DMU/DSL/SPA metadata"?  But it looks like you are encrypting DMU 
metadata (indirect blocks -- db_level > 0), and you are not encrypting 
any metadata, not even ZPL metadata like directories (from ot_metadata).

eventually, the zil's blocks will need to be encrypted as well.

--matt

Reply via email to