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

Re: [dm-devel] [PATCH v2] multipath: change default configuration parameters



On 01/10/2013 09:05 PM, Benjamin Marzinski wrote:
This patch makes multipath devices default to setting fast_io_fail,
switches the default selector to service-time and removes the
round-robin setting of the builtin device configurations. The goal is
to give multipath devices better performance "out of the box".

Couldn't you split it off in three patches?
(Bad) experience tells me one always run into problems when
munching together unrelated pieces ...

Signed-off-by: Benjamin Marzinski <bmarzins redhat com>
---
  libmultipath/config.c   |    1
  libmultipath/defaults.h |    2 -
  libmultipath/hwtable.c  |   65 ------------------------------------------------
  3 files changed, 2 insertions(+), 66 deletions(-)

Index: multipath-tools-120821/libmultipath/config.c
===================================================================
--- multipath-tools-120821.orig/libmultipath/config.c
+++ multipath-tools-120821/libmultipath/config.c
@@ -516,6 +516,7 @@ load_config (char * file)
  	conf->reassign_maps = DEFAULT_REASSIGN_MAPS;
  	conf->checkint = DEFAULT_CHECKINT;
  	conf->max_checkint = MAX_CHECKINT(conf->checkint);
+	conf->fast_io_fail = 5;

  	/*
  	 * preload default hwtable

Please use #define DEFAULT_FAST_IO_FAIL here.

Index: multipath-tools-120821/libmultipath/hwtable.c
===================================================================
--- multipath-tools-120821.orig/libmultipath/hwtable.c
+++ multipath-tools-120821/libmultipath/hwtable.c
@@ -28,7 +28,6 @@ static struct hwentry default_hw[] = {
  		.product       = "Compellent Vol",
  		.features      = DEFAULT_FEATURES,
  		.hwhandler     = DEFAULT_HWHANDLER,
-		.selector      = DEFAULT_SELECTOR,
  		.pgpolicy      = MULTIBUS,
  		.pgfailback    = -FAILBACK_IMMEDIATE,
  		.rr_weight     = RR_WEIGHT_NONE,

Can't we get rid of _all_ 'DEFAULT_XXX' settings in the
hardware table? They _should_ be set to default during
init anyway, shouldn't they?
And that would shorten the hardware table by quite a lot ...


Index: multipath-tools-120821/libmultipath/defaults.h
===================================================================
--- multipath-tools-120821.orig/libmultipath/defaults.h
+++ multipath-tools-120821/libmultipath/defaults.h
@@ -1,7 +1,7 @@
  #define DEFAULT_UID_ATTRIBUTE	"ID_SERIAL"
  #define DEFAULT_UDEVDIR		"/dev"
  #define DEFAULT_MULTIPATHDIR	"/" LIB_STRING "/multipath"
-#define DEFAULT_SELECTOR	"round-robin 0"
+#define DEFAULT_SELECTOR	"service-time 0"
  #define DEFAULT_ALIAS_PREFIX	"mpath"
  #define DEFAULT_FEATURES	"0"
  #define DEFAULT_HWHANDLER	"0"

Hmm.

And you _have_ tested 'service-time' thoroughly, now have you?

Cheers,

Hannes
--
Dr. Hannes Reinecke		      zSeries & Storage
hare suse de			      +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)


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