[PATCH 0/4] Fix freezing multiple arrays in container during assembly (2)

[PATCH 0/4] Fix freezing multiple arrays in container during assembly (2)

[Adam Kwolek]

The following series corrects freezing reshaped array in container during
assembly while other arrays are present.

The goal of the patches is to get entire assembled container (all arrays inside)
to be blocked from monitoring when reshape is in progress /exactly as when
reshape is initialized from mdadm command line/.

This time, freezing decision is taken based upon information from metadata,
not /sysfs entries. For this purpose recovery_blocked field is added to mdinfo.


Note: First patch in this series for mdadm throwing core file problem
      is resent as reminder.

BR
Adam

---

Adam Kwolek (4):
      Remove freeze() call from Grow_continue()
      imsm: Fill recovery_blocked field present in mdinfo
      Add recovery blocked field to mdinfo
      FIX: restore_backup() throws core dump


 Assemble.c    |    2 +-
 Grow.c        |   13 ++++++-------
 mdadm.h       |    7 +++++++
 super-ddf.c   |    2 ++
 super-intel.c |    6 ++++--
 super0.c      |    2 ++
 super1.c      |    2 ++
 7 files changed, 24 insertions(+), 10 deletions(-)

-- 
Signature

[PATCH 1/4] FIX: restore_backup() throws core dump

[Adam Kwolek]

1. restore_backup() throws core dump during releasing fdlist.
Loop for closing handlers checks next_spare variable,
but iterates disk_count.

2. fdlist initialization/close is corrected to initialize/close
   whole allocated array

3. next_spare variable name is replaced by spares

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 Grow.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Grow.c b/Grow.c
index de177d8..ae0d112 100644
--- a/Grow.c
+++ b/Grow.c
@@ -38,7 +38,7 @@
 int restore_backup(struct supertype *st,
 		   struct mdinfo *content,
 		   int working_disks,
-		   int next_spare,
+		   int spares,
 		   char *backup_file,
 		   int verbose)
 {
@@ -46,7 +46,7 @@ int restore_backup(struct supertype *st,
 	int *fdlist;
 	struct mdinfo *dev;
 	int err;
-	int disk_count = next_spare + working_disks;
+	int disk_count = working_disks + spares;
 
 	dprintf("Called restore_backup()\n");
 	fdlist = malloc(sizeof(int) * disk_count);
@@ -55,7 +55,7 @@ int restore_backup(struct supertype *st,
 			Name ": cannot allocate memory for disk list\n");
 		return 1;
 	}
-	for (i = 0; i < next_spare; i++)
+	for (i = 0; i < disk_count; i++)
 		fdlist[i] = -1;
 	for (dev = content->devs; dev; dev = dev->next) {
 		char buf[22];
@@ -68,16 +68,16 @@ int restore_backup(struct supertype *st,
 		if (dev->disk.raid_disk >= 0)
 			fdlist[dev->disk.raid_disk] = fd;
 		else
-			fdlist[next_spare++] = fd;
+			fdlist[working_disks++] = fd;
 	}
 
 	if (st->ss->external && st->ss->recover_backup)
 		err = st->ss->recover_backup(st, content);
 	else
-		err = Grow_restart(st, content, fdlist, next_spare,
+		err = Grow_restart(st, content, fdlist, working_disks,
 				   backup_file, verbose > 0);
 
-	while (next_spare > 0) {
+	while (disk_count > 0) {
 		disk_count--;
 		if (fdlist[disk_count] >= 0)
 			close(fdlist[disk_count]);

Re: [PATCH 1/4] FIX: restore_backup() throws core dump

[NeilBrown]

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

On Mon, 03 Oct 2011 18:13:38 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> 1. restore_backup() throws core dump during releasing fdlist.
> Loop for closing handlers checks next_spare variable,
> but iterates disk_count.

The best way to fix this is simply to just fix this.. See below.

> 
> 2. fdlist initialization/close is corrected to initialize/close
>    whole allocated array

This is unnecessary.

> 
> 3. next_spare variable name is replaced by spares

But the variable doesn't contain a count of the number of spares.  It contains
the number of the next spare...

NeilBrown

commit cc7f63e55319b5c372af20ce528e7e7230746d92
Author: NeilBrown <neilb@suse.de>
Date:   Wed Oct 5 13:29:16 2011 +1100

    restore_backup() throws core dump
    
    restore_backup() throws core dump during releasing fdlist.
    Loop for closing handlers checks next_spare variable,
    but iterates disk_count.
    
    Reported-by: Adam Kwolek <adam.kwolek@intel.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/Grow.c b/Grow.c
index de177d8..9fa2d6b 100644
--- a/Grow.c
+++ b/Grow.c
@@ -78,9 +78,9 @@ int restore_backup(struct supertype *st,
 				   backup_file, verbose > 0);
 
 	while (next_spare > 0) {
-		disk_count--;
-		if (fdlist[disk_count] >= 0)
-			close(fdlist[disk_count]);
+		next_spare--;
+		if (fdlist[next_spare] >= 0)
+			close(fdlist[next_spare]);
 	}
 	free(fdlist);
 	if (err) {

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

[PATCH 2/4] Add recovery blocked field to mdinfo

[Adam Kwolek]

When container is assembled while reshape is active on one of its member
whole container can be required to be blocked from monitoring.
For such purpose field recovery blocked is added to mdinfo structure.

When metadata handler finds active reshape in container it should set
recovery_blocked field to disable whole container monitoring during
reshape.

For arrays that doesn't use containers, recovery_blocked field
has the same value as reshape_active field e.g. super0/1.
In fact,recovery is blocked during reshape for such arrays.
For ddf, metadata handler doesn't set reshape_active field,
so recovery_blocked is not set also.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 Assemble.c  |    2 +-
 mdadm.h     |    7 +++++++
 super-ddf.c |    2 ++
 super0.c    |    2 ++
 super1.c    |    2 ++
 5 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index afca38e..4511f4d 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1527,7 +1527,7 @@ int assemble_container_content(struct supertype *st, int mdfd,
 		if (sysfs_set_array(content, md_get_version(mdfd)) != 0)
 			return 1;
 
-	if (content->reshape_active)
+	if (st->ss->external && content->recovery_blocked)
 		block_subarray(content);
 
 	if (sra)
diff --git a/mdadm.h b/mdadm.h
index f219c95..c397045 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -196,6 +196,13 @@ struct mdinfo {
 						    */
 	int			reshape_active;
 	unsigned long long	reshape_progress;
+	int			recovery_blocked; /* for external metadata it
+						   * indicates that there is
+						   * reshape in progress in
+						   * container,
+						   * for native metadata it is
+						   * reshape_active field mirror
+						   */
 	union {
 		unsigned long long resync_start; /* per-array resync position */
 		unsigned long long recovery_start; /* per-device rebuild position */
diff --git a/super-ddf.c b/super-ddf.c
index 7312ba4..d3f8b29 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1374,6 +1374,7 @@ static void getinfo_super_ddf(struct supertype *st, struct mdinfo *info, char *m
 
 	info->recovery_start = MaxSector;
 	info->reshape_active = 0;
+	info->recovery_blocked = 0;
 	info->name[0] = 0;
 
 	info->array.major_version = -1;
@@ -1449,6 +1450,7 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
 	info->recovery_start = MaxSector;
 	info->resync_start = 0;
 	info->reshape_active = 0;
+	info->recovery_blocked = 0;
 	if (!(ddf->virt->entries[info->container_member].state
 	      & DDF_state_inconsistent)  &&
 	    (ddf->virt->entries[info->container_member].init_state
diff --git a/super0.c b/super0.c
index f791e9d..3061ecf 100644
--- a/super0.c
+++ b/super0.c
@@ -387,6 +387,8 @@ static void getinfo_super0(struct supertype *st, struct mdinfo *info, char *map)
 	} else
 		info->reshape_active = 0;
 
+	info->recovery_blocked = info->reshape_active;
+
 	sprintf(info->name, "%d", sb->md_minor);
 	/* work_disks is calculated rather than read directly */
 	for (i=0; i < MD_SB_DISKS; i++)
diff --git a/super1.c b/super1.c
index 0cd4124..9a72681 100644
--- a/super1.c
+++ b/super1.c
@@ -626,6 +626,8 @@ static void getinfo_super1(struct supertype *st, struct mdinfo *info, char *map)
 	} else
 		info->reshape_active = 0;
 
+	info->recovery_blocked = info->reshape_active;
+
 	if (map)
 		for (i=0; i<map_disks; i++)
 			map[i] = 0;

[PATCH 3/4] imsm: Fill recovery_blocked field present in mdinfo

[Adam Kwolek]

If any reshape in container is active set recovery_blocked field.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 super-intel.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 8e9e977..e761819 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2251,8 +2251,7 @@ int imsm_reshape_blocks_arrays_changes(struct intel_super *super)
 	 */
 	for (i_dev = super->devlist; i_dev; i_dev = i_dev->next) {
 		dev = i_dev->dev;
-		if (dev->vol.migr_state &&
-		    dev->vol.migr_type == MIGR_GEN_MIGR) {
+		if (is_gen_migration(dev)) {
 			/* No repair during any migration in container
 			 */
 			rv = 1;
@@ -2294,6 +2293,8 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
 	info->custom_array_size   = __le32_to_cpu(dev->size_high);
 	info->custom_array_size   <<= 32;
 	info->custom_array_size   |= __le32_to_cpu(dev->size_low);
+	info->recovery_blocked = imsm_reshape_blocks_arrays_changes(st->sb);
+
 	if (prev_map && map->map_state == prev_map->map_state) {
 		info->reshape_active = 1;
 		info->new_level = get_imsm_raid_level(map);
@@ -2516,6 +2517,7 @@ static void getinfo_super_imsm(struct supertype *st, struct mdinfo *info, char *
 	info->disk.state = 0;
 	info->name[0] = 0;
 	info->recovery_start = MaxSector;
+	info->recovery_blocked = imsm_reshape_blocks_arrays_changes(st->sb);
 
 	/* do we have the all the insync disks that we expect? */
 	mpb = super->anchor;

[PATCH 4/4] Remove freeze() call from Grow_continue()

[Adam Kwolek]

Grow_continue() for external metadata should be executed on blocked
from monitoring array(s)/container.
Additional call to freeze() is not necessary in such case.
It produces meaningless error message only.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 Grow.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index ae0d112..0b58d5e 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3807,7 +3807,6 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
 	if (st->ss->external) {
 		fmt_devname(buf, st->container_dev);
 		container = buf;
-		freeze(st);
 
 		if (!mdmon_running(st->container_dev))
 			start_mdmon(st->container_dev);

Re: [PATCH 0/4] Fix freezing multiple arrays in container during assembly (2)

[NeilBrown]

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

On Mon, 03 Oct 2011 18:13:30 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> The following series corrects freezing reshaped array in container during
> assembly while other arrays are present.
> 
> The goal of the patches is to get entire assembled container (all arrays inside)
> to be blocked from monitoring when reshape is in progress /exactly as when
> reshape is initialized from mdadm command line/.
> 
> This time, freezing decision is taken based upon information from metadata,
> not /sysfs entries. For this purpose recovery_blocked field is added to mdinfo.
> 
> 
> Note: First patch in this series for mdadm throwing core file problem
>       is resent as reminder.
> 
> BR
> Adam
> 
> ---
> 
> Adam Kwolek (4):
>       Remove freeze() call from Grow_continue()
>       imsm: Fill recovery_blocked field present in mdinfo
>       Add recovery blocked field to mdinfo

These three all applied - thanks.

>       FIX: restore_backup() throws core dump

Fixed differently as described in separate email.

Thanks,
NeilBrown


> 
> 
>  Assemble.c    |    2 +-
>  Grow.c        |   13 ++++++-------
>  mdadm.h       |    7 +++++++
>  super-ddf.c   |    2 ++
>  super-intel.c |    6 ++++--
>  super0.c      |    2 ++
>  super1.c      |    2 ++
>  7 files changed, 24 insertions(+), 10 deletions(-)
> 


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