From: Steve Dickson <SteveD@redhat.com>
To: Jeff Layton <jlayton@redhat.com>
Cc: linux-nfs@vger.kernel.org, bfields@fieldses.org, chuck.lever@oracle.com
Subject: Re: [PATCH] rpc.nfsd: mount up nfsdfs is it doesn't appear to be mounted yet
Date: Tue, 31 Aug 2010 10:49:56 -0400 [thread overview]
Message-ID: <4C7D1694.6000708@RedHat.com> (raw)
In-Reply-To: <20100831084323.02a1abf5@corrin.poochiereds.net>
On 08/31/2010 08:43 AM, Jeff Layton wrote:
> On Tue, 31 Aug 2010 08:24:57 -0400
> Steve Dickson <SteveD@redhat.com> wrote:
>
>>
>>
>> On 08/30/2010 01:48 PM, Jeff Layton wrote:
>>> On Mon, 30 Aug 2010 12:53:16 -0400
>>> Steve Dickson <SteveD@redhat.com> wrote:
>>>>>
>>>>> Hmmm...if I had known that it was ok to stop supporting old kernels,
>>>>> the IPv6 support for rpc.nfsd would have been a heck of a lot easier to
>>>>> implement.
>>>> I thought you said the legacy interface would not work with IPV6 or
>>>> did I misunderstand you?
>>>>
>>>
>>> Sorry, forgot to answer this part. IPv6 indeed will not work when using
>>> the legacy interface. In order to get the IPv6 code in though, I ended
>>> up rewriting a large swath of rpc.nfsd. That task would have been
>>> easier had I not needed to deal with the legacy nfsctl() interface.
>>>
>> So I'm thinking this the final nail in the coffin of the legacy interface
>> at least in rpc.nfsd. So if /proc/fs/nfsd filesystem is not or can not
>> be mounted then rpc.nfsd should fail... IMHO...
>>
>> Maybe something like:
>>
>> if (stat("/proc/fs/nfsd") < 0) {
>
> This needs to check a file within /proc/fs/nfsd. The nfsd "dir" may
> already exist even if it's not mounted. I chose "threads" but any file
> in there would do...
>
>> if (system("mount /proc/fs/nfsd") < 0) {
>> xlog(L_ERROR, "Unable to mount /proc/fs/nfsd");
>> exit 0;
>> }
>> }
>>
>> steved.
>
> That will universally fail, at least on most distros.
By no means was I meaning this was working code...
I was just trying to introduced logic of: check to see if
/proc/fs/nfsd exists, if not, mount it. If the mount fails, exit.
Detail to be worked out later...
>
>
> When you go to mount /proc/fs/nfsd, the system does a request_module()
> for nfsd.ko. On most distros (e.g. Fedora or RHEL), modprobe will then
> automatically mount up /proc/fs/nfsd. request_module will then return
> and the mount attempt will proceed.
Yeah I know... I am one that taught modprobe how to that... at least
in the RH distros... ;-) Although I believe the actual mechanics
have changed over the years...
> The mount will then fail because
> it's now already mounted, and the system() call will return non-zero.
Isn't this reason to do the stat() before the mount? So you will not try to
mount the FS if is already mounted?
>
> That's the main reason I ignored the return code from system() in the
> original patch. What you could do is recheck whether the file exists
> after the mount attempt, and then fail if it doesn't.
My point was we need to know if the mount did or did not
work to take the appropriate action...
>
> Is that really what we want to do here though? How does ditching the
> legacy interface move us forward now? If we had decided to do this a
> year ago when I was overhauling rpc.nfsd, it might have made sense. The
> code exists now though and it works. Ripping this out seems more
> disruptive than just leaving it in.
I believe we are talking about 4 lines of code at the bottom of nfssvc_threads()
I don't think that would be too disruptive...
steved.
>
> To keep this on track, I think we need to treat this as 2 separate
> discussions:
>
> 1) how to we ensure that /proc/fs/nfsd is actually mounted when
> rpc.nfsd is run?
>
> 2) what do we do about the legacy nfsctl() interface?
>
> These are separate but related questions...
next prev parent reply other threads:[~2010-08-31 14:50 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-28 11:35 [PATCH] rpc.nfsd: mount up nfsdfs is it doesn't appear to be mounted yet Jeff Layton
2010-08-28 22:29 ` Neil Brown
2010-08-28 22:38 ` Neil Brown
2010-08-29 2:24 ` Jeff Layton
2010-08-29 19:31 ` J. Bruce Fields
2010-08-29 19:37 ` J. Bruce Fields
2010-08-29 22:12 ` Neil Brown
2010-08-30 15:51 ` Steve Dickson
2010-08-30 16:16 ` Jeff Layton
2010-08-30 16:53 ` Steve Dickson
2010-08-30 17:04 ` J. Bruce Fields
2010-08-30 17:22 ` Jeff Layton
2010-08-31 12:14 ` Steve Dickson
2010-08-30 17:48 ` Jeff Layton
2010-08-31 12:24 ` Steve Dickson
2010-08-31 12:43 ` Jeff Layton
2010-08-31 14:49 ` Steve Dickson [this message]
2010-08-31 15:10 ` Jeff Layton
2010-08-31 15:13 ` J. Bruce Fields
2010-08-31 15:18 ` Steve Dickson
2010-08-31 15:51 ` J. Bruce Fields
2010-08-31 16:13 ` Steve Dickson
2010-08-31 16:15 ` J. Bruce Fields
2010-08-31 17:18 ` Steve Dickson
2010-08-31 18:07 ` J. Bruce Fields
2010-08-31 18:59 ` Steve Dickson
2010-08-31 19:02 ` Jeff Layton
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=4C7D1694.6000708@RedHat.com \
--to=steved@redhat.com \
--cc=bfields@fieldses.org \
--cc=chuck.lever@oracle.com \
--cc=jlayton@redhat.com \
--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.