linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] Reshape restart from checkpoint
@ 2011-03-09 13:45 Adam Kwolek
  2011-03-09 13:45 ` [PATCH 1/9] FIX: Load container content for container reshape continuation Adam Kwolek
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:45 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

This series contains fixes for reshape restart (external metadata)
and makes process workable.

In general your idea of reusing reshape_container() works. The main
problem I've found is opening read only array for writing.
Fortunately during reshape restart we do not need any array writes, 
so read only access is enough.

In patch 'FIX: Unfreeze array on success only during reshape continuation'
I've blocked unfreezing array after reshape restart in case of error.
This causes mdmon to not touch such array and user can take some actions.

I've fixed problem dirty array assembly also. Fix is almost as you proposed.

Known issues:
1. Container reshape half way (corner case)
  When 2 arrays are in container and first is under near finished reshape (during restart), 
  it can occur that reshape_container() could want to reshape second array before it is assembled
  Probably for container reshape continuation we should wait
  (after fork() in reshape_container()) for assembly finish.
2. when first array in container reshape is restarted, second array is not frozen

BR
Adam


---

Adam Kwolek (9):
      imsm : FIX: Assemble dirty array when reshape is in progress
      FIX: Set 'active' array state before array configuration
      FIX: Array cannot be opened for writing on restart
      FIX: Cannot continue reshape if mdmon is not run
      FIX: Unfreeze array on success only during reshape continuation
      imsm: FIX: Do not clean checkpoint for active reshape
      FIX: Make expansion counter usable
      FIX: Block reshaped array monitoring
      FIX: Load container content for container reshape continuation


 Assemble.c    |   39 ++++++++++++++++++++++++---------------
 Grow.c        |   38 +++++++++++++++++++++++++++++++++-----
 super-intel.c |   19 +++++++++++++------
 3 files changed, 70 insertions(+), 26 deletions(-)

-- 
Signature

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] FIX: Load container content for container reshape continuation
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
@ 2011-03-09 13:45 ` Adam Kwolek
  2011-03-10  0:27   ` NeilBrown
  2011-03-09 13:45 ` [PATCH 2/9] FIX: Block reshaped array monitoring Adam Kwolek
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:45 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

st->sb is null. This is exception cause.
reshape_container() function expects that super block will be loaded.

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

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

diff --git a/Grow.c b/Grow.c
index 40cb929..e37fc57 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3370,10 +3370,18 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
 		fmt_devname(buf, st->container_dev);
 		container = buf;
 		freeze(st);
-		if (info->reshape_active == 2)
-			return reshape_container(container, NULL,
+
+		if (info->reshape_active == 2) {
+			int cfd = open_dev(st->container_dev);
+			if (cfd >= 0) {
+				st->ss->load_container(st, cfd, container);
+				close(cfd);
+				return reshape_container(container, NULL,
 						 st, info, 0, backup_file,
 						 0, 1);
+			}
+			return 1;
+		}
 	}
 	return reshape_array(container, mdfd, "array", st, info, 1,
 			     backup_file, 0, 0, 1);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/9] FIX: Block reshaped array monitoring
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
  2011-03-09 13:45 ` [PATCH 1/9] FIX: Load container content for container reshape continuation Adam Kwolek
@ 2011-03-09 13:45 ` Adam Kwolek
  2011-03-10  0:28   ` NeilBrown
  2011-03-09 13:45 ` [PATCH 3/9] FIX: Make expansion counter usable Adam Kwolek
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:45 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

When array under reshape is is assembled it has be disabled from monitoring
as soon as possible. It can accrue that this is i.e second array in container
and mdmon is loaded already.
Lack of blocking monitoring can cause change array state to active,
and reshape continuation will be not possible.

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

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

diff --git a/Assemble.c b/Assemble.c
index 20c27eb..fe917b2 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1519,6 +1519,9 @@ 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)
+		block_subarray(content);
+
 	if (sra)
 		sysfs_free(sra);
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] FIX: Make expansion counter usable
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
  2011-03-09 13:45 ` [PATCH 1/9] FIX: Load container content for container reshape continuation Adam Kwolek
  2011-03-09 13:45 ` [PATCH 2/9] FIX: Block reshaped array monitoring Adam Kwolek
@ 2011-03-09 13:45 ` Adam Kwolek
  2011-03-10  0:28   ` NeilBrown
  2011-03-09 13:46 ` [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape Adam Kwolek
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:45 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

Currently whole array geometry is set in sysfs_set_array(),
so none of disks (even for expansion) should fail during sysfs_add_disk()
Due to this expansion counter should be used for reshaped array when
disk slot is bigger than number of disks in array.

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

 Assemble.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index fe917b2..0ffbbc9 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1526,13 +1526,14 @@ int assemble_container_content(struct supertype *st, int mdfd,
 		sysfs_free(sra);
 
 	for (dev = content->devs; dev; dev = dev->next)
-		if (sysfs_add_disk(content, dev, 1) == 0)
-			working++;
-		else if (errno == EEXIST)
+		if (sysfs_add_disk(content, dev, 1) == 0) {
+			if (dev->disk.raid_disk >= content->array.raid_disks &&
+			    content->reshape_active)
+				expansion++;
+			else
+				working++;
+		} else if (errno == EEXIST)
 			preexist++;
-		else if (dev->disk.raid_disk >= content->array.raid_disks &&
-			  content->reshape_active)
-			expansion++;
 	if (working == 0)
 		return 1;/* Nothing new, don't try to start */
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
                   ` (2 preceding siblings ...)
  2011-03-09 13:45 ` [PATCH 3/9] FIX: Make expansion counter usable Adam Kwolek
@ 2011-03-09 13:46 ` Adam Kwolek
  2011-03-10  0:33   ` NeilBrown
  2011-03-09 13:46 ` [PATCH 5/9] FIX: Unfreeze array on success only during reshape continuation Adam Kwolek
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:46 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

Smaller checkpoint can be stored when metadata is not in migration state
(this means when there is no second map).
Without this additional condition it can occur that during array start/stop
sequence checkpoint was roll back to 0.

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

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

diff --git a/super-intel.c b/super-intel.c
index cd409e9..4e3de8a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5335,9 +5335,15 @@ mark_checkpoint:
 		 */
 		if (units32 == units &&
 		    __le32_to_cpu(dev->vol.curr_migr_unit) != units32) {
-			dprintf("imsm: mark checkpoint (%u)\n", units32);
-			dev->vol.curr_migr_unit = __cpu_to_le32(units32);
-			super->updates_pending++;
+			struct imsm_map *map2 = get_imsm_map(dev, 1);
+			if ((__le32_to_cpu(dev->vol.curr_migr_unit) < units32)
+			    || (map2 == NULL)) {
+				dprintf("imsm: mark checkpoint (%u)\n",
+					units32);
+				dev->vol.curr_migr_unit =
+					__cpu_to_le32(units32);
+				super->updates_pending++;
+			}
 		}
 	}
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/9] FIX: Unfreeze array on success only during reshape continuation
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
                   ` (3 preceding siblings ...)
  2011-03-09 13:46 ` [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape Adam Kwolek
@ 2011-03-09 13:46 ` Adam Kwolek
  2011-03-10  0:36   ` NeilBrown
  2011-03-09 13:46 ` [PATCH 6/9] FIX: Cannot continue reshape if mdmon is not run Adam Kwolek
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:46 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

When reshape continuation fails, reshaped array should remain frozen
to allow user for repair action (mdmon will not change array state).
Setting restart to 0 was moved down to allow failure detection.

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

 Grow.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Grow.c b/Grow.c
index e37fc57..67bbb26 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2255,11 +2255,12 @@ int reshape_container(char *container, char *devname,
 				   content, force,
 				   backup_file, quiet, 1, restart);
 		close(fd);
-		restart = 0;
 		if (rv)
 			break;
+		restart = 0;
 	}
-	unfreeze(st);
+	if (restart == 0)
+		unfreeze(st);
 	sysfs_free(cc);
 	exit(0);
 }


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] FIX: Cannot continue reshape if mdmon is not run
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
                   ` (4 preceding siblings ...)
  2011-03-09 13:46 ` [PATCH 5/9] FIX: Unfreeze array on success only during reshape continuation Adam Kwolek
@ 2011-03-09 13:46 ` Adam Kwolek
  2011-03-10  0:37   ` NeilBrown
  2011-03-09 13:46 ` [PATCH 7/9] FIX: Array cannot be opened for writing on restart Adam Kwolek
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:46 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

Grow_continue() call was moved down in assemble_container_content()
because it needs running mdmon. Mdmon is running at the end of this function.

Due to mdmon is required to run setting array_state was disabled for reshaped array.
This causes that md will keep stable reshape in sync_action.

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

 Assemble.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index 0ffbbc9..3771ee1 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1544,7 +1544,7 @@ int assemble_container_content(struct supertype *st, int mdfd,
 	if (runstop > 0 ||
 		 (working + preexist + expansion) >=
 			content->array.working_disks) {
-		int err;
+		int err = 0;
 
 		if (content->reshape_active) {
 			int spare = content->array.raid_disks + expansion;
@@ -1577,18 +1577,19 @@ int assemble_container_content(struct supertype *st, int mdfd,
 						" to specify a --backup-file\n");
 				return 1;
 			}
-
-			err = Grow_continue(mdfd, st, content, backup_file);
-		} else switch(content->array.level) {
+		}
+		switch (content->array.level) {
 		case LEVEL_LINEAR:
 		case LEVEL_MULTIPATH:
 		case 0:
-			err = sysfs_set_str(content, NULL, "array_state",
-					    "active");
+			if (!content->reshape_active)
+				err = sysfs_set_str(content, NULL,
+						    "array_state", "active");
 			break;
 		default:
-			err = sysfs_set_str(content, NULL, "array_state",
-				      "readonly");
+			if (!content->reshape_active)
+				err = sysfs_set_str(content, NULL,
+						    "array_state", "readonly");
 			/* start mdmon if needed. */
 			if (!err) {
 				if (!mdmon_running(st->container_dev))
@@ -1615,8 +1616,12 @@ int assemble_container_content(struct supertype *st, int mdfd,
 					expansion);
 			fprintf(stderr, "\n");
 		}
-		if (!err)
+		if (!err) {
 			wait_for(chosen_name, mdfd);
+			if (content->reshape_active)
+				err = Grow_continue(mdfd, st, content,
+						    backup_file);
+		}
 		return err;
 		/* FIXME should have an O_EXCL and wait for read-auto */
 	} else {


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] FIX: Array cannot be opened for writing on restart
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
                   ` (5 preceding siblings ...)
  2011-03-09 13:46 ` [PATCH 6/9] FIX: Cannot continue reshape if mdmon is not run Adam Kwolek
@ 2011-03-09 13:46 ` Adam Kwolek
  2011-03-10  0:38   ` NeilBrown
  2011-03-09 13:46 ` [PATCH 8/9] FIX: Set 'active' array state before array configuration Adam Kwolek
  2011-03-09 13:46 ` [PATCH 9/9] imsm : FIX: Assemble dirty array when reshape is in progress Adam Kwolek
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:46 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

When reshape is restarted array (in readonly state) cannot be opened for writing.
For reshape restart open array for reading only.
Array is configured already and no writes are required.

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

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

diff --git a/Grow.c b/Grow.c
index 67bbb26..e392290 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2240,7 +2240,17 @@ int reshape_container(char *container, char *devname,
 		if (!content)
 			break;
 
-		fd = open_dev(mdstat->devnum);
+		if (restart) {
+			char buf[20];
+			/* array is in readonly state,
+			 * so cannot be opened for writting
+			 */
+			sprintf(buf, "%d:%d",
+				dev2major(mdstat->devnum),
+				dev2minor(mdstat->devnum));
+			fd = dev_open(buf, O_RDONLY);
+		} else
+			fd = open_dev(mdstat->devnum);
 		if (fd < 0)
 			break;
 		adev = map_dev(dev2major(mdstat->devnum),


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] FIX: Set 'active' array state before array configuration
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
                   ` (6 preceding siblings ...)
  2011-03-09 13:46 ` [PATCH 7/9] FIX: Array cannot be opened for writing on restart Adam Kwolek
@ 2011-03-09 13:46 ` Adam Kwolek
  2011-03-10  0:39   ` NeilBrown
  2011-03-09 13:46 ` [PATCH 9/9] imsm : FIX: Assemble dirty array when reshape is in progress Adam Kwolek
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:46 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

For not reshaped array in container during assembly array is in auto-read-only state.
It is not possible to set disk slot for such array and later reshape cannot be started also.
To move array from 'auto-read-only' to 'active' state storing 'active' state
to sysfs is added. This allows for disks configuration and reshape.

During reshaped array restart it is disabled by condition on restart variable.

When reshape is starting, storing 'active' state
to already active array should not matter.

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

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

diff --git a/Grow.c b/Grow.c
index e392290..9a831f4 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1654,6 +1654,15 @@ static int reshape_array(char *container, int fd, char *devname,
 		info->new_level = UnSet;
 		msg = analyse_change(info, &reshape);
 		info->new_level = new_level;
+		if (!restart) {
+			sra = sysfs_read(fd, 0, GET_VERSION);
+			if (sra) {
+				sysfs_set_str(sra, NULL,
+					      "array_state", "active");
+				sysfs_free(sra);
+				sra = NULL;
+			}
+		}
 	} else
 		msg = analyse_change(info, &reshape);
 	if (msg) {


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] imsm : FIX: Assemble dirty array when reshape is in progress
  2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
                   ` (7 preceding siblings ...)
  2011-03-09 13:46 ` [PATCH 8/9] FIX: Set 'active' array state before array configuration Adam Kwolek
@ 2011-03-09 13:46 ` Adam Kwolek
  2011-03-10  0:39   ` NeilBrown
  8 siblings, 1 reply; 19+ messages in thread
From: Adam Kwolek @ 2011-03-09 13:46 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

During reshape for dirty volumes reshape_progress has to be calculated also.
To keep the same logic for array creation:
not setting info->resync_start = MaxSector when first condition is true,
resync_start is initialized by MaxSector to allow proper array initialization.

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

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

diff --git a/super-intel.c b/super-intel.c
index 4e3de8a..38756e2 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1820,10 +1820,12 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
 	info->recovery_start = MaxSector;
 
 	info->reshape_progress = 0;
+	info->resync_start = MaxSector;
 	if (map_to_analyse->map_state == IMSM_T_STATE_UNINITIALIZED ||
 	    dev->vol.dirty) {
 		info->resync_start = 0;
-	} else if (dev->vol.migr_state) {
+	}
+	if (dev->vol.migr_state) {
 		switch (migr_type(dev)) {
 		case MIGR_REPAIR:
 		case MIGR_INIT: {
@@ -1868,8 +1870,7 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
 			/* we are not dirty, so... */
 			info->resync_start = MaxSector;
 		}
-	} else
-		info->resync_start = MaxSector;
+	}
 
 	strncpy(info->name, (char *) dev->volume, MAX_RAID_SERIAL_LEN);
 	info->name[MAX_RAID_SERIAL_LEN] = 0;


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/9] FIX: Load container content for container reshape continuation
  2011-03-09 13:45 ` [PATCH 1/9] FIX: Load container content for container reshape continuation Adam Kwolek
@ 2011-03-10  0:27   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:27 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:45:37 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> st->sb is null. This is exception cause.
> reshape_container() function expects that super block will be loaded.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 40cb929..e37fc57 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3370,10 +3370,18 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
>  		fmt_devname(buf, st->container_dev);
>  		container = buf;
>  		freeze(st);
> -		if (info->reshape_active == 2)
> -			return reshape_container(container, NULL,
> +
> +		if (info->reshape_active == 2) {
> +			int cfd = open_dev(st->container_dev);
> +			if (cfd >= 0) {
> +				st->ss->load_container(st, cfd, container);
> +				close(cfd);
> +				return reshape_container(container, NULL,
>  						 st, info, 0, backup_file,
>  						 0, 1);
> +			}
> +			return 1;
> +		}
>  	}
>  	return reshape_array(container, mdfd, "array", st, info, 1,
>  			     backup_file, 0, 0, 1);
> 
> --
> 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

Applied, thanks.

NeilBrown


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/9] FIX: Block reshaped array monitoring
  2011-03-09 13:45 ` [PATCH 2/9] FIX: Block reshaped array monitoring Adam Kwolek
@ 2011-03-10  0:28   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:28 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:45:45 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When array under reshape is is assembled it has be disabled from monitoring
> as soon as possible. It can accrue that this is i.e second array in container
> and mdmon is loaded already.
> Lack of blocking monitoring can cause change array state to active,
> and reshape continuation will be not possible.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Assemble.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 20c27eb..fe917b2 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1519,6 +1519,9 @@ 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)
> +		block_subarray(content);
> +
>  	if (sra)
>  		sysfs_free(sra);
>  
> 
> --
> 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


Applied, thanks.


I think I need to review all the 'blocking' and see if it can be tidied up,
but this is OK for now.

NeilBrown


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/9] FIX: Make expansion counter usable
  2011-03-09 13:45 ` [PATCH 3/9] FIX: Make expansion counter usable Adam Kwolek
@ 2011-03-10  0:28   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:28 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:45:54 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Currently whole array geometry is set in sysfs_set_array(),
> so none of disks (even for expansion) should fail during sysfs_add_disk()
> Due to this expansion counter should be used for reshaped array when
> disk slot is bigger than number of disks in array.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Assemble.c |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index fe917b2..0ffbbc9 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1526,13 +1526,14 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  		sysfs_free(sra);
>  
>  	for (dev = content->devs; dev; dev = dev->next)
> -		if (sysfs_add_disk(content, dev, 1) == 0)
> -			working++;
> -		else if (errno == EEXIST)
> +		if (sysfs_add_disk(content, dev, 1) == 0) {
> +			if (dev->disk.raid_disk >= content->array.raid_disks &&
> +			    content->reshape_active)
> +				expansion++;
> +			else
> +				working++;
> +		} else if (errno == EEXIST)
>  			preexist++;
> -		else if (dev->disk.raid_disk >= content->array.raid_disks &&
> -			  content->reshape_active)
> -			expansion++;
>  	if (working == 0)
>  		return 1;/* Nothing new, don't try to start */
>  
> 

Applied, thanks.

NeilBrown

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape
  2011-03-09 13:46 ` [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape Adam Kwolek
@ 2011-03-10  0:33   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:33 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:46:02 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Smaller checkpoint can be stored when metadata is not in migration state
> (this means when there is no second map).
> Without this additional condition it can occur that during array start/stop
> sequence checkpoint was roll back to 0.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index cd409e9..4e3de8a 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5335,9 +5335,15 @@ mark_checkpoint:
>  		 */
>  		if (units32 == units &&
>  		    __le32_to_cpu(dev->vol.curr_migr_unit) != units32) {
> -			dprintf("imsm: mark checkpoint (%u)\n", units32);
> -			dev->vol.curr_migr_unit = __cpu_to_le32(units32);
> -			super->updates_pending++;
> +			struct imsm_map *map2 = get_imsm_map(dev, 1);
> +			if ((__le32_to_cpu(dev->vol.curr_migr_unit) < units32)
> +			    || (map2 == NULL)) {
> +				dprintf("imsm: mark checkpoint (%u)\n",
> +					units32);
> +				dev->vol.curr_migr_unit =
> +					__cpu_to_le32(units32);
> +				super->updates_pending++;
> +			}
>  		}
>  	}
>  
> 

Not applied.

I don't really understand the change you are making here.

As it seems to me that:
  - before the patch, we only update the 'curr_migr_unit' when
    units32 is different
  - after the patch, we only update it if units32 is bigger, or if
    map2==NULL (meaning this isn't a migration?).

So when would units32 be smaller??  What is the important case that this
fixes?

From the comment, it seems to be to avoid 'roll back to 0'.   I don't see
how it does that, and I don't see why that is a problem.

If some reshape has happened but curr_migr_unit is 0, that just means
that we need to replay the backed-up data and set curr_migr_unit
accordingly (or reflect that the backed-up data has been migrated).
So after Grow_restart has run, curr_migr_unit should never be zero.
You will need to get update_super_imsm to understand '_reshape_progress'
for this to work properly.

NeilBrown


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/9] FIX: Unfreeze array on success only during reshape continuation
  2011-03-09 13:46 ` [PATCH 5/9] FIX: Unfreeze array on success only during reshape continuation Adam Kwolek
@ 2011-03-10  0:36   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:36 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:46:10 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When reshape continuation fails, reshaped array should remain frozen
> to allow user for repair action (mdmon will not change array state).
> Setting restart to 0 was moved down to allow failure detection.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index e37fc57..67bbb26 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2255,11 +2255,12 @@ int reshape_container(char *container, char *devname,
>  				   content, force,
>  				   backup_file, quiet, 1, restart);
>  		close(fd);
> -		restart = 0;
>  		if (rv)
>  			break;
> +		restart = 0;
>  	}
> -	unfreeze(st);
> +	if (restart == 0)
> +		unfreeze(st);
>  	sysfs_free(cc);
>  	exit(0);
>  }
> 

Not applied... it is probably close to right, but I'm a little confused.
Surely we should never never unfreeze on an error - not only in the restart
case...
And the code doesn't make it obvious that the unfreeze is only in the
error case...
Maybe
  if (!rv)
     unfreeze(st);
??

NeilBrown

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/9] FIX: Cannot continue reshape if mdmon is not run
  2011-03-09 13:46 ` [PATCH 6/9] FIX: Cannot continue reshape if mdmon is not run Adam Kwolek
@ 2011-03-10  0:37   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:37 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:46:18 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Grow_continue() call was moved down in assemble_container_content()
> because it needs running mdmon. Mdmon is running at the end of this function.
> 
> Due to mdmon is required to run setting array_state was disabled for reshaped array.
> This causes that md will keep stable reshape in sync_action.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Assemble.c |   23 ++++++++++++++---------
>  1 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 0ffbbc9..3771ee1 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1544,7 +1544,7 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  	if (runstop > 0 ||
>  		 (working + preexist + expansion) >=
>  			content->array.working_disks) {
> -		int err;
> +		int err = 0;
>  
>  		if (content->reshape_active) {
>  			int spare = content->array.raid_disks + expansion;
> @@ -1577,18 +1577,19 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  						" to specify a --backup-file\n");
>  				return 1;
>  			}
> -
> -			err = Grow_continue(mdfd, st, content, backup_file);
> -		} else switch(content->array.level) {
> +		}
> +		switch (content->array.level) {
>  		case LEVEL_LINEAR:
>  		case LEVEL_MULTIPATH:
>  		case 0:
> -			err = sysfs_set_str(content, NULL, "array_state",
> -					    "active");
> +			if (!content->reshape_active)
> +				err = sysfs_set_str(content, NULL,
> +						    "array_state", "active");
>  			break;
>  		default:
> -			err = sysfs_set_str(content, NULL, "array_state",
> -				      "readonly");
> +			if (!content->reshape_active)
> +				err = sysfs_set_str(content, NULL,
> +						    "array_state", "readonly");
>  			/* start mdmon if needed. */
>  			if (!err) {
>  				if (!mdmon_running(st->container_dev))
> @@ -1615,8 +1616,12 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  					expansion);
>  			fprintf(stderr, "\n");
>  		}
> -		if (!err)
> +		if (!err) {
>  			wait_for(chosen_name, mdfd);
> +			if (content->reshape_active)
> +				err = Grow_continue(mdfd, st, content,
> +						    backup_file);
> +		}
>  		return err;
>  		/* FIXME should have an O_EXCL and wait for read-auto */
>  	} else {
> 

This is needlessly complex.

I have applied the following instead.

Thanks,
NeilBrown

commit f362d22b5bc492c60fa1ea7e0f8346b7837dd7da
Author: NeilBrown <neilb@suse.de>
Date:   Thu Mar 10 11:36:47 2011 +1100

    Grow: make sure mdmon is running for Grow_continue arrays.
    
    when starting an array that is in the middle of a migration,
    we need to start mdmon, just as we do for arrays which are not
    in the middle of a migration.
    
    Repored-by: Adam Kwolek <adam.kwolek@intel.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/Grow.c b/Grow.c
index 5acd94d..734fa6d 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3371,6 +3371,11 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
                container = buf;
                freeze(st);
 
+               if (!mdmon_running(st->container_dev))
+                       start_mdmon(st->container_dev);
+               ping_monitor(devnum2devname(st->container_dev));
+
+
                if (info->reshape_active == 2) {
                        int cfd = open_dev(st->container_dev);
                        if (cfd < 0)



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/9] FIX: Array cannot be opened for writing on restart
  2011-03-09 13:46 ` [PATCH 7/9] FIX: Array cannot be opened for writing on restart Adam Kwolek
@ 2011-03-10  0:38   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:38 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:46:26 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When reshape is restarted array (in readonly state) cannot be opened for writing.
> For reshape restart open array for reading only.
> Array is configured already and no writes are required.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 67bbb26..e392290 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2240,7 +2240,17 @@ int reshape_container(char *container, char *devname,
>  		if (!content)
>  			break;
>  
> -		fd = open_dev(mdstat->devnum);
> +		if (restart) {
> +			char buf[20];
> +			/* array is in readonly state,
> +			 * so cannot be opened for writting
> +			 */
> +			sprintf(buf, "%d:%d",
> +				dev2major(mdstat->devnum),
> +				dev2minor(mdstat->devnum));
> +			fd = dev_open(buf, O_RDONLY);
> +		} else
> +			fd = open_dev(mdstat->devnum);
>  		if (fd < 0)
>  			break;
>  		adev = map_dev(dev2major(mdstat->devnum),
> 

I changed open_dev to always open O_RDONLY instead.  That
seems to be quite sufficient.

Thanks,
NeilBrown


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/9] FIX: Set 'active' array state before array configuration
  2011-03-09 13:46 ` [PATCH 8/9] FIX: Set 'active' array state before array configuration Adam Kwolek
@ 2011-03-10  0:39   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:39 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:46:34 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> For not reshaped array in container during assembly array is in auto-read-only state.
> It is not possible to set disk slot for such array and later reshape cannot be started also.
> To move array from 'auto-read-only' to 'active' state storing 'active' state
> to sysfs is added. This allows for disks configuration and reshape.
> 
> During reshaped array restart it is disabled by condition on restart variable.
> 
> When reshape is starting, storing 'active' state
> to already active array should not matter.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index e392290..9a831f4 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1654,6 +1654,15 @@ static int reshape_array(char *container, int fd, char *devname,
>  		info->new_level = UnSet;
>  		msg = analyse_change(info, &reshape);
>  		info->new_level = new_level;
> +		if (!restart) {
> +			sra = sysfs_read(fd, 0, GET_VERSION);
> +			if (sra) {
> +				sysfs_set_str(sra, NULL,
> +					      "array_state", "active");
> +				sysfs_free(sra);
> +				sra = NULL;
> +			}
> +		}
>  	} else
>  		msg = analyse_change(info, &reshape);
>  	if (msg) {
> 

I changed this to just use the RESTART_ARRAY_RW ioctl instead.

Thanks,
NeilBrown


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 9/9] imsm : FIX: Assemble dirty array when reshape is in progress
  2011-03-09 13:46 ` [PATCH 9/9] imsm : FIX: Assemble dirty array when reshape is in progress Adam Kwolek
@ 2011-03-10  0:39   ` NeilBrown
  0 siblings, 0 replies; 19+ messages in thread
From: NeilBrown @ 2011-03-10  0:39 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

On Wed, 09 Mar 2011 14:46:42 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> During reshape for dirty volumes reshape_progress has to be calculated also.
> To keep the same logic for array creation:
> not setting info->resync_start = MaxSector when first condition is true,
> resync_start is initialized by MaxSector to allow proper array initialization.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 4e3de8a..38756e2 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1820,10 +1820,12 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
>  	info->recovery_start = MaxSector;
>  
>  	info->reshape_progress = 0;
> +	info->resync_start = MaxSector;
>  	if (map_to_analyse->map_state == IMSM_T_STATE_UNINITIALIZED ||
>  	    dev->vol.dirty) {
>  		info->resync_start = 0;
> -	} else if (dev->vol.migr_state) {
> +	}
> +	if (dev->vol.migr_state) {
>  		switch (migr_type(dev)) {
>  		case MIGR_REPAIR:
>  		case MIGR_INIT: {
> @@ -1868,8 +1870,7 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
>  			/* we are not dirty, so... */
>  			info->resync_start = MaxSector;
>  		}
> -	} else
> -		info->resync_start = MaxSector;
> +	}
>  
>  	strncpy(info->name, (char *) dev->volume, MAX_RAID_SERIAL_LEN);
>  	info->name[MAX_RAID_SERIAL_LEN] = 0;

Applied, thanks.

NeilBrown

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-03-10  0:39 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-09 13:45 [PATCH 0/9] Reshape restart from checkpoint Adam Kwolek
2011-03-09 13:45 ` [PATCH 1/9] FIX: Load container content for container reshape continuation Adam Kwolek
2011-03-10  0:27   ` NeilBrown
2011-03-09 13:45 ` [PATCH 2/9] FIX: Block reshaped array monitoring Adam Kwolek
2011-03-10  0:28   ` NeilBrown
2011-03-09 13:45 ` [PATCH 3/9] FIX: Make expansion counter usable Adam Kwolek
2011-03-10  0:28   ` NeilBrown
2011-03-09 13:46 ` [PATCH 4/9] imsm: FIX: Do not clean checkpoint for active reshape Adam Kwolek
2011-03-10  0:33   ` NeilBrown
2011-03-09 13:46 ` [PATCH 5/9] FIX: Unfreeze array on success only during reshape continuation Adam Kwolek
2011-03-10  0:36   ` NeilBrown
2011-03-09 13:46 ` [PATCH 6/9] FIX: Cannot continue reshape if mdmon is not run Adam Kwolek
2011-03-10  0:37   ` NeilBrown
2011-03-09 13:46 ` [PATCH 7/9] FIX: Array cannot be opened for writing on restart Adam Kwolek
2011-03-10  0:38   ` NeilBrown
2011-03-09 13:46 ` [PATCH 8/9] FIX: Set 'active' array state before array configuration Adam Kwolek
2011-03-10  0:39   ` NeilBrown
2011-03-09 13:46 ` [PATCH 9/9] imsm : FIX: Assemble dirty array when reshape is in progress Adam Kwolek
2011-03-10  0:39   ` NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).