SLES resync
Subsystems (2)
cover SLES resync
Hi Christophe,
here are some patches and bugfixes with have accumulated on my SLES11 branch. Please apply.
(Patch 37 is blank due to merging, so it'll be missing from
the patch set).
Hannes Reinecke (41):
libmultipath: Invalid check for mpp->wwid in dm_addmap() Remove newline from condlog()
Fixup pathgroup allocation in disassemble_map()
libmultipath: resource leak in read_value_block() Accept several whitespaces in bindings file
Add TAGS makefile target
libmultipath: Fix typo in mp_prio_handler()
Do not trigger a map reload on priority updates Introduce MP_FAST_IO_FAIL_UNSET
Checker name is not displayed on failure Valgrind fixes for prioritizer
Incorrect inquiry vendor length in hds prioritizer Print out multipath alias for flush_on_last_del messages Clarify setting origin in propsel.c
libmultipath: error checking in remove_features() Increase parameter buffer
Check return code from pathinfo()
Inconsistent string quoting
Switch off 'queue_if_no_path' before removing maps Double free in disassemble_map()
libmultipath: prio keyword ignored for multipath config Path checker should return PATH_DOWN when no path is found Do not call sysfs_get_timeout for non-SCSI devices Make log_pthread more robust
Print log messages when updating tables failed Update 'no_path_retry' correctly for failed paths Clean up uevent queue on shutdown
libmultipath: Print out uevent sequence number Fix race condition in stop_waiter_thread()
Use VECTOR_SIZE() defines
Make 'allocated' an integer in vector.h
Syntax error in /etc/init.d/boot.multipath multipath.init.suse: Update usage message multipath.conf.5: Clarify dev_loss_tmo settings Clarify dev_loss_tmo capping in multipath.conf.5
multipathd: Ignore errors when creating pidfile multipathd deadlocks during restart
multipathd: sighandlers might use uninitialized gvecs
multipathd: crash in reconfigure CLI command
multipathd: lock vectors during initial configuration
Petr Uzel (1):
prio: fix merging of prioritizers with different args
Makefile | 7 ++ libmultipath/alias.c | 2 +- libmultipath/config.h | 1 + libmultipath/configure.c | 13 ++-- libmultipath/configure.h | 2 +- libmultipath/devmapper.c | 36 ++++++++-- libmultipath/devmapper.h | 1 + libmultipath/dict.c | 108 +++++++++++++++--------------- libmultipath/discovery.c | 38 ++++++----- libmultipath/dmparser.c | 20 +++--- libmultipath/log_pthread.c | 101 +++++++++++++++++++--------- libmultipath/log_pthread.h | 10 ++- libmultipath/parser.c | 32 +++++---- libmultipath/prio.c | 51 ++++++++++++++- libmultipath/prio.h | 9 ++- libmultipath/prioritizers/alua.c | 1 + libmultipath/prioritizers/datacore.c | 1 + libmultipath/prioritizers/emc.c | 1 + libmultipath/prioritizers/hds.c | 6 +- libmultipath/prioritizers/hp_sw.c | 1 + libmultipath/prioritizers/ontap.c | 2 + libmultipath/prioritizers/rdac.c | 2 + libmultipath/prioritizers/weightedpath.c | 6 ++- libmultipath/propsel.c | 53 ++++++++------- libmultipath/structs.c | 7 ++ libmultipath/structs.h | 8 ++- libmultipath/structs_vec.c | 29 +++++--- libmultipath/sysfs.c | 2 +- libmultipath/uevent.c | 27 +++++++- libmultipath/uevent.h | 1 + libmultipath/util.c | 12 ++-- libmultipath/vector.c | 10 ++-- libmultipath/vector.h | 12 ++-- libmultipath/waiter.c | 5 +- multipath.conf.annotated | 1 + multipath/main.c | 8 +- multipath/multipath.conf.5 | 9 ++- multipath/multipath.init.suse | 6 +- multipathd/cli_handlers.c | 2 +- multipathd/main.c | 98 ++++++++++++--------------- 40 files changed, 468 insertions(+), 273 deletions(-)
Signature · 1 line
1.7.4.2
PATCH 1/42 libmultipath: Invalid check for mpp->wwid in dm_addmap()
mpp->wwid is an array, and so a check against NULL is wrong; we need to use strlen here. Found by coverity.
--- libmultipath/devmapper.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
libmultipath/devmapper.c +1 -1
@@ -281,7 +281,7 @@ dm_addmap (int task, const char *target, struct multipath *mpp, char * params,if (ro)dm_task_set_ro (dmt); -if (use_uuid && mpp->wwid){ +if (use_uuid &&strlen (mpp->wwid) >0 ){ prefixed_uuid =MALLOC (UUID_PREFIX_LEN +strlen (mpp->wwid) +1 );if (!prefixed_uuid) { condlog(0, "cannot create prefixed uuid : %s\n",
Signature · 1 line
1.7.4.2
PATCH 2/42 Remove newline from condlog()
In reply to Hannes Reineckecondlog() already adds a newline to each message.
--- libmultipath/devmapper.c | 8 ++++---- libmultipath/dmparser.c | 4 ++-- libmultipath/structs_vec.c | 2 +- libmultipath/sysfs.c | 2 +- libmultipath/uevent.c | 4 ++-- libmultipath/util.c | 12 ++++++------ multipath/main.c | 2 +- multipathd/main.c | 27 ++++++++++++++------------- 8 files changed, 31 insertions(+), 30 deletions(-)
libmultipath/devmapper.c +4 -4
@@ -284,7 +284,7 @@ dm_addmap (int task, const char *target, struct multipath *mpp, char * params,if (use_uuid &&strlen (mpp->wwid) >0 ){ prefixed_uuid =MALLOC (UUID_PREFIX_LEN +strlen (mpp->wwid) +1 );if (!prefixed_uuid) { -condlog (0 ,"cannot create prefixed uuid : %s\n" , +condlog (0 ,"cannot create prefixed uuid : %s" ,strerror (errno)); goto addout; }
@@ -302,7 +302,7 @@ dm_addmap (int task, const char *target, struct multipath *mpp, char * params,if (mpp->attribute_flags & (1 <<ATTR_GID ) && !dm_task_set_gid (dmt, mpp->gid)) goto freeout; -condlog (4 ,"%s: addmap [0 %llu %s %s]\n" , mpp->alias, mpp->size, +condlog (4 ,"%s: addmap [0 %llu %s %s]" , mpp->alias, mpp->size, target, params);dm_task_no_open_count (dmt);
@@ -931,7 +931,7 @@ dm_get_name(char *uuid) prefixed_uuid =MALLOC (UUID_PREFIX_LEN +strlen (uuid) +1 );if (!prefixed_uuid) { -condlog (0 ,"cannot create prefixed uuid : %s\n" , +condlog (0 ,"cannot create prefixed uuid : %s" ,strerror (errno)); goto freeout; }
@@ -1366,7 +1366,7 @@ int dm_reassign(const char *mapname)int r =0 , i;if (dm_dev_t (mapname, &dev_t[0 ],32 )) { -condlog (3 ,"%s: failed to get device number\n" , mapname); +condlog (3 ,"%s: failed to get device number" , mapname);return 1 ; }
libmultipath/dmparser.c +2 -2
@@ -100,7 +100,7 @@ assemble_map (struct multipath * mp, char * params, int len) shift =snprintf (p, freechar," %s %i 1" , mp->selector,VECTOR_SIZE (pgp->paths));if (shift >= freechar) { -condlog (0 ,"%s: params too small\n" , mp->alias); +condlog (0 ,"%s: params too small" , mp->alias);return 1 ; } p += shift;
@@ -113,7 +113,7 @@ assemble_map (struct multipath * mp, char * params, int len) && pp->priority > 0) tmp_minio = minio * pp->priority;if (!strlen (pp->dev_t) ) { -condlog (0 ,"dev_t not set for '%s'\n" , pp->dev); +condlog (0 ,"dev_t not set for '%s'" , pp->dev);return 1 ; } shift = snprintf(p, freechar, " %s %d",
libmultipath/structs_vec.c +1 -1
@@ -483,7 +483,7 @@ int update_multipath (struct vectors *vecs, char *mapname, int reset) mpp =find_mp_by_alias (vecs->mpvec, mapname);if (!mpp) { -condlog (3 ,"%s: multipath map not found\n" , mapname); +condlog (3 ,"%s: multipath map not found" , mapname);return 2 ; }
libmultipath/sysfs.c +1 -1
@@ -77,7 +77,7 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,condlog (4 ,"write to %s failed: %s" , devpath,strerror (errno)); size =0 ; }else if (size < value_len) { -condlog (4 ,"tried to write %ld to %s. Wrote %ld\n" , +condlog (4 ,"tried to write %ld to %s. Wrote %ld" , (long )value_len, devpath, (long )size); size =0 ; }
libmultipath/uevent.c +2 -2
@@ -422,10 +422,10 @@ int uevent_listen(void) err =udev_monitor_filter_add_match_subsystem_devtype (monitor,"block" ,NULL );if (err) -condlog (2 ,"failed to create filter : %s\n" ,strerror (-err)); +condlog (2 ,"failed to create filter : %s" ,strerror (-err)); err =udev_monitor_enable_receiving (monitor);if (err) { -condlog (2 ,"failed to enable receiving : %s\n" ,strerror (-err)); +condlog (2 ,"failed to enable receiving : %s" ,strerror (-err)); goto out; }while (1 ) {
libmultipath/util.c +6 -6
@@ -86,7 +86,7 @@ get_word (char * sentence, char ** word) *word =MALLOC (len +1 );if (!*word) { -condlog (0 ,"get_word : oom\n" ); +condlog (0 ,"get_word : oom" );return 0 ; }strncpy (*word, sentence, len);
@@ -178,7 +178,7 @@ devt2devname (char *devname, int devname_len, char *devt)char *p =strrchr (dev,'/' );if (!p) { -condlog (0 ,"No sysfs entry for %s\n" , +condlog (0 ,"No sysfs entry for %s" , block_path);return 1 ; }
@@ -208,7 +208,7 @@ devt2devname (char *devname, int devname_len, char *devt)if ((major == tmpmaj) && (minor == tmpmin)) {if (snprintf (block_path,sizeof (block_path),"/sys/block/%s" , dev) >=sizeof (block_path)) { -condlog (0 ,"device name %s is too long\n" , dev); +condlog (0 ,"device name %s is too long" , dev);fclose (fd);return 1 ; }
@@ -218,17 +218,17 @@ devt2devname (char *devname, int devname_len, char *devt)fclose (fd); skip_proc:if (strncmp (block_path,"/sys/block" ,10 )) { -condlog (3 ,"No device found for %u:%u\n" , major, minor); +condlog (3 ,"No device found for %u:%u" , major, minor);return 1 ; }if (stat (block_path, &statbuf) <0 ) { -condlog (0 ,"No sysfs entry for %s\n" , block_path); +condlog (0 ,"No sysfs entry for %s" , block_path);return 1 ; }if (S_ISDIR (statbuf.st_mode) ==0 ) { -condlog (0 ,"sysfs entry %s is not a directory\n" , block_path); +condlog (0 ,"sysfs entry %s is not a directory" , block_path);return 1 ; }basenamecpy ((const char *)block_path, devname, devname_len);
multipath/main.c +1 -1
@@ -535,7 +535,7 @@ main (int argc, char *argv[]) fd_limit.rlim_cur = conf->max_fds; fd_limit.rlim_max = conf->max_fds;if (setrlimit (RLIMIT_NOFILE , &fd_limit) <0 ) -condlog (0 ,"can't set open fds limit to %d : %s\n" , +condlog (0 ,"can't set open fds limit to %d : %s" , conf->max_fds,strerror (errno)); }
multipathd/main.c +14 -13
@@ -1154,7 +1154,7 @@ check_path (struct vectors * vecs, struct path * pp)* Synchronize with kernel state */ if (update_multipath_strings (pp->mpp, vecs->pathvec)) { -condlog (1 ,"%s: Could not synchronize with kernel state\n" , +condlog (1 ,"%s: Could not synchronize with kernel state" , pp->dev); pp->dmstate =PSTATE_UNDEF ; }
@@ -1611,7 +1611,7 @@ child (void * param)struct rlimit fd_limit;if (getrlimit (RLIMIT_NOFILE , &fd_limit) <0 ) { -condlog (0 ,"can't get open fds limit: %s\n" , +condlog (0 ,"can't get open fds limit: %s" ,strerror (errno)); fd_limit.rlim_cur =0 ; fd_limit.rlim_max =0 ;
@@ -1622,11 +1622,11 @@ child (void * param) fd_limit.rlim_max = conf->max_fds;if (setrlimit (RLIMIT_NOFILE , &fd_limit) <0 ) {condlog (0 ,"can't set open fds limit to " -"%lu/%lu : %s\n" , +"%lu/%lu : %s" , fd_limit.rlim_cur, fd_limit.rlim_max,strerror (errno)); }else { -condlog (3 ,"set open fds limit to %lu/%lu\n" , +condlog (3 ,"set open fds limit to %lu/%lu" , fd_limit.rlim_cur, fd_limit.rlim_max); } }
@@ -1713,7 +1713,8 @@ child (void * param)/* Now all the waitevent threads will start rushing in. */ while (vecs->lock.depth >0 ) {sleep (1 );/* This is weak. */ -condlog (3 ,"Have %d wait event checkers threads to de-alloc, waiting..\n" , vecs->lock.depth); +condlog (3 ,"Have %d wait event checkers threads to de-alloc," +" waiting..." , vecs->lock.depth); }pthread_mutex_destroy (vecs->lock.mutex);FREE (vecs->lock.mutex);
@@ -1900,18 +1901,18 @@ void * mpath_pr_event_handler_fn (void * pathp ) resp =mpath_alloc_prin_response (MPATH_PRIN_RKEY_SA );if (!resp){ -condlog (0 ,"%s Alloc failed for prin response \n" , pp->dev); +condlog (0 ,"%s Alloc failed for prin response" , pp->dev);return NULL ; } ret =prin_do_scsi_ioctl (pp->dev,MPATH_PRIN_RKEY_SA , resp,0 );if (ret !=MPATH_PR_SUCCESS ) { -condlog (0 ,"%s : pr in read keys service action failed. Error=%d\n" , pp->dev, ret); +condlog (0 ,"%s : pr in read keys service action failed. Error=%d" , pp->dev, ret); goto out; } -condlog (3 ," event pr=%d addlen=%d\n" ,resp->prin_descriptor.prin_readkeys.prgeneration, +condlog (3 ," event pr=%d addlen=%d" ,resp->prin_descriptor.prin_readkeys.prgeneration, resp->prin_descriptor.prin_readkeys.additional_length );if (resp->prin_descriptor.prin_readkeys.additional_length ==0 )
@@ -1933,7 +1934,7 @@ void * mpath_pr_event_handler_fn (void * pathp ) isFound =0 ;for (i =0 ; i < resp->prin_descriptor.prin_readkeys.additional_length/8 ; i++ ) { -condlog (2 ,"PR IN READKEYS[%d] reservation key:\n" ,i); +condlog (2 ,"PR IN READKEYS[%d] reservation key:" ,i);dumpHex ((char *)&resp->prin_descriptor.prin_readkeys.key_list[i*8 ],8 ,-1 );if (!memcmp (mpp->reservation_key, &resp->prin_descriptor.prin_readkeys.key_list[i*8 ],8 )) {
@@ -1945,7 +1946,7 @@ void * mpath_pr_event_handler_fn (void * pathp )if (!isFound) {condlog (0 ,"%s: Either device not registered or " , pp->dev); -condlog (0 ,"host is not authorised for registration. Skip path\n" ); +condlog (0 ,"host is not authorised for registration. Skip path" ); ret =MPATH_PR_OTHER ; goto out; }
@@ -1959,12 +1960,12 @@ void * mpath_pr_event_handler_fn (void * pathp ) } param->num_transportid =0 ; -condlog (3 ,"device %s:%s \n" , pp->dev, pp->mpp->wwid); +condlog (3 ,"device %s:%s" , pp->dev, pp->mpp->wwid); ret =prout_do_scsi_ioctl (pp->dev,MPATH_PROUT_REG_IGN_SA ,0 ,0 , param,0 );if (ret !=MPATH_PR_SUCCESS ) { -condlog (0 ,"%s: Reservation registration failed. Error: %d\n" , pp->dev, ret); +condlog (0 ,"%s: Reservation registration failed. Error: %d" , pp->dev, ret); } mpp->prflag =1 ;
@@ -1991,7 +1992,7 @@ int mpath_pr_event_handle(struct path *pp) rc =pthread_create (&thread,NULL , mpath_pr_event_handler_fn, pp);if (rc) { -condlog (0 ,"%s: ERROR; return code from pthread_create() is %d\n" , pp->dev, rc); +condlog (0 ,"%s: ERROR; return code from pthread_create() is %d" , pp->dev, rc);return -1 ; }pthread_attr_destroy (&attr);
Signature · 1 line
1.7.4.2
PATCH 3/42 Fixup pathgroup allocation in disassemble_map()
In reply to Hannes ReineckeThe check for empty path groups in disassemble_map() is not quite correct; we might end up removing the pathgroup vector even though there are some entries in it.
--- libmultipath/dmparser.c | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-)
libmultipath/dmparser.c +9 -6
@@ -232,13 +232,16 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp) num_pg =atoi (word);FREE (word); -if (num_pg >0 && !mpp->pg) { - mpp->pg =vector_alloc (); - -if (!mpp->pg) -return 1 ; - }else +if (num_pg >0 ) { +if (!mpp->pg) { + mpp->pg =vector_alloc (); +if (!mpp->pg) +return 1 ; + } + }else { +free_pgvec (mpp->pg,KEEP_PATHS ); mpp->pg =NULL ; + }/* * first pg to try
Signature · 1 line
1.7.4.2
PATCH 4/42 libmultipath: resource leak in read_value_block()
In reply to Hannes Reineckeread_value_block() allocates the vector 'elements', but doesn't free it on error.
--- libmultipath/parser.c | 32 +++++++++++++++++--------------- 1 files changed, 17 insertions(+), 15 deletions(-)
libmultipath/parser.c +16 -14
@@ -311,8 +311,10 @@ read_value_block(void) buf = (char *)MALLOC (MAXBUF ); -if (!buf) +if (!buf) { +vector_free (elements);return NULL ; + }while (read_line (buf,MAXBUF )) { vec =alloc_strvec (buf);
@@ -323,21 +325,20 @@ read_value_block(void)break ; } -if (VECTOR_SIZE (vec)) -for (i =0 ; i <VECTOR_SIZE (vec); i++) { - str =VECTOR_SLOT (vec, i); - dup = (char *)MALLOC (strlen (str) +1 ); -if (!dup) - goto out; -memcpy (dup, str,strlen (str)); - -if (!vector_alloc_slot (elements)) { -free_strvec (vec); - goto out1; - } +for (i =0 ; i <VECTOR_SIZE (vec); i++) { + str =VECTOR_SLOT (vec, i); + dup = (char *)MALLOC (strlen (str) +1 ); +if (!dup) + goto out; +memcpy (dup, str,strlen (str)); -vector_set_slot (elements, dup); +if (!vector_alloc_slot (elements)) { +free_strvec (vec); + goto out1; } + +vector_set_slot (elements, dup); + }free_strvec (vec); }memset (buf,0 ,MAXBUF );
@@ -348,6 +349,7 @@ out1:FREE (dup); out:FREE (buf); +vector_free (elements);return NULL ; }
Signature · 1 line
1.7.4.2
PATCH 5/42 prio: fix merging of prioritizers with different args
In reply to Hannes ReineckeFrom: Petr Uzel <[email protected]>
--- libmultipath/discovery.c | 12 ++++--- libmultipath/prio.c | 49 +++++++++++++++++++++++++++++- libmultipath/prio.h | 9 +++++- libmultipath/prioritizers/alua.c | 1 + libmultipath/prioritizers/datacore.c | 1 + libmultipath/prioritizers/emc.c | 1 + libmultipath/prioritizers/hds.c | 1 + libmultipath/prioritizers/hp_sw.c | 1 + libmultipath/prioritizers/ontap.c | 1 + libmultipath/prioritizers/rdac.c | 1 + libmultipath/prioritizers/weightedpath.c | 6 +++- libmultipath/propsel.c | 15 ++++----- libmultipath/structs.c | 3 ++ libmultipath/structs.h | 6 +++- libmultipath/structs_vec.c | 2 +- 15 files changed, 90 insertions(+), 19 deletions(-)
libmultipath/discovery.c +7 -5
@@ -777,21 +777,23 @@ get_prio (struct path * pp)if (!pp)return 0 ; -if (!pp->prio) { +struct prio * p = &pp->prio; + +if (!prio_selected (p)) {select_prio (pp); -if (!pp->prio) { +if (!prio_selected (p)) {condlog (3 ,"%s: no prio selected" , pp->dev);return 1 ; } } - pp->priority =prio_getprio (pp->prio, pp); + pp->priority =prio_getprio (p, pp);if (pp->priority <0 ) { -condlog (3 ,"%s: %s prio error" , pp->dev,prio_name (pp->prio)); +condlog (3 ,"%s: %s prio error" , pp->dev,prio_name (p)); pp->priority =PRIO_UNDEF ;return 1 ; }condlog (3 ,"%s: %s prio = %u" , - pp->dev,prio_name (pp->prio), pp->priority); + pp->dev,prio_name (p), pp->priority);return 0 ; }
libmultipath/prio.c +48 -1
70 lines not on this page · raw message
libmultipath/prio.h +8 -1
@@ -6,7 +6,10 @@*/ #include "checkers.h" #include "vector.h" -#include "structs.h" + +/* forward declaration to avoid circular dependency */ +struct path ; +#include "list.h" #include "memory.h"
@@ -41,6 +44,7 @@struct prio {void *handle; +int refcount;struct list_head node;char name[PRIO_NAME_LEN ];char args[PRIO_ARGS_LEN ];
@@ -52,6 +56,9 @@ void cleanup_prio (void);struct prio *add_prio (char *);struct prio *prio_lookup (char *);int prio_getprio (struct prio *,struct path *); +void prio_get (struct prio *,char *,char *); +void prio_put (struct prio *); +int prio_selected (struct prio *);char *prio_name (struct prio *);char *prio_args (struct prio *);int prio_set_args (struct prio *,char *);
libmultipath/prioritizers/alua.c +1 -0
@@ -16,6 +16,7 @@#include <debug.h> #include <prio.h> +#include <structs.h> #include "alua.h"
libmultipath/prioritizers/datacore.c +1 -0
@@ -24,6 +24,7 @@#include <sg_include.h> #include <debug.h> #include <prio.h> +#include <structs.h> #define INQ_REPLY_LEN 255#define INQ_CMD_CODE 0x12
libmultipath/prioritizers/emc.c +1 -0
@@ -5,6 +5,7 @@#include <sg_include.h> #include <debug.h> #include <prio.h> +#include <structs.h> #define INQUIRY_CMD 0x12#define INQUIRY_CMDLEN 6
libmultipath/prioritizers/hds.c +1 -0
@@ -75,6 +75,7 @@#include <sg_include.h> #include <debug.h> #include <prio.h> +#include <structs.h> #define INQ_REPLY_LEN 255#define INQ_CMD_CODE 0x12
libmultipath/prioritizers/hp_sw.c +1 -0
@@ -15,6 +15,7 @@#include <sg_include.h> #include <debug.h> #include <prio.h> +#include <structs.h> #define TUR_CMD_LEN 6#define SCSI_CHECK_CONDITION 0x2
libmultipath/prioritizers/ontap.c +1 -0
@@ -22,6 +22,7 @@#include <sg_include.h> #include <debug.h> #include <prio.h> +#include <structs.h> #define INQUIRY_CMD 0x12#define INQUIRY_CMDLEN 6
libmultipath/prioritizers/rdac.c +1 -0
@@ -5,6 +5,7 @@#include <sg_include.h> #include <debug.h> #include <prio.h> +#include <structs.h> #define INQUIRY_CMD 0x12#define INQUIRY_CMDLEN 6
libmultipath/prioritizers/weightedpath.c +5 -1
@@ -60,6 +60,10 @@ int prio_path_weight(struct path *pp, char *prio_args) regex =get_next_string (&temp, split_char); +/* Return default priority if the argument is not parseable */ +if (!regex) +return priority; +if (!strcmp (regex,HBTL )) {sprintf (path,"%d:%d:%d:%d" , pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.scsi_id, pp->sg_id.lun);
@@ -67,7 +71,7 @@ int prio_path_weight(struct path *pp, char *prio_args)strcpy (path, pp->dev); }else {condlog (0 ,"%s: %s - Invalid arguments" , pp->dev, - pp->prio->name); + pp->prio.name);return priority; }
libmultipath/propsel.c +6 -9
@@ -383,20 +383,19 @@ extern int select_prio (struct path * pp) {struct mpentry * mpe; +struct prio * p = &pp->prio;if ((mpe =find_mpe (pp->wwid))) {if (mpe->prio_name) { - pp->prio =prio_lookup (mpe->prio_name); -prio_set_args (pp->prio, mpe->prio_args); +prio_get (p, mpe->prio_name, mpe->prio_args);condlog (3 ,"%s: prio = %s (LUN setting)" , - pp->dev, pp->prio->name); + pp->dev,prio_name (p));return 0 ; } }if (pp->hwe && pp->hwe->prio_name) { - pp->prio =prio_lookup (pp->hwe->prio_name); -prio_set_args (pp->prio, pp->hwe->prio_args); +prio_get (p, pp->hwe->prio_name, pp->hwe->prio_name);condlog (3 ,"%s: prio = %s (controller setting)" , pp->dev, pp->hwe->prio_name); condlog(3, "%s: prio args = %s (controller setting)",
@@ -404,16 +403,14 @@ select_prio (struct path * pp)return 0 ; }if (conf->prio_name) { - pp->prio =prio_lookup (conf->prio_name); -prio_set_args (pp->prio, conf->prio_args); +prio_get (p, conf->prio_name, conf->prio_args);condlog (3 ,"%s: prio = %s (config file default)" , pp->dev, conf->prio_name);condlog (3 ,"%s: prio args = %s (config file default)" , pp->dev, conf->prio_args);return 0 ; } - pp->prio =prio_lookup (DEFAULT_PRIO ); -prio_set_args (pp->prio,DEFAULT_PRIO_ARGS ); +prio_get (p,DEFAULT_PRIO ,DEFAULT_PRIO_ARGS );condlog (3 ,"%s: prio = %s (internal default)" , pp->dev,DEFAULT_PRIO ); condlog(3, "%s: prio = %s (internal default)",
libmultipath/structs.c +3 -0
@@ -45,6 +45,9 @@ free_path (struct path * pp)if (checker_selected (&pp->checker))checker_put (&pp->checker); +if (prio_selected (&pp->prio)) +prio_put (&pp->prio); +if (pp->fd >=0 )close (pp->fd);
libmultipath/structs.h +5 -1
@@ -3,6 +3,8 @@#include <sys/types.h> +#include "prio.h" +#define WWID_SIZE 128#define SERIAL_SIZE 65#define NODE_NAME_SIZE 224
@@ -132,6 +134,7 @@ struct hd_geometry {unsigned long start; };#endif +struct path {char dev[FILE_NAME_SIZE ];char dev_t[BLK_DEV_SIZE ];
@@ -157,7 +160,8 @@ struct path {int priority;int pgindex;char * uid_attribute; -struct prio * prio; +struct prio prio; +char * prio_args;struct checker checker;struct multipath * mpp;int fd;
libmultipath/structs_vec.c +1 -1
@@ -82,7 +82,7 @@ orphan_path (struct path * pp) pp->mpp =NULL ; pp->dmstate =PSTATE_UNDEF ; pp->uid_attribute =NULL ; - pp->prio =NULL ; +prio_put (&pp->prio);checker_put (&pp->checker);if (pp->fd >=0 )close (pp->fd);
Signature · 1 line
1.7.4.2
PATCH 6/42 Accept several whitespaces in bindings file
In reply to Hannes ReineckePrior versions of multipathd would accept several whitespaces in the bindings file.
--- libmultipath/alias.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
libmultipath/alias.c +1 -1
@@ -99,7 +99,7 @@ lookup_binding(FILE *f, char *map_wwid, char **map_alias, char *prefix) curr_id =scan_devname (alias, prefix);if (curr_id >= id) id = curr_id +1 ; - wwid =strtok (NULL ,"" ); + wwid =strtok (NULL ," \t" );if (!wwid){ condlog(3, "Ignoring malformed line %u in bindings file",
Signature · 1 line
1.7.4.2
PATCH 7/42 Add TAGS makefile target
In reply to Hannes Reinecke--- Makefile | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
Makefile +7 -0
@@ -66,6 +66,13 @@ install: recurse_install uninstall: recurse_uninstall +.PHONY: TAGS +TAGS: + etags -a libmultipath/*.c + etags -a libmultipath/*.h + etags -a multipathd/*.c + etags -a multipathd/*.h + release: sed -e "s/__VERSION__/${VERSION}/" \ multipath-tools.spec.in > multipath-tools.spec
Signature · 1 line
1.7.4.2
PATCH 8/42 libmultipath: Fix typo in mp_prio_handler()
In reply to Hannes ReineckeThe mpentry is found in conf->mptable, not conf->hwtable.
--- libmultipath/dict.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
libmultipath/dict.c +1 -1
@@ -1622,7 +1622,7 @@ mp_flush_on_last_del_handler(vector strvec)static int mp_prio_handler (vector strvec) { -struct mpentry * mpe =VECTOR_LAST_SLOT (conf->hwtable); +struct mpentry * mpe =VECTOR_LAST_SLOT (conf->mptable);if (!mpe)return 1 ;
Signature · 1 line
1.7.4.2
PATCH 9/42 Do not trigger a map reload on priority updates
In reply to Hannes Reineckeupdate_path_groups() is just there to update the priority groups, so it should trigger a table reload only if the priority has indeed changed.
--- libmultipath/configure.c | 13 ++++++++----- libmultipath/configure.h | 2 +- multipathd/cli_handlers.c | 2 +- multipathd/main.c | 27 ++++++--------------------- 4 files changed, 16 insertions(+), 28 deletions(-)
libmultipath/configure.c +8 -5
@@ -765,14 +765,17 @@ out:return NULL ; } -extern int reload_map (struct vectors *vecs,struct multipath *mpp) +extern int reload_map (struct vectors *vecs,struct multipath *mpp,int refresh) { -char params[PARAMS_SIZE ]; -int r; +char params[PARAMS_SIZE ] = {0 }; +struct path *pp; +int i, r;update_mpp_paths (mpp, vecs->pathvec); - - params[0 ] ='\0' ; +if (refresh) { + vector_foreach_slot (mpp->paths, pp, i) +pathinfo (pp, conf->hwtable,DI_PRIO ); + }if (setup_map (mpp, params,PARAMS_SIZE )) {condlog (0 ,"%s: failed to setup map" , mpp->alias);return 1 ;
libmultipath/configure.h +1 -1
@@ -28,5 +28,5 @@ int domap (struct multipath * mpp, char * params);int reinstate_paths (struct multipath *mpp);int coalesce_paths (struct vectors *vecs,vector curmp,char * refwwid,int force_reload);char *get_refwwid (char * dev,enum devtypes dev_type,vector pathvec); -int reload_map (struct vectors *vecs,struct multipath *mpp); +int reload_map (struct vectors *vecs,struct multipath *mpp,int refresh);
multipathd/cli_handlers.c +1 -1
@@ -561,7 +561,7 @@ cli_reload(void *v, char **reply, int *len, void *data)return 1 ; } -return reload_map (vecs, mpp); +return reload_map (vecs, mpp,0 ); } int resize_map(struct multipath *mpp, unsigned long long size,
multipathd/main.c +6 -21
@@ -674,11 +674,12 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) uev->kernel);return 1 ; } -if (pp->mpp) - retval =reload_map (vecs, pp->mpp); +if (pp->mpp) { + retval =reload_map (vecs, pp->mpp,0 ); -condlog (2 ,"%s: map %s reloaded (retval %d)" , - uev->kernel, pp->mpp->alias, retval); +condlog (2 ,"%s: map %s reloaded (retval %d)" , + uev->kernel, pp->mpp->alias, retval); + } }
@@ -1086,25 +1087,9 @@ int update_prio(struct path *pp, int refresh_all)int update_path_groups (struct multipath *mpp,struct vectors *vecs,int refresh) { -int i; -struct path * pp; -char params[PARAMS_SIZE ]; - -update_mpp_paths (mpp, vecs->pathvec); -if (refresh) { - vector_foreach_slot (mpp->paths, pp, i) -pathinfo (pp, conf->hwtable,DI_PRIO ); - } - params[0 ] ='\0' ; -if (setup_map (mpp, params,PARAMS_SIZE )) +if (reload_map (vecs, mpp, refresh))return 1 ; - mpp->action =ACT_RELOAD ; -if (domap (mpp, params) <=0 ) { -condlog (0 ,"%s: failed to update map : %s" , mpp->alias, -strerror (errno)); -return 1 ; - }dm_lib_release ();if (setup_multipath (vecs, mpp) !=0 )return 1 ;
Signature · 1 line
1.7.4.2
PATCH 10/42 Introduce MP_FAST_IO_FAIL_UNSET
In reply to Hannes ReineckeFor completeness; all other special values are encoded with defines, so 'unset' should be, too.
--- libmultipath/config.h | 1 + libmultipath/dict.c | 8 ++++---- libmultipath/discovery.c | 6 +++--- libmultipath/propsel.c | 6 +++--- libmultipath/structs.c | 1 + 5 files changed, 12 insertions(+), 10 deletions(-)
libmultipath/config.h +1 -0
@@ -11,6 +11,7 @@* In kernel, fast_io_fail == 0 means immediate failure on rport delete. * OTOH '0' means not-configured in various places in multipath-tools. */ +#define MP_FAST_IO_FAIL_UNSET (0)#define MP_FAST_IO_FAIL_OFF (-1)#define MP_FAST_IO_FAIL_ZERO (-2)
libmultipath/dict.c +4 -4
@@ -47,7 +47,7 @@ def_fast_io_fail_handler(vector strvec) conf->fast_io_fail =MP_FAST_IO_FAIL_OFF ; else if (sscanf(buff, "%d", &conf->fast_io_fail) != 1 || conf->fast_io_fail < MP_FAST_IO_FAIL_ZERO) - conf->fast_io_fail =0 ; + conf->fast_io_fail =MP_FAST_IO_FAIL_UNSET ; else if (conf->fast_io_fail == 0) conf->fast_io_fail = MP_FAST_IO_FAIL_ZERO;
@@ -882,7 +882,7 @@ hw_fast_io_fail_handler(vector strvec) hwe->fast_io_fail =MP_FAST_IO_FAIL_OFF ; else if (sscanf(buff, "%d", &hwe->fast_io_fail) != 1 || hwe->fast_io_fail < MP_FAST_IO_FAIL_ZERO) - hwe->fast_io_fail =0 ; + hwe->fast_io_fail =MP_FAST_IO_FAIL_UNSET ; else if (hwe->fast_io_fail == 0) hwe->fast_io_fail = MP_FAST_IO_FAIL_ZERO;
@@ -1973,7 +1973,7 @@ static int snprint_hw_fast_io_fail(char * buff, int len, void * data) {struct hwentry * hwe = (struct hwentry *)data; -if (!hwe->fast_io_fail) +if (hwe->fast_io_fail ==MP_FAST_IO_FAIL_UNSET )return 0 ;if (hwe->fast_io_fail == conf->fast_io_fail)return 0 ;
@@ -2282,7 +2282,7 @@ snprint_def_polling_interval (char * buff, int len, void * data)static int snprint_def_fast_io_fail (char * buff,int len,void * data) { -if (!conf->fast_io_fail) +if (conf->fast_io_fail ==MP_FAST_IO_FAIL_UNSET )return 0 ;if (conf->fast_io_fail ==MP_FAST_IO_FAIL_OFF )return snprintf (buff, len,"off" );
libmultipath/discovery.c +3 -3
@@ -283,7 +283,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)snprintf (value,11 ,"%u" , mpp->dev_loss);if (mpp->dev_loss &&sysfs_attr_set_value (rport_dev,"dev_loss_tmo" , value,11 ) <=0 ) { -if ((!mpp->fast_io_fail || +if ((mpp->fast_io_fail ==MP_FAST_IO_FAIL_UNSET || mpp->fast_io_fail ==MP_FAST_IO_FAIL_OFF ) && mpp->dev_loss >600 ) { condlog(3, "%s: limiting dev_loss_tmo to 600, since "
@@ -296,7 +296,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp) goto out; } } -if (mpp->fast_io_fail){ +if (mpp->fast_io_fail !=MP_FAST_IO_FAIL_UNSET ){if (mpp->fast_io_fail ==MP_FAST_IO_FAIL_OFF )sprintf (value,"off" );else if (mpp->fast_io_fail ==MP_FAST_IO_FAIL_ZERO )
@@ -340,7 +340,7 @@ sysfs_set_scsi_tmo (struct multipath *mpp) mpp->alias, mpp->fast_io_fail); mpp->fast_io_fail =MP_FAST_IO_FAIL_OFF ; } -if (!mpp->dev_loss && !mpp->fast_io_fail) +if (!mpp->dev_loss && mpp->fast_io_fail ==MP_FAST_IO_FAIL_UNSET )return 0 ; vector_foreach_slot(mpp->paths, pp, i) {
libmultipath/propsel.c +3 -3
@@ -558,7 +558,7 @@ select_pg_timeout(struct multipath *mp)extern int select_fast_io_fail (struct multipath *mp) { -if (mp->hwe && mp->hwe->fast_io_fail) { +if (mp->hwe && mp->hwe->fast_io_fail !=MP_FAST_IO_FAIL_UNSET ) { mp->fast_io_fail = mp->hwe->fast_io_fail;if (mp->fast_io_fail ==MP_FAST_IO_FAIL_OFF )condlog (3 ,"%s: fast_io_fail_tmo = off (controller default)" , mp->alias);
@@ -567,7 +567,7 @@ select_fast_io_fail(struct multipath *mp) mp->fast_io_fail == MP_FAST_IO_FAIL_ZERO ? 0 : mp->fast_io_fail);return 0 ; } -if (conf->fast_io_fail) { +if (conf->fast_io_fail !=MP_FAST_IO_FAIL_UNSET ) { mp->fast_io_fail = conf->fast_io_fail;if (mp->fast_io_fail ==MP_FAST_IO_FAIL_OFF )condlog (3 ,"%s: fast_io_fail_tmo = off (config file default)" , mp->alias);
@@ -576,7 +576,7 @@ select_fast_io_fail(struct multipath *mp) mp->fast_io_fail == MP_FAST_IO_FAIL_ZERO ? 0 : mp->fast_io_fail);return 0 ; } - mp->fast_io_fail =0 ; + mp->fast_io_fail =MP_FAST_IO_FAIL_UNSET ;return 0 ; }
libmultipath/structs.c +1 -0
@@ -131,6 +131,7 @@ alloc_multipath (void) mpp->bestpg =1 ; mpp->mpcontext =NULL ; mpp->no_path_retry =NO_PATH_RETRY_UNDEF ; + mpp->fast_io_fail =MP_FAST_IO_FAIL_UNSET ; }return mpp; }
Signature · 1 line
1.7.4.2
PATCH 11/42 Checker name is not displayed on failure
In reply to Hannes ReineckeIf add_checker() isn't able to locate the checker
it won't display the name in free_checker().
--- libmultipath/prio.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
libmultipath/prio.c +1 -1
@@ -86,6 +86,7 @@ struct prio * add_prio (char * name) p =alloc_prio ();if (!p)return NULL ; +snprintf (p->name,PRIO_NAME_LEN ,"%s" , name);snprintf (libname,LIB_PRIO_NAMELEN ,"%s/libprio%s.so" , conf->multipath_dir, name);if (stat (libname,&stbuf) <0 ) {
@@ -107,7 +108,6 @@ struct prio * add_prio (char * name)condlog (0 ,"A dynamic linking error occurred: (%s)" , errstr);if (!p->getprio) goto out; -snprintf (p->name,PRIO_NAME_LEN ,"%s" , name);list_add (&p->node, &prioritizers);return p; out:
Signature · 1 line
1.7.4.2
PATCH 12/42 Valgrind fixes for prioritizer
In reply to Hannes ReineckeDeclaring an array does not zero out its contents. So we might be reading random garbage here.
--- libmultipath/prioritizers/hds.c | 1 + libmultipath/prioritizers/ontap.c | 1 + libmultipath/prioritizers/rdac.c | 1 + 3 files changed, 3 insertions(+), 0 deletions(-)
libmultipath/prioritizers/hds.c +1 -0
@@ -105,6 +105,7 @@ int hds_modular_prio (const char *dev, int fd) }memset (&io_hdr,0 ,sizeof (sg_io_hdr_t)); +memset (inqBuff,0 ,INQ_REPLY_LEN ); io_hdr.interface_id ='S' ; io_hdr.cmd_len =sizeof (inqCmdBlk); io_hdr.mx_sb_len =sizeof (sense_buffer);
libmultipath/prioritizers/ontap.c +1 -0
@@ -80,6 +80,7 @@ static int send_gva(const char *dev, int fd, unsigned char pg,int ret =-1 ;memset (&io_hdr,0 ,sizeof (struct sg_io_hdr )); +memset (results,0 , *results_size); io_hdr.interface_id ='S' ; io_hdr.cmd_len =sizeof (cdb); io_hdr.mx_sb_len =sizeof (sb);
libmultipath/prioritizers/rdac.c +1 -0
@@ -22,6 +22,7 @@ int rdac_prio(const char *dev, int fd)int ret =0 ;memset (&io_hdr,0 ,sizeof (struct sg_io_hdr )); +memset (sense_buffer,0 ,256 ); io_hdr.interface_id ='S' ; io_hdr.cmd_len =sizeof (inqCmdBlk); io_hdr.mx_sb_len =sizeof (sb);
Signature · 1 line
1.7.4.2
PATCH 13/42 Incorrect inquiry vendor length in hds prioritizer
In reply to Hannes ReineckeThe inquiry vendor length is 8 bytes, but snprintf writes the given number of bytes _including_ the NULL byte. So we need to supply a 9 byte buffer here.
--- libmultipath/prioritizers/hds.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
libmultipath/prioritizers/hds.c +2 -2
@@ -87,7 +87,7 @@int hds_modular_prio (const char *dev,int fd) {int k; -char vendor[8 ]; +char vendor[9 ];char product[32 ];char serial[32 ];char ldev[32 ];
@@ -125,7 +125,7 @@ int hds_modular_prio (const char *dev, int fd)return -1 ; } -snprintf (vendor,8 ,"%.8s" , inqBuffp +8 ); +snprintf (vendor,9 ,"%.8s" , inqBuffp +8 );snprintf (product,17 ,"%.16s" , inqBuffp +16 );snprintf (serial,5 ,"%.4s" , inqBuffp +40 );snprintf (ldev,5 ,"%.4s" , inqBuffp +44 );
Signature · 1 line
1.7.4.2
PATCH 14/42 Print out multipath alias for flush_on_last_del messages
In reply to Hannes ReineckeAdded for consistency.
--- libmultipath/propsel.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
libmultipath/propsel.c +8 -7
@@ -606,24 +606,25 @@ select_flush_on_last_del(struct multipath *mp)return 0 ;if (mp->mpe && mp->mpe->flush_on_last_del !=FLUSH_UNDEF ) { mp->flush_on_last_del = mp->mpe->flush_on_last_del; -condlog (3 ,"flush_on_last_del = %i (multipath setting)" , - mp->flush_on_last_del); +condlog (3 ,"%s: flush_on_last_del = %i (multipath setting)" , + mp->alias, mp->flush_on_last_del);return 0 ; }if (mp->hwe && mp->hwe->flush_on_last_del !=FLUSH_UNDEF ) { mp->flush_on_last_del = mp->hwe->flush_on_last_del; -condlog (3 ,"flush_on_last_del = %i (controler setting)" , - mp->flush_on_last_del); +condlog (3 ,"%s: flush_on_last_del = %i (controler setting)" , + mp->alias, mp->flush_on_last_del);return 0 ; }if (conf->flush_on_last_del !=FLUSH_UNDEF ) { mp->flush_on_last_del = conf->flush_on_last_del; -condlog (3 ,"flush_on_last_del = %i (config file default)" , - mp->flush_on_last_del); +condlog (3 ,"%s: flush_on_last_del = %i (config file default)" , + mp->alias, mp->flush_on_last_del);return 0 ; } mp->flush_on_last_del =FLUSH_UNDEF ; -condlog (3 ,"flush_on_last_del = DISABLED (internal default)" ); +condlog (3 ,"%s: flush_on_last_del = DISABLED (internal default)" , + mp->alias);return 0 ; }
Signature · 1 line
1.7.4.2
PATCH 15/42 Clarify setting origin in propsel.c
In reply to Hannes ReineckeWe should differentiate between config file and internal default. And the controller settings are not defaults.
--- libmultipath/propsel.c | 17 ++++++++++++----- 1 files changed, 12 insertions(+), 5 deletions(-)
libmultipath/propsel.c +12 -5
@@ -282,8 +282,11 @@ select_features (struct multipath * mp) }else if (mp->hwe && mp->hwe->features) { mp->features =STRDUP (mp->hwe->features); origin ="controller setting" ; - }else { + }else if (conf->features) { mp->features =STRDUP (conf->features); + origin ="config file default" ; + }else { + mp->features =set_default (DEFAULT_FEATURES ); origin ="internal default" ; } condlog(3, "%s: features = %s (%s)",
@@ -561,18 +564,22 @@ select_fast_io_fail(struct multipath *mp)if (mp->hwe && mp->hwe->fast_io_fail !=MP_FAST_IO_FAIL_UNSET ) { mp->fast_io_fail = mp->hwe->fast_io_fail;if (mp->fast_io_fail ==MP_FAST_IO_FAIL_OFF ) -condlog (3 ,"%s: fast_io_fail_tmo = off (controller default)" , mp->alias); +condlog (3 ,"%s: fast_io_fail_tmo = off " +"(controller setting)" , mp->alias);else -condlog (3 ,"%s: fast_io_fail_tmo = %d (controller default)" , mp->alias, +condlog (3 ,"%s: fast_io_fail_tmo = %d " +"(controller setting)" , mp->alias, mp->fast_io_fail ==MP_FAST_IO_FAIL_ZERO ?0 : mp->fast_io_fail);return 0 ; }if (conf->fast_io_fail !=MP_FAST_IO_FAIL_UNSET ) { mp->fast_io_fail = conf->fast_io_fail;if (mp->fast_io_fail ==MP_FAST_IO_FAIL_OFF ) -condlog (3 ,"%s: fast_io_fail_tmo = off (config file default)" , mp->alias); +condlog (3 ,"%s: fast_io_fail_tmo = off " +"(config file default)" , mp->alias);else -condlog (3 ,"%s: fast_io_fail_tmo = %d (config file default)" , mp->alias, +condlog (3 ,"%s: fast_io_fail_tmo = %d " +"(config file default)" , mp->alias, mp->fast_io_fail ==MP_FAST_IO_FAIL_ZERO ?0 : mp->fast_io_fail);return 0 ; }
Signature · 1 line
1.7.4.2
PATCH 16/42 libmultipath: error checking in remove_features()
In reply to Hannes ReineckeAn error check was missing in remove_features().
--- libmultipath/structs.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
libmultipath/structs.c +3 -0
@@ -559,6 +559,9 @@ remove_feature(char **f, char *o)* about to be removed */ p =strchr (*f,' ' ); +if (!p) +/* Internal error, feature string inconsistent */ +return 1 ;while (*p ==' ' ) p++; p--;
Signature · 1 line
1.7.4.2
PATCH 17/42 Increase parameter buffer
In reply to Hannes ReineckeMultipath is using an internal static buffer for assembling device-mapper tables, which might be too small for large setups.
--- libmultipath/structs.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
libmultipath/structs.h +1 -1
@@ -9,7 +9,7 @@#define SERIAL_SIZE 65#define NODE_NAME_SIZE 224#define PATH_STR_SIZE 16 -#define PARAMS_SIZE 1024 +#define PARAMS_SIZE 4096#define FILE_NAME_SIZE 256#define CALLOUT_MAX_SIZE 256#define BLK_DEV_SIZE 33
Signature · 1 line
1.7.4.2
PATCH 18/42 Check return code from pathinfo()
In reply to Hannes ReineckePathinfo might fail, which indicates that the path is not available anymore. So check the return value and take appropriate action.
--- libmultipath/discovery.c | 2 +- libmultipath/structs_vec.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
libmultipath/discovery.c +1 -1
@@ -680,7 +680,7 @@ path_offline (struct path * pp)return PATH_DOWN ; } -extern int +int sysfs_pathinfo (struct path * pp) {if (common_sysfs_pathinfo (pp))
libmultipath/structs_vec.c +3 -3
@@ -68,9 +68,9 @@ adopt_paths (vector pathvec, struct multipath * mpp, int get_info)if (!find_path_by_dev (mpp->paths, pp->dev) &&store_path (mpp->paths, pp))return 1 ; -if (get_info) -pathinfo (pp, conf->hwtable, -DI_PRIO |DI_CHECKER ); +if (get_info &&pathinfo (pp, conf->hwtable, +DI_PRIO |DI_CHECKER )) +return 1 ; } }return 0 ;
Signature · 1 line
1.7.4.2
PATCH 19/42 Inconsistent string quoting
In reply to Hannes ReineckeWhen printing the hardware table strings are quoted twice, and even numerical values have single quotes. This patch removes the double quotes and retains single quotes only for string values.
--- libmultipath/dict.c | 98 +++++++++++++++++++++++++------------------------- 1 files changed, 49 insertions(+), 49 deletions(-)
libmultipath/dict.c +49 -49
@@ -1062,11 +1062,11 @@ hw_failback_handler(vector strvec) buff =set_value (strvec); -if (strlen (buff) ==6 && !strcmp (buff,"manual" )) +if (strlen (buff) ==6 && !strcmp (buff,"\"manual\"" )) hwe->pgfailback = -FAILBACK_MANUAL ; -else if (strlen (buff) ==9 && !strcmp (buff,"immediate" )) +else if (strlen (buff) ==9 && !strcmp (buff,"\"immediate\"" )) hwe->pgfailback = -FAILBACK_IMMEDIATE ; -else if (strlen (buff) ==10 && !strcmp (buff,"followover" )) +else if (strlen (buff) ==10 && !strcmp (buff,"\"followover\"" )) hwe->pgfailback = -FAILBACK_FOLLOWOVER ;else hwe->pgfailback =atoi (buff);
@@ -1757,7 +1757,7 @@ snprint_mp_path_grouping_policy (char * buff, int len, void * data)return 0 ;get_pgpolicy_name (str,POLICY_NAME_SIZE , mpe->pgpolicy); -return snprintf (buff, len,"%s" , str); +return snprintf (buff, len,"\"%s\"" , str); } static int
@@ -1832,9 +1832,9 @@ snprint_mp_rr_weight (char * buff, int len, void * data)if (!mpe->rr_weight)return 0 ;if (mpe->rr_weight ==RR_WEIGHT_PRIO ) -return snprintf (buff, len,"priorities" ); +return snprintf (buff, len,"\"priorities\"" );if (mpe->rr_weight ==RR_WEIGHT_NONE ) -return snprintf (buff, len,"uniform" ); +return snprintf (buff, len,"\"uniform\"" );return 0 ; }
@@ -1851,9 +1851,9 @@ snprint_mp_no_path_retry (char * buff, int len, void * data)case NO_PATH_RETRY_UNDEF :break ;case NO_PATH_RETRY_FAIL : -return snprintf (buff, len,"fail" ); +return snprintf (buff, len,"\"fail\"" );case NO_PATH_RETRY_QUEUE : -return snprintf (buff, len,"queue" ); +return snprintf (buff, len,"\"queue\"" );default :return snprintf (buff, len,"%i" , mpe->no_path_retry);
@@ -1892,7 +1892,7 @@ snprint_mp_pg_timeout (char * buff, int len, void * data)case PGTIMEOUT_UNDEF :break ;case -PGTIMEOUT_NONE : -return snprintf (buff, len,"none" ); +return snprintf (buff, len,"\"none\"" );default :return snprintf (buff, len,"%i" , mpe->pg_timeout); }
@@ -1910,7 +1910,7 @@ snprint_mp_features (char * buff, int len, void * data) !strcmp(mpe->features, conf->features))return 0 ; -return snprintf (buff, len,"%s" , mpe->features); +return snprintf (buff, len,"\"%s\"" , mpe->features); } static int
@@ -1920,9 +1920,9 @@ snprint_mp_flush_on_last_del (char * buff, int len, void * data)switch (mpe->flush_on_last_del) {case FLUSH_DISABLED : -return snprintf (buff, len,"no" ); +return snprintf (buff, len,"\"no\"" );case FLUSH_ENABLED : -return snprintf (buff, len,"yes" ); +return snprintf (buff, len,"\"yes\"" ); }return 0 ; }
@@ -1935,7 +1935,7 @@ snprint_mp_prio(char * buff, int len, void * data)if (!mpe->prio_name)return 0 ; -return snprintf (buff, len,"%s" , mpe->prio_name); +return snprintf (buff, len,"\"%s\"" , mpe->prio_name); } static int
@@ -1946,7 +1946,7 @@ snprint_mp_prio_args(char * buff, int len, void * data)if (!mpe->prio_args)return 0 ; -return snprintf (buff, len,"%s" , mpe->prio_args); +return snprintf (buff, len,"\"%s\"" , mpe->prio_args); } static int
@@ -1978,7 +1978,7 @@ snprint_hw_fast_io_fail(char * buff, int len, void * data)if (hwe->fast_io_fail == conf->fast_io_fail)return 0 ;if (hwe->fast_io_fail ==MP_FAST_IO_FAIL_OFF ) -return snprintf (buff, len,"off" ); +return snprintf (buff, len,"\"off\"" );if (hwe->fast_io_fail ==MP_FAST_IO_FAIL_ZERO )return snprintf (buff, len,"0" );return snprintf (buff, len,"%d" , hwe->fast_io_fail);
@@ -1993,7 +1993,7 @@ snprint_hw_dev_loss(char * buff, int len, void * data)if (hwe->dev_loss == conf->dev_loss)return 0 ;if (hwe->dev_loss >=MAX_DEV_LOSS_TMO ) -return snprintf (buff, len,"infinity" ); +return snprintf (buff, len,"\"infinity\"" );return snprintf (buff, len,"%u" , hwe->dev_loss); }
@@ -2061,7 +2061,7 @@ snprint_hw_prio (char * buff, int len, void * data)if (!hwe->prio_name)return 0 ; -return snprintf (buff, len,"%s" , hwe->prio_name); +return snprintf (buff, len,"\"%s\"" , hwe->prio_name); } static int
@@ -2131,7 +2131,7 @@ snprint_hw_path_grouping_policy (char * buff, int len, void * data)get_pgpolicy_name (str,POLICY_NAME_SIZE , hwe->pgpolicy); -return snprintf (buff, len,"%s" , str); +return snprintf (buff, len,"\"%s\"" , str); } static int
@@ -2165,9 +2165,9 @@ snprint_hw_rr_weight (char * buff, int len, void * data)if (!hwe->rr_weight)return 0 ;if (hwe->rr_weight ==RR_WEIGHT_PRIO ) -return snprintf (buff, len,"priorities" ); +return snprintf (buff, len,"\"priorities\"" );if (hwe->rr_weight ==RR_WEIGHT_NONE ) -return snprintf (buff, len,"uniform" ); +return snprintf (buff, len,"\"uniform\"" );return 0 ; }
@@ -2184,9 +2184,9 @@ snprint_hw_no_path_retry (char * buff, int len, void * data)case NO_PATH_RETRY_UNDEF :break ;case NO_PATH_RETRY_FAIL : -return snprintf (buff, len,"fail" ); +return snprintf (buff, len,"\"fail\"" );case NO_PATH_RETRY_QUEUE : -return snprintf (buff, len,"queue" ); +return snprintf (buff, len,"\"queue\"" );default :return snprintf (buff, len,"%i" , hwe->no_path_retry);
@@ -2228,7 +2228,7 @@ snprint_hw_pg_timeout (char * buff, int len, void * data)case PGTIMEOUT_UNDEF :break ;case -PGTIMEOUT_NONE : -return snprintf (buff, len,"none" ); +return snprintf (buff, len,"\"none\"" );default :return snprintf (buff, len,"%i" , hwe->pg_timeout); }
@@ -2242,9 +2242,9 @@ snprint_hw_flush_on_last_del (char * buff, int len, void * data)switch (hwe->flush_on_last_del) {case FLUSH_DISABLED : -return snprintf (buff, len,"no" ); +return snprintf (buff, len,"\"no\"" );case FLUSH_ENABLED : -return snprintf (buff, len,"yes" ); +return snprintf (buff, len,"\"yes\"" ); }return 0 ; }
@@ -2257,7 +2257,7 @@ snprint_hw_path_checker (char * buff, int len, void * data)if (!hwe->checker_name)return 0 ; -return snprintf (buff, len,"%s" , hwe->checker_name); +return snprintf (buff, len,"\"%s\"" , hwe->checker_name); } static int
@@ -2285,7 +2285,7 @@ snprint_def_fast_io_fail(char * buff, int len, void * data)if (conf->fast_io_fail ==MP_FAST_IO_FAIL_UNSET )return 0 ;if (conf->fast_io_fail ==MP_FAST_IO_FAIL_OFF ) -return snprintf (buff, len,"off" ); +return snprintf (buff, len,"\"off\"" );if (conf->fast_io_fail ==MP_FAST_IO_FAIL_ZERO )return snprintf (buff, len,"0" );return snprintf (buff, len,"%d" , conf->fast_io_fail);
@@ -2297,7 +2297,7 @@ snprint_def_dev_loss(char * buff, int len, void * data)if (!conf->dev_loss)return 0 ;if (conf->dev_loss >=MAX_DEV_LOSS_TMO ) -return snprintf (buff, len,"infinity" ); +return snprintf (buff, len,"\"infinity\"" );return snprintf (buff, len,"%u" , conf->dev_loss); }
@@ -2320,7 +2320,7 @@ snprint_reassign_maps (char * buff, int len, void * data) {if (conf->reassign_maps ==DEFAULT_REASSIGN_MAPS )return 0 ; -return snprintf (buff, len,"%s" , +return snprintf (buff, len,"\"%s\"" , conf->reassign_maps?"yes" :"no" ); }
@@ -2353,7 +2353,7 @@ snprint_def_path_grouping_policy (char * buff, int len, void * data)get_pgpolicy_name (str,POLICY_NAME_SIZE , pgpolicy); -return snprintf (buff, len,"%s" , str); +return snprintf (buff, len,"\"%s\"" , str); } static int
@@ -2369,9 +2369,9 @@ static int snprint_def_prio (char * buff, int len, void * data) {if (!conf->prio_name) -return snprintf (buff, len,"%s" ,DEFAULT_PRIO ); +return snprintf (buff, len,"\"%s\"" ,DEFAULT_PRIO ); -return snprintf (buff, len,"%s" , conf->prio_name); +return snprintf (buff, len,"\"%s\"" , conf->prio_name); } static int
@@ -2396,9 +2396,9 @@ static int snprint_def_path_checker (char * buff, int len, void * data) {if (!conf->checker_name) -return snprintf (buff, len,"%s" ,DEFAULT_CHECKER ); +return snprintf (buff, len,"\"%s\"" ,DEFAULT_CHECKER ); -return snprintf (buff, len,"%s" , conf->checker_name); +return snprintf (buff, len,"\"%s\"" , conf->checker_name); } static int
@@ -2412,11 +2412,11 @@ snprint_def_failback (char * buff, int len, void * data)case FAILBACK_UNDEF :break ;case -FAILBACK_MANUAL : -return snprintf (buff, len,"manual" ); +return snprintf (buff, len,"\"manual\"" );case -FAILBACK_IMMEDIATE : -return snprintf (buff, len,"immediate" ); +return snprintf (buff, len,"\"immediate\"" );case -FAILBACK_FOLLOWOVER : -return snprintf (buff, len,"followover" ); +return snprintf (buff, len,"\"followover\"" );default :return snprintf (buff, len,"%i" , conf->pgfailback); }
@@ -2478,9 +2478,9 @@ static int snprint_def_rr_weight (char * buff, int len, void * data) {if (!conf->rr_weight || conf->rr_weight ==RR_WEIGHT_NONE ) -return snprintf (buff, len,"uniform" ); +return snprintf (buff, len,"\"uniform\"" );if (conf->rr_weight ==RR_WEIGHT_PRIO ) -return snprintf (buff, len,"priorities" ); +return snprintf (buff, len,"\"priorities\"" );return 0 ; }
@@ -2492,9 +2492,9 @@ snprint_def_no_path_retry (char * buff, int len, void * data)case NO_PATH_RETRY_UNDEF :break ;case NO_PATH_RETRY_FAIL : -return snprintf (buff, len,"fail" ); +return snprintf (buff, len,"\"fail\"" );case NO_PATH_RETRY_QUEUE : -return snprintf (buff, len,"queue" ); +return snprintf (buff, len,"\"queue\"" );default :return snprintf (buff, len,"%i" , conf->no_path_retry);
@@ -2507,10 +2507,10 @@ snprint_def_queue_without_daemon (char * buff, int len, void * data) {switch (conf->queue_without_daemon) {case QUE_NO_DAEMON_OFF : -return snprintf (buff, len,"no" ); +return snprintf (buff, len,"\"no\"" );case QUE_NO_DAEMON_ON :case QUE_NO_DAEMON_UNDEF : -return snprintf (buff, len,"yes" ); +return snprintf (buff, len,"\"yes\"" ); }return 0 ; }
@@ -2530,7 +2530,7 @@ snprint_def_pg_timeout (char * buff, int len, void * data)switch (conf->pg_timeout) {case PGTIMEOUT_UNDEF :case -PGTIMEOUT_NONE : -return snprintf (buff, len,"none" ); +return snprintf (buff, len,"\"none\"" );default :return snprintf (buff, len,"%i" , conf->pg_timeout); }
@@ -2543,10 +2543,10 @@ snprint_def_flush_on_last_del (char * buff, int len, void * data)switch (conf->flush_on_last_del) {case FLUSH_UNDEF :case FLUSH_DISABLED : -return snprintf (buff, len,"no" ); +return snprintf (buff, len,"\"no\"" );case FLUSH_ENABLED :case FLUSH_IN_PROGRESS : -return snprintf (buff, len,"yes" ); +return snprintf (buff, len,"\"yes\"" ); }return 0 ; }
@@ -2563,9 +2563,9 @@ static int snprint_def_user_friendly_names (char * buff, int len, void * data) {if (conf->user_friendly_names ==USER_FRIENDLY_NAMES_ON ) -return snprintf (buff, len,"yes" ); +return snprintf (buff, len,"\"yes\"" );else -return snprintf (buff, len,"no" ); +return snprintf (buff, len,"\"no\"" ); } static int
@@ -2581,7 +2581,7 @@ snprint_def_bindings_file (char * buff, int len, void * data) {if (conf->bindings_file ==NULL )return 0 ; -return snprintf (buff, len,"%s" , conf->bindings_file); +return snprintf (buff, len,"\"%s\"" , conf->bindings_file); } static int
Signature · 1 line
1.7.4.2
PATCH 20/42 Switch off 'queue_if_no_path' before removing maps
In reply to Hannes ReineckeBefore we try to flush a map we have to switch off the 'queue_if_no_path' setting to flush any outstanding I/O.
--- libmultipath/devmapper.c | 26 +++++++++++++++++++++++++- libmultipath/devmapper.h | 1 + multipath/main.c | 6 +++--- 3 files changed, 29 insertions(+), 4 deletions(-)
libmultipath/devmapper.c +25 -1
@@ -707,6 +707,30 @@ _dm_flush_map (const char * mapname, int need_sync) }extern int +dm_suspend_and_flush_map (const char * mapname) +{ +int s; + +if (!dm_map_present (mapname)) +return 0 ; + +if (dm_type (mapname,TGT_MPATH ) <=0 ) +return 0 ;/* nothing to do */ + + s =dm_queue_if_no_path ((char *)mapname,0 ); +if (!s) + s =dm_simplecmd_flush (DM_DEVICE_SUSPEND , mapname,0 ); + +if (!dm_flush_map (mapname)) { +condlog (4 ,"multipath map %s removed" , mapname); +return 0 ; + } +condlog (2 ,"failed to remove multipath map %s" , mapname); +dm_simplecmd_noflush (DM_DEVICE_RESUME , mapname); +return 1 ; +} + +extern int dm_flush_maps (void ) {int r =0 ;
@@ -729,7 +753,7 @@ dm_flush_maps (void) goto out;do { - r |=dm_flush_map (names->name); + r |=dm_suspend_and_flush_map (names->name); next = names->next; names = (void *) names + next; }while (next);
libmultipath/devmapper.h +1 -0
@@ -22,6 +22,7 @@ int dm_type(const char *, char *);int _dm_flush_map (const char *,int );#define dm_flush_map (mapname) _dm_flush_map(mapname, 1)#define dm_flush_map_nosync (mapname) _dm_flush_map(mapname, 0) +int dm_suspend_and_flush_map (const char * mapname);int dm_flush_maps (void );int dm_fail_path (char * mapname,char * path);int dm_reinstate_path (char * mapname,char * path);
multipath/main.c +3 -3
@@ -555,9 +555,9 @@ main (int argc, char *argv[]) goto out; }if (conf->remove ==FLUSH_ONE ) { -if (conf->dev_type ==DEV_DEVMAP ) - r =dm_flush_map (conf->dev); -else +if (conf->dev_type ==DEV_DEVMAP ) { + r =dm_suspend_and_flush_map (conf->dev); + }else condlog (0 ,"must provide a map name to remove" ); goto out;
Signature · 1 line
1.7.4.2
PATCH 21/42 Double free in disassemble_map()
In reply to Hannes ReineckeLabel 'out1' already frees 'word'; no need to do it here.
--- libmultipath/dmparser.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
libmultipath/dmparser.c +0 -1
@@ -276,7 +276,6 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp) num_pg_args =atoi (word);if (merge_words (&mpp->selector, word,1 )) { -FREE (word); goto out1; }FREE (word);
Signature · 1 line
1.7.4.2
PATCH 22/42 libmultipath: prio keyword ignored for multipath config
In reply to Hannes ReineckeWhen specifying the 'prio' keyword in the multipath section of the configuration file the value is ignored. Problem is that the 'wwid' value is set only after the call to select_prio(), so the correct definition couldn't been found in the config file.
--- libmultipath/discovery.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
libmultipath/discovery.c +3 -3
@@ -891,6 +891,9 @@ pathinfo (struct path *pp, vector hwtable, int mask) } } +if (path_state ==PATH_UP && (mask &DI_WWID ) && !strlen (pp->wwid)) +get_uid (pp); +/* * Retrieve path priority, even for PATH_DOWN paths if it has never * been successfully obtained before.
@@ -905,9 +908,6 @@ pathinfo (struct path *pp, vector hwtable, int mask) } } -if (path_state ==PATH_UP && (mask &DI_WWID ) && !strlen (pp->wwid)) -get_uid (pp); -return 0 ; blank:
Signature · 1 line
1.7.4.2
PATCH 23/42 Path checker should return PATH_DOWN when no path is found
In reply to Hannes ReineckeIf the path checker fails to lookup the path in sysfs it's already gone, so we should rather return 'PATH_DOWN' here. Otherwise the path will never marked failed and no failover will happen.
--- libmultipath/discovery.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
libmultipath/discovery.c +2 -2
@@ -659,11 +659,11 @@ path_offline (struct path * pp)if (!parent) {condlog (1 ,"%s: failed to get sysfs information" , pp->dev); -return PATH_WILD ; +return PATH_DOWN ; }if (sysfs_get_state (parent, buff,SCSI_STATE_SIZE )) -return PATH_WILD ; +return PATH_DOWN ;condlog (3 ,"%s: path state = %s" , pp->dev, buff);
Signature · 1 line
1.7.4.2
PATCH 24/42 Do not call sysfs_get_timeout for non-SCSI devices
In reply to Hannes ReineckeOnly SCSI devices have a timeout, so there is no point in trying to set a timeout for other types.
--- libmultipath/discovery.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
libmultipath/discovery.c +5 -3
@@ -761,11 +761,13 @@ get_state (struct path * pp, int daemon)checker_clear_message (c);if (daemon)checker_set_async (c); -if (!conf->checker_timeout) -sysfs_get_timeout (pp, &(c->timeout)); +if (!conf->checker_timeout && + (pp->bus !=SYSFS_BUS_SCSI || +sysfs_get_timeout (pp, &(c->timeout)))) + c->timeout =DEF_TIMEOUT ; state =checker_check (c);condlog (3 ,"%s: state = %s" , pp->dev,checker_state_name (state)); -if (state ==PATH_DOWN &&strlen (checker_message (c))) +if (state !=PATH_UP &&strlen (checker_message (c)))condlog (3 ,"%s: checker msg is \"%s\"" , pp->dev,checker_message (c));return state;
Signature · 1 line
1.7.4.2
PATCH 25/42 Make log_pthread more robust
In reply to Hannes ReineckeWe don't need to allocate memory for mutexes, we can just be using static variables. And valgrind complained about logqueue flush from shutdown, so don't do this. The normal shutdown process should be flushing the log queue anyway.
--- libmultipath/log_pthread.c | 101 +++++++++++++++++++++++++++++-------------- libmultipath/log_pthread.h | 10 +++-- 2 files changed, 74 insertions(+), 37 deletions(-)
libmultipath/log_pthread.c +68 -33
@@ -13,20 +13,41 @@#include "log.h" #include "lock.h" +pthread_t log_thr; + +pthread_mutex_t logq_lock; +pthread_mutex_t logev_lock; +pthread_cond_t logev_cond; + +int logq_running; + +static void +sigusr1 (int sig) +{ +pthread_mutex_lock (&logq_lock); +log_reset ("multipathd" ); +pthread_mutex_unlock (&logq_lock); +} +void log_safe (int prio,const char * fmt,va_list ap) {sigset_t old; +if (log_thr == (pthread_t )0 ) { +syslog (prio, fmt, ap); +return ; + } +block_signal (SIGUSR1 , &old);block_signal (SIGHUP ,NULL ); -pthread_mutex_lock (logq_lock); +pthread_mutex_lock (&logq_lock);log_enqueue (prio, fmt, ap); -pthread_mutex_unlock (logq_lock); +pthread_mutex_unlock (&logq_lock); -pthread_mutex_lock (logev_lock); -pthread_cond_signal (logev_cond); -pthread_mutex_unlock (logev_lock); +pthread_mutex_lock (&logev_lock); +pthread_cond_signal (&logev_cond); +pthread_mutex_unlock (&logev_lock);pthread_sigmask (SIG_SETMASK , &old,NULL ); }
@@ -36,9 +57,9 @@ static void flush_logqueue (void)int empty;do { -pthread_mutex_lock (logq_lock); +pthread_mutex_lock (&logq_lock); empty =log_dequeue (la->buff); -pthread_mutex_unlock (logq_lock); +pthread_mutex_unlock (&logq_lock);if (!empty)log_syslog (la->buff); }while (empty ==0 );
@@ -46,15 +67,30 @@ static void flush_logqueue (void)static void *log_thread (void * et) { +struct sigaction sig; +int running; + + sig.sa_handler = sigusr1; +sigemptyset (&sig.sa_mask); + sig.sa_flags =0 ; +if (sigaction (SIGUSR1 , &sig,NULL ) <0 ) +logdbg (stderr,"Cannot set signal handler" ); + +pthread_mutex_lock (&logev_lock); + logq_running =1 ; +pthread_mutex_unlock (&logev_lock); +mlockall (MCL_CURRENT |MCL_FUTURE );logdbg (stderr,"enter log_thread\n" );while (1 ) { -pthread_mutex_lock (logev_lock); -pthread_cond_wait (logev_cond, logev_lock); -pthread_mutex_unlock (logev_lock); - -flush_logqueue (); +pthread_mutex_lock (&logev_lock); +pthread_cond_wait (&logev_cond, &logev_lock); + running = logq_running; +pthread_mutex_unlock (&logev_lock); +if (!running) +break ; +log_thread_flush (); }return NULL ; }
@@ -63,19 +99,18 @@ void log_thread_start (pthread_attr_t *attr) {logdbg (stderr,"enter log_thread_start\n" ); - logq_lock = (pthread_mutex_t *)malloc (sizeof (pthread_mutex_t)); - logev_lock = (pthread_mutex_t *)malloc (sizeof (pthread_mutex_t)); - logev_cond = (pthread_cond_t *)malloc (sizeof (pthread_cond_t)); - -pthread_mutex_init (logq_lock,NULL ); -pthread_mutex_init (logev_lock,NULL ); -pthread_cond_init (logev_cond,NULL ); +pthread_mutex_init (&logq_lock,NULL ); +pthread_mutex_init (&logev_lock,NULL ); +pthread_cond_init (&logev_cond,NULL );if (log_init ("multipathd" ,0 )) {fprintf (stderr,"can't initialize log buffer\n" );exit (1 ); } -pthread_create (&log_thr, attr, log_thread,NULL ); +if (pthread_create (&log_thr, attr, log_thread,NULL )) { +fprintf (stderr,"can't start log thread\n" ); +exit (1 ); + }return ; }
@@ -84,22 +119,22 @@ void log_thread_stop (void) {logdbg (stderr,"enter log_thread_stop\n" ); -pthread_mutex_lock (logq_lock); +pthread_mutex_lock (&logev_lock); + logq_running =0 ; +pthread_cond_signal (&logev_cond); +pthread_mutex_unlock (&logev_lock); + +pthread_mutex_lock (&logq_lock);pthread_cancel (log_thr); -pthread_mutex_unlock (logq_lock); +pthread_mutex_unlock (&logq_lock);pthread_join (log_thr,NULL ); + log_thr = (pthread_t )0 ;flush_logqueue (); -pthread_mutex_destroy (logq_lock); -pthread_mutex_destroy (logev_lock); -pthread_cond_destroy (logev_cond); - -free (logq_lock); - logq_lock =NULL ; -free (logev_lock); - logev_lock =NULL ; -free (logev_cond); - logev_cond =NULL ; -free_logarea (); +pthread_mutex_destroy (&logq_lock); +pthread_mutex_destroy (&logev_lock); +pthread_cond_destroy (&logev_cond); + +log_close (); }
libmultipath/log_pthread.h +6 -4
@@ -3,11 +3,13 @@#include <pthread.h> -pthread_t log_thr; +extern pthread_t log_thr; -pthread_mutex_t *logq_lock; -pthread_mutex_t *logev_lock; -pthread_cond_t *logev_cond; +extern pthread_mutex_t logq_lock; +extern pthread_mutex_t logev_lock; +extern pthread_cond_t logev_cond; + +extern int logq_running;void log_safe (int prio,const char * fmt,va_list ap);void log_thread_start (pthread_attr_t *attr);
Signature · 1 line
1.7.4.2
Re: [PATCH 25/42] Make log_pthread more robust
Quoted 6 lines · Hannes Reinecke
We don't need to allocate memory for mutexes, we can just be using static variables. And valgrind complained about logqueue flush from shutdown, so don't do this. The normal shutdown process should be flushing the log queue anyway.
It seems the log_thread_flush() function is missing from the patchset. Care to send the incremental patch ?
Regards,
Christophe Varoqui
www.opensvc.com
Quoted 190 lines
Signed-off-by Hannes Reinecke <[email protected]>--- libmultipath/log_pthread.c | 101 +++++++++++++++++++++++++++++-------------- libmultipath/log_pthread.h | 10 +++-- 2 files changed, 74 insertions(+), 37 deletions(-)libmultipath/log_pthread.c +68
-33@@ -13,20 +13,41 @@#include "log.h" #include "lock.h" +pthread_t log_thr; + +pthread_mutex_t logq_lock; +pthread_mutex_t logev_lock; +pthread_cond_t logev_cond; + +int logq_running; + +static void +sigusr1 (int sig) +{ +pthread_mutex_lock (&logq_lock); +log_reset ("multipathd" ); +pthread_mutex_unlock (&logq_lock); +} +void log_safe (int prio,const char * fmt,va_list ap) {sigset_t old; +if (log_thr == (pthread_t )0 ) { +syslog (prio, fmt, ap); +return ; + } +block_signal (SIGUSR1 , &old);block_signal (SIGHUP ,NULL ); -pthread_mutex_lock (logq_lock); +pthread_mutex_lock (&logq_lock);log_enqueue (prio, fmt, ap); -pthread_mutex_unlock (logq_lock); +pthread_mutex_unlock (&logq_lock); -pthread_mutex_lock (logev_lock); -pthread_cond_signal (logev_cond); -pthread_mutex_unlock (logev_lock); +pthread_mutex_lock (&logev_lock); +pthread_cond_signal (&logev_cond); +pthread_mutex_unlock (&logev_lock);pthread_sigmask (SIG_SETMASK , &old,NULL ); }@@ -36,9 +57,9 @@ static void flush_logqueue (void)int empty;do { -pthread_mutex_lock (logq_lock); +pthread_mutex_lock (&logq_lock); empty =log_dequeue (la->buff); -pthread_mutex_unlock (logq_lock); +pthread_mutex_unlock (&logq_lock);if (!empty)log_syslog (la->buff); }while (empty ==0 );@@ -46,15 +67,30 @@ static void flush_logqueue (void)static void *log_thread (void * et) { +struct sigaction sig; +int running; + + sig.sa_handler = sigusr1; +sigemptyset (&sig.sa_mask); + sig.sa_flags =0 ; +if (sigaction (SIGUSR1 , &sig,NULL ) <0 ) +logdbg (stderr,"Cannot set signal handler" ); + +pthread_mutex_lock (&logev_lock); + logq_running =1 ; +pthread_mutex_unlock (&logev_lock); +mlockall (MCL_CURRENT |MCL_FUTURE );logdbg (stderr,"enter log_thread\n" );while (1 ) { -pthread_mutex_lock (logev_lock); -pthread_cond_wait (logev_cond, logev_lock); -pthread_mutex_unlock (logev_lock); - -flush_logqueue (); +pthread_mutex_lock (&logev_lock); +pthread_cond_wait (&logev_cond, &logev_lock); + running = logq_running; +pthread_mutex_unlock (&logev_lock); +if (!running) +break ; +log_thread_flush (); }return NULL ; }@@ -63,19 +99,18 @@ void log_thread_start (pthread_attr_t *attr) {logdbg (stderr,"enter log_thread_start\n" ); - logq_lock = (pthread_mutex_t *)malloc (sizeof (pthread_mutex_t)); - logev_lock = (pthread_mutex_t *)malloc (sizeof (pthread_mutex_t)); - logev_cond = (pthread_cond_t *)malloc (sizeof (pthread_cond_t)); - -pthread_mutex_init (logq_lock,NULL ); -pthread_mutex_init (logev_lock,NULL ); -pthread_cond_init (logev_cond,NULL ); +pthread_mutex_init (&logq_lock,NULL ); +pthread_mutex_init (&logev_lock,NULL ); +pthread_cond_init (&logev_cond,NULL );if (log_init ("multipathd" ,0 )) {fprintf (stderr,"can't initialize log buffer\n" );exit (1 ); } -pthread_create (&log_thr, attr, log_thread,NULL ); +if (pthread_create (&log_thr, attr, log_thread,NULL )) { +fprintf (stderr,"can't start log thread\n" ); +exit (1 ); + }return ; }@@ -84,22 +119,22 @@ void log_thread_stop (void) {logdbg (stderr,"enter log_thread_stop\n" ); -pthread_mutex_lock (logq_lock); +pthread_mutex_lock (&logev_lock); + logq_running =0 ; +pthread_cond_signal (&logev_cond); +pthread_mutex_unlock (&logev_lock); + +pthread_mutex_lock (&logq_lock);pthread_cancel (log_thr); -pthread_mutex_unlock (logq_lock); +pthread_mutex_unlock (&logq_lock);pthread_join (log_thr,NULL ); + log_thr = (pthread_t )0 ;flush_logqueue (); -pthread_mutex_destroy (logq_lock); -pthread_mutex_destroy (logev_lock); -pthread_cond_destroy (logev_cond); - -free (logq_lock); - logq_lock =NULL ; -free (logev_lock); - logev_lock =NULL ; -free (logev_cond); - logev_cond =NULL ; -free_logarea (); +pthread_mutex_destroy (&logq_lock); +pthread_mutex_destroy (&logev_lock); +pthread_cond_destroy (&logev_cond); + +log_close (); }libmultipath/log_pthread.h +6
-4@@ -3,11 +3,13 @@#include <pthread.h> -pthread_t log_thr; +extern pthread_t log_thr; -pthread_mutex_t *logq_lock; -pthread_mutex_t *logev_lock; -pthread_cond_t *logev_cond; +extern pthread_mutex_t logq_lock; +extern pthread_mutex_t logev_lock; +extern pthread_cond_t logev_cond; + +extern int logq_running;void log_safe (int prio,const char * fmt,va_list ap);void log_thread_start (pthread_attr_t *attr);
Re: [PATCH 25/42] Make log_pthread more robust
Quoted 9 lines · Christophe Varoqui
Quoted 6 lines · Hannes Reinecke
We don't need to allocate memory for mutexes, we can just be using static variables. And valgrind complained about logqueue flush from shutdown, so don't do this. The normal shutdown process should be flushing the log queue anyway.
It seems the
log_thread_flush()function is missing from the patchset. Care to send the incremental patch ?
and also log_reset()
chuchi:~/tmp/multipath-tools $ make -i
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmultipath'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmultipath'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmultipath/prioritizers'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmultipath/prioritizers'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmultipath/checkers'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmultipath/checkers'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/libmpathpersist'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/libmpathpersist'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/multipath'
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wunused -Wstrict-prototypes -fPIC -DLIB_STRING=\"lib64\" -I../libmultipath main.o -o multipath -lpthread -ldevmapper -ldl -L../libmultipath -lmultipath
../libmultipath/libmultipath.so: undefined reference to `log_thread_flush'
../libmultipath/libmultipath.so: undefined reference to `log_reset'
collect2: error: ld returned 1 exit status
make[1]: [multipath] Error 1 (ignored)/bin/gzip -9 -c multipath.8 > multipath.8.gz/bin/gzip -9 -c multipath.conf.5 > multipath.conf.5.gz
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/multipath'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/multipathd'
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wunused -Wstrict-prototypes -fPIC -DLIB_STRING=\"lib64\" -I../libmultipath -I../libmpathpersist main.o pidfile.o uxlsnr.o uxclnt.o cli.o cli_handlers.o -lpthread -ldevmapper -lreadline -lncurses -ludev -ldl -L../libmultipath -lmultipath -L../libmpathpersist -lmpathpersist -o multipathd ../libmultipath/libmultipath.so: undefined reference to `log_thread_flush'
../libmultipath/libmultipath.so: undefined reference to `log_reset'
collect2: error: ld returned 1 exit status
make[1]: [multipathd] Error 1 (ignored)/bin/gzip -9 -c multipathd.8 > multipathd.8.gz
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/multipathd'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/mpathpersist'
cc -g main.o -o mpathpersist -lpthread -ldevmapper -L../libmpathpersist -lmpathpersist -L../libmultipath -lmultipath -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -Wunused -Wstrict-prototypes -fPIC -DLIB_STRING=\"lib64\" -I../libmultipath -I../libmpathpersist ../libmultipath/libmultipath.so: undefined reference to log_thread_flush' ../libmultipath/libmultipath.so: undefined reference to log_reset'
collect2: error: ld returned 1 exit status
make[1]: [mpathpersist] Error 1 (ignored)/bin/gzip -9 -c mpathpersist.8 > mpathpersist.8.gz
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/mpathpersist'
make[1]: Entering directory `/home/xose/tmp/multipath-tools/kpartx'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/xose/tmp/multipath-tools/kpartx'
PATCH 26/42 Print log messages when updating tables failed
In reply to Hannes ReineckeAdd some logging messages to identify the case of failure.
--- libmultipath/structs_vec.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
libmultipath/structs_vec.c +12 -4
@@ -246,11 +246,15 @@ update_multipath_table (struct multipath *mpp, vector pathvec)if (!mpp)return 1 ; -if (dm_get_map (mpp->alias, &mpp->size, params)) +if (dm_get_map (mpp->alias, &mpp->size, params)) { +condlog (3 ,"%s: cannot get map" , mpp->alias);return 1 ; + } -if (disassemble_map (pathvec, params, mpp)) +if (disassemble_map (pathvec, params, mpp)) { +condlog (3 ,"%s: cannot disassemble map" , mpp->alias);return 1 ; + }return 0 ; }
@@ -263,11 +267,15 @@ update_multipath_status (struct multipath *mpp)if (!mpp)return 1 ; -if (dm_get_status (mpp->alias, status)) +if (dm_get_status (mpp->alias, status)) { +condlog (3 ,"%s: cannot get status" , mpp->alias);return 1 ; + } -if (disassemble_status (status, mpp)) +if (disassemble_status (status, mpp)) { +condlog (3 ,"%s: cannot disassemble status" , mpp->alias);return 1 ; + }return 0 ; }
Signature · 1 line
1.7.4.2
PATCH 27/42 Update 'no_path_retry' correctly for failed paths
In reply to Hannes ReineckeThe bug is triggered if path failed event is received by multipathd after all paths have been already marked as failed. Surprisingly enough, it seems to happen quite often; colleague of mine who tested this hit this bug every time.
Here is event sequence that explains this bug. I left some messages for clarity; full log is available on request. We have completed initialization and set feature queue_if_no_path for map CX_201 by virtue of using no_path_retry > 0.
Aug 31 10:49:09 | CX_201: devmap event #18 Aug 31 10:49:09 | CX_201: discover Aug 31 10:49:09 | CX_201: rr_weight = 1 (internal default) Aug 31 10:49:09 | CX_201: pgfailback = -2 (controller setting) Aug 31 10:49:09 | CX_201: no_path_retry = 2 (controller setting) Aug 31 10:49:09 | pg_timeout = NONE (internal default) Aug 31 10:49:09 | 65:192: mark as failed Aug 31 10:49:09 | CX_201: remaining active paths: 3 Aug 31 10:49:09 | 8:192: mark as failed Aug 31 10:49:09 | CX_201: remaining active paths: 2 Aug 31 10:49:09 | CX_201: devmap event #19 Aug 31 10:49:09 | CX_201: discover Aug 31 10:49:09 | CX_201: rr_weight = 1 (internal default) Aug 31 10:49:09 | CX_201: pgfailback = -2 (controller setting) Aug 31 10:49:09 | CX_201: no_path_retry = 2 (controller setting) Aug 31 10:49:09 | pg_timeout = NONE (internal default)
Two paths failed by driver, multipahd marked them as failed.
Aug 31 10:49:09 | checker failed path 66:0 in map CX_201 Aug 31 10:49:09 | CX_201: remaining active paths: 1
Checker failed third path
Aug 31 10:49:09 | checker failed path 8:96 in map CX_201 Aug 31 10:49:09 | CX_201: Entering recovery mode: max_retries=2 Aug 31 10:49:09 | CX_201: remaining active paths: 0
Checker failed last path; multipathd entered retry loop.
Aug 31 10:49:10 | CX_201: devmap event #20
We got late event about failed path
Aug 31 10:49:10 | CX_201: discover
Start discovery. Call update_multipath -> setup_multipath -> update_multipath_strings -> update_multipath_tablle -> disassemble_map.
Now disassemble_map tries to set no_path_retry value from kernel. This obviously is not going to work as kernel is able remembering only Boolean (queue/fail), while no_path_retry is arbitrary integer. So no_path_retry is set to NO_PATH_RETRY_QUEUE from kernel.
Aug 31 10:49:10 | CX_201: rr_weight = 1 (internal default) Aug 31 10:49:10 | CX_201: pgfailback = -2 (controller setting)
At this point we call set_no_path_retry:
set_no_path_retry(struct multipath *mpp)
{
mpp->retry_tick = 0;
mpp->nr_active = pathcount(mpp, PATH_UP) + pathcount(mpp, PATH_GHOST);
if (mpp->nr_active > 0)
select_no_path_retry(mpp);So
1) retry_tick is reset
2) nr_active = 0 (no active path)
3) we do not set no_path_retry from config file because nr_active == 0 => left with NO_PATH_RETRY_QUEUE.
Aug 31 10:49:10 | pg_timeout = NONE (internal default)
From now on there is no state changes, so map is hung forever.
--- libmultipath/structs_vec.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
libmultipath/structs_vec.c +1 -2
@@ -306,8 +306,7 @@ set_no_path_retry(struct multipath *mpp) { mpp->retry_tick =0 ; mpp->nr_active =pathcount (mpp,PATH_UP ) +pathcount (mpp,PATH_GHOST ); -if (mpp->nr_active >0 ) -select_no_path_retry (mpp); +select_no_path_retry (mpp);switch (mpp->no_path_retry) {case NO_PATH_RETRY_UNDEF :
Signature · 1 line
1.7.4.2
PATCH 28/42 Clean up uevent queue on shutdown
In reply to Hannes ReineckeDuring shutdown there might be some unprocessed events in the queue. So clear them up.
--- libmultipath/uevent.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
libmultipath/uevent.c +12 -0
@@ -132,6 +132,17 @@ static void uevq_stop(void *arg)pthread_mutex_unlock (uevq_lockp); } +void +uevq_cleanup (struct list_head *tmpq) +{ +struct uevent *uev, *tmp; + +list_for_each_entry_safe (uev, tmp, tmpq, node) { +list_del_init (&uev->node); +FREE (uev); + } +} +/* * Service the uevent queue. */
@@ -163,6 +174,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, void * trigger_data),service_uevq (&uevq_tmp); }condlog (3 ,"Terminating uev service queue" ); +uevq_cleanup (&uevq);return 0 ; }
Signature · 1 line
1.7.4.2
PATCH 29/42 libmultipath: Print out uevent sequence number
In reply to Hannes ReineckeFor debugging we should be printing out the sequence number.
--- libmultipath/uevent.c | 11 ++++++++++- libmultipath/uevent.h | 1 + 2 files changed, 11 insertions(+), 1 deletions(-)
libmultipath/uevent.c +10 -1
@@ -361,11 +361,20 @@ int failback_listen(void) key = &buffer[bufpos]; keylen =strlen (key); uev->envp[i] = key; +/* Filter out sequence number */ +if (strncmp (key,"SEQNUM=" ,7 ) ==0 ) { +char *eptr; + + uev->seqnum =strtoul (key +7 , &eptr,10 ); +if (eptr == key +7 ) + uev->seqnum =-1 ; + } bufpos += keylen +1 ; } uev->envp[i] =NULL ; -condlog (3 ,"uevent '%s' from '%s'" , uev->action, uev->devpath); +condlog (3 ,"uevent %ld '%s' from '%s'" , uev->seqnum, + uev->action, uev->devpath); uev->kernel =strrchr (uev->devpath,'/' );if (uev->kernel) uev->kernel++;
libmultipath/uevent.h +1 -0
@@ -20,6 +20,7 @@ struct uevent {char *devpath;char *action;char *kernel; +unsigned long seqnum;char *envp[HOTPLUG_NUM_ENVP ]; };
Signature · 1 line
1.7.4.2
PATCH 30/42 Fix race condition in stop_waiter_thread()
In reply to Hannes ReineckeThe signal handler might run before we had a chance to set the 'waiter' context to '0', so better do it previously.
--- libmultipath/waiter.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
libmultipath/waiter.c +4 -1
@@ -45,6 +45,8 @@ void free_waiter (void *data)void stop_waiter_thread (struct multipath *mpp,struct vectors *vecs) { +pthread_t thread; +if (mpp->waiter == (pthread_t )0 ) {condlog (3 ,"%s: event checker thread already stopped" , mpp->alias);
@@ -52,8 +54,9 @@ void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs) }condlog (2 ,"%s: stop event checker thread (%lu)" , mpp->alias, mpp->waiter); -pthread_cancel (mpp->waiter); + thread = mpp->waiter; mpp->waiter = (pthread_t )0 ; +pthread_cancel (thread); }/*
Signature · 1 line
1.7.4.2
PATCH 31/42 Use VECTOR_SIZE() defines
In reply to Hannes ReineckeThe size of a vector slot might be larger than one, so we should be using the VECTOR_SIZE() define everywhere.
--- libmultipath/vector.c | 8 ++++---- libmultipath/vector.h | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-)
libmultipath/vector.c +4 -4
@@ -81,7 +81,7 @@ vector_insert_slot(vector v, int slot, void *value)if (!vector_alloc_slot (v))return NULL ; -for (i = (v->allocated /VECTOR_DEFAULT_SIZE ) -2 ; i >= slot; i--) +for (i =VECTOR_SIZE (v) -2 ; i >= slot; i--) v->slot[i +1 ] = v->slot[i]; v->slot[slot] = value;
@@ -94,7 +94,7 @@ find_slot(vector v, void * addr) {int i; -for (i =0 ; i < (v->allocated /VECTOR_DEFAULT_SIZE ); i++) +for (i =0 ; i <VECTOR_SIZE (v); i++)if (v->slot[i] == addr)return i;
@@ -109,7 +109,7 @@ vector_del_slot(vector v, int slot)if (!v || !v->allocated || slot <0 || slot >VECTOR_SIZE (v))return ; -for (i = slot +1 ; i < (v->allocated /VECTOR_DEFAULT_SIZE ); i++) +for (i = slot +1 ; i <VECTOR_SIZE (v); i++) v->slot[i-1 ] = v->slot[i]; v->allocated -=VECTOR_DEFAULT_SIZE ;
@@ -137,7 +137,7 @@ vector_repack(vector v)if (!v || !v->allocated)return ; -for (i =0 ; i < (v->allocated /VECTOR_DEFAULT_SIZE ); i++) +for (i =0 ; i <VECTOR_SIZE (v); i++)if (i >0 && v->slot[i] ==NULL )vector_del_slot (v, i--); }
libmultipath/vector.h +5 -5
@@ -31,14 +31,14 @@ struct _vector {typedef struct _vector *vector ;#define VECTOR_DEFAULT_SIZE 1 -#define VECTOR_SLOT (V ,E ) (((V) && (E) < (V)->allocated) ? (V)->slot[(E)] : NULL) -#define VECTOR_SIZE (V ) ((V) ? (V)->allocated : 0) -#define VECTOR_LAST_SLOT (V ) (((V) && (V)->allocated) ? (V)->slot[((V)->allocated - 1)] : NULL) +#define VECTOR_SIZE (V ) ((V) ? ((V)->allocated) / VECTOR_DEFAULT_SIZE : 0) +#define VECTOR_SLOT (V ,E ) (((V) && (E) < VECTOR_SIZE(V)) ? (V)->slot[(E)] : NULL) +#define VECTOR_LAST_SLOT (V ) (((V) && VECTOR_SIZE(V) > 0) ? (V)->slot[(VECTOR_SIZE(V) - 1)] : NULL)#define vector_foreach_slot (v,p,i) \ - for (i = 0; (v) && i < (v)->allocated && ((p) = (v)->slot[i]); i++) + for (i = 0; (v) && i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)#define vector_foreach_slot_after (v,p,i) \ - for (; (v) && i < (v)->allocated && ((p) = (v)->slot[i]); i++) + for (; (v) && i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)#define vector_foreach_slot_backwards (v,p,i) \ for (i = VECTOR_SIZE(v); i > 0 && ((p) = (v)->slot[i-1]); i--)
Signature · 1 line
1.7.4.2
PATCH 32/42 Make 'allocated' an integer in vector.h
In reply to Hannes ReineckeI don't trust the programmers here, as we're unconditionally decreasing the 'allocated' setting on vector_free(). So better make that an integer to catch underflows.
--- libmultipath/vector.c | 2 +- libmultipath/vector.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
libmultipath/vector.c +1 -1
@@ -114,7 +114,7 @@ vector_del_slot(vector v, int slot) v->allocated -=VECTOR_DEFAULT_SIZE ; -if (!v->allocated) { +if (v->allocated <=0 ) {FREE (v->slot); v->slot =NULL ; v->allocated =0 ;
libmultipath/vector.h +1 -1
@@ -25,7 +25,7 @@/* vector definition */ struct _vector { -unsigned int allocated; +int allocated;void **slot; };typedef struct _vector *vector ;
Signature · 1 line
1.7.4.2
PATCH 33/42 Syntax error in /etc/init.d/boot.multipath
In reply to Hannes ReineckeThe multipath init script has a syntax error in line 113.
--- multipath/multipath.init.suse | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
multipath/multipath.init.suse +1 -1
@@ -110,7 +110,7 @@ case "$1" in partlist=$(/sbin/kpartx -l -p _part /dev/disk/by-id/dm-name-$map | sed 's/\([^ ]*\) :.*/\1/p') for part in $partlist; do [ -e /dev/disk/by-id/dm-name-$part ] && continue - num$((num + 1)) + num=$((num + 1)) done done [ $num -eq 0 ] && break
Signature · 1 line
1.7.4.2
PATCH 34/42 multipath.init.suse: Update usage message
In reply to Hannes ReineckeThe usage message in multipath.init.suse doesn't list 'reload'. And 'reload' is a misnomer, as it's actually a restart. So rename 'reload' to 'restart' and add to the usage message.
--- multipath/multipath.init.suse | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
multipath/multipath.init.suse +2 -2
@@ -146,12 +146,12 @@ case "$1" in fi rc_status -v ;; - reload) + restart) $0 stop $0 start ;; *) - echo "Usage: $0 {start|stop|status}" + echo "Usage: $0 {start|stop|status|restart}" exit 1 ;; esac
Signature · 1 line
1.7.4.2
PATCH 35/42 multipath.conf.5: Clarify dev_loss_tmo settings
In reply to Hannes ReineckeWe need to document that dev_loss_tmo is in fact modified by no_path_retry.
--- multipath/multipath.conf.5 | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
multipath/multipath.conf.5 +6 -1
@@ -334,9 +334,14 @@ will disable the timeout. .TP .B dev_loss_tmo Specify the number of seconds the scsi layer will wait after a problem has +<<<<<<< HEAD been detected on a FC remote port before removing it from the system. This can be set to "infinity" which sets it to the max value of 2147483647 -seconds, or 68 years. +seconds, or 68 years. It will be automatically adjusted to the overall +retry interval +\fIno_path_retry\fR * \fIpolling_interval\fR +if a number of retries is given with \fIno_path_retry\fR and the +overall retry interval is longer than the specified \fIdev_loss_tmo\fR value. .TP .B queue_without_daemon If set to
Signature · 1 line
1.7.4.2
PATCH 36/42 Clarify dev_loss_tmo capping in multipath.conf.5
In reply to Hannes ReineckeThe linux kernel will not allow any dev_loss_tmo setting larger than 300 if fast_io_fail is not set. So we should document this in the manpage.
--- multipath/multipath.conf.5 | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
multipath/multipath.conf.5 +2 -0
@@ -342,6 +342,8 @@ retry interval \fIno_path_retry\fR * \fIpolling_interval\fR if a number of retries is given with \fIno_path_retry\fR and the overall retry interval is longer than the specified \fIdev_loss_tmo\fR value. +The linux kernel will cap this value to \fI300\fR if \fBfast_io_fail_tmo\fR +is not set. .TP .B queue_without_daemon If set to
Signature · 1 line
1.7.4.2
PATCH 38/42 multipathd: Ignore errors when creating pidfile
In reply to Hannes ReineckeWe can use CLI commands to communicate with the daemon, so we don't need the pidfile for correct operation. Hence any errors from creating the pidfile can be safely ignored.
--- multipathd/main.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)
multipathd/main.c +7 -6
@@ -1560,7 +1560,7 @@ child (void * param)struct vectors * vecs;struct multipath * mpp;int i; -int rc; +int rc, pid_rc;mlockall (MCL_CURRENT |MCL_FUTURE );
@@ -1667,9 +1667,8 @@ child (void * param)pthread_mutex_lock (&exit_mutex);/* Startup complete, create logfile */ -if (pidfile_create (DEFAULT_PIDFILE , daemon_pid)) -/* Ignore errors, we can live without */ -condlog (1 ,"failed to create pidfile" ); + pid_rc =pidfile_create (DEFAULT_PIDFILE , daemon_pid); +/* Ignore errors, we can live without */ running_state =DAEMON_RUNNING ;pthread_cond_wait (&exit_cond, &exit_mutex);
@@ -1715,8 +1714,10 @@ child (void * param)dm_lib_exit ();/* We're done here */ -condlog (3 ,"unlink pidfile" ); -unlink (DEFAULT_PIDFILE ); +if (!pid_rc) { +condlog (3 ,"unlink pidfile" ); +unlink (DEFAULT_PIDFILE ); + }condlog (2 ,"--------shut down-------" );
Signature · 1 line
1.7.4.2
PATCH 39/42 multipathd deadlocks during restart
In reply to Hannes ReineckeDuring restart multipathd might deadlock as the uevent handler is missing a cleanup handler. Thus the thread might be terminated while it still holds the vector lock.
--- multipathd/main.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
multipathd/main.c +12 -7
@@ -895,13 +895,11 @@ exit_daemon (int status)if (status !=0 )fprintf (stderr,"bad exit status. see daemon.log\n" ); -condlog (3 ,"unlink pidfile" ); -unlink (DEFAULT_PIDFILE ); - -pthread_mutex_lock (&exit_mutex); -pthread_cond_signal (&exit_cond); -pthread_mutex_unlock (&exit_mutex); - +if (running_state !=DAEMON_SHUTDOWN ) { +pthread_mutex_lock (&exit_mutex); +pthread_cond_signal (&exit_cond); +pthread_mutex_unlock (&exit_mutex); + }return status; }
@@ -1560,6 +1558,7 @@ child (void * param)struct vectors * vecs;struct multipath * mpp;int i; +sigset_t set;int rc, pid_rc;mlockall (MCL_CURRENT |MCL_FUTURE );
@@ -1672,11 +1671,17 @@ child (void * param) running_state =DAEMON_RUNNING ;pthread_cond_wait (&exit_cond, &exit_mutex); +/* Need to block these to avoid deadlocking */ +sigemptyset (&set); +sigaddset (&set,SIGTERM ); +sigaddset (&set,SIGINT ); +pthread_sigmask (SIG_BLOCK , &set,NULL );/* * exit path */ running_state =DAEMON_SHUTDOWN ; +pthread_sigmask (SIG_UNBLOCK , &set,NULL );block_signal (SIGHUP ,NULL );lock (vecs->lock);if (conf->queue_without_daemon ==QUE_NO_DAEMON_OFF )
Signature · 1 line
1.7.4.2
PATCH 40/42 multipathd: sighandlers might use uninitialized gvecs
In reply to Hannes Reineckegvecs are initialized after signal handlers, which in turn might access the vectors.
So the signal handlers might access uninitialized variables.
--- multipathd/main.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
multipathd/main.c +4 -4
@@ -1617,14 +1617,14 @@ child (void * param) } -signal_init (); -setscheduler (); -set_oom_adj (); vecs = gvecs =init_vecs (); -if (!vecs)exit (1 ); +signal_init (); +setscheduler (); +set_oom_adj (); + conf->daemon =1 ;udev_set_sync_support (0 );/*
Signature · 1 line
1.7.4.2
PATCH 41/42 multipathd: crash in reconfigure CLI command
In reply to Hannes ReineckeThe 'reconfigure' CLI command doesn't take the vector lock, so if multipathd is processing a table / udev event at the same time it'll crash.
--- multipathd/main.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
multipathd/main.c +2 -2
@@ -1389,6 +1389,7 @@ reconfigure (struct vectors * vecs)struct config * old = conf;int retval =1 ; +lock (vecs->lock);/* * free old map and path vectors ... they use old conf state */
@@ -1409,6 +1410,7 @@ reconfigure (struct vectors * vecs) retval =0 ; } +unlock (vecs->lock);return retval; }
@@ -1466,9 +1468,7 @@ sighup (int sig)if (running_state !=DAEMON_RUNNING )return ; -lock (gvecs->lock);reconfigure (gvecs); -unlock (gvecs->lock);#ifdef _DEBUG_dbg_free_final (NULL );
Signature · 1 line
1.7.4.2
PATCH 42/42 multipathd: lock vectors during initial configuration
In reply to Hannes ReineckeDuring initial configuration the CLI thread is already running, so we need to lock the vectors here to not race with the 'reconfigure' CLI command.
--- multipathd/main.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
multipathd/main.c +1 -3
@@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs)struct config * old = conf;int retval =1 ; -lock (vecs->lock);/* * free old map and path vectors ... they use old conf state */
@@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs) retval =0 ; } -unlock (vecs->lock);return retval; }
@@ -1641,9 +1639,9 @@ child (void * param)/* * fetch and configure both paths and multipaths */ -lock (vecs->lock); running_state =DAEMON_CONFIGURE ; +lock (vecs->lock);if (configure (vecs,1 )) {unlock (vecs->lock);condlog (0 ,"failure during configuration" );
Signature · 1 line
1.7.4.2
Re: [PATCH 42/42] multipathd: lock vectors during initial configuration
Quoted 4 lines · Hannes Reinecke
During initial configuration the CLI thread is already running, so we need to lock the vectors here to not race with the 'reconfigure' CLI command.
Are you sure of the 41 - 42 patches ?
- 41 moves the vecs locking from reconfigure caller to reconfigure
- 42 removes the locking from reconfigure
Regards,
Christophe Varoqui
www.opensvc.com
Quoted 36 lines
Signed-off-by Hannes Reinecke <[email protected]>--- multipathd/main.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)multipathd/main.c +1
-3@@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs)struct config * old = conf;int retval =1 ; -lock (vecs->lock);/* * free old map and path vectors ... they use old conf state */ @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs) retval =0 ; } -unlock (vecs->lock);return retval; }@@ -1641,9 +1639,9 @@ child (void * param)/* * fetch and configure both paths and multipaths */ -lock (vecs->lock); running_state =DAEMON_CONFIGURE ; +lock (vecs->lock);if (configure (vecs,1 )) {unlock (vecs->lock);condlog (0 ,"failure during configuration" );
Re: [PATCH 42/42] multipathd: lock vectors during initial configuration
Quoted 46 lines · Christophe Varoqui
Quoted 4 lines · Hannes Reinecke
During initial configuration the CLI thread is already running, so we need to lock the vectors here to not race with the 'reconfigure' CLI command.
Are you sure of the 41 - 42 patches ?
- 41 moves the vecs locking from reconfigure caller to reconfigure
- 42 removes the locking from reconfigureRegards,
Christophe Varoqui
www.opensvc.comQuoted 32 lines
Signed-off-by Hannes Reinecke <[email protected]>--- multipathd/main.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)multipathd/main.c +0
-3@@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs)struct config * old = conf;int retval =1 ; -lock (vecs->lock);/* * free old map and path vectors ... they use old conf state */ @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs) retval =0 ; } -unlock (vecs->lock);return retval; }@@ -1641,9 +1639,9 @@ child (void * param)/* * fetch and configure both paths and multipaths */ -lock (vecs->lock); running_state =DAEMON_CONFIGURE ;
Quoted 7 lines
multipathd/main.c child (void * param) · line 1647
Quoted 4 lines
multipathd/main.c +1
-0+lock (vecs->lock);if (configure (vecs,1 )) {unlock (vecs->lock);condlog (0 ,"failure during configuration" );
Yes.
Problem is we're already taking the lock prior to running any CLI commands in multipathd/main.c:uxsock_trigger().
So we cannot take the lock in reconfigure().
We also should not take the lock from the interrupt handler, as we might be in god knows which state.
If you're bothered by this I'd rather remove the SIGHUP signal handler; we can achieve very much the same with the 'reconfigure' CLI command.
Cheers,
Hannes
Signature · 9 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
Quoted 8 lines · Hannes Reinecke
Hi Christophe,
here are some patches and bugfixes with have accumulated on my SLES11 branch. Please apply.
(Patch 37 is blank due to merging, so it'll be missing from
the patch set).
Applied the full set before Ben's set ... this one big larger. Ruined my evening, but definitely worth it.
I only add to remove a merge resolution conflict label from "Clarify dev_loss_tmo settings" and resolve a conflict with latest vector.c::find_slot() patch from Mike.
Benjamin, please let me know if you will rebase your set. If not I'll be able to work on rebasing myself next week.
Thanks,
Christophe Varoqui
www.opensvc.com
Quoted 87 lines
Hannes Reinecke (41):
libmultipath: Invalid check formpp->wwidindm_addmap()Remove newline fromcondlog()
Fixup pathgroup allocation indisassemble_map()
libmultipath: resource leak inread_value_block()Accept several whitespaces in bindings file
Add TAGS makefile target
libmultipath: Fix typo inmp_prio_handler()
Do not trigger a map reload on priority updates IntroduceMP_FAST_IO_FAIL_UNSET
Checker name is not displayed on failure Valgrind fixes for prioritizer
Incorrect inquiry vendor length in hds prioritizer Print out multipath alias forflush_on_last_delmessages Clarify setting origin inpropsel.c
libmultipath: error checking inremove_features()Increase parameter buffer
Check return code frompathinfo()
Inconsistent string quoting
Switch off 'queue_if_no_path' before removing maps Double free indisassemble_map()
libmultipath: prio keyword ignored for multipath config Path checker should returnPATH_DOWNwhen no path is found Do not callsysfs_get_timeoutfor non-SCSI devices Makelog_pthreadmore robust
Print log messages when updating tables failed Update 'no_path_retry' correctly for failed paths Clean up uevent queue on shutdown
libmultipath: Print out uevent sequence number Fix race condition instop_waiter_thread()
UseVECTOR_SIZE()defines
Make 'allocated' an integer invector.h
Syntax error in/etc/init.d/boot.multipathmultipath.init.suse: Update usage message multipath.conf.5: Clarifydev_loss_tmosettings Clarifydev_loss_tmocapping in multipath.conf.5
multipathd: Ignore errors when creating pidfile multipathd deadlocks during restart
multipathd: sighandlers might use uninitialized gvecs
multipathd: crash in reconfigure CLI command
multipathd: lock vectors during initial configurationPetr Uzel (1):
prio: fix merging of prioritizers with different argsMakefile | 7 ++ libmultipath/alias.c | 2 +- libmultipath/config.h | 1 + libmultipath/configure.c | 13 ++-- libmultipath/configure.h | 2 +- libmultipath/devmapper.c | 36 ++++++++-- libmultipath/devmapper.h | 1 + libmultipath/dict.c | 108 +++++++++++++++--------------- libmultipath/discovery.c | 38 ++++++----- libmultipath/dmparser.c | 20 +++--- libmultipath/log_pthread.c | 101 +++++++++++++++++++--------- libmultipath/log_pthread.h | 10 ++- libmultipath/parser.c | 32 +++++---- libmultipath/prio.c | 51 ++++++++++++++- libmultipath/prio.h | 9 ++- libmultipath/prioritizers/alua.c | 1 + libmultipath/prioritizers/datacore.c | 1 + libmultipath/prioritizers/emc.c | 1 + libmultipath/prioritizers/hds.c | 6 +- libmultipath/prioritizers/hp_sw.c | 1 + libmultipath/prioritizers/ontap.c | 2 + libmultipath/prioritizers/rdac.c | 2 + libmultipath/prioritizers/weightedpath.c | 6 ++- libmultipath/propsel.c | 53 ++++++++------- libmultipath/structs.c | 7 ++ libmultipath/structs.h | 8 ++- libmultipath/structs_vec.c | 29 +++++--- libmultipath/sysfs.c | 2 +- libmultipath/uevent.c | 27 +++++++- libmultipath/uevent.h | 1 + libmultipath/util.c | 12 ++-- libmultipath/vector.c | 10 ++-- libmultipath/vector.h | 12 ++-- libmultipath/waiter.c | 5 +- multipath.conf.annotated | 1 + multipath/main.c | 8 +- multipath/multipath.conf.5 | 9 ++- multipath/multipath.init.suse | 6 +- multipathd/cli_handlers.c | 2 +- multipathd/main.c | 98 ++++++++++++--------------- 40 files changed, 468 insertions(+), 273 deletions(-)
Benjamin Marzinski
maintainerQuoted 12 lines · Christophe Varoqui
Quoted 8 lines · Hannes Reinecke
Hi Christophe,
here are some patches and bugfixes with have accumulated on my SLES11 branch. Please apply.
(Patch 37 is blank due to merging, so it'll be missing from
the patch set).Applied the full set before Ben's set ... this one big larger. Ruined my evening, but definitely worth it.
Quoted 6 lines
I only add to remove a merge resolution conflict label from "Clarify
dev_loss_tmosettings" and resolve a conflict with latestvector.c::find_slot()patch from Mike.Benjamin, please let me know if you will rebase your set. If not I'll be able to work on rebasing myself next week.
Yeah. I should be able to get to it tomorrow.
-Ben
Quoted 100 lines
Thanks,
Christophe Varoqui
www.opensvc.comQuoted 78 lines
Hannes Reinecke (41):
libmultipath: Invalid check formpp->wwidindm_addmap()Remove newline fromcondlog()
Fixup pathgroup allocation indisassemble_map()
libmultipath: resource leak inread_value_block()Accept several whitespaces in bindings file
Add TAGS makefile target
libmultipath: Fix typo inmp_prio_handler()
Do not trigger a map reload on priority updates IntroduceMP_FAST_IO_FAIL_UNSET
Checker name is not displayed on failure Valgrind fixes for prioritizer
Incorrect inquiry vendor length in hds prioritizer Print out multipath alias forflush_on_last_delmessages Clarify setting origin inpropsel.c
libmultipath: error checking inremove_features()Increase parameter buffer
Check return code frompathinfo()
Inconsistent string quoting
Switch off 'queue_if_no_path' before removing maps Double free indisassemble_map()
libmultipath: prio keyword ignored for multipath config Path checker should returnPATH_DOWNwhen no path is found Do not callsysfs_get_timeoutfor non-SCSI devices Makelog_pthreadmore robust
Print log messages when updating tables failed Update 'no_path_retry' correctly for failed paths Clean up uevent queue on shutdown
libmultipath: Print out uevent sequence number Fix race condition instop_waiter_thread()
UseVECTOR_SIZE()defines
Make 'allocated' an integer invector.h
Syntax error in/etc/init.d/boot.multipathmultipath.init.suse: Update usage message multipath.conf.5: Clarifydev_loss_tmosettings Clarifydev_loss_tmocapping in multipath.conf.5
multipathd: Ignore errors when creating pidfile multipathd deadlocks during restart
multipathd: sighandlers might use uninitialized gvecs
multipathd: crash in reconfigure CLI command
multipathd: lock vectors during initial configurationPetr Uzel (1):
prio: fix merging of prioritizers with different argsMakefile | 7 ++ libmultipath/alias.c | 2 +- libmultipath/config.h | 1 + libmultipath/configure.c | 13 ++-- libmultipath/configure.h | 2 +- libmultipath/devmapper.c | 36 ++++++++-- libmultipath/devmapper.h | 1 + libmultipath/dict.c | 108 +++++++++++++++--------------- libmultipath/discovery.c | 38 ++++++----- libmultipath/dmparser.c | 20 +++--- libmultipath/log_pthread.c | 101 +++++++++++++++++++--------- libmultipath/log_pthread.h | 10 ++- libmultipath/parser.c | 32 +++++---- libmultipath/prio.c | 51 ++++++++++++++- libmultipath/prio.h | 9 ++- libmultipath/prioritizers/alua.c | 1 + libmultipath/prioritizers/datacore.c | 1 + libmultipath/prioritizers/emc.c | 1 + libmultipath/prioritizers/hds.c | 6 +- libmultipath/prioritizers/hp_sw.c | 1 + libmultipath/prioritizers/ontap.c | 2 + libmultipath/prioritizers/rdac.c | 2 + libmultipath/prioritizers/weightedpath.c | 6 ++- libmultipath/propsel.c | 53 ++++++++------- libmultipath/structs.c | 7 ++ libmultipath/structs.h | 8 ++- libmultipath/structs_vec.c | 29 +++++--- libmultipath/sysfs.c | 2 +- libmultipath/uevent.c | 27 +++++++- libmultipath/uevent.h | 1 + libmultipath/util.c | 12 ++-- libmultipath/vector.c | 10 ++-- libmultipath/vector.h | 12 ++--Quoted 9 lines
libmultipath/waiter.c | 5 +- multipath.conf.annotated | 1 + multipath/main.c | 8 +- multipath/multipath.conf.5 | 9 ++- multipath/multipath.init.suse | 6 +- multipathd/cli_handlers.c | 2 +- multipathd/main.c | 98 ++++++++++++--------------- 40 files changed, 468 insertions(+), 273 deletions(-)--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel