login
Header Space

 
 

Re: [PATCH 4/4] autofs4 - add miscelaneous device for ioctls

Score:
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Ian Kent <raven@...>
Cc: Kernel Mailing List <linux-kernel@...>, autofs mailing list <autofs@...>, linux-fsdevel <linux-fsdevel@...>, Christoph Hellwig <hch@...>, Al Viro <viro@...>
Date: Thursday, February 28, 2008 - 1:17 am

On Tue, 26 Feb 2008 12:23:55 +0900 (WST) Ian Kent <raven@themaw.net> wrote:


Could you please document the new kernel interface which you're proposing? 
In Docmentation/ or in the changelog?

We seem to be passing some string into a miscdevice ioctl and getting some
results back.  Be aware that this won't be a terribly popular proposal, so
I'd suggest that you fully describe the problem which it's trying to solve,
and how it solves it, and why the various alternatives (sysfs, netlink,
mount options, etc) were judged unsuitable.



We should run unregister_filesystem() if autofs_dev_ioctl_init() fails.


Please always pass all diffs through scripts/checkpatch.pl.


We prefer not to bother with the filename-in-the-file thing.  You know what
file you're reading, and these things tend to not get updated across
renames.


Please include <linux/foo.h> rather than <asm/foo.h> if the former exists.


This needs parentheses.

Shouldn't these be in a header file, exported to userspace builds?


`f' cannot be NULL here.


Don't bother inlining things - the compiler will do it.


hm.  possibly-interested parties cc'ed.


That's fd_install() plus an add-on.  It's not autofs-specific.  Should be
in fs/open.c, methinks?


hm.


We have a new filesystem type, a misc device with a mysterious ioctl,
hand-rolled mountpoint chasing, hand-rolled fd installation and now pipes
too.

This is a complex interface.  We really need to see the overall problem
statement, design and interface description, please.


uninline everything...


especially that - it's only ever called indirectly anwyay!


Are there races around the modification of ino->flags here?


uninline..


Have you really carefully reviewed and tested what happens when non-autofs
fds are fed into all the ioctl modes?

I hope all these ioctl entrypoints are root-only.  What determines that? 
The miscdevice permissions?


`idx' should have unsigned type.


OK, I guess that answers my above question.


whoa, what's this doing here?


uninline..


Can we do this automatically via preprocessor tricks, or whatever?


--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH 4/4] autofs4 - add miscelaneous device for ioctls, Andrew Morton, (Thu Feb 28, 1:17 am)
Re: [PATCH 3/4] autofs4 - track uid and gid of last mount re..., Eric W. Biederman, (Thu Feb 28, 4:33 pm)
Re: [autofs] [PATCH 3/4] autofs4 - track uid and gid of last..., Fabio Olive Leite, (Thu Feb 28, 8:31 am)
speck-geostationary