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

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

[Lukasz Orlowski]

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);

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

[NeilBrown]

[-- 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 --]

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

[Orlowski, Lukasz]

Thanks for the feedback. I'll keep this in mind, while producing my next patches.

Pozdrawiam / Regards,
Łukasz Orłowski

-----Original Message-----
From: NeilBrown [mailto:neilb@suse.de] 
Sent: Monday, November 07, 2011 2:23 AM
To: Orlowski, Lukasz
Cc: linux-raid@vger.kernel.org; Williams, Dan J
Subject: Re: [PATCH] fix: Allowed to assemble 2 volumes with the same names from config file.

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

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html