Re: multipath: change the DEFAULT_MINIO for the request based multipath

[Malahal Naineni <malahal@us.ibm.com>]

>Christophe Varoqui [christophe.varoqui@gmail.com] wrote:

>+dm_drv_get_rq (void)
>+{
>+	unsigned int minv_dmrq[3] = {1, 1, 0};
>+	unsigned int *v;
>+
>+	v = zalloc(3);
>+	if (!v)
>+		return 0;
>+
>+	if (dm_drv_version(v, TGT_MPATH)) {
>+		/* in doubt return least capable */
>+		return 0;
>+	}

Looks like the 'v' is NOT freed. Local stack allocation looks much
cleaner, why not do that? You missed the same thing at other places, so
I imagine you started with the on stack local structure but changed
later???

>+static int
>+dm_drvprereq (char * str)
>+{
>+	unsigned int minv[3] = {1, 0, 3};
>+	unsigned int *v;
>+
>+	v = zalloc(3);
>+	if (!v)
>+		return 0;
>+
>+	if (dm_drv_version(v, str)) {
>+		/* in doubt return not capable */
>+		return 1;
>+	}

Missed freeing 'v'. Also, this function taking the target driver name as
'str' doesn't make sense as the minimum version is hard coded internally
to this function. Take no arguments and pass 'TGT_MPATH' while calling
dm_drv_version.

> static int
>+def_minio_rq_handler(vector strvec)
>+{
>+	char * buff;
>+
>+	buff = set_value(strvec);
>+
>+	if (!buff)
>+		return 1;
>+
>+	conf->minio_rq = atoi(buff);
>+	FREE(buff);
>+
>+	return 0;
>+}

I was thinking why introduce minio and minio_rq in the
/etc/multipath.conf file. By default we ship empty /etc/multipath.conf
file. If the admin wants to override the default, he knows if he is
going to use the BIO or REQUEST based multipath. So here is my approach
to avoid introducing another similar looking config keyword:

If we detect minio setting from the config file (either in the
multipath, device or default section), we use it. The code internally
has minio_rq just as you did. In other words, the only change I am
proposing is, use the existing routines that handle minio keyword and
set minio_rq there.

E.g: def_minio_handler() will set conf->minio_rq = conf->minio
hw_minio_handler() will set hwe->minio_rq = hwe->minio
mp_minio_handler() will set mpe->minio_rq = mpe->minio

Thanks, Malahal.