Re: [PATCH] fix: Allowed to assemble 2 volumes with the same names from config file.

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3197 bytes --]

On Fri, 04 Nov 2011 09:51:48 +0100 Lukasz Orlowski
<lukasz.orlowski@intel.com> wrote:

> mdadm allowes to assemble 2 volumes with the same names based on the
> config file. The issue is fixed by iterating over the list of md device
> identifiers and comparing the names of md devices against each other,
> detecting identical names and blocking the assembly should the same names
> be found.
> Now having detected duplicate names, mdadm terminates without assembling
> the container, displaying appropriate prompt.
> 
> Signed-off-by: Lukasz Orlowski <lukasz.orlowski@intel.com>
> ---
>  config.c |   25 +++++++++++++++++++++++++
>  mdadm.c  |    7 +++++++
>  mdadm.h  |    1 +
>  3 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/config.c b/config.c
> index c0a6baa..9b337eb 100644
> --- a/config.c
> +++ b/config.c
> @@ -1088,3 +1088,28 @@ struct mddev_ident *conf_match(struct mdinfo *info, struct supertype *st)
>  	}
>  	return match;
>  }
> +
> +int conf_verify_devnames(struct mddev_ident *array_list)
> +{
> +	struct mddev_ident *a1, *a2;
> +
> +	for (a1 = array_list; a1; a1 = a1->next) {
> +		if (!a1->devname)
> +			continue;
> +		for (a2 = a1->next; a2; a2 = a2->next) {
> +			if (!a2->devname)
> +				continue;
> +			if (!strcmp(a1->devname, a2->devname)) {
> +				fprintf(stderr,
> +					Name ": Devices %x:%x:%x:%x and "
> +					"%x:%x:%x:%x have the same name: %s\n",
> +					a1->uuid[0], a1->uuid[1], a1->uuid[2],
> +					a1->uuid[3], a2->uuid[0], a2->uuid[1],
> +					a2->uuid[2], a2->uuid[3], a1->devname);
> +				return 1;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/mdadm.c b/mdadm.c
> index 56de7b7..ebf4c4d 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -1282,6 +1282,13 @@ int main(int argc, char *argv[])
>  			struct map_ent *map = NULL;
>  			int cnt = 0;
>  			int failures, successes;
> +
> +			if (conf_verify_devnames(array_list)) {
> +				fprintf(stderr, Name
> +					": Duplicate MD device names in "
> +					"conf file were found.\n");
> +				exit(1);
> +			}
>  			if (devlist == NULL) {
>  				fprintf(stderr, Name ": No devices listed in conf file were found.\n");
>  				exit(1);
> diff --git a/mdadm.h b/mdadm.h
> index 6dbc5ea..bff454f 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1145,6 +1145,7 @@ extern char *conf_get_homehost(int *require_homehostp);
>  extern char *conf_line(FILE *file);
>  extern char *conf_word(FILE *file, int allow_key);
>  extern int conf_name_is_free(char *name);
> +extern int conf_verify_devnames(struct mddev_ident *array_list);
>  extern int devname_matches(char *name, char *match);
>  extern struct mddev_ident *conf_match(struct mdinfo *info, struct supertype *st);
>  extern int experimental(void);
> 

Thanks.
I've applied this patch, though with a few changes.

1/ I personally despise the "!strcmp()" usage.  It seems to be saying "not"
   but it is actually saying "is equal".  I always write
       strcmp(a,c) == 0
   or != or < or whatever comparison is wanted.
2/ You assume that both names have a uuid but that may not be correct.  You
   need to check 'uuid_set'.

Thanks,
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]