Date: Mon, 16 Mar 2015 11:00:31 +0100 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: "Matthew D. Fuller" <fullermd@over-yonder.net> Cc: freebsd-geom@freebsd.org Subject: Re: RFC: Pass TRIM through GELI Message-ID: <20150316100030.GB1515@garage.freebsd.pl> In-Reply-To: <20150316092126.GC52331@over-yonder.net> 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> <20150316092126.GC52331@over-yonder.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Mar 16, 2015 at 04:21:26AM -0500, Matthew D. Fuller wrote:
> 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...
You are right. I read your reasoning you've sent earlier after sending
my e-mail.
> > 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.
> */
Looks good, thanks.
> > + 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.
Rejecting -b/-B for one-time providers is of course fine. Accepting
-t/-T for one-time providers would be nice to have, but of course
nothing critical. Still would be good to know the root cause of what you
are seeing.
--
Pawel Jakub Dawidek http://www.wheelsystems.com
FreeBSD committer http://www.FreeBSD.org
Am I Evil? Yes, I Am! http://mobter.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150316100030.GB1515>