[PATCH 0/9] OLCE for external meta (cont.)

[PATCH 0/9] OLCE for external meta (cont.)

[Adam Kwolek]

The following series continues OLCE implementation for external metadata.

Main changes:
- Raid0 OLCE is possible
- OLCE for container with 2 arrays works (imsm) 
- Spares can be added to raid5 container (roll back of some changes)
- some bugs fixed

Problems:
- checkpointing sometimes hangs 
  reshape in md is finished, but mdadm stops responding before size set is executed,
  somewhere in child_monitor()
- when second array in container is raid0, it is not reshaped during OLCE 
  for imsm no one at this moment can update metadata:
     - mdadm is decided not to do this
     - mdmon is not running for this array yet (array_set_state() cannot be called for metadata update)
     - mdmon can be run later in reshape_array(), but we need meta update earlier to get reshape_active variable updated

BR
Adam



     

---

Adam Kwolek (9):
      imsm: Proceed with second array reshape only for frozen container
      FIX: container has to be frozen during reshape
      imsm: FIX: monitor should initialize 2nd reshape only
      imsm: FIX: container content gathering is not needed for size set
      imsm: Update metadata for second array
      imsm: FIX: only one spare is passed in update
      imsm:FIX: one spare can be added to raid0 only
      FIX: Cannot add spare to monitored container
      imsm: FIX: set correct slot information in metadata (raid0)


 Grow.c        |   62 ++++++++++++++----------------
 Manage.c      |    8 ++--
 super-intel.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 149 insertions(+), 40 deletions(-)

-- 
Signature

[PATCH 1/9] imsm: FIX: set correct slot information in metadata (raid0)

[Adam Kwolek]

Slot was set based on anchor information.
Disks information was copied outside disk list area.

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 2700401..90bff47 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5648,6 +5648,7 @@ static int apply_reshape_container_disks_update(struct imsm_update_reshape *u,
 	struct intel_dev *id;
 	int i;
 	int delta_disks = u->new_raid_disks - u->old_raid_disks;
+	int disk_count = u->old_raid_disks;
 	void **tofree = NULL;
 	int devices_to_reshape = 1;
 	struct imsm_super *mpb = super->anchor;
@@ -5668,7 +5669,7 @@ static int apply_reshape_container_disks_update(struct imsm_update_reshape *u,
 		    ((new_disk->index >= 0) &&
 		     (new_disk->index < u->old_raid_disks)))
 			goto update_reshape_exit;
-		new_disk->index = mpb->num_disks++;
+		new_disk->index = disk_count++;
 		/* slot to fill in autolayout
 		 */
 		new_disk->raiddisk = new_disk->index;
@@ -6484,7 +6485,8 @@ static int imsm_create_metadata_update_for_reshape(
 		u->new_disks[i] = makedev(dev->disk.major,
 					  dev->disk.minor);
 		dl = get_disk_super(super, dev->disk.major, dev->disk.minor);
-		dl->index = mpb->num_disks++;
+		dl->index = mpb->num_disks;
+		mpb->num_disks++;
 	}
 
 abort:

[PATCH 2/9] FIX: Cannot add spare to monitored container

[Adam Kwolek]

This fix rolls backs some changes introduced by patch:
	"Add spares to raid0 in mdadm"

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

 Manage.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Manage.c b/Manage.c
index 4e6afb5..fa1b13f 100644
--- a/Manage.c
+++ b/Manage.c
@@ -800,16 +800,16 @@ int Manage_subdevs(char *devname, int fd,
 				if (dv->writemostly == 1)
 					disc.state |= 1 << MD_DISK_WRITEMOSTLY;
 				dfd = dev_open(dv->devname, O_RDWR | O_EXCL|O_DIRECT);
-				if (tst->ss->external &&
-				    mdmon_running(tst->container_dev))
-					tst->update_tail = &tst->updates;
 				if (tst->ss->add_to_super(tst, &disc, dfd,
 							  dv->devname)) {
 					close(dfd);
 					return 1;
 				}
 				/* write_init_super will close 'dfd' */
-				if (tst->ss->write_init_super(tst))
+				if (tst->ss->external)
+					/* mdmon will write the metadata */
+					close(dfd);
+				else if (tst->ss->write_init_super(tst))
 					return 1;
 			} else if (dv->re_add) {
 				/*  this had better be raid1.

[PATCH 3/9] imsm:FIX: one spare can be added to raid0 only

[Adam Kwolek]

This is part of original commit "Add spares to raid0 in mdadm"

Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 super-intel.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 90bff47..ef51000 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -3442,6 +3442,8 @@ static int add_to_super_imsm_volume(struct supertype *st, mdu_disk_info_t *dk,
 	return 0;
 }
 
+static int write_super_imsm(struct supertype *st, int doclose);
+
 static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
 			      int fd, char *devname)
 {
@@ -3506,6 +3508,12 @@ static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
 		super->disks = dd;
 	}
 
+	if (!mdmon_running(st->container_dev)) {
+		dprintf("imsm: mdmon is not active- write metadata by mdadm\n");
+		super->updates_pending++;
+		write_super_imsm(st, 0);
+	}
+
 	return 0;
 }
 

[PATCH 4/9] imsm: FIX: only one spare is passed in update

[Adam Kwolek]

Only one spare is passed in update.
When more than one disk is added first spare is passed multiple times.

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

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

diff --git a/super-intel.c b/super-intel.c
index ef51000..321dc47 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6448,6 +6448,7 @@ static int imsm_create_metadata_update_for_reshape(
 	struct mdinfo *spares = NULL;
 	int i;
 	int delta_disks = 0;
+	struct mdinfo *dev;
 
 	dprintf("imsm_update_metadata_for_reshape(enter) raid_disks = %i\n",
 		geo->raid_disks);
@@ -6486,15 +6487,18 @@ static int imsm_create_metadata_update_for_reshape(
 	dprintf("imsm: %i spares are available.\n\n",
 		spares->array.spare_disks);
 
+	dev = spares->devs;
 	for (i = 0; i < delta_disks; i++) {
-		struct mdinfo *dev = spares->devs;
 		struct dl *dl;
 
+		if (dev == NULL)
+			break;
 		u->new_disks[i] = makedev(dev->disk.major,
 					  dev->disk.minor);
 		dl = get_disk_super(super, dev->disk.major, dev->disk.minor);
 		dl->index = mpb->num_disks;
 		mpb->num_disks++;
+		dev = dev->next;
 	}
 
 abort:

[PATCH 5/9] imsm: Update metadata for second array

[Adam Kwolek]

When second array reshape is about to start metadata should be updated
by mdmon in imsm_set_array_state().

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

 super-intel.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 321dc47..1db15c2 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5069,6 +5069,83 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
 
 		for (mdi = a->info.devs; mdi; mdi = mdi->next)
 			imsm_set_disk(a, mdi->disk.raid_disk, mdi->curr_state);
+	} else if (dev->vol.migr_state == 0) {
+		/* check here if this is container action
+		 * and next metadata should be prepared
+		*/
+		struct active_array *aa = a->container->arrays;
+		int arrays_under_reshape = 0;
+
+		/* check if no other arrays is under reshape
+		 */
+		while (aa) {
+			if (aa->container) {
+				int inst = aa->info.container_member;
+				struct intel_super *super = aa->container->sb;
+				struct imsm_dev *dev =
+						get_imsm_dev(super, inst);
+				struct imsm_map *map2 = get_imsm_map(dev, 1);
+
+				if (map2)
+					arrays_under_reshape++;
+			}
+			aa = aa->next;
+		}
+		if (arrays_under_reshape == 0) {
+			struct imsm_map *map = get_imsm_map(dev, 0);
+			struct dl *dl = NULL;
+			int disks_count = 0;
+
+			/* check if this array should be reshaped
+			*/
+			for (dl = super->disks; dl; dl = dl->next)
+				if (dl->index >= 0)
+					disks_count++;
+			if (disks_count > map->num_members) {
+				/* manage changes in volume
+				 */
+				struct imsm_map *map, *map2;
+				int prev_num_members;
+				int used_disks;
+				int i;
+
+				map = get_imsm_map(dev, 0);
+				prev_num_members = map->num_members;
+				map->num_members = disks_count;
+				dev->vol.migr_state = 1;
+				dev->vol.curr_migr_unit = 0;
+				dev->vol.migr_type = MIGR_GEN_MIGR;
+				for (i = prev_num_members;
+					i < map->num_members; i++)
+					set_imsm_ord_tbl_ent(map, i, i);
+				map2 = get_imsm_map(dev, 1);
+				/* Copy the current map */
+				memcpy(map2, map, sizeof_imsm_map(map));
+				map2->num_members = prev_num_members;
+
+				/* calculate new size
+				 */
+				used_disks = imsm_num_data_members(dev, 0);
+				if (used_disks) {
+					unsigned long long array_blocks;
+
+					array_blocks =
+						map->blocks_per_member
+						* used_disks;
+					/* round array size down to closest MB
+					 */
+					array_blocks = (array_blocks
+							>> SECT_PER_MB_SHIFT)
+							<< SECT_PER_MB_SHIFT;
+					dev->size_low =
+					     __cpu_to_le32((__u32)array_blocks);
+					dev->size_high =
+						__cpu_to_le32(
+						(__u32)(array_blocks >> 32));
+				}
+				super->updates_pending++;
+			}
+		}
 	}
 
 	return consistent;

[PATCH 6/9] imsm: FIX: container content gathering is not needed for size set

[Adam Kwolek]

Size information is loaded already and there is no need to load it again,
when metadata is not reloaded.

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

 Grow.c |   58 +++++++++++++++++++++++++---------------------------------
 1 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/Grow.c b/Grow.c
index 2ca9835..6238eb3 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2019,44 +2019,36 @@ static int reshape_array(char *container, int fd, char *devname,
 		if (reshape.before.data_disks !=
 		    reshape.after.data_disks &&
 		    info->custom_array_size) {
-			struct mdinfo *info2;
-			char *subarray = strchr(info->text_version+1, '/')+1;
+			unsigned long long current_size = 0;
+			unsigned long long new_size = info->custom_array_size/2;
+			unsigned long long suspend_value;
 
 			wait_reshape(sra);
 			ping_monitor(container);
 
-			info2 = st->ss->container_content(st, subarray);
-			if (info2) {
-				unsigned long long current_size = 0;
-				unsigned long long new_size =
-					info2->custom_array_size/2;
-				unsigned long long suspend_value;
-
-				if (sysfs_get_ll(sra,
-						 NULL,
-						 "array_size",
-						 &current_size) == 0 &&
-				    new_size > current_size) {
-					if (sysfs_set_num(sra, NULL,
-							  "array_size", new_size)
-					    < 0)
-						dprintf("Error: Cannot"
-							" set array size");
-					else
-						dprintf("Array size "
-							"changed");
-					dprintf(" from %llu to %llu.\n",
-						current_size, new_size);
-				}
-				/* manage suspend_* entries
-				 * set suspend_lo to suspend_hi value
-				 * to unblock array
-				 */
-				sysfs_get_ll(sra, NULL, "suspend_hi", &suspend_value);
-				sysfs_set_num(sra, NULL, "suspend_lo", suspend_value);
-
-				sysfs_free(info2);
+
+			if (sysfs_get_ll(sra,
+					 NULL,
+					 "array_size",
+					 &current_size) == 0 &&
+			    new_size > current_size) {
+				if (sysfs_set_num(sra, NULL,
+						  "array_size", new_size)
+				    < 0)
+					dprintf("Error: Cannot"
+						" set array size");
+				else
+					dprintf("Array size "
+						"changed");
+				dprintf(" from %llu to %llu.\n",
+					current_size, new_size);
 			}
+			/* manage suspend_* entries
+			 * set suspend_lo to suspend_hi value
+			 * to unblock array
+			 */
+			sysfs_get_ll(sra, NULL, "suspend_hi", &suspend_value);
+			sysfs_set_num(sra, NULL, "suspend_lo", suspend_value);
 		}
 
 		if (info->new_level != reshape.level) {

[PATCH 7/9] imsm: FIX: monitor should initialize 2nd reshape only

[Adam Kwolek]

in case configuration:
	raid5
	raid0
first array is monitored only.
During reshape of such container first array (raid5), in metadata
migration is initialized for both arrays.
This is imsm incompatible case.

Allow for 2nd array in metadata reshape initialization when there is more than 1 array monitored.

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

 super-intel.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 1db15c2..c465288 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5075,10 +5075,20 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
 		*/
 		struct active_array *aa = a->container->arrays;
 		int arrays_under_reshape = 0;
+		int array_counter = 0;
 
 		/* check if no other arrays is under reshape
 		 */
 		while (aa) {
+			array_counter++;
+			/* when we have takeovered array even single array
+			 * can be initialized for reshape, if mdadm didn't
+			 * do this.
+			 * This is due to fact, that raid0 (takeovered to raid4)
+			 * appears in monitor one by one
+			 */
+			if (aa->info.array.level == 4)
+				array_counter++;
 			if (aa->container) {
 				int inst = aa->info.container_member;
 				struct intel_super *super = aa->container->sb;
@@ -5091,7 +5101,7 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
 			}
 			aa = aa->next;
 		}
-		if (arrays_under_reshape == 0) {
+		if ((arrays_under_reshape == 0) && (array_counter > 1)) {
 			struct imsm_map *map = get_imsm_map(dev, 0);
 			struct dl *dl = NULL;
 			int disks_count = 0;

[PATCH 8/9] FIX: container has to be frozen during reshape

[Adam Kwolek]

Parent process unfreezes container when reshape was forked.
Container has to be frozen during entire reshape for container case.
Based on reshape flag "next array" reshape decision is taken also.

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

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

diff --git a/Grow.c b/Grow.c
index 6238eb3..29a78a6 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1579,6 +1579,7 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
 		 */
 		rv = reshape_container(container, fd, devname, st, &info,
 				       force, backup_file, quiet);
+		frozen = rv;
 	} else {
 		/* Impose these changes on a single array.  First
 		 * check that the metadata is OK with the change. */
@@ -2201,6 +2202,9 @@ int reshape_container(char *container, int cfd, char *devname,
 			break;
 	}
 	sysfs_free(cc);
+
+	unfreeze(st, 1);
+
 	exit(0);
 }
 

[PATCH 9/9] imsm: Proceed with second array reshape only for frozen container

[Adam Kwolek]

In IMSM case container operation is set of array operations.
We have 2 cases:
1. reshape initiated by mdadm grow
2. continue reshape from checkpoint after i.e. reboot.

In first case we have continue reshape for next array.
In second case we cannot be sure that assembly procedure will assembly
arrays in required order, and can potentially have parallel reshapes.

Monitor cannot keep information for this because for raid0 it will not be started.
Difference between 2 cases is that. In first case we have frozen container,
so only for frozen container 2nd array reshape is allowed.

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

 super-intel.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index c465288..d3da6f9 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5076,10 +5076,22 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
 		struct active_array *aa = a->container->arrays;
 		int arrays_under_reshape = 0;
 		int array_counter = 0;
+		int frozen = 0;
+		char buf[PATH_MAX];
 
+		/* do not process next volume if container is not frozen.
+		 * 'not frozen' case can occure,
+		 * when array in migration state  is assembled
+		 */			/* do not process next volume if container is not frozen
+			 */
+
+		if (sysfs_get_str(&a->info, NULL, "metadata_version", buf, sizeof(buf)) > 0)
+			frozen = buf[9] == '-';
+		if (frozen == 0)
+			arrays_under_reshape = -1;
 		/* check if no other arrays is under reshape
 		 */
-		while (aa) {
+		while (aa && frozen) {
 			array_counter++;
 			/* when we have takeovered array even single array
 			 * can be initialized for reshape, if mdadm didn't

[PATCH 0/4] RAID 0 to/from RAID5 migration

[Labun, Marcin]

The first three patches are mandatory modifications in common code.
The last one contains portion of IMSM raid 5 to raid 0 and raid0 to raid5 migration.

Regards,
Marcin Labun

Marcin Labun (4):
  Grow_reshape: need text version of device name in mdinfo.
  Grow_reshape: need text_version for sub-array processing.
  Grow_reshape: need sub-array component size in data migration.
  imsm: support for raid0 <-> raid5 migration

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

Re: [PATCH 2/9] FIX: Cannot add spare to monitored container

[NeilBrown]

On Tue, 11 Jan 2011 15:04:03 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> This fix rolls backs some changes introduced by patch:
> 	"Add spares to raid0 in mdadm"
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>

It would help a lot if you explained *why* you were rolling back these
changes.

Not applied.

NeilBrown


> ---
> 
>  Manage.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Manage.c b/Manage.c
> index 4e6afb5..fa1b13f 100644
> --- a/Manage.c
> +++ b/Manage.c
> @@ -800,16 +800,16 @@ int Manage_subdevs(char *devname, int fd,
>  				if (dv->writemostly == 1)
>  					disc.state |= 1 << MD_DISK_WRITEMOSTLY;
>  				dfd = dev_open(dv->devname, O_RDWR | O_EXCL|O_DIRECT);
> -				if (tst->ss->external &&
> -				    mdmon_running(tst->container_dev))
> -					tst->update_tail = &tst->updates;
>  				if (tst->ss->add_to_super(tst, &disc, dfd,
>  							  dv->devname)) {
>  					close(dfd);
>  					return 1;
>  				}
>  				/* write_init_super will close 'dfd' */
> -				if (tst->ss->write_init_super(tst))
> +				if (tst->ss->external)
> +					/* mdmon will write the metadata */
> +					close(dfd);
> +				else if (tst->ss->write_init_super(tst))
>  					return 1;
>  			} else if (dv->re_add) {
>  				/*  this had better be raid1.

Re: [PATCH 3/9] imsm:FIX: one spare can be added to raid0 only

[NeilBrown]

On Tue, 11 Jan 2011 15:04:11 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> This is part of original commit "Add spares to raid0 in mdadm"
> 
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 90bff47..ef51000 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -3442,6 +3442,8 @@ static int add_to_super_imsm_volume(struct supertype *st, mdu_disk_info_t *dk,
>  	return 0;
>  }
>  
> +static int write_super_imsm(struct supertype *st, int doclose);
> +
>  static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
>  			      int fd, char *devname)
>  {
> @@ -3506,6 +3508,12 @@ static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
>  		super->disks = dd;
>  	}
>  
> +	if (!mdmon_running(st->container_dev)) {
> +		dprintf("imsm: mdmon is not active- write metadata by mdadm\n");
> +		super->updates_pending++;
> +		write_super_imsm(st, 0);
> +	}
> +
>  	return 0;
>  }
>  


No.
I don't want "mdmon_running" tests in super-intel.c

What is the problem you are trying to fix (the comment in the patch should
*always* answer that question for me - I shouldn't have to ask).

NeilBrown

Re: [PATCH 4/9] imsm: FIX: only one spare is passed in update

[NeilBrown]

On Tue, 11 Jan 2011 15:04:19 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Only one spare is passed in update.
> When more than one disk is added first spare is passed multiple times.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index ef51000..321dc47 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6448,6 +6448,7 @@ static int imsm_create_metadata_update_for_reshape(
>  	struct mdinfo *spares = NULL;
>  	int i;
>  	int delta_disks = 0;
> +	struct mdinfo *dev;
>  
>  	dprintf("imsm_update_metadata_for_reshape(enter) raid_disks = %i\n",
>  		geo->raid_disks);
> @@ -6486,15 +6487,18 @@ static int imsm_create_metadata_update_for_reshape(
>  	dprintf("imsm: %i spares are available.\n\n",
>  		spares->array.spare_disks);
>  
> +	dev = spares->devs;
>  	for (i = 0; i < delta_disks; i++) {
> -		struct mdinfo *dev = spares->devs;
>  		struct dl *dl;
>  
> +		if (dev == NULL)
> +			break;
>  		u->new_disks[i] = makedev(dev->disk.major,
>  					  dev->disk.minor);
>  		dl = get_disk_super(super, dev->disk.major, dev->disk.minor);
>  		dl->index = mpb->num_disks;
>  		mpb->num_disks++;
> +		dev = dev->next;
>  	}
>  
>  abort:


Applied, thanks.

NeilBrown

Re: [PATCH 5/9] imsm: Update metadata for second array

[NeilBrown]

On Tue, 11 Jan 2011 15:04:27 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When second array reshape is about to start metadata should be updated
> by mdmon in imsm_set_array_state().
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 77 insertions(+), 0 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 321dc47..1db15c2 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5069,6 +5069,83 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
>  
>  		for (mdi = a->info.devs; mdi; mdi = mdi->next)
>  			imsm_set_disk(a, mdi->disk.raid_disk, mdi->curr_state);
> +	} else if (dev->vol.migr_state == 0) {
> +		/* check here if this is container action
> +		 * and next metadata should be prepared
> +		*/
> +		struct active_array *aa = a->container->arrays;
> +		int arrays_under_reshape = 0;
> +
> +		/* check if no other arrays is under reshape
> +		 */
> +		while (aa) {
> +			if (aa->container) {
> +				int inst = aa->info.container_member;
> +				struct intel_super *super = aa->container->sb;
> +				struct imsm_dev *dev =
> +						get_imsm_dev(super, inst);
> +				struct imsm_map *map2 = get_imsm_map(dev, 1);
> +
> +				if (map2)
> +					arrays_under_reshape++;
> +			}
> +			aa = aa->next;
> +		}
> +		if (arrays_under_reshape == 0) {
> +			struct imsm_map *map = get_imsm_map(dev, 0);
> +			struct dl *dl = NULL;
> +			int disks_count = 0;
> +
> +			/* check if this array should be reshaped
> +			*/
> +			for (dl = super->disks; dl; dl = dl->next)
> +				if (dl->index >= 0)
> +					disks_count++;
> +			if (disks_count > map->num_members) {
> +				/* manage changes in volume
> +				 */
> +				struct imsm_map *map, *map2;
> +				int prev_num_members;
> +				int used_disks;
> +				int i;
> +
> +				map = get_imsm_map(dev, 0);
> +				prev_num_members = map->num_members;
> +				map->num_members = disks_count;
> +				dev->vol.migr_state = 1;
> +				dev->vol.curr_migr_unit = 0;
> +				dev->vol.migr_type = MIGR_GEN_MIGR;
> +				for (i = prev_num_members;
> +					i < map->num_members; i++)
> +					set_imsm_ord_tbl_ent(map, i, i);
> +				map2 = get_imsm_map(dev, 1);
> +				/* Copy the current map */
> +				memcpy(map2, map, sizeof_imsm_map(map));
> +				map2->num_members = prev_num_members;
> +
> +				/* calculate new size
> +				 */
> +				used_disks = imsm_num_data_members(dev, 0);
> +				if (used_disks) {
> +					unsigned long long array_blocks;
> +
> +					array_blocks =
> +						map->blocks_per_member
> +						* used_disks;
> +					/* round array size down to closest MB
> +					 */
> +					array_blocks = (array_blocks
> +							>> SECT_PER_MB_SHIFT)
> +							<< SECT_PER_MB_SHIFT;
> +					dev->size_low =
> +					     __cpu_to_le32((__u32)array_blocks);
> +					dev->size_high =
> +						__cpu_to_le32(
> +						(__u32)(array_blocks >> 32));
> +				}
> +				super->updates_pending++;
> +			}
> +		}
>  	}
>  
>  	return consistent;


Most of this code is identical to  imsm_progress_container_reshape.
Maybe you just want to put another call to that function in somewhere.

NeilBrown

Re: [PATCH 6/9] imsm: FIX: container content gathering is not needed for size set

[NeilBrown]

On Tue, 11 Jan 2011 15:04:35 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Size information is loaded already and there is no need to load it again,
> when metadata is not reloaded.

Why do you say that?  It seems wrong.

When growing an array, the size will not change until the reshape completes.
When it does complete, it will be mdmon which updates the metadata and
records in it the desired size of the array.

The only way mdadm can find this number out is by loading the metadata.

Where am I wrong?

NeilBrown


> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   58 +++++++++++++++++++++++++---------------------------------
>  1 files changed, 25 insertions(+), 33 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 2ca9835..6238eb3 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2019,44 +2019,36 @@ static int reshape_array(char *container, int fd, char *devname,
>  		if (reshape.before.data_disks !=
>  		    reshape.after.data_disks &&
>  		    info->custom_array_size) {
> -			struct mdinfo *info2;
> -			char *subarray = strchr(info->text_version+1, '/')+1;
> +			unsigned long long current_size = 0;
> +			unsigned long long new_size = info->custom_array_size/2;
> +			unsigned long long suspend_value;
>  
>  			wait_reshape(sra);
>  			ping_monitor(container);
>  
> -			info2 = st->ss->container_content(st, subarray);
> -			if (info2) {
> -				unsigned long long current_size = 0;
> -				unsigned long long new_size =
> -					info2->custom_array_size/2;
> -				unsigned long long suspend_value;
> -
> -				if (sysfs_get_ll(sra,
> -						 NULL,
> -						 "array_size",
> -						 &current_size) == 0 &&
> -				    new_size > current_size) {
> -					if (sysfs_set_num(sra, NULL,
> -							  "array_size", new_size)
> -					    < 0)
> -						dprintf("Error: Cannot"
> -							" set array size");
> -					else
> -						dprintf("Array size "
> -							"changed");
> -					dprintf(" from %llu to %llu.\n",
> -						current_size, new_size);
> -				}
> -				/* manage suspend_* entries
> -				 * set suspend_lo to suspend_hi value
> -				 * to unblock array
> -				 */
> -				sysfs_get_ll(sra, NULL, "suspend_hi", &suspend_value);
> -				sysfs_set_num(sra, NULL, "suspend_lo", suspend_value);
> -
> -				sysfs_free(info2);
> +
> +			if (sysfs_get_ll(sra,
> +					 NULL,
> +					 "array_size",
> +					 &current_size) == 0 &&
> +			    new_size > current_size) {
> +				if (sysfs_set_num(sra, NULL,
> +						  "array_size", new_size)
> +				    < 0)
> +					dprintf("Error: Cannot"
> +						" set array size");
> +				else
> +					dprintf("Array size "
> +						"changed");
> +				dprintf(" from %llu to %llu.\n",
> +					current_size, new_size);
>  			}
> +			/* manage suspend_* entries
> +			 * set suspend_lo to suspend_hi value
> +			 * to unblock array
> +			 */
> +			sysfs_get_ll(sra, NULL, "suspend_hi", &suspend_value);
> +			sysfs_set_num(sra, NULL, "suspend_lo", suspend_value);
>  		}
>  
>  		if (info->new_level != reshape.level) {

Re: [PATCH 7/9] imsm: FIX: monitor should initialize 2nd reshape only

[NeilBrown]

On Tue, 11 Jan 2011 15:04:43 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> in case configuration:
> 	raid5
> 	raid0
> first array is monitored only.
> During reshape of such container first array (raid5), in metadata
> migration is initialized for both arrays.
> This is imsm incompatible case.
> 
> Allow for 2nd array in metadata reshape initialization when there is more than 1 array monitored.

I'm sorry, but I cannot follow what you are saying here.

Maybe if you could explain what you see happening, and what should actually
happen instead.  At which stop in the process does something go wrong?

NeilBrown

> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 1db15c2..c465288 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5075,10 +5075,20 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
>  		*/
>  		struct active_array *aa = a->container->arrays;
>  		int arrays_under_reshape = 0;
> +		int array_counter = 0;
>  
>  		/* check if no other arrays is under reshape
>  		 */
>  		while (aa) {
> +			array_counter++;
> +			/* when we have takeovered array even single array
> +			 * can be initialized for reshape, if mdadm didn't
> +			 * do this.
> +			 * This is due to fact, that raid0 (takeovered to raid4)
> +			 * appears in monitor one by one
> +			 */
> +			if (aa->info.array.level == 4)
> +				array_counter++;
>  			if (aa->container) {
>  				int inst = aa->info.container_member;
>  				struct intel_super *super = aa->container->sb;
> @@ -5091,7 +5101,7 @@ static int imsm_set_array_state(struct active_array *a, int consistent)
>  			}
>  			aa = aa->next;
>  		}
> -		if (arrays_under_reshape == 0) {
> +		if ((arrays_under_reshape == 0) && (array_counter > 1)) {
>  			struct imsm_map *map = get_imsm_map(dev, 0);
>  			struct dl *dl = NULL;
>  			int disks_count = 0;

RE: [PATCH 3/9] imsm:FIX: one spare can be added to raid0 only

[Kwolek, Adam]

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, January 12, 2011 6:45 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 3/9] imsm:FIX: one spare can be added to raid0 only
> 
> On Tue, 11 Jan 2011 15:04:11 +0100 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > This is part of original commit "Add spares to raid0 in mdadm"
> >
> > Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  super-intel.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index 90bff47..ef51000 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -3442,6 +3442,8 @@ static int add_to_super_imsm_volume(struct
> supertype *st, mdu_disk_info_t *dk,
> >  	return 0;
> >  }
> >
> > +static int write_super_imsm(struct supertype *st, int doclose);
> > +
> >  static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t
> *dk,
> >  			      int fd, char *devname)
> >  {
> > @@ -3506,6 +3508,12 @@ static int add_to_super_imsm(struct supertype
> *st, mdu_disk_info_t *dk,
> >  		super->disks = dd;
> >  	}
> >
> > +	if (!mdmon_running(st->container_dev)) {
> > +		dprintf("imsm: mdmon is not active- write metadata by
> mdadm\n");
> > +		super->updates_pending++;
> > +		write_super_imsm(st, 0);
> > +	}
> > +
> >  	return 0;
> >  }
> >
> 
> 
> No.
> I don't want "mdmon_running" tests in super-intel.c
> 
> What is the problem you are trying to fix (the comment in the patch
> should
> *always* answer that question for me - I shouldn't have to ask).
> 
> NeilBrown

In new code (I've posted today) that new update mechanism for raid0 is used this patch is not necessary.

Thanks
Adam

RE: [PATCH 6/9] imsm: FIX: container content gathering is not needed for size set

[Kwolek, Adam]

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, January 12, 2011 6:53 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 6/9] imsm: FIX: container content gathering is not
> needed for size set
> 
> On Tue, 11 Jan 2011 15:04:35 +0100 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > Size information is loaded already and there is no need to load it
> again,
> > when metadata is not reloaded.
> 
> Why do you say that?  It seems wrong.
> 
> When growing an array, the size will not change until the reshape
> completes.
> When it does complete, it will be mdmon which updates the metadata and
> records in it the desired size of the array.
> 
> The only way mdadm can find this number out is by loading the metadata.
> 
> Where am I wrong?
> 
> NeilBrown

Size is set in metadata, before reshape start in reshape_super() and remains unchanged during whole reshape.
After reshape this value doesn't change in metadata also.
It is possible that this behavior is imsm specific, and for general case reload is required as you describes.

BR
Adam



> 
> 
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  Grow.c |   58 +++++++++++++++++++++++++-----------------------------
> ----
> >  1 files changed, 25 insertions(+), 33 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 2ca9835..6238eb3 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -2019,44 +2019,36 @@ static int reshape_array(char *container, int
> fd, char *devname,
> >  		if (reshape.before.data_disks !=
> >  		    reshape.after.data_disks &&
> >  		    info->custom_array_size) {
> > -			struct mdinfo *info2;
> > -			char *subarray = strchr(info->text_version+1, '/')+1;
> > +			unsigned long long current_size = 0;
> > +			unsigned long long new_size = info-
> >custom_array_size/2;
> > +			unsigned long long suspend_value;
> >
> >  			wait_reshape(sra);
> >  			ping_monitor(container);
> >
> > -			info2 = st->ss->container_content(st, subarray);
> > -			if (info2) {
> > -				unsigned long long current_size = 0;
> > -				unsigned long long new_size =
> > -					info2->custom_array_size/2;
> > -				unsigned long long suspend_value;
> > -
> > -				if (sysfs_get_ll(sra,
> > -						 NULL,
> > -						 "array_size",
> > -						 &current_size) == 0 &&
> > -				    new_size > current_size) {
> > -					if (sysfs_set_num(sra, NULL,
> > -							  "array_size", new_size)
> > -					    < 0)
> > -						dprintf("Error: Cannot"
> > -							" set array size");
> > -					else
> > -						dprintf("Array size "
> > -							"changed");
> > -					dprintf(" from %llu to %llu.\n",
> > -						current_size, new_size);
> > -				}
> > -				/* manage suspend_* entries
> > -				 * set suspend_lo to suspend_hi value
> > -				 * to unblock array
> > -				 */
> > -				sysfs_get_ll(sra, NULL, "suspend_hi",
> &suspend_value);
> > -				sysfs_set_num(sra, NULL, "suspend_lo",
> suspend_value);
> > -
> > -				sysfs_free(info2);
> > +
> > +			if (sysfs_get_ll(sra,
> > +					 NULL,
> > +					 "array_size",
> > +					 &current_size) == 0 &&
> > +			    new_size > current_size) {
> > +				if (sysfs_set_num(sra, NULL,
> > +						  "array_size", new_size)
> > +				    < 0)
> > +					dprintf("Error: Cannot"
> > +						" set array size");
> > +				else
> > +					dprintf("Array size "
> > +						"changed");
> > +				dprintf(" from %llu to %llu.\n",
> > +					current_size, new_size);
> >  			}
> > +			/* manage suspend_* entries
> > +			 * set suspend_lo to suspend_hi value
> > +			 * to unblock array
> > +			 */
> > +			sysfs_get_ll(sra, NULL, "suspend_hi",
> &suspend_value);
> > +			sysfs_set_num(sra, NULL, "suspend_lo",
> suspend_value);
> >  		}
> >
> >  		if (info->new_level != reshape.level) {

RE: [PATCH 2/9] FIX: Cannot add spare to monitored container

[Kwolek, Adam]

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, January 12, 2011 6:43 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 2/9] FIX: Cannot add spare to monitored container
> 
> On Tue, 11 Jan 2011 15:04:03 +0100 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > This fix rolls backs some changes introduced by patch:
> > 	"Add spares to raid0 in mdadm"
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> 
> It would help a lot if you explained *why* you were rolling back these
> changes.
> 
> Not applied.
> 
> NeilBrown


I cannot add spare to i.e. raid5 array. In my unit tests, I'm creating raid5 array and then I'm adding spares for expansion (mdadm --add).
If I've create container with more disks than create array that is using fewer disks, I can test expansion.
Spares can be added to raid0 with no monitor usage without problems.
I've started to observe this problem when patch for adding spares to raid0 was applied.
I've identify code change that makes problem. This causes this code rollback. 

Problem that is visible is  that. Monitor cannot write major:minor to new_dev in sysfs /without code roll back/.
I didn't dig this problem deeply. This patch is rather reminder to signalize problem /and not forget about it/.

BR
Adam

> 
> > ---
> >
> >  Manage.c |    8 ++++----
> >  1 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/Manage.c b/Manage.c
> > index 4e6afb5..fa1b13f 100644
> > --- a/Manage.c
> > +++ b/Manage.c
> > @@ -800,16 +800,16 @@ int Manage_subdevs(char *devname, int fd,
> >  				if (dv->writemostly == 1)
> >  					disc.state |= 1 << MD_DISK_WRITEMOSTLY;
> >  				dfd = dev_open(dv->devname, O_RDWR |
> O_EXCL|O_DIRECT);
> > -				if (tst->ss->external &&
> > -				    mdmon_running(tst->container_dev))
> > -					tst->update_tail = &tst->updates;
> >  				if (tst->ss->add_to_super(tst, &disc, dfd,
> >  							  dv->devname)) {
> >  					close(dfd);
> >  					return 1;
> >  				}
> >  				/* write_init_super will close 'dfd' */
> > -				if (tst->ss->write_init_super(tst))
> > +				if (tst->ss->external)
> > +					/* mdmon will write the metadata */
> > +					close(dfd);
> > +				else if (tst->ss->write_init_super(tst))
> >  					return 1;
> >  			} else if (dv->re_add) {
> >  				/*  this had better be raid1.

Re: [PATCH 6/9] imsm: FIX: container content gathering is not needed for size set

[NeilBrown]

On Wed, 12 Jan 2011 15:30:50 +0000 "Kwolek, Adam" <adam.kwolek@intel.com>
wrote:

> 
> 
> > -----Original Message-----
> > From: NeilBrown [mailto:neilb@suse.de]
> > Sent: Wednesday, January 12, 2011 6:53 AM
> > To: Kwolek, Adam
> > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> > Neubauer, Wojciech
> > Subject: Re: [PATCH 6/9] imsm: FIX: container content gathering is not
> > needed for size set
> > 
> > On Tue, 11 Jan 2011 15:04:35 +0100 Adam Kwolek <adam.kwolek@intel.com>
> > wrote:
> > 
> > > Size information is loaded already and there is no need to load it
> > again,
> > > when metadata is not reloaded.
> > 
> > Why do you say that?  It seems wrong.
> > 
> > When growing an array, the size will not change until the reshape
> > completes.
> > When it does complete, it will be mdmon which updates the metadata and
> > records in it the desired size of the array.
> > 
> > The only way mdadm can find this number out is by loading the metadata.
> > 
> > Where am I wrong?
> > 
> > NeilBrown
> 
> Size is set in metadata, before reshape start in reshape_super() and remains unchanged during whole reshape.
> After reshape this value doesn't change in metadata also.
> It is possible that this behavior is imsm specific, and for general case reload is required as you describes.
> 

If reshape_super does to that (and it looks like it does) then I strongly
suspect that it is wrong.

While an array is in the middle of a migration from one configuration to
another its effective size must be the smaller of those two sizes.
So when we add a device to a RAID5, the device size must stay unchanged until
the migration completes.  At the moment when the migration completes the size
can change, not before.  So I suspect that the size recorded in the metadata
should be the smaller size.

So I think that apply_reshape_container_disks_update and
imsm_progress_container_reshape need to be fixed to change the size after the migration.

NeilBrown

Re: [PATCH 2/9] FIX: Cannot add spare to monitored container

[NeilBrown]

On Wed, 12 Jan 2011 15:57:45 +0000 "Kwolek, Adam" <adam.kwolek@intel.com>
wrote:

> > -----Original Message-----
> > From: NeilBrown [mailto:neilb@suse.de]
> > Sent: Wednesday, January 12, 2011 6:43 AM
> > To: Kwolek, Adam
> > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> > Neubauer, Wojciech
> > Subject: Re: [PATCH 2/9] FIX: Cannot add spare to monitored container
> > 
> > On Tue, 11 Jan 2011 15:04:03 +0100 Adam Kwolek <adam.kwolek@intel.com>
> > wrote:
> > 
> > > This fix rolls backs some changes introduced by patch:
> > > 	"Add spares to raid0 in mdadm"
> > >
> > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > 
> > It would help a lot if you explained *why* you were rolling back these
> > changes.
> > 
> > Not applied.
> > 
> > NeilBrown
> 
> 
> I cannot add spare to i.e. raid5 array. In my unit tests, I'm creating raid5 array and then I'm adding spares for expansion (mdadm --add).
> If I've create container with more disks than create array that is using fewer disks, I can test expansion.
> Spares can be added to raid0 with no monitor usage without problems.
> I've started to observe this problem when patch for adding spares to raid0 was applied.
> I've identify code change that makes problem. This causes this code rollback. 
> 
> Problem that is visible is  that. Monitor cannot write major:minor to new_dev in sysfs /without code roll back/.
> I didn't dig this problem deeply. This patch is rather reminder to signalize problem /and not forget about it/.
> 

Fair enough.
Having a patch which works around a problem and reminds you to fix it later
is a good idea.  However you should make sure that the notes at the top of
the patch make it clear that is the purpose of the patch  - especially when
you post it to me.
It is just SO important to include completed details in the notes at the top
of the patch!!!!

NeilBrown


> BR
> Adam
> 
> > 
> > > ---
> > >
> > >  Manage.c |    8 ++++----
> > >  1 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Manage.c b/Manage.c
> > > index 4e6afb5..fa1b13f 100644
> > > --- a/Manage.c
> > > +++ b/Manage.c
> > > @@ -800,16 +800,16 @@ int Manage_subdevs(char *devname, int fd,
> > >  				if (dv->writemostly == 1)
> > >  					disc.state |= 1 << MD_DISK_WRITEMOSTLY;
> > >  				dfd = dev_open(dv->devname, O_RDWR |
> > O_EXCL|O_DIRECT);
> > > -				if (tst->ss->external &&
> > > -				    mdmon_running(tst->container_dev))
> > > -					tst->update_tail = &tst->updates;
> > >  				if (tst->ss->add_to_super(tst, &disc, dfd,
> > >  							  dv->devname)) {
> > >  					close(dfd);
> > >  					return 1;
> > >  				}
> > >  				/* write_init_super will close 'dfd' */
> > > -				if (tst->ss->write_init_super(tst))
> > > +				if (tst->ss->external)
> > > +					/* mdmon will write the metadata */
> > > +					close(dfd);
> > > +				else if (tst->ss->write_init_super(tst))
> > >  					return 1;
> > >  			} else if (dv->re_add) {
> > >  				/*  this had better be raid1.
> 
> --
> 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

RE: [PATCH 2/9] FIX: Cannot add spare to monitored container

[Czarnowska, Anna]

With this problem we are not able to move any spare to imsm container 
and so most of Monitor tests fail at the moment.
Regards 
Anna

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Thursday, January 13, 2011 4:07 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 2/9] FIX: Cannot add spare to monitored container
> 
> On Wed, 12 Jan 2011 15:57:45 +0000 "Kwolek, Adam"
> <adam.kwolek@intel.com>
> wrote:
> 
> > > -----Original Message-----
> > > From: NeilBrown [mailto:neilb@suse.de]
> > > Sent: Wednesday, January 12, 2011 6:43 AM
> > > To: Kwolek, Adam
> > > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> > > Neubauer, Wojciech
> > > Subject: Re: [PATCH 2/9] FIX: Cannot add spare to monitored
> container
> > >
> > > On Tue, 11 Jan 2011 15:04:03 +0100 Adam Kwolek
> <adam.kwolek@intel.com>
> > > wrote:
> > >
> > > > This fix rolls backs some changes introduced by patch:
> > > > 	"Add spares to raid0 in mdadm"
> > > >
> > > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > >
> > > It would help a lot if you explained *why* you were rolling back
> these
> > > changes.
> > >
> > > Not applied.
> > >
> > > NeilBrown
> >
> >
> > I cannot add spare to i.e. raid5 array. In my unit tests, I'm
> creating raid5 array and then I'm adding spares for expansion (mdadm --
> add).
> > If I've create container with more disks than create array that is
> using fewer disks, I can test expansion.
> > Spares can be added to raid0 with no monitor usage without problems.
> > I've started to observe this problem when patch for adding spares to
> raid0 was applied.
> > I've identify code change that makes problem. This causes this code
> rollback.
> >
> > Problem that is visible is  that. Monitor cannot write major:minor to
> new_dev in sysfs /without code roll back/.
> > I didn't dig this problem deeply. This patch is rather reminder to
> signalize problem /and not forget about it/.
> >
> 
> Fair enough.
> Having a patch which works around a problem and reminds you to fix it
> later
> is a good idea.  However you should make sure that the notes at the top
> of
> the patch make it clear that is the purpose of the patch  - especially
> when
> you post it to me.
> It is just SO important to include completed details in the notes at
> the top
> of the patch!!!!
> 
> NeilBrown
> 
> 
> > BR
> > Adam
> >
> > >
> > > > ---
> > > >
> > > >  Manage.c |    8 ++++----
> > > >  1 files changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/Manage.c b/Manage.c
> > > > index 4e6afb5..fa1b13f 100644
> > > > --- a/Manage.c
> > > > +++ b/Manage.c
> > > > @@ -800,16 +800,16 @@ int Manage_subdevs(char *devname, int fd,
> > > >  				if (dv->writemostly == 1)
> > > >  					disc.state |= 1 <<
> MD_DISK_WRITEMOSTLY;
> > > >  				dfd = dev_open(dv->devname, O_RDWR |
> > > O_EXCL|O_DIRECT);
> > > > -				if (tst->ss->external &&
> > > > -				    mdmon_running(tst->container_dev))
> > > > -					tst->update_tail = &tst->updates;
> > > >  				if (tst->ss->add_to_super(tst, &disc,
> dfd,
> > > >  							  dv->devname)) {
> > > >  					close(dfd);
> > > >  					return 1;
> > > >  				}
> > > >  				/* write_init_super will close 'dfd' */
> > > > -				if (tst->ss->write_init_super(tst))
> > > > +				if (tst->ss->external)
> > > > +					/* mdmon will write the metadata */
> > > > +					close(dfd);
> > > > +				else if (tst->ss->write_init_super(tst))
> > > >  					return 1;
> > > >  			} else if (dv->re_add) {
> > > >  				/*  this had better be raid1.
> >
> > --
> > 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
> 
> --
> 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