* [PATCH v2 1/3] nfsd: add a usermodehelper upcall for NFSv4 client ID tracking
2012-10-03 12:40 [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking Jeff Layton
@ 2012-10-03 12:40 ` Jeff Layton
2012-10-05 14:36 ` J. Bruce Fields
2012-10-03 12:40 ` [PATCH v2 2/3] nfsd: change heuristic for selecting the client_tracking_ops Jeff Layton
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Jeff Layton @ 2012-10-03 12:40 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Add a new client tracker upcall type that uses call_usermodehelper to
call out to a program. This seems to be the preferred method of
calling out to usermode these days for seldom-called upcalls. It's
simple and doesn't require a running daemon, so it should "just work"
as long as the binary is installed.
The client tracking exit operation is also changed to check for a
NULL pointer before running. The UMH upcall doesn't need to do anything
at module teardown time.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4recover.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 138 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 43295d4..481cb3e 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -926,6 +926,142 @@ static struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops = {
.grace_done = nfsd4_cld_grace_done,
};
+/* upcall via usermodehelper */
+static char cltrack_prog[PATH_MAX] = "/sbin/nfsdcltrack";
+module_param_string(cltrack_prog, cltrack_prog, sizeof(cltrack_prog),
+ S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
+
+static int
+nfsd4_cltrack_upcall(char *cmd, char *arg)
+{
+ char *envp[] = { "HOME=/",
+ "TERM=linux",
+ "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
+ NULL
+ };
+ char *argv[4];
+ int ret;
+
+ if (unlikely(!cltrack_prog[0])) {
+ dprintk("%s: cltrack_prog is disabled\n", __func__);
+ return -EACCES;
+ }
+
+ dprintk("%s: cmd: %s\n", __func__, cmd);
+ dprintk("%s: arg: %s\n", __func__, arg ? arg : "(null)");
+
+ argv[0] = (char *)cltrack_prog;
+ argv[1] = cmd;
+ argv[2] = arg;
+ argv[3] = NULL;
+
+ ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
+ /*
+ * Disable the upcall mechanism if we're getting an ENOENT
+ * error. The admin can re-enable it on the fly by using sysfs
+ * once the problem has been fixed.
+ */
+ if (ret == -ENOENT || ret == -EACCES) {
+ printk(KERN_ERR "NFSD: %s was not found or isn't executable. "
+ "Please reset nfsd.cltrack_prog module parameter "
+ "once problem is resolved (%d)!\n",
+ cltrack_prog, ret);
+ cltrack_prog[0] = '\0';
+ }
+ dprintk("%s: %s return value: %d\n", __func__, cltrack_prog, ret);
+
+ return ret;
+}
+
+static char *
+bin_to_hex_dup(const unsigned char *src, int srclen)
+{
+ int i;
+ char *buf, *hex;
+
+ /* +1 for terminating NULL */
+ buf = kmalloc((srclen * 2) + 1, GFP_KERNEL);
+ if (!buf)
+ return buf;
+
+ hex = buf;
+ for (i = 0; i < srclen; i++) {
+ sprintf(hex, "%2.2x", *src++);
+ hex += 2;
+ }
+ return buf;
+}
+
+static int
+nfsd4_umh_cltrack_init(struct net __attribute__((unused)) *net)
+{
+ return nfsd4_cltrack_upcall("init", NULL);
+}
+
+static void
+nfsd4_umh_cltrack_create(struct nfs4_client *clp)
+{
+ char *hexid;
+
+ hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
+ if (!hexid) {
+ dprintk("%s: can't allocate memory for upcall!\n", __func__);
+ return;
+ }
+ nfsd4_cltrack_upcall("create", hexid);
+ kfree(hexid);
+}
+
+static void
+nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
+{
+ char *hexid;
+
+ hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
+ if (!hexid) {
+ dprintk("%s: can't allocate memory for upcall!\n", __func__);
+ return;
+ }
+ nfsd4_cltrack_upcall("remove", hexid);
+ kfree(hexid);
+}
+
+static int
+nfsd4_umh_cltrack_check(struct nfs4_client *clp)
+{
+ int ret;
+ char *hexid;
+
+ hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
+ if (!hexid) {
+ dprintk("%s: can't allocate memory for upcall!\n", __func__);
+ return -ENOMEM;
+ }
+ ret = nfsd4_cltrack_upcall("check", hexid);
+ kfree(hexid);
+ return ret;
+}
+
+static void
+nfsd4_umh_cltrack_grace_done(struct net __attribute__((unused)) *net,
+ time_t boot_time)
+{
+ char timestr[22]; /* FIXME: better way to determine max size? */
+
+ sprintf(timestr, "%ld", boot_time);
+ nfsd4_cltrack_upcall("gracedone", timestr);
+}
+
+static struct nfsd4_client_tracking_ops nfsd4_umh_tracking_ops = {
+ .init = nfsd4_umh_cltrack_init,
+ .exit = NULL,
+ .create = nfsd4_umh_cltrack_create,
+ .remove = nfsd4_umh_cltrack_remove,
+ .check = nfsd4_umh_cltrack_check,
+ .grace_done = nfsd4_umh_cltrack_grace_done,
+};
+
int
nfsd4_client_tracking_init(struct net *net)
{
@@ -956,7 +1092,8 @@ void
nfsd4_client_tracking_exit(struct net *net)
{
if (client_tracking_ops) {
- client_tracking_ops->exit(net);
+ if (client_tracking_ops->exit)
+ client_tracking_ops->exit(net);
client_tracking_ops = NULL;
}
}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] nfsd: add a usermodehelper upcall for NFSv4 client ID tracking
2012-10-03 12:40 ` [PATCH v2 1/3] nfsd: add a usermodehelper upcall for NFSv4 client ID tracking Jeff Layton
@ 2012-10-05 14:36 ` J. Bruce Fields
2012-10-05 14:47 ` Jeff Layton
0 siblings, 1 reply; 7+ messages in thread
From: J. Bruce Fields @ 2012-10-05 14:36 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs
On Wed, Oct 03, 2012 at 08:40:29AM -0400, Jeff Layton wrote:
> Add a new client tracker upcall type that uses call_usermodehelper to
> call out to a program. This seems to be the preferred method of
> calling out to usermode these days for seldom-called upcalls. It's
> simple and doesn't require a running daemon, so it should "just work"
> as long as the binary is installed.
>
> The client tracking exit operation is also changed to check for a
> NULL pointer before running. The UMH upcall doesn't need to do anything
> at module teardown time.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfsd/nfs4recover.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 43295d4..481cb3e 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -926,6 +926,142 @@ static struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops = {
> .grace_done = nfsd4_cld_grace_done,
> };
>
> +/* upcall via usermodehelper */
> +static char cltrack_prog[PATH_MAX] = "/sbin/nfsdcltrack";
> +module_param_string(cltrack_prog, cltrack_prog, sizeof(cltrack_prog),
> + S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
> +
> +static int
> +nfsd4_cltrack_upcall(char *cmd, char *arg)
> +{
> + char *envp[] = { "HOME=/",
> + "TERM=linux",
> + "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
Dumb question: why does the upcall program need these environment
variables?
--b.
> + NULL
> + };
> + char *argv[4];
> + int ret;
> +
> + if (unlikely(!cltrack_prog[0])) {
> + dprintk("%s: cltrack_prog is disabled\n", __func__);
> + return -EACCES;
> + }
> +
> + dprintk("%s: cmd: %s\n", __func__, cmd);
> + dprintk("%s: arg: %s\n", __func__, arg ? arg : "(null)");
> +
> + argv[0] = (char *)cltrack_prog;
> + argv[1] = cmd;
> + argv[2] = arg;
> + argv[3] = NULL;
> +
> + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
> + /*
> + * Disable the upcall mechanism if we're getting an ENOENT
> + * error. The admin can re-enable it on the fly by using sysfs
> + * once the problem has been fixed.
> + */
> + if (ret == -ENOENT || ret == -EACCES) {
> + printk(KERN_ERR "NFSD: %s was not found or isn't executable. "
> + "Please reset nfsd.cltrack_prog module parameter "
> + "once problem is resolved (%d)!\n",
> + cltrack_prog, ret);
> + cltrack_prog[0] = '\0';
> + }
> + dprintk("%s: %s return value: %d\n", __func__, cltrack_prog, ret);
> +
> + return ret;
> +}
> +
> +static char *
> +bin_to_hex_dup(const unsigned char *src, int srclen)
> +{
> + int i;
> + char *buf, *hex;
> +
> + /* +1 for terminating NULL */
> + buf = kmalloc((srclen * 2) + 1, GFP_KERNEL);
> + if (!buf)
> + return buf;
> +
> + hex = buf;
> + for (i = 0; i < srclen; i++) {
> + sprintf(hex, "%2.2x", *src++);
> + hex += 2;
> + }
> + return buf;
> +}
> +
> +static int
> +nfsd4_umh_cltrack_init(struct net __attribute__((unused)) *net)
> +{
> + return nfsd4_cltrack_upcall("init", NULL);
> +}
> +
> +static void
> +nfsd4_umh_cltrack_create(struct nfs4_client *clp)
> +{
> + char *hexid;
> +
> + hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> + if (!hexid) {
> + dprintk("%s: can't allocate memory for upcall!\n", __func__);
> + return;
> + }
> + nfsd4_cltrack_upcall("create", hexid);
> + kfree(hexid);
> +}
> +
> +static void
> +nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
> +{
> + char *hexid;
> +
> + hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> + if (!hexid) {
> + dprintk("%s: can't allocate memory for upcall!\n", __func__);
> + return;
> + }
> + nfsd4_cltrack_upcall("remove", hexid);
> + kfree(hexid);
> +}
> +
> +static int
> +nfsd4_umh_cltrack_check(struct nfs4_client *clp)
> +{
> + int ret;
> + char *hexid;
> +
> + hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> + if (!hexid) {
> + dprintk("%s: can't allocate memory for upcall!\n", __func__);
> + return -ENOMEM;
> + }
> + ret = nfsd4_cltrack_upcall("check", hexid);
> + kfree(hexid);
> + return ret;
> +}
> +
> +static void
> +nfsd4_umh_cltrack_grace_done(struct net __attribute__((unused)) *net,
> + time_t boot_time)
> +{
> + char timestr[22]; /* FIXME: better way to determine max size? */
> +
> + sprintf(timestr, "%ld", boot_time);
> + nfsd4_cltrack_upcall("gracedone", timestr);
> +}
> +
> +static struct nfsd4_client_tracking_ops nfsd4_umh_tracking_ops = {
> + .init = nfsd4_umh_cltrack_init,
> + .exit = NULL,
> + .create = nfsd4_umh_cltrack_create,
> + .remove = nfsd4_umh_cltrack_remove,
> + .check = nfsd4_umh_cltrack_check,
> + .grace_done = nfsd4_umh_cltrack_grace_done,
> +};
> +
> int
> nfsd4_client_tracking_init(struct net *net)
> {
> @@ -956,7 +1092,8 @@ void
> nfsd4_client_tracking_exit(struct net *net)
> {
> if (client_tracking_ops) {
> - client_tracking_ops->exit(net);
> + if (client_tracking_ops->exit)
> + client_tracking_ops->exit(net);
> client_tracking_ops = NULL;
> }
> }
> --
> 1.7.11.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] nfsd: add a usermodehelper upcall for NFSv4 client ID tracking
2012-10-05 14:36 ` J. Bruce Fields
@ 2012-10-05 14:47 ` Jeff Layton
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2012-10-05 14:47 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
On Fri, 5 Oct 2012 10:36:10 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Wed, Oct 03, 2012 at 08:40:29AM -0400, Jeff Layton wrote:
> > Add a new client tracker upcall type that uses call_usermodehelper to
> > call out to a program. This seems to be the preferred method of
> > calling out to usermode these days for seldom-called upcalls. It's
> > simple and doesn't require a running daemon, so it should "just work"
> > as long as the binary is installed.
> >
> > The client tracking exit operation is also changed to check for a
> > NULL pointer before running. The UMH upcall doesn't need to do anything
> > at module teardown time.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfsd/nfs4recover.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 138 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > index 43295d4..481cb3e 100644
> > --- a/fs/nfsd/nfs4recover.c
> > +++ b/fs/nfsd/nfs4recover.c
> > @@ -926,6 +926,142 @@ static struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops = {
> > .grace_done = nfsd4_cld_grace_done,
> > };
> >
> > +/* upcall via usermodehelper */
> > +static char cltrack_prog[PATH_MAX] = "/sbin/nfsdcltrack";
> > +module_param_string(cltrack_prog, cltrack_prog, sizeof(cltrack_prog),
> > + S_IRUGO|S_IWUSR);
> > +MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
> > +
> > +static int
> > +nfsd4_cltrack_upcall(char *cmd, char *arg)
> > +{
> > + char *envp[] = { "HOME=/",
> > + "TERM=linux",
> > + "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
>
> Dumb question: why does the upcall program need these environment
> variables?
>
> --b.
>
It probably doesn't. I copied this code from the osd_login upcall. I
think it sets those because the osd_login program is a shell script and
some of the stuff it calls expects them. We could probably remove
them here.
BTW, I agree that this code is probably best deferred until 3.8...
> > + NULL
> > + };
> > + char *argv[4];
> > + int ret;
> > +
> > + if (unlikely(!cltrack_prog[0])) {
> > + dprintk("%s: cltrack_prog is disabled\n", __func__);
> > + return -EACCES;
> > + }
> > +
> > + dprintk("%s: cmd: %s\n", __func__, cmd);
> > + dprintk("%s: arg: %s\n", __func__, arg ? arg : "(null)");
> > +
> > + argv[0] = (char *)cltrack_prog;
> > + argv[1] = cmd;
> > + argv[2] = arg;
> > + argv[3] = NULL;
> > +
> > + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
> > + /*
> > + * Disable the upcall mechanism if we're getting an ENOENT
> > + * error. The admin can re-enable it on the fly by using sysfs
> > + * once the problem has been fixed.
> > + */
> > + if (ret == -ENOENT || ret == -EACCES) {
> > + printk(KERN_ERR "NFSD: %s was not found or isn't executable. "
> > + "Please reset nfsd.cltrack_prog module parameter "
> > + "once problem is resolved (%d)!\n",
> > + cltrack_prog, ret);
> > + cltrack_prog[0] = '\0';
> > + }
> > + dprintk("%s: %s return value: %d\n", __func__, cltrack_prog, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static char *
> > +bin_to_hex_dup(const unsigned char *src, int srclen)
> > +{
> > + int i;
> > + char *buf, *hex;
> > +
> > + /* +1 for terminating NULL */
> > + buf = kmalloc((srclen * 2) + 1, GFP_KERNEL);
> > + if (!buf)
> > + return buf;
> > +
> > + hex = buf;
> > + for (i = 0; i < srclen; i++) {
> > + sprintf(hex, "%2.2x", *src++);
> > + hex += 2;
> > + }
> > + return buf;
> > +}
> > +
> > +static int
> > +nfsd4_umh_cltrack_init(struct net __attribute__((unused)) *net)
> > +{
> > + return nfsd4_cltrack_upcall("init", NULL);
> > +}
> > +
> > +static void
> > +nfsd4_umh_cltrack_create(struct nfs4_client *clp)
> > +{
> > + char *hexid;
> > +
> > + hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> > + if (!hexid) {
> > + dprintk("%s: can't allocate memory for upcall!\n", __func__);
> > + return;
> > + }
> > + nfsd4_cltrack_upcall("create", hexid);
> > + kfree(hexid);
> > +}
> > +
> > +static void
> > +nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
> > +{
> > + char *hexid;
> > +
> > + hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> > + if (!hexid) {
> > + dprintk("%s: can't allocate memory for upcall!\n", __func__);
> > + return;
> > + }
> > + nfsd4_cltrack_upcall("remove", hexid);
> > + kfree(hexid);
> > +}
> > +
> > +static int
> > +nfsd4_umh_cltrack_check(struct nfs4_client *clp)
> > +{
> > + int ret;
> > + char *hexid;
> > +
> > + hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
> > + if (!hexid) {
> > + dprintk("%s: can't allocate memory for upcall!\n", __func__);
> > + return -ENOMEM;
> > + }
> > + ret = nfsd4_cltrack_upcall("check", hexid);
> > + kfree(hexid);
> > + return ret;
> > +}
> > +
> > +static void
> > +nfsd4_umh_cltrack_grace_done(struct net __attribute__((unused)) *net,
> > + time_t boot_time)
> > +{
> > + char timestr[22]; /* FIXME: better way to determine max size? */
> > +
> > + sprintf(timestr, "%ld", boot_time);
> > + nfsd4_cltrack_upcall("gracedone", timestr);
> > +}
> > +
> > +static struct nfsd4_client_tracking_ops nfsd4_umh_tracking_ops = {
> > + .init = nfsd4_umh_cltrack_init,
> > + .exit = NULL,
> > + .create = nfsd4_umh_cltrack_create,
> > + .remove = nfsd4_umh_cltrack_remove,
> > + .check = nfsd4_umh_cltrack_check,
> > + .grace_done = nfsd4_umh_cltrack_grace_done,
> > +};
> > +
> > int
> > nfsd4_client_tracking_init(struct net *net)
> > {
> > @@ -956,7 +1092,8 @@ void
> > nfsd4_client_tracking_exit(struct net *net)
> > {
> > if (client_tracking_ops) {
> > - client_tracking_ops->exit(net);
> > + if (client_tracking_ops->exit)
> > + client_tracking_ops->exit(net);
> > client_tracking_ops = NULL;
> > }
> > }
> > --
> > 1.7.11.4
> >
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] nfsd: change heuristic for selecting the client_tracking_ops
2012-10-03 12:40 [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking Jeff Layton
2012-10-03 12:40 ` [PATCH v2 1/3] nfsd: add a usermodehelper upcall for NFSv4 client ID tracking Jeff Layton
@ 2012-10-03 12:40 ` Jeff Layton
2012-10-03 12:40 ` [PATCH v2 3/3] nfsd: pass info about the legacy recoverydir in environment variables Jeff Layton
2012-10-05 14:34 ` [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking J. Bruce Fields
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2012-10-03 12:40 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
First, try to use the new usermodehelper upcall. It should succeed or
fail quickly, so there's little cost to doing so.
If it fails, and the legacy tracking dir exists, use that. If it
doesn't exist then fall back to using nfsdcld.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4recover.c | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 481cb3e..5a74037 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -1068,17 +1068,35 @@ nfsd4_client_tracking_init(struct net *net)
int status;
struct path path;
- if (!client_tracking_ops) {
- client_tracking_ops = &nfsd4_cld_tracking_ops;
- status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW, &path);
- if (!status) {
- if (S_ISDIR(path.dentry->d_inode->i_mode))
- client_tracking_ops =
- &nfsd4_legacy_tracking_ops;
- path_put(&path);
- }
+ /* just run the init if it the method is already decided */
+ if (client_tracking_ops)
+ goto do_init;
+
+ /*
+ * First, try a UMH upcall. It should succeed or fail quickly, so
+ * there's little harm in trying that first.
+ */
+ client_tracking_ops = &nfsd4_umh_tracking_ops;
+ status = client_tracking_ops->init(net);
+ if (!status)
+ return status;
+
+ /*
+ * See if the recoverydir exists and is a directory. If it is,
+ * then use the legacy ops.
+ */
+ client_tracking_ops = &nfsd4_legacy_tracking_ops;
+ status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW, &path);
+ if (!status) {
+ status = S_ISDIR(path.dentry->d_inode->i_mode);
+ path_put(&path);
+ if (status)
+ goto do_init;
}
+ /* Finally, try to use nfsdcld */
+ client_tracking_ops = &nfsd4_cld_tracking_ops;
+do_init:
status = client_tracking_ops->init(net);
if (status) {
printk(KERN_WARNING "NFSD: Unable to initialize client "
--
1.7.11.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/3] nfsd: pass info about the legacy recoverydir in environment variables
2012-10-03 12:40 [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking Jeff Layton
2012-10-03 12:40 ` [PATCH v2 1/3] nfsd: add a usermodehelper upcall for NFSv4 client ID tracking Jeff Layton
2012-10-03 12:40 ` [PATCH v2 2/3] nfsd: change heuristic for selecting the client_tracking_ops Jeff Layton
@ 2012-10-03 12:40 ` Jeff Layton
2012-10-05 14:34 ` [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking J. Bruce Fields
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Layton @ 2012-10-03 12:40 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
The usermodehelper upcall program can then decide to use this info as
a (one-way) transition mechanism to the new scheme. When a "check"
upcall occurs and the client doesn't exist in the database, we can
look to see whether the directory exists. If it does, then we'd add
the client to the database, remove the legacy recdir, and return
success to the kernel to allow the recovery to proceed.
For gracedone, we simply pass the v4recovery "topdir" so that the
upcall can clean it out prior to returning to the kernel.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfsd/nfs4recover.c | 99 ++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 87 insertions(+), 12 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 5a74037..e1d9e2e 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -932,14 +932,77 @@ module_param_string(cltrack_prog, cltrack_prog, sizeof(cltrack_prog),
S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
+static bool cltrack_legacy_disable;
+module_param(cltrack_legacy_disable, bool, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(cltrack_legacy_disable,
+ "Disable legacy recoverydir conversion. Default: false");
+
+#define LEGACY_TOPDIR_ENV_PREFIX "NFSDCLTRACK_LEGACY_TOPDIR="
+#define LEGACY_RECDIR_ENV_PREFIX "NFSDCLTRACK_LEGACY_RECDIR="
+
+static char *
+nfsd4_cltrack_legacy_topdir(void)
+{
+ int copied;
+ size_t len;
+ char *result;
+
+ /* don't build the string if legacy conversion is disabled */
+ if (cltrack_legacy_disable)
+ return NULL;
+
+ len = strlen(LEGACY_TOPDIR_ENV_PREFIX) +
+ strlen(nfs4_recoverydir()) + 1;
+
+ result = kmalloc(len, GFP_KERNEL);
+ if (!result)
+ return result;
+
+ copied = snprintf(result, len, LEGACY_TOPDIR_ENV_PREFIX "%s",
+ nfs4_recoverydir());
+ if (copied >= len) {
+ /* just return nothing if output was truncated */
+ kfree(result);
+ return NULL;
+ }
+
+ return result;
+}
+
+static char *
+nfsd4_cltrack_legacy_recdir(const char *recdir)
+{
+ int copied;
+ size_t len;
+ char *result;
+
+ /* don't build the string if legacy conversion is disabled */
+ if (cltrack_legacy_disable)
+ return NULL;
+
+ /* +1 is for '/' between "topdir" and "recdir" */
+ len = strlen(LEGACY_RECDIR_ENV_PREFIX) +
+ strlen(nfs4_recoverydir()) + 1 + HEXDIR_LEN;
+
+ result = kmalloc(len, GFP_KERNEL);
+ if (!result)
+ return result;
+
+ copied = snprintf(result, len, LEGACY_RECDIR_ENV_PREFIX "%s/%s",
+ nfs4_recoverydir(), recdir);
+ if (copied >= len) {
+ /* just return nothing if output was truncated */
+ kfree(result);
+ return NULL;
+ }
+
+ return result;
+}
+
static int
-nfsd4_cltrack_upcall(char *cmd, char *arg)
+nfsd4_cltrack_upcall(char *cmd, char *arg, char *legacy)
{
- char *envp[] = { "HOME=/",
- "TERM=linux",
- "PATH=/sbin:/usr/sbin:/bin:/usr/bin",
- NULL
- };
+ char *envp[5];
char *argv[4];
int ret;
@@ -950,6 +1013,13 @@ nfsd4_cltrack_upcall(char *cmd, char *arg)
dprintk("%s: cmd: %s\n", __func__, cmd);
dprintk("%s: arg: %s\n", __func__, arg ? arg : "(null)");
+ dprintk("%s: legacy: %s\n", __func__, legacy ? legacy : "(null)");
+
+ envp[0] = "HOME=/";
+ envp[1] = "TERM=linux";
+ envp[2] = "PATH=/sbin:/usr/sbin:/bin:/usr/bin";
+ envp[3] = legacy;
+ envp[4] = NULL;
argv[0] = (char *)cltrack_prog;
argv[1] = cmd;
@@ -996,7 +1066,7 @@ bin_to_hex_dup(const unsigned char *src, int srclen)
static int
nfsd4_umh_cltrack_init(struct net __attribute__((unused)) *net)
{
- return nfsd4_cltrack_upcall("init", NULL);
+ return nfsd4_cltrack_upcall("init", NULL, NULL);
}
static void
@@ -1009,7 +1079,7 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)
dprintk("%s: can't allocate memory for upcall!\n", __func__);
return;
}
- nfsd4_cltrack_upcall("create", hexid);
+ nfsd4_cltrack_upcall("create", hexid, NULL);
kfree(hexid);
}
@@ -1023,7 +1093,7 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
dprintk("%s: can't allocate memory for upcall!\n", __func__);
return;
}
- nfsd4_cltrack_upcall("remove", hexid);
+ nfsd4_cltrack_upcall("remove", hexid, NULL);
kfree(hexid);
}
@@ -1031,14 +1101,16 @@ static int
nfsd4_umh_cltrack_check(struct nfs4_client *clp)
{
int ret;
- char *hexid;
+ char *hexid, *legacy;
hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
if (!hexid) {
dprintk("%s: can't allocate memory for upcall!\n", __func__);
return -ENOMEM;
}
- ret = nfsd4_cltrack_upcall("check", hexid);
+ legacy = nfsd4_cltrack_legacy_recdir(clp->cl_recdir);
+ ret = nfsd4_cltrack_upcall("check", hexid, legacy);
+ kfree(legacy);
kfree(hexid);
return ret;
}
@@ -1047,10 +1119,13 @@ static void
nfsd4_umh_cltrack_grace_done(struct net __attribute__((unused)) *net,
time_t boot_time)
{
+ char *legacy;
char timestr[22]; /* FIXME: better way to determine max size? */
sprintf(timestr, "%ld", boot_time);
- nfsd4_cltrack_upcall("gracedone", timestr);
+ legacy = nfsd4_cltrack_legacy_topdir();
+ nfsd4_cltrack_upcall("gracedone", timestr, legacy);
+ kfree(legacy);
}
static struct nfsd4_client_tracking_ops nfsd4_umh_tracking_ops = {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking
2012-10-03 12:40 [PATCH v2 0/3] nfsd: add a usermodehelper upcall for client id tracking Jeff Layton
` (2 preceding siblings ...)
2012-10-03 12:40 ` [PATCH v2 3/3] nfsd: pass info about the legacy recoverydir in environment variables Jeff Layton
@ 2012-10-05 14:34 ` J. Bruce Fields
3 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-10-05 14:34 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs
On Wed, Oct 03, 2012 at 08:40:28AM -0400, Jeff Layton wrote:
> This is a respin of the set with the same title that I sent on Monday
> (Oct 1st). This one has a few bugfixes, and adds some code to allow the
> upcall program to convert from the legacy database to the new one. I'll
> also be posting a respin of the companion nfs-utils set too.
>
> The idea here is to allow a one-way conversion from the legacy clientid
> tracking code to the new one, which should allow for seamless kernel and
> nfs-utils upgrades.
>
> In order to handle that, this set also changes the heuristic that the
> kernel uses to decide what client tracker to use. We now must now try
> the usermodehelper upcall first and only fall back to other schemes if
> its initialization fails.
>
> Note that I'm not 100% sold on the idea to convert from the legacy db
> format. It's doable but somewhat "fiddly" to handle. There are probably
> corner cases where it will fall down, so it may be best not to even try
> to do that. I won't be offended if you decide not to take the last
> patch.
>
> Comments? Discuss!
So that's take 3 at reboot recovery. Take 1 is what people actually use
(despite it being ugly), take 2 nobody's using yet--so it could probably
be ripped out fairly quickly.
I don't see anything objectionable on a quick skim.
I'd rather wait a little longer and consider it for the next merge
window.
--b.
^ permalink raw reply [flat|nested] 7+ messages in thread