From: "J. Bruce Fields" <bfields@fieldses.org>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 5/6] nfsd: take struct file setup fully into nfs4_preprocess_stateid_op
Date: Tue, 28 Apr 2015 16:50:49 -0400 [thread overview]
Message-ID: <20150428205049.GD16090@fieldses.org> (raw)
In-Reply-To: <20150428204455.GC16090@fieldses.org>
On Tue, Apr 28, 2015 at 04:44:55PM -0400, J. Bruce Fields wrote:
> On Tue, Apr 28, 2015 at 03:41:19PM +0200, Christoph Hellwig wrote:
> > This patch changes nfs4_preprocess_stateid_op so it always returns
> > a valid struct file if it has been asked for that. For that we
> > now allocate a temporary struct file for special stateids, and check
> > permissions if we got the file structure from the stateid. This
> > ensures that all callers will get their handling of special stateids
> > right, and avoids code duplication.
> >
> > There is a little wart in here because the read code needs to know
> > if we allocated a file structure so that it can copy around the
> > read-ahead parameters. In the long run we should probably aim to
> > cache full file structures used with special stateids instead.
>
> This causes a failure on pynfs OPEN23b.
>
> It's doing a READ using a stateid from a write open. We previously
> returned NFS_OK, taking the "may" option from:
>
> http://tools.ietf.org/html/rfc7530#page-111
>
> In the case of READ, the server may perform the corresponding
> check on the access mode, or it may choose to allow READ on
> opens for WRITE only, to accommodate clients whose write
> implementation may unavoidably do reads (e.g., due to buffer
> cache constraints).
>
> OPENMODE might also have been OK, but we're returning SERVERFAULT. I
> guess the old code was passing preprocess_stateid_op without returning a
> file, then relying on a temporary open for the read? Ugh.
Hm, and writes with special stateids (e.g. WRT1) are timing out, I
haven't figured that out.
--b.
next prev parent reply other threads:[~2015-04-28 20:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-28 13:41 refactor stateid checking and file allocation Christoph Hellwig
2015-04-28 13:41 ` [PATCH 1/6] nfsd: fix the check for confirmed openowner in nfs4_preprocess_stateid_op Christoph Hellwig
2015-04-28 19:12 ` J. Bruce Fields
2015-04-28 13:41 ` [PATCH 2/6] nfsd: remove nfsd_close Christoph Hellwig
2015-04-28 13:41 ` [PATCH 3/6] nfsd: refactor nfs4_preprocess_stateid_op Christoph Hellwig
2015-04-28 13:41 ` [PATCH 4/6] nfsd: clean up raparams handling Christoph Hellwig
2015-04-28 13:41 ` [PATCH 5/6] nfsd: take struct file setup fully into nfs4_preprocess_stateid_op Christoph Hellwig
2015-04-28 20:44 ` J. Bruce Fields
2015-04-28 20:50 ` J. Bruce Fields [this message]
2015-04-29 13:11 ` Christoph Hellwig
2015-04-29 14:16 ` J. Bruce Fields
2015-04-28 13:41 ` [PATCH 6/6] nfsd: drop the file argument to nfsd_write Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150428205049.GD16090@fieldses.org \
--to=bfields@fieldses.org \
--cc=hch@lst.de \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.