Re: [PATCH] libmultipath: fix multipath -q command logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hannes, Ben,

are you ok with the solution to these two issues.
Seems sane to me.

Thanks,
Christophe

On Tue, Oct 11, 2016 at 8:46 AM, <tang.junhui@xxxxxxxxxx> wrote:
Please have a review for this patch, any comment will be highly appreciated.




发件人:         tang.junhui@xxxxxxxxxx
收件人:         christophe varoqui <christophe.varoqui@xxxxxxx>,
抄送:        dm-devel@xxxxxxxxxx, zhang.kai16@xxxxxxxxxx, "tang.junhui" <tang.junhui@xxxxxxxxxx>
日期:         2016/08/16 19:33
主题:        [PATCH] libmultipath: fix multipath -q command logic




From: "tang.junhui" <tang.junhui@xxxxxxxxxx>

multipath judged whether multipathd service in running by check_daemon() when executing
mutipath commands, check_daemon() try to connect to the multipathd service and execute
"show dameon" command. The expected result is that the command will be failed when the
service is not running, however, mpath_connect() will activate the multipathd service when
the service is not running, so check_daemon() always return with true. Another problem is that
multipath command with -q parameter is not processed in coalesce_paths(). This patch fix for
those two problems.

Signed-off-by: tang.junhui <tang.junhui@xxxxxxxxxx>
---
libmultipath/configure.c | 85 +++++++++++++++++++++++++++---------------------
1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 707e6be..d8a17a6 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -715,36 +715,36 @@ deadmap (struct multipath * mpp)
                 return 1; /* dead */
}

-int check_daemon(void)
+static int
+check_daemon(void)
{
                 int fd;
-                 char *reply;
-                 int ret = 0;
-                 unsigned int timeout;
-                 struct config *conf;
-
-                 fd = mpath_connect();
-                 if (fd == -1)
-                                  return 0;
+                 struct flock lock;

-                 if (send_packet(fd, "show daemon") != 0)
-                                  goto out;
-                 conf = get_multipath_config();
-                 timeout = conf->uxsock_timeout;
-                 put_multipath_config(conf);
-                 if (recv_packet(fd, &reply, timeout) != 0)
-                                  goto out;
-
-                 if (strstr(reply, "shutdown"))
-                                  goto out_free;
-
-                 ret = 1;
+                 fd = open(DEFAULT_PIDFILE, O_RDONLY);
+                 if (fd < 0) {
+                                  if (errno == ENOENT)
+                                                   return 0;
+                                  if (errno == EMFILE)
+                                                   condlog(0, "failed to open file, increase max_fds at %s", DEFAULT_CONFIGFILE);
+                                  else
+                                                   condlog(0, "can not open pidfile %s: %s", DEFAULT_PIDFILE, strerror(errno));
+                                  return -1;
+                 }

-out_free:
-                 FREE(reply);
-out:
-                 mpath_disconnect(fd);
-                 return ret;
+                 lock.l_type = F_WRLCK;
+                 lock.l_start = 0;
+                 lock.l_whence = SEEK_SET;
+                 lock.l_len = 0;
+                 if (fcntl(fd, F_GETLK, &lock) < 0) {
+                                  condlog(0, "can not get file locker, %s: %s", DEFAULT_PIDFILE, strerror(errno));
+                                  close(fd);
+                                  return -1;
+                 }
+                 close(fd);
+                 if (lock.l_type == F_UNLCK)
+                                  return 0;
+                 return 1;
}

extern int
@@ -873,17 +873,28 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
                                  if (r == DOMAP_DRY)
                                                   continue;

-                                  conf = get_multipath_config();
-                                  allow_queueing = conf->allow_queueing;
-                                  put_multipath_config(conf);
-                                  if (!is_daemon && !allow_queueing && !check_daemon()) {
-                                                   if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF &&
-                                                       mpp->no_path_retry != NO_PATH_RETRY_FAIL)
-                                                                    condlog(3, "%s: multipathd not running, unset "
-                                                                                     "queue_if_no_path feature", mpp->alias);
-                                                   if (!dm_queue_if_no_path(mpp->alias, 0))
-                                                                    remove_feature(&mpp->features,
-                                                                                            "queue_if_no_path");
+                                  /* run as multipath command and the service is not running */
+                                  if (!is_daemon && !check_daemon()) {
+                                                   conf = get_multipath_config();
+                                                   allow_queueing = conf->allow_queueing;
+                                                   put_multipath_config(conf);
+                                                   /* no -q choice */
+                                                   if (!allow_queueing) {
+                                                                    if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF &&
+                                                                                     mpp->no_path_retry != NO_PATH_RETRY_FAIL)
+                                                                                     condlog(3, "%s: multipathd not running, unset "
+                                                                                                      "queue_if_no_path feature", mpp->alias);
+                                                                    if (!dm_queue_if_no_path(mpp->alias, 0))
+                                                                                     remove_feature(&mpp->features,
+                                                                                                                       "queue_if_no_path");
+                                                   } else { /* with -q choice */
+                                                                    if (mpp->no_path_retry == NO_PATH_RETRY_UNDEF ||
+                                                                                     mpp->no_path_retry == NO_PATH_RETRY_FAIL)
+                                                                                     condlog(3, "%s: multipathd not running, set "
+                                                                                                      "queue_if_no_path feature", mpp->alias);
+                                                                    if (!dm_queue_if_no_path(mpp->alias, 1))
+                                                                                     add_feature(&mpp->features, "queue_if_no_path");
+                                                   }
                                  }
                                  else if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF) {
                                                   if (mpp->no_path_retry == NO_PATH_RETRY_FAIL) {
--
2.8.1.windows.1



--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel

[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux