[PATCH 0/5] Autorebuild, spare/spare-same-slot fixes

[PATCH 0/5] Autorebuild, spare/spare-same-slot fixes

[Przemyslaw Czarnowski]

The following series changes (improves) behaviors implemented so far in 
Incremental for bare disks.
It enables some features for containers working so far only for native
metadata (size check and degradation) and fixes domain checking for 
"same slot" action.

---

Przemyslaw Czarnowski (5):
      external: get number of failed disks for container
      Added test for array degradation for spare-same-slot
      Skip domain check for spare-same-slot
      Validate size of potential spare disk for external metadata (with containers)
      Consider target only for spare-same-domain


 Incremental.c |   79 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 66 insertions(+), 13 deletions(-)

-- 
Best Regards
Przemyslaw Czarnowski

[PATCH 1/5] external: get number of failed disks for container

[Przemyslaw Czarnowski]

Container degradation here is defined as the number of failed disks in
mostly degraded sub-array. This number is used as value for
array.failed_disks and used in comparison to find best match.

Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
---
 Incremental.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index a4ac1b5..ef719fa 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -820,6 +820,34 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
 	return cnt;
 }
 
+/* test if container has degraded member(s) */
+int container_members_max_degradation(struct map_ent *map, struct map_ent *me)
+{
+	mdu_array_info_t array;
+	int afd;
+	int max_degraded = 0;
+	char devname[100];
+
+	snprintf(devname, sizeof(devname), "md%d", me->devnum);
+	for(; map; map = map->next) {
+		if (!is_subarray(map->metadata) ||
+		    strncmp(me->metadata+1, devname, strlen(devname) != 0))
+			continue;
+		afd = open_dev(map->devnum);
+		if (afd < 0)
+			continue;
+		/* most accurate information regarding array degradation */
+		if (ioctl(afd, GET_ARRAY_INFO, &array) >= 0) {
+			int degraded = array.raid_disks - array.active_disks -
+				       array.spare_disks;
+			if (degraded > max_degraded)
+				max_degraded = degraded;
+		}
+		close(afd);
+	}
+	return (max_degraded);
+}
+
 static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 			   struct map_ent *target, int bare,
 			   struct supertype *st, int verbose)
@@ -887,7 +915,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 					 GET_DEVS|GET_OFFSET|GET_SIZE|GET_STATE|
 					 GET_COMPONENT|GET_VERSION);
 			if (sra)
-				sra->array.failed_disks = 0;
+				sra->array.failed_disks = -1;
 		}
 		if (!sra)
 			continue;
@@ -914,6 +942,11 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 				goto next;
 		} else
 			st2 = st;
+		/* update number of failed disks for mostly degraded
+		 * container member */
+		if (sra->array.failed_disks == -1)
+			sra->array.failed_disks = container_members_max_degradation(map, mp);
+
 		get_dev_size(dfd, NULL, &devsize);
 		if (st2->ss->avail_size(st2, devsize) < sra->component_size) {
 			if (verbose > 1)

Re: [PATCH 1/5] external: get number of failed disks for container

[Neil Brown]

On Thu, 23 Dec 2010 17:02:33 +0100 Przemyslaw Czarnowski
<przemyslaw.hawrylewicz.czarnowski@intel.com> wrote:

> Container degradation here is defined as the number of failed disks in
> mostly degraded sub-array. This number is used as value for
> array.failed_disks and used in comparison to find best match.
> 
> Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>

Applied... but ...

> ---
>  Incremental.c |   35 ++++++++++++++++++++++++++++++++++-
>  1 files changed, 34 insertions(+), 1 deletions(-)
> 
> diff --git a/Incremental.c b/Incremental.c
> index a4ac1b5..ef719fa 100644
> --- a/Incremental.c
> +++ b/Incremental.c
> @@ -820,6 +820,34 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
>  	return cnt;
>  }
>  
> +/* test if container has degraded member(s) */
> +int container_members_max_degradation(struct map_ent *map, struct map_ent *me)

should be 'static'.


> +{
> +	mdu_array_info_t array;
> +	int afd;
> +	int max_degraded = 0;
> +	char devname[100];
> +
> +	snprintf(devname, sizeof(devname), "md%d", me->devnum);
> +	for(; map; map = map->next) {
> +		if (!is_subarray(map->metadata) ||
> +		    strncmp(me->metadata+1, devname, strlen(devname) != 0))
> +			continue;

Rather than formatting devnum as a devname, you can use devname2devnum on
me->metadata+1 and compare that with me->devnum.

And the strncmp should have been for map->metadata+1, not me->metadata+1 !!!!

I have fixed all these.

Thanks,
NeilBrown



> +		afd = open_dev(map->devnum);
> +		if (afd < 0)
> +			continue;
> +		/* most accurate information regarding array degradation */
> +		if (ioctl(afd, GET_ARRAY_INFO, &array) >= 0) {
> +			int degraded = array.raid_disks - array.active_disks -
> +				       array.spare_disks;
> +			if (degraded > max_degraded)
> +				max_degraded = degraded;
> +		}
> +		close(afd);
> +	}
> +	return (max_degraded);
> +}
> +
>  static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
>  			   struct map_ent *target, int bare,
>  			   struct supertype *st, int verbose)
> @@ -887,7 +915,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
>  					 GET_DEVS|GET_OFFSET|GET_SIZE|GET_STATE|
>  					 GET_COMPONENT|GET_VERSION);
>  			if (sra)
> -				sra->array.failed_disks = 0;
> +				sra->array.failed_disks = -1;
>  		}
>  		if (!sra)
>  			continue;
> @@ -914,6 +942,11 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
>  				goto next;
>  		} else
>  			st2 = st;
> +		/* update number of failed disks for mostly degraded
> +		 * container member */
> +		if (sra->array.failed_disks == -1)
> +			sra->array.failed_disks = container_members_max_degradation(map, mp);
> +
>  		get_dev_size(dfd, NULL, &devsize);
>  		if (st2->ss->avail_size(st2, devsize) < sra->component_size) {
>  			if (verbose > 1)

[PATCH 2/5] Added test for array degradation for spare-same-slot

[Przemyslaw Czarnowski]

spare-same-slot allows re-adding of missing array member with disk
re-inserted into the same slot where previous member was plugged in.
If in the meantime another spare has been used for recovery, same slot
cookie should be ignored.

Signed-off-by: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
---
 Incremental.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index ef719fa..137bd75 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -968,11 +968,13 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		 * arrays/containers that match 'target'.
 		 * If 'target' is set and 'bare' is true, we prefer the
 		 * array which matches 'target'.
+		 * target is considered only if we deal with degraded array
 		 */
 		if (target) {
 			if (strcmp(target->metadata, mp->metadata) == 0 &&
 			    memcmp(target->uuid, mp->uuid,
-				   sizeof(target->uuid)) == 0) {
+				   sizeof(target->uuid)) == 0 && 
+			    sra->array.failed_disks > 0) {
 				/* This is our target!! */
 				if (chosen)
 					sysfs_free(chosen);

[PATCH 3/5] Skip domain check for spare-same-slot

[Przemyslaw Czarnowski]

If lost disk was the only one that belonged to particular domain, array
won't match with that domain any longer. We can achieve this by moving
domain check below the 'target' test.
---
 Incremental.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index 137bd75..8d9f825 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -954,15 +954,6 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 					devname, mp->path);
 			goto next;
 		}
-		dl = domain_from_array(sra, st2->ss->name);
-		if (!domain_test(dl, pol, st2->ss->name)) {
-			/* domain test fails */
-			if (verbose > 1)
-				fprintf(stderr, Name ": not adding %s to %s as it is not in a compatible domain\n",
-					devname, mp->path);
-
-			goto next;
-		}
 		/* test against target.
 		 * If 'target' is set and 'bare' is false, we only accept
 		 * arrays/containers that match 'target'.
@@ -990,6 +981,15 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 				goto next;
 		}
 
+		dl = domain_from_array(sra, st2->ss->name);
+		if (!domain_test(dl, pol, st2->ss->name)) {
+			/* domain test fails */
+			if (verbose > 1)
+				fprintf(stderr, Name ": not adding %s to %s as it is not in a compatible domain\n",
+					devname, mp->path);
+
+			goto next;
+		}
 		/* all tests passed, OK to add to this array */
 		if (!chosen) {
 			chosen = sra;

[PATCH 4/5] Validate size of potential spare disk for external metadata (with containers)

[Przemyslaw Czarnowski]

mdinfo read with sysfs_read do not contain information about the space
needed to store data of all volumes created in that container, so that
spare can be used as replacement for existing subarrays in the future.
---
 Incremental.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index 8d9f825..e022506 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -890,6 +890,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		struct domainlist *dl = NULL;
 		struct mdinfo *sra;
 		unsigned long long devsize;
+		unsigned long long component_size;
 
 		if (is_subarray(mp->metadata))
 			continue;
@@ -948,7 +949,23 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 			sra->array.failed_disks = container_members_max_degradation(map, mp);
 
 		get_dev_size(dfd, NULL, &devsize);
-		if (st2->ss->avail_size(st2, devsize) < sra->component_size) {
+		if (sra->component_size == 0) {
+			/* true for containers, here we must read superblock
+			 * to obtain minimum spare size */
+			struct supertype *st3 = dup_super(st2);
+			int mdfd = open_dev(mp->devnum);
+			if (!mdfd)
+				goto next;
+			if (st3->ss->load_container &&
+			    !st3->ss->load_container(st3, mdfd, mp->path)) {
+				component_size = st3->ss->min_acceptable_spare_size(st3);
+				st3->ss->free_super(st3);
+			}
+			free(st3);
+			close(mdfd);
+		}
+		if ((sra->component_size > 0 && st2->ss->avail_size(st2, devsize) < sra->component_size) ||
+		    (sra->component_size == 0 && devsize < component_size)) {
 			if (verbose > 1)
 				fprintf(stderr, Name ": not adding %s to %s as it is too small\n",
 					devname, mp->path);

[PATCH 5/5] Consider target only for spare-same-domain

[Przemyslaw Czarnowski]

otherwise, matching target will force spare-same-domain regardless of
action that comes in domain.
---
 Incremental.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index e022506..24015ca 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -978,7 +978,8 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		 * array which matches 'target'.
 		 * target is considered only if we deal with degraded array
 		 */
-		if (target) {
+		if (target && policy_action_allows(pol, st2->ss->name,
+						   act_spare_same_slot)) {
 			if (strcmp(target->metadata, mp->metadata) == 0 &&
 			    memcmp(target->uuid, mp->uuid,
 				   sizeof(target->uuid)) == 0 && 

Re: [PATCH 0/5] Autorebuild, spare/spare-same-slot fixes

[Neil Brown]

On Thu, 23 Dec 2010 17:02:00 +0100 Przemyslaw Czarnowski
<przemyslaw.hawrylewicz.czarnowski@intel.com> wrote:

> The following series changes (improves) behaviors implemented so far in 
> Incremental for bare disks.
> It enables some features for containers working so far only for native
> metadata (size check and degradation) and fixes domain checking for 
> "same slot" action.
> 
> ---
> 
> Przemyslaw Czarnowski (5):
>       external: get number of failed disks for container
>       Added test for array degradation for spare-same-slot
>       Skip domain check for spare-same-slot
>       Validate size of potential spare disk for external metadata (with containers)
>       Consider target only for spare-same-domain
> 

Thanks. All of these applied, with one changes as I described separately.

NeilBrown