From: Frank S Filz Subject: Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS Date: Mon, 11 Mar 2013 13:25:20 -0700 Message-ID: References: <1362065133-9490-1-git-send-email-piastry@etersoft.ru> <1362065133-9490-8-git-send-email-piastry@etersoft.ru> <20130311150540.119abd63@corrin.poochiereds.net> <20130311193638.GB642@fieldses.org> <20130311160844.7dedf15a@corrin.poochiereds.net> <20130311201108.GE642@fieldses.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6535880014786133835==" Cc: linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, Jeff Layton , wine-devel@winehq.org, linux-kernel@vger.kernel.org, Pavel Shilovsky , linux-fsdevel@vger.kernel.org, linux-nfs-owner@vger.kernel.org To: "J. Bruce Fields" Return-path: In-Reply-To: <20130311201108.GE642@fieldses.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: wine-devel-bounces@winehq.org Errors-To: wine-devel-bounces@winehq.org List-ID: --===============6535880014786133835== Content-type: multipart/alternative; Boundary="0__=08BBF1B8DFFC3E598f9e8a93df938690918c08BBF1B8DFFC3E59" Content-Disposition: inline --0__=08BBF1B8DFFC3E598f9e8a93df938690918c08BBF1B8DFFC3E59 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: quoted-printable "J. Bruce Fields" wrote > On Mon, Mar 11, 2013 at 04:08:44PM -0400, Jeff Layton wrote: > > On Mon, 11 Mar 2013 15:36:38 -0400 > > "J. Bruce Fields" wrote: > > > > It doesn't look like this patch removes any of that old code though. I > > > > think it probably should, or there ought to be some considerati= on of > > > > how this new stuff will mesh with it. > > > > > > > > I think you have 2 choices here: > > > > > > > > 1/ rip out the old share reservation code altogether and requir= e that > > > > filesystems mount with -o sharemand or whatever if they want to= allow > > > > their enforcement > > > > > > > > 2/ make knfsd fall back to using the internal share reservation= code > > > > when the mount option isn't enabled > > > > > > > > Personally, I think #1 would be fine, but Bruce may want to wei= gh in on > > > > what he'd prefer. > > > > > > #1 sounds good. Clients that use deny bits are few. My preferen= ce > > > would be to return an error to such clients in the case share loc= ks > > > aren't available. > > > > > > (We're a little out of spec there, so I'm not sure which error. = I think > > > the goal is to notify a human there's a problem with minimal collateral > > > damange. > > > > > > NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry about that!") wou= ld > > > probably result in an IO error to the application. > > > > > > SHARE_DENIED strikes me as unsafe: an application would be in its= rights > > > not to even check for that e.g. in the case of an exclusive creat= e. Hmm, shouldn't the client catch that with a "default" case at least? > > > Maybe DELAY? Kind of ridiculous, but blocking the application > > > indefinitely would probably get someone's attention quickly enoug= h > > > without doing any damnage.) > > > > > > > I agree that we should return an error, but hadn't considered what > > error. Given that hardly any NFS clients use them, I'd probably jus= t go > > with NFS4ERR_SERVERFAULT, and maybe throw a printk or something on = the > > server about enabling share reservations for superblock x:y. > > Sounds reasonable. If I'm understanding, the suggestion is a mount option to enable share reservations and if so, they will be mandatory? In that case, perhaps we want to keep the existing knfsd code as a fallback, someone might want to support them, but not have them be mandatory (if nothing else, you may cause consternation from folks runn= ing pynfs against a default configured knfsd server....). In the Ganesha project, we provide an internal implementation of share reservations for when the underlying system can not support them. Another bit to consider, does lockd provide share reservations for NLM?= Frank= --0__=08BBF1B8DFFC3E598f9e8a93df938690918c08BBF1B8DFFC3E59 Content-type: text/html; charset=US-ASCII Content-Disposition: inline Content-transfer-encoding: quoted-printable

"J. Bruce Fields" <bfields@fieldse= s.org>  wrote
> On Mon, Mar 11, 2013 at 04:08:44PM -0400, Jef= f Layton wrote:
> > On Mon, 11 Mar 2013 15:36:38 -0400
> > "J. Bruce Fields" <bfields@fieldses.org> wrot= e:
> > > > It doesn't look like this patch removes any of that= old code though. I
> > > > think it probably should, or there ought to be some= consideration of
> > > > how this new stuff will mesh with it.
> > > >
> > > > I think you have 2 choices here:
> > > >
> > > > 1/ rip out the old share reservation code altogethe= r and require that
> > > > filesystems mount with -o sharemand or whatever if = they want to allow
> > > > their enforcement
> > > >
> > > > 2/ make knfsd fall back to using the internal share= reservation code
> > > > when the mount option isn't enabled
> > > >
> > > > Personally, I think #1 would be fine, but Bruce may= want to weigh in on
> > > > what he'd prefer.
> > >
> > > #1 sounds good.  Clients that use deny bits are few= .  My preference
> > > would be to return an error to such clients in the case = share locks
> > > aren't available.
> > >
> > > (We're a little out of spec there, so I'm not sure which= error.  I think
> > > the goal is to notify a human there's a problem with min= imal collateral
> > > damange.
> > >
> > > NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry abo= ut that!") would
> > > probably result in an IO error to the application.
> > >
> > > SHARE_DENIED strikes me as unsafe: an application would = be in its rights
> > > not to even check for that e.g. in the case of an exclus= ive create.

Hmm, shouldn't the client catch that with a "= default" case at least?

> > > Maybe DELAY?  Kind of ridiculo= us, but blocking the application
> > > indefinitely would probably get someone's attention quic= kly enough
> > > without doing any damnage.)
> > >
> >
> > I agree that we should return an error, but hadn't considered= what
> > error. Given that hardly any NFS clients use them, I'd probab= ly just go
> > with NFS4ERR_SERVERFAULT, and maybe throw a printk or somethi= ng on the
> > server about enabling share reservations for superblock x:y.<= br> >
> Sounds reasonable.

If I'm understanding, the suggestion is a mount op= tion to enable share reservations and if so, they will be mandatory?

In that case, perhaps we want to keep the existing= knfsd code as a fallback, someone might want to support them, but not = have them be mandatory (if nothing else, you may cause consternation fr= om folks running pynfs against a default configured knfsd server....).<= /font>

In the Ganesha project, we provide an internal imp= lementation of share reservations for when the underlying system can no= t support them.

Another bit to consider, does lockd provide share = reservations for NLM?

Frank
= --0__=08BBF1B8DFFC3E598f9e8a93df938690918c08BBF1B8DFFC3E59-- --===============6535880014786133835== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============6535880014786133835==--