Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 16 Mar 2015 04:21:26 -0500
From:      "Matthew D. Fuller" <fullermd@over-yonder.net>
To:        Pawel Jakub Dawidek <pjd@FreeBSD.org>
Cc:        freebsd-geom@freebsd.org
Subject:   Re: RFC: Pass TRIM through GELI
Message-ID:  <20150316092126.GC52331@over-yonder.net>
In-Reply-To: <20150316010845.GA1515@garage.freebsd.pl>
References:  <20150308000131.GP1742@over-yonder.net> <20150314151453.GJ24274@over-yonder.net> <501bca86.508d8e26@fabiankeil.de> <20150315145324.GA52331@over-yonder.net> <20150315182444.GB52331@over-yonder.net> <20150316010845.GA1515@garage.freebsd.pl>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Mar 16, 2015 at 02:08:45AM +0100 I heard the voice of
Pawel Jakub Dawidek, and lo! it spake thus:
> 
> Overall the patch looks good. The main concern I have is that we do
> nothing if the underlying provider returns EOPNOTSUPP on BIO_DELETE,
> we will just keep sending those requests.

Well, they're all coming from the layer above us, and it'll get back
the EOPNOTSUPP's, so if it keeps sending them anyway that seems more
like its problem than ours.  It seems like intercepting the response
(that would mean making our own new request, getting the response,
then making that response ourselves to the original?) wouldn't really
save much of anything, and adds a lot of extra bug-havens...


> Few small nits inline.

Thanks, addressed in geli-delete.4.patch.

> I've been told some time ago that one should use 'you are', etc. in
> manual pages.

Rewrote away from the 2nd person, avoiding the issue entirely.

> This whole comment needs to be rewritten.

How about:

		/*
		 * If the user has set the DELETE flag, we just pass it
		 * down the stack and let the layers beneath us do (or
		 * not) whatever they do with it.  Else we currently just
		 * reject it.  A possible extension would be to take it
		 * as a hint to shred the data with [multiple?]
		 * overwrites.
		 */


> +		if (sc->sc_flags & G_ELI_FLAG_ONETIME) {
> +			gctl_error(req, "Cannot change configuration of "
> +			    "onetime provider %s.", prov);
> +			continue;
> +		}
> 
> Actually, nothing stops us from allowing to change trim/notrim for
> one-time providers.

Well, what stops us right now is the bit where it somehow finds
metadata version 2468898832 when it tries, and then panics  :(.  It
happened with both my changes and the existing code, so I figured it
was just an artifact of something onetime did, and nobody ever cared
(or tried) configure'ing one.

In a quick re-test, I actually can't trigger it on a bare device, but
on top of gnop it happens every time (with both original and my code).
Nothing special, just an arg-less nop, and an arg-less onetime we
'configure -b' afterward.  That makes it a little more concerning...
I guess it needs more looking into; 0'd out the conditional in the
.4.patch pending more investigation.


-- 
Matthew Fuller     (MF4839)   |  fullermd@over-yonder.net
Systems/Network Administrator |  http://www.over-yonder.net/~fullermd/
           On the Internet, nobody can hear you scream.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150316092126.GC52331>