[PATCH 1/5] FIX: Unfreeze array if reshape_array wasn't succeded

[PATCH 1/5] FIX: Unfreeze array if reshape_array wasn't succeded

[Krzysztof Wojcik]

If reshape_array does not success we should not leave
array freezed.

Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
---
 Grow.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index c5f83a8..7c5edae 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1567,7 +1567,10 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
 		sync_metadata(st);
 		rv = reshape_array(container, cfd, fd, devname,
 				   st, &info, force, backup_file, quiet, 0);
-		frozen = 0;
+		if (rv)
+			frozen = 1;
+		else
+			frozen = 0;
 	}
 release:
 	if (frozen > 0)

[PATCH 2/5] Add raid1->raid0 takeover support

[Krzysztof Wojcik]

Add support for raid1 to raid0 takeover operation in user space.
This patch includes support for native and imsm metadata.

Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
---
 Grow.c        |   39 ++++++++++++++++++++++++++++-----------
 super-intel.c |    6 ++++++
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/Grow.c b/Grow.c
index 7c5edae..a8da20a 100644
--- a/Grow.c
+++ b/Grow.c
@@ -650,15 +650,20 @@ void abort_reshape(struct mdinfo *sra)
 	sysfs_set_str(sra, NULL, "sync_max", "max");
 }
 
-int remove_disks_on_raid10_to_raid0_takeover(struct supertype *st,
-					     struct mdinfo *sra,
-					     int layout)
+int remove_disks_for_takeover(struct supertype *st,
+			     struct mdinfo *sra,
+			     int layout)
 {
 	int nr_of_copies;
 	struct mdinfo *remaining;
 	int slot;
 
-	nr_of_copies = layout & 0xff;
+	if (sra->array.level == 10)
+		nr_of_copies = layout & 0xff;
+	else if (sra->array/level == 1)
+		nr_of_copies = sra->array.raid_disks;
+	else
+		return 1;
 
 	remaining = sra->devs;
 	sra->devs = NULL;
@@ -911,8 +916,18 @@ char *analyse_change(struct mdinfo *info, struct reshape *re)
 	switch (info->array.level) {
 	case 1:
 		/* RAID1 can convert to RAID1 with different disks, or
-		 * raid5 with 2 disks
+		 * raid5 with 2 disks, or
+		 * raid0 with 1 disk
 		 */
+		if (info->new_level == 0) {
+			re->level = 0;
+			re->before.data_disks = info->array.raid_disks / 2;
+			re->after.data_disks = re->before.data_disks;
+			re->before.layout = 0;
+			re->backup_blocks = 0;
+			re->parity = 0;
+			return NULL;
+		}
 		if (info->new_level == 1) {
 			if (info->delta_disks == UnSet)
 				/* Don't know what to do */
@@ -1449,15 +1464,17 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
 			size = array.size;
 	}
 
-	/* ========= check for Raid10 -> Raid0 conversion ===============
+	/* ========= check for Raid10/Raid1 -> Raid0 conversion ===============
 	 * current implementation assumes that following conditions must be met:
-	 * - far_copies == 1
-	 * - near_copies == 2
+	 * - RAID10:
+	 * 	- far_copies == 1
+	 * 	- near_copies == 2
 	 */
-	if (level == 0 && array.level == 10 && sra &&
-	    array.layout == ((1 << 8) + 2) && !(array.raid_disks & 1)) {
+	if ((level == 0 && array.level == 10 && sra &&
+	    array.layout == ((1 << 8) + 2) && !(array.raid_disks & 1)) ||
+	    (level == 0 && array.level == 1 && sra)) {
 		int err;
-		err = remove_disks_on_raid10_to_raid0_takeover(st, sra, array.layout);
+		err = remove_disks_for_takeover(st, sra, array.layout);
 		if (err) {
 			dprintf(Name": Array cannot be reshaped\n");
 			if (cfd > -1)
diff --git a/super-intel.c b/super-intel.c
index b25d4fb..461fb0c 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6785,6 +6785,12 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
 				check_devs = 1;
 			}
 			break;
+		case 1:
+			if (geo->level == 0) {
+				change = CH_TAKEOVER;
+				check_devs = 1;
+			}
+			break;
 		case 5:
 			if (geo->level != 0)
 				change = CH_LEVEL_MIGRATION;

[PATCH 3/5] Mistake in raid1->raid5 migration

[Krzysztof Wojcik]

1. Mistake in target level comparison.
2. Initialize reshape->after.data_disks field
to proper spares_needed calculation
---
 Grow.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index a8da20a..e1e509e 100644
--- a/Grow.c
+++ b/Grow.c
@@ -941,11 +941,12 @@ char *analyse_change(struct mdinfo *info, struct reshape *re)
 			return NULL;
 		}
 		if (info->array.raid_disks == 2 &&
-		    info->array.raid_disks == 5) {
+		    info->new_level == 5) {
 			/* simple in-place conversion */
 			re->level = 5;
 			re->parity = 1;
 			re->before.data_disks = 1;
+			re->after.data_disks = 1;
 			re->before.layout = ALGORITHM_LEFT_SYMMETRIC;
 			re->backup_blocks = 0;
 			return NULL;

[PATCH 4/5] FIX: Validate input in ping_monitor function

[Krzysztof Wojcik]

For native case we do not have monitor running so we have to
return without pinging.
Moreover we have NULL as input parameter. We should to avoid
segmentation fault.

Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
---
 msg.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/msg.c b/msg.c
index 76e74e7..b97ebec 100644
--- a/msg.c
+++ b/msg.c
@@ -206,8 +206,13 @@ int fping_monitor(int sfd)
 /* give the monitor a chance to update the metadata */
 int ping_monitor(char *devname)
 {
-	int sfd = connect_monitor(devname);
-	int err = fping_monitor(sfd);
+	int sfd, err;
+
+	if (!devname)
+		return -1;
+
+	sfd = connect_monitor(devname);
+	err = fping_monitor(sfd);
 
 	close(sfd);
 	return err;

[PATCH 5/5] FIX: Validate input in ping_manager function

[Krzysztof Wojcik]

For native case we do not have manager running so we have to
return without pinging.
Moreover we have NULL as input parameter. We should to avoid
segmentation fault.

Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
---
 msg.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/msg.c b/msg.c
index b97ebec..2c770ab 100644
--- a/msg.c
+++ b/msg.c
@@ -432,7 +432,13 @@ void unblock_monitor(char *container, const int unfreeze)
  */
 int ping_manager(char *devname)
 {
-	int sfd = connect_monitor(devname);
+	int sfd;
+
+	if (!devname)
+		return -1;
+
+	sfd = connect_monitor(devname);
+
 	struct metadata_update msg = { .len = -1 };
 	int err = 0;
 

Re: [PATCH 1/5] FIX: Unfreeze array if reshape_array wasn't succeded

[Neil Brown]

On Mon, 24 Jan 2011 15:17:04 +0100
Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote:

> If reshape_array does not success we should not leave
> array freezed.
> 
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> ---
>  Grow.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index c5f83a8..7c5edae 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1567,7 +1567,10 @@ int Grow_reshape(char *devname, int fd, int
> quiet, char *backup_file, sync_metadata(st);
>  		rv = reshape_array(container, cfd, fd, devname,
>  				   st, &info, force, backup_file,
> quiet, 0);
> -		frozen = 0;
> +		if (rv)
> +			frozen = 1;
> +		else
> +			frozen = 0;

reshape_array is responsible for unfreezing the array in this case, and
I'm fairly sure that it already does!

NeilBrown

>  	}
>  release:
>  	if (frozen > 0)

Re: [PATCH 2/5] Add raid1->raid0 takeover support

[Neil Brown]

On Mon, 24 Jan 2011 15:17:12 +0100
Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote:

> Add support for raid1 to raid0 takeover operation in user space.
> This patch includes support for native and imsm metadata.
> 
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> ---
>  Grow.c        |   39 ++++++++++++++++++++++++++++-----------
>  super-intel.c |    6 ++++++
>  2 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 7c5edae..a8da20a 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -650,15 +650,20 @@ void abort_reshape(struct mdinfo *sra)
>  	sysfs_set_str(sra, NULL, "sync_max", "max");
>  }
>  
> -int remove_disks_on_raid10_to_raid0_takeover(struct supertype *st,
> -					     struct mdinfo *sra,
> -					     int layout)
> +int remove_disks_for_takeover(struct supertype *st,
> +			     struct mdinfo *sra,
> +			     int layout)
>  {
>  	int nr_of_copies;
>  	struct mdinfo *remaining;
>  	int slot;
>  
> -	nr_of_copies = layout & 0xff;
> +	if (sra->array.level == 10)
> +		nr_of_copies = layout & 0xff;
> +	else if (sra->array/level == 1)
> +		nr_of_copies = sra->array.raid_disks;
> +	else
> +		return 1;
>  
>  	remaining = sra->devs;
>  	sra->devs = NULL;
> @@ -911,8 +916,18 @@ char *analyse_change(struct mdinfo *info, struct
> reshape *re) switch (info->array.level) {
>  	case 1:
>  		/* RAID1 can convert to RAID1 with different disks,
> or
> -		 * raid5 with 2 disks
> +		 * raid5 with 2 disks, or
> +		 * raid0 with 1 disk
>  		 */
> +		if (info->new_level == 0) {
> +			re->level = 0;
> +			re->before.data_disks =
> info->array.raid_disks / 2;
> +			re->after.data_disks = re->before.data_disks;

dividing raid_disks by 2 doesn't make any sense at all.
It have fixed it so that it does make sense, and applied.

Thanks,
NeilBrown


> +			re->before.layout = 0;
> +			re->backup_blocks = 0;
> +			re->parity = 0;
> +			return NULL;
> +		}
>  		if (info->new_level == 1) {
>  			if (info->delta_disks == UnSet)
>  				/* Don't know what to do */
> @@ -1449,15 +1464,17 @@ int Grow_reshape(char *devname, int fd, int
> quiet, char *backup_file, size = array.size;
>  	}
>  
> -	/* ========= check for Raid10 -> Raid0 conversion
> ===============
> +	/* ========= check for Raid10/Raid1 -> Raid0 conversion
> ===============
>  	 * current implementation assumes that following conditions
> must be met:
> -	 * - far_copies == 1
> -	 * - near_copies == 2
> +	 * - RAID10:
> +	 * 	- far_copies == 1
> +	 * 	- near_copies == 2
>  	 */
> -	if (level == 0 && array.level == 10 && sra &&
> -	    array.layout == ((1 << 8) + 2) && !(array.raid_disks &
> 1)) {
> +	if ((level == 0 && array.level == 10 && sra &&
> +	    array.layout == ((1 << 8) + 2) && !(array.raid_disks &
> 1)) ||
> +	    (level == 0 && array.level == 1 && sra)) {
>  		int err;
> -		err = remove_disks_on_raid10_to_raid0_takeover(st,
> sra, array.layout);
> +		err = remove_disks_for_takeover(st, sra,
> array.layout); if (err) {
>  			dprintf(Name": Array cannot be reshaped\n");
>  			if (cfd > -1)
> diff --git a/super-intel.c b/super-intel.c
> index b25d4fb..461fb0c 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6785,6 +6785,12 @@ enum imsm_reshape_type
> imsm_analyze_change(struct supertype *st, check_devs = 1;
>  			}
>  			break;
> +		case 1:
> +			if (geo->level == 0) {
> +				change = CH_TAKEOVER;
> +				check_devs = 1;
> +			}
> +			break;
>  		case 5:
>  			if (geo->level != 0)
>  				change = CH_LEVEL_MIGRATION;
> 
> --
> 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 3/5] Mistake in raid1->raid5 migration

[Neil Brown]

On Mon, 24 Jan 2011 15:17:20 +0100
Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote:

> 1. Mistake in target level comparison.

I had already fixed that one thanks...

> 2. Initialize reshape->after.data_disks field

But not that one.  fixed now.

Thanks.
NeilBrown

> to proper spares_needed calculation
> ---
>  Grow.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index a8da20a..e1e509e 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -941,11 +941,12 @@ char *analyse_change(struct mdinfo *info,
> struct reshape *re) return NULL;
>  		}
>  		if (info->array.raid_disks == 2 &&
> -		    info->array.raid_disks == 5) {
> +		    info->new_level == 5) {
>  			/* simple in-place conversion */
>  			re->level = 5;
>  			re->parity = 1;
>  			re->before.data_disks = 1;
> +			re->after.data_disks = 1;
>  			re->before.layout = ALGORITHM_LEFT_SYMMETRIC;
>  			re->backup_blocks = 0;
>  			return NULL;

Re: [PATCH 4/5] FIX: Validate input in ping_monitor function

[Neil Brown]

On Mon, 24 Jan 2011 15:17:29 +0100
Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote:

> For native case we do not have monitor running so we have to
> return without pinging.
> Moreover we have NULL as input parameter. We should to avoid
> segmentation fault.

I have not applied this patch or the next one.
I would rather not call ping_XXX when not needed, rather than have it
check if it was needed or not.

NeilBrown

> 
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> ---
>  msg.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/msg.c b/msg.c
> index 76e74e7..b97ebec 100644
> --- a/msg.c
> +++ b/msg.c
> @@ -206,8 +206,13 @@ int fping_monitor(int sfd)
>  /* give the monitor a chance to update the metadata */
>  int ping_monitor(char *devname)
>  {
> -	int sfd = connect_monitor(devname);
> -	int err = fping_monitor(sfd);
> +	int sfd, err;
> +
> +	if (!devname)
> +		return -1;
> +
> +	sfd = connect_monitor(devname);
> +	err = fping_monitor(sfd);
>  
>  	close(sfd);
>  	return err;
> 
> --
> 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