Re: multipath disk size

[Arsene Gschwind <arsene.gschwind@unibas.ch>]

Hello Christophe,

I've applied the patchs editing by hand.
Doing tests after applied the first Patch results in an empty serial 
printout, but the volume size returned is korrekt and the multipath 
driver works fine.
Doing tests after applied the second Patch results with a correct serial 
printout and correct volume size.
It seems that this 2 patches resolved my issue, thanks a lot.

Are you going to release a new tarball version which includes this 2 
patches?

Thanks a lot et "Joyeuse Pâques" avec beaucoup de chocolat :-)
Arsène

Christophe Varoqui wrote:

> Arsene Gschwind a écrit :
>
>> I've found something strange, 2 declaration of SERIAL_SIZE with 
>> different sizes
>> libmultipath/structs.h:#define SERIAL_SIZE              17
>> path_priority/pp_balance_units/pp_balance_units.c:#define SERIAL_SIZE 
>> 255
>>
>> In my case the first buffer size is to small because pp->serial has a 
>> size of 17 chars and when you look at my output it has 40 chars.
>> By setting libmultipath/structs.h:#define SERIAL_SIZE    to 255 it 
>> works for me and the returned volume size is correct.
>> I'm not sure if this is the right solution, it would be great if 
>> someone could verify that issue.
>>
>> Thanks a lot for your work
>> Arsène
>>
>>
> Very good catch.
> get_serial() was happily overflowing.
> Sorry for your being the first with a long-serial-hardware :)
>
>
> Please try the following 2 patchs, with testing between to 2, please.
>
> === 1===
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -339,24 +339,26 @@ do_inq(int sg_fd, int cmddt, int evpd, u
>         return -1;
> }
>
> -int
> -get_serial (char * str, int fd)
> +static int
> +get_serial (char * str, int maxlen, int fd)
> {
>         int len = 0;
>         char buff[MX_ALLOC_LEN + 1] = {0};
>
>        if (fd < 0)
> -                return 0;
> +                return 1;
>
>        if (0 == do_inq(fd, 0, 1, 0x80, buff, MX_ALLOC_LEN, 0)) {
>                len = buff[3];
> +               if (len >= maxlen)
> +                       return 1;
>                if (len > 0) {
>                        memcpy(str, buff + 4, len);
>                        str[len] = '\0';
>                }
> -               return 1;
> +               return 0;
>        }
> -        return 0;
> +        return 1;
> }
>
> static int
> @@ -597,7 +599,7 @@ static int
> scsi_ioctl_pathinfo (struct path * pp, int mask)
> {
>        if (mask & DI_SERIAL) {
> -               get_serial(pp->serial, pp->fd);
> +               get_serial(pp->serial, SERIAL_SIZE, pp->fd);
>                condlog(3, "%s: serial = %s", pp->dev, pp->serial);
>        }
>
> diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
> --- a/libmultipath/discovery.h
> +++ b/libmultipath/discovery.h
> @@ -30,7 +30,6 @@ int sysfs_get_size (char * sysfs_path, c
> int path_discovery (vector pathvec, struct config * conf, int flag);
>
> void basename (char *, char *);
> -int get_serial (char * buff, int fd);
> int do_tur (char *);
> int devt2devname (char *, char *);
> int pathinfo (struct path *, vector hwtable, int mask);
> diff --git a/path_priority/pp_balance_units/pp_balance_units.c 
> b/path_priority/pp_balance_units/pp_balance_units.c
> --- a/path_priority/pp_balance_units/pp_balance_units.c
> +++ b/path_priority/pp_balance_units/pp_balance_units.c
> @@ -172,7 +172,7 @@ do_inq(int sg_fd, int cmddt, int evpd, u
> }
>
> static int
> -get_serial (char * str, char * devt)
> +get_serial (char * str, int maxlen, char * devt)
> {
>        int fd;
>         int len;
> @@ -181,20 +181,22 @@ get_serial (char * str, char * devt)
>        fd = opennode(devt, O_RDONLY);
>
>        if (fd < 0)
> -                return 0;
> +                return 1;
>
>        if (0 == do_inq(fd, 0, 1, 0x80, buff, MX_ALLOC_LEN, 0)) {
>                len = buff[3];
> +               if (len >= maxlen)
> +                       return 1;
>                if (len > 0) {
>                        memcpy(str, buff + 4, len);
>                        buff[len] = '\0';
>                }
>                close(fd);
> -               return 1;
> +               return 0;
>        }
>
>        closenode(devt, fd);
> -        return 0;
> +        return 1;
> }
>
> static void *
> @@ -358,7 +360,7 @@ get_paths (vector pathvec)
>                        if (pos == BEFOREPG)
>                                pos = INPG;
>
> -                       get_serial(pp->serial, pp->dev_t);
> +                       get_serial(pp->serial, SERIAL_SIZE, pp->dev_t);
>                        vector_alloc_slot(pathvec);
>                        vector_set_slot(pathvec, pp);
>                        debug("store %s [%s]",
> @@ -449,7 +451,7 @@ main (int argc, char **argv)
>        if (optind<argc)
>                strncpy(ref_path->dev_t, argv[optind], WORD_SIZE);
>
> -       get_serial(ref_path->serial, ref_path->dev_t);
> +       get_serial(ref_path->serial, SERIAL_SIZE, ref_path->dev_t);
>
>        if (!ref_path->serial || !strlen(ref_path->serial))
>                exit_tool(0);
>
>
> ==== 2 ====
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -2,7 +2,7 @@
> #define _STRUCTS_H
>
> #define WWID_SIZE              64
> -#define SERIAL_SIZE            17
> +#define SERIAL_SIZE            64
> #define NODE_NAME_SIZE         19
> #define PATH_STR_SIZE                  16
> #define PARAMS_SIZE            1024
>
>
>
> -- 
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel


-- 
***********************************************************
Gschwind Arsene			
Universitaet Rechenzentrum (URZ)
Klingelbergstrasse 70
CH-4056 Basel
SWITZERLAND

Languages : F/E/D
WWW:  <http://www.urz.unibas.ch>
Mail: <Arsene DOT Gschwind AT unibas DOT ch>
************************************************************