[illumos-Developer] iSCSI UNMAP with DKIOCFREE

[George Wilson]

On 2/21/11 6:36 PM, Dan McDonald wrote:
> On Mon, Feb 21, 2011 at 09:21:55PM -0500, Dan McDonald wrote:
>> On Mon, Feb 21, 2011 at 06:09:40PM -0800, George Wilson wrote:
>>> zvol.c:
>>>
>>> - need error checking for the range [df_start, df_start + df_length)
>>>      - ensure that df_start + df_length<= zv_volsize
>> Will do!
> Done, returning EINVAL on failure.

BTW, if you don't want to return error you could do:

if (df.df_start + df.df_length > zv->zv_volsize)
     df.df_length = DMU_OBJECT_END


>>>      - ensure that df_start>= 0
>> I thought that data type was unsigned...
>>
>> everywhere(~)[0]% grep diskaddr_t /usr/include/sys/types.h
>> typedef	u_longlong_t	diskaddr_t;
>> everywhere(~)[0]%
>>
>> Is a>= 0 check still valid?  Should I do some unsigned equivalent thereof
>> (<= LLONG_MAX)?
> I went ahead and did this check, also returning EINVAL on failure.

Sorry, I should have looked closer at my response, instead I led you 
down the wrong path. What I was trying to say was add a add a check when 
you're freeing an area which is already free:

/* Nothing to do */
if (df.df_start >= zv->zv_volsize)
     break;
>>> - need to call zfs_range_lock() for the range specified by df (need
>>> unlock too)
>> Makes sense and will do.
> Done, with one question (see below).
>
>>> - I think we also need to log this operation to the zil so that it
>>> can be replayed on reboot (I'm digging into dmu_free_long_range() to
>>> see if it tracks the deletion)
>> Please report back with these results, and an answer to my LLONG_MAX-ish
>> question?
> So I've implemented all of your suggested changes, modulo the zil logging.  I
> have one question, and that's whether or not I need to place the
> zfs_range_unlock() *after* the optional txg_wait_synced().  The previous
> webrev is now updated:

Will let you know what I uncover with dmu_free_long_range(). As for your 
location of zfs_range_unlock() it looks fine.

- George
> 	http://www.kebe.com/~danmcd/webrevs/unmap/
>
> Thanks,
> Dan
>
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer