Patchwork [BUG:2469] mgmt/glusterd: check process mode before performing mount

login
register
Submitter Kaushal M
Date 2011-07-18 07:34:50
Message ID <1310974490-18369-1-git-send-email-kaushal@gluster.com>
Download mbox | patch
Permalink /patch/7961/
State Changes Requested
Headers show

Comments

Kaushal M - 2011-07-18 07:34:50
Signed-off-by: Kaushal M <kaushal@gluster.com>
---
 glusterfsd/src/glusterfsd.c  |   16 +++++++++++++---
 libglusterfs/src/glusterfs.h |    1 +
 2 files changed, 14 insertions(+), 3 deletions(-)
Amar Tumballi - 2011-07-18 07:52:15
>
>
> +        if (process_mode != GF_CLIENT_PROCESS) {
> +                gf_log("", GF_LOG_TRACE,
> +                       "not a client process, not mounting to mount
> point");
> +                return 0;
> +        }
>

multiple things about this section.

* no need of another variable 'process_mode' as you can access
'ctx->process_mode' directly here.
* logging with 'GF_LOG_TRACE' level for a error case is not valid.
* you should not be returning '0' if it is an error.

Please fix all these, and re-send the patch.
Pavan - 2011-07-18 08:10:30
An alternate approach -

Why call create_fuse_mount, if you don't actually intend to do that ?
You could have something to the effect of the following in main():

if (ctx->process_mode == GF_CLIENT_PROCESS) {
         create_fuse_mount();
}

Pavan

On Monday 18 July 2011 01:04 PM, Kaushal M wrote:
>
> Signed-off-by: Kaushal M<kaushal@gluster.com>
> ---
>   glusterfsd/src/glusterfsd.c  |   16 +++++++++++++---
>   libglusterfs/src/glusterfs.h |    1 +
>   2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
> index b5d6bd6..796fd8d 100644
> --- a/glusterfsd/src/glusterfsd.c
> +++ b/glusterfsd/src/glusterfsd.c
> @@ -78,6 +78,10 @@
>
>   #include "daemon.h"
>
> +/* process mode definitions */
> +#define GF_SERVER_PROCESS   0
> +#define GF_CLIENT_PROCESS   1
> +#define GF_GLUSTERD_PROCESS 2
>
>   /* using argp for command line parsing */
>   static char gf_doc[] = "";
> @@ -190,10 +194,12 @@ int
>   create_fuse_mount (glusterfs_ctx_t *ctx)
>   {
>           int              ret = 0;
> +	int              process_mode = 0;
>           cmd_args_t      *cmd_args = NULL;
>           xlator_t        *master = NULL;
>
>           cmd_args =&ctx->cmd_args;
> +        process_mode = ctx->process_mode;
>
>           if (!cmd_args->mount_point) {
>                   gf_log ("", GF_LOG_TRACE,
> @@ -201,6 +207,12 @@ create_fuse_mount (glusterfs_ctx_t *ctx)
>                   return 0;
>           }
>
> +        if (process_mode != GF_CLIENT_PROCESS) {
> +                gf_log("", GF_LOG_TRACE,
> +                       "not a client process, not mounting to mount point");
> +                return 0;
> +        }
> +
>           master = GF_CALLOC (1, sizeof (*master),
>                               gfd_mt_xlator_t);
>           if (!master)
> @@ -834,9 +846,6 @@ generate_uuid ()
>           return gf_strdup (tmp_str);
>   }
>
> -#define GF_SERVER_PROCESS   0
> -#define GF_CLIENT_PROCESS   1
> -#define GF_GLUSTERD_PROCESS 2
>
>   static uint8_t
>   gf_get_process_mode (char *exec_name)
> @@ -1073,6 +1082,7 @@ parse_cmdline (int argc, char *argv[], glusterfs_ctx_t *ctx)
>           }
>
>           process_mode = gf_get_process_mode (argv[0]);
> +        ctx->process_mode = process_mode;
>
>           /* Make sure after the parsing cli, if '--volfile-server' option is
>              given, then '--volfile-id' is mandatory */
> diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
> index 78af68c..c746cfe 100644
> --- a/libglusterfs/src/glusterfs.h
> +++ b/libglusterfs/src/glusterfs.h
> @@ -343,6 +343,7 @@ struct _glusterfs_ctx {
>                                            indicate how many times the graph has
>                                            got changed */
>           pid_t               mtab_pid; /* pid of the process which updates the mtab */
> +        int                 process_mode; /*mode in which process is runninng*/
>   };
>   typedef struct _glusterfs_ctx glusterfs_ctx_t;
>

Patch

diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c
index b5d6bd6..796fd8d 100644
--- a/glusterfsd/src/glusterfsd.c
+++ b/glusterfsd/src/glusterfsd.c
@@ -78,6 +78,10 @@ 
 
 #include "daemon.h"
 
+/* process mode definitions */
+#define GF_SERVER_PROCESS   0
+#define GF_CLIENT_PROCESS   1
+#define GF_GLUSTERD_PROCESS 2
 
 /* using argp for command line parsing */
 static char gf_doc[] = "";
@@ -190,10 +194,12 @@  int
 create_fuse_mount (glusterfs_ctx_t *ctx)
 {
         int              ret = 0;
+	int              process_mode = 0;
         cmd_args_t      *cmd_args = NULL;
         xlator_t        *master = NULL;
 
         cmd_args = &ctx->cmd_args;
+        process_mode = ctx->process_mode;
 
         if (!cmd_args->mount_point) {
                 gf_log ("", GF_LOG_TRACE,
@@ -201,6 +207,12 @@  create_fuse_mount (glusterfs_ctx_t *ctx)
                 return 0;
         }
 
+        if (process_mode != GF_CLIENT_PROCESS) {
+                gf_log("", GF_LOG_TRACE,
+                       "not a client process, not mounting to mount point");
+                return 0;
+        }
+
         master = GF_CALLOC (1, sizeof (*master),
                             gfd_mt_xlator_t);
         if (!master)
@@ -834,9 +846,6 @@  generate_uuid ()
         return gf_strdup (tmp_str);
 }
 
-#define GF_SERVER_PROCESS   0
-#define GF_CLIENT_PROCESS   1
-#define GF_GLUSTERD_PROCESS 2
 
 static uint8_t
 gf_get_process_mode (char *exec_name)
@@ -1073,6 +1082,7 @@  parse_cmdline (int argc, char *argv[], glusterfs_ctx_t *ctx)
         }
 
         process_mode = gf_get_process_mode (argv[0]);
+        ctx->process_mode = process_mode;
 
         /* Make sure after the parsing cli, if '--volfile-server' option is
            given, then '--volfile-id' is mandatory */
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 78af68c..c746cfe 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -343,6 +343,7 @@  struct _glusterfs_ctx {
                                          indicate how many times the graph has
                                          got changed */
         pid_t               mtab_pid; /* pid of the process which updates the mtab */
+        int                 process_mode; /*mode in which process is runninng*/
 };
 typedef struct _glusterfs_ctx glusterfs_ctx_t;