[illumos-Developer] iSCSI UNMAP with DKIOCFREE

[Dan McDonald]

On Wed, Feb 23, 2011 at 09:38:47AM -0800, George Wilson wrote:
> Dan,
> 
> The latest changes look good. I've also finished looking at
> dmu_free_long_range() and as feared we're going to need to log an
> intention for this in the zil so that we can replay this if a  crash
> were ever to happen.

So I see two possibilities after an initial exploration:

1.) Have DKIOCFREE code do a distinct ZIL intention.  I see two possible
    places...

		rl = zfs_range_lock(&zv->zv_znode, df.df_start, df.df_length,
		    RL_WRITER);
		error = dmu_free_long_range(zv->zv_objset, ZVOL_OBJ,
		    df.df_start, df.df_length);
		/* XXX KEBE ASKS log an intention ZIL here? */
		zfs_range_unlock(rl);

		if (error == 0) {
			/* XXX KEBE ASKS or log an intention ZIL here? */

			if (df.df_flags & DF_WAIT_SYNC) {
				txg_wait_synced(
				    dmu_objset_pool(zv->zv_objset), 0);
			}
		}

    If I understand other such code in zvol.c (e.g. zvol_write()), I need a
    dmu tx object, a call to zvol_log_write(), and then dmu_tx_commit().
    zvol_write() does it inside the range lock, so probably at the first
    place.

2.) We could modify dmu_free_long_range() or ..._long_range_impl() to commit
    to the zil itself, but that seems crufty and interface-breaking.

    Unlike the dmu_write...() calls zvol_write() makes, dmu_free_long_range()
    requires no dmu_tx_t, as it allocates and commits one itself.  And it
    does so multiple times if "length" comprises multiple chunks (see
    dmu_free_long_range_impl()).


IMHO either one of these feels wrong w.r.t. the integrity of the system.
Perhaps we introduce dmu_free_long_range_zil() which also logs to the zil,
and then dmu_free_long_range_impl() takes another flag indicating zil
commitment?  That'd be option #3...

Again... any clues are, as always, welcome.

Thanks,
Dan