[PATCH 00/14] Series short description

[PATCH 00/14] Series short description

[Adam Kwolek]

This patch series (14) implements manual reshape restart/continuation
in the following scenario:

1. Condition:
   During system boot mdadm finds array under reshape that should be
   assembled. This happens while initramfs is mounted
   (before file system pivot)

2. Scenario:
 a) Assembly procedure assembles array and runs reshape continuation
    procedure (as usual). Reshape procedure continues reshape and during
    file system pivot lost file system context and cannot control process
    using check-pointing
    To avoid this now reshape procedure detects that initramfs is mounted
    (before pivot) and reshape critical section is restored and array
    is prepared for reshape only. At this point mdadm finishes his work
    for this array and array is fully functional/accessible now.

        0001-Stop-array-reshape-when-mounted-initramfs-is-detecte.patch
        0002-Stop-container-reshape-while-using-initramfs.patch
        0003-FIX-Do-not-unblock-array-accidentally.patch
        0007-FIX-Memory-leak-during-Assembly.patch

 b) Afters system boot user manually can invoke reshape continuation.
    New mdadm option '--continue' for grow command was added.
    This allows for reshape continuation. mdadm for reshape continuation
    uses parameters read from metadata. Command line can looks as follows:

      mdadm -G --continue device_name [--backup-file=file_name]

      where:
          device_name : device that reshape should be continued on
                        e.g. /dev/md/container_name or /dev/md/raid_name
          backup-file : optional parameter required when backup-file 
                        was use previously for reshape execution


    For external metadata mdadm takes carry for metadata compatibility
    e.g. container operation can be continued on array device /and oposit/.

       0004-Add-continue-option-to-grow-command.patch
       0005-Add-Grow_continue_command.patch
       0006-Check-and-run-mdmon.patch
       0007-FIX-Memory-leak-during-Assembly.patch
       0008-Check-if-reshape-can-be-restarted.patch
       0009-Move-restore-backup-code-to-function.patch
       0010-Perform-restore-backup-for-reshape-continuation.patch
       0011-Add-possibility-to-restart-reshape-for-native-metada.patch
       0012-Early-reshape-backup-verification.patch
       0013-Check-if-md-allows-to-control-reshape.patch

3. man update:
   The last patch provides madam's man update for '--continue' grow option
       0014-Manual-update-for-continue-option.patch
BR
Adam

---

Adam Kwolek (14):
      Manual update for --continue option
      Check if md allows to control reshape
      Early reshape backup verification
      Add possibility to restart reshape for native metadata
      Perform restore backup for reshape continuation
      Move restore backup code to function
      Check if reshape can be restarted
      FIX: Memory leak during Assembly
      Check and run mdmon
      Add Grow_continue_command
      Add continue option to grow command
      FIX: Do not unblock array accidentally
      Stop container reshape while using initramfs
      Stop array reshape when mounted initramfs is detected


 Assemble.c |   39 +-------
 Grow.c     |  290 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 ReadMe.c   |    1 
 mdadm.8.in |   24 +++++
 mdadm.c    |   24 +++++
 mdadm.h    |   14 +++
 sysfs.c    |   10 ++
 util.c     |   59 ++++++++++++
 8 files changed, 419 insertions(+), 42 deletions(-)

-- 
Signature

[PATCH 01/14] Stop array reshape when mounted initramfs is detected

[Adam Kwolek]

During remounting file system from initramfs to real one,
mdadm is not able to continue reshape procedure due to lost
filesystem context.
To avoid this mdadm detects mounted initramfs and performs
critical section restore only. Array is set for later
reshape continuation.

Reshape continuation will be possible to invoking manually,
using '--continue' option.

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

 Grow.c  |   25 +++++++++++++++++++++++--
 mdadm.h |    4 ++++
 util.c  |   22 ++++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/Grow.c b/Grow.c
index 17d14b6..420177c 100644
--- a/Grow.c
+++ b/Grow.c
@@ -635,10 +635,21 @@ static int subarray_set_num(char *container, struct mdinfo *sra, char *name, int
 	return rc;
 }
 
+/* possible return values:
+ * 0 : success
+ * -1: error
+ * INITRAMFS_IS_IN_USE: continue reshape later
+ */
 int start_reshape(struct mdinfo *sra, int already_running)
 {
 	int err;
-	sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
+
+	/* do not block array as we not continue reshape this time
+	 */
+	if (initramfs_is_in_use() == INITRAMFS_NOT_USED)
+		sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
+	else
+		sysfs_set_num(sra, NULL, "suspend_lo", 0);
 	err = sysfs_set_num(sra, NULL, "suspend_hi", 0);
 	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo", 0);
 	if (!already_running)
@@ -2190,6 +2201,14 @@ started:
 	}
 	if (restart)
 		sysfs_set_str(sra, NULL, "array_state", "active");
+	if (initramfs_is_in_use() == INITRAMFS_IS_IN_USE) {
+		free(fdlist);
+		free(offsets);
+		sysfs_free(sra);
+		fprintf(stderr, Name ": Reshape has to be continued "
+			"when root fileststem will be mounted\n");
+		return 1;
+	}
 
 	/* Now we just need to kick off the reshape and watch, while
 	 * handling backups of the data...
@@ -2357,7 +2376,9 @@ int reshape_container(char *container, char *devname,
 		unfreeze(st);
 		return 1;
 	default: /* parent */
-		printf(Name ": multi-array reshape continues in background\n");
+		if (initramfs_is_in_use() == INITRAMFS_NOT_USED)
+			printf(Name ": multi-array reshape continues "
+			       "in background\n");
 		return 0;
 	case 0: /* child */
 		break;
diff --git a/mdadm.h b/mdadm.h
index d616966..9035e67 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1161,6 +1161,10 @@ extern char *human_size(long long bytes);
 extern char *human_size_brief(long long bytes);
 extern void print_r10_layout(int layout);
 
+extern int initramfs_is_in_use(void);
+#define INITRAMFS_IS_IN_USE		1
+#define INITRAMFS_NOT_USED		0
+
 #define NoMdDev (1<<23)
 extern int find_free_devnum(int use_partitions);
 
diff --git a/util.c b/util.c
index 0ea7e0d..7186f3f 100644
--- a/util.c
+++ b/util.c
@@ -1768,3 +1768,25 @@ struct mdinfo *container_choose_spares(struct supertype *st,
 	}
 	return disks;
 }
+
+int initramfs_is_in_use(void)
+{
+	int ret_val;
+	struct stat sts;
+
+	/* /init
+	 *	common script for many Linux distribution
+	 * /sysboot
+	 *	is specific for RH
+	 * /mkinitrd.config
+	 *	is specific for SLES
+	 */
+	if ((stat("/init", &sts) == -1) ||
+	    ((stat("/sysroot", &sts) == -1) &&
+	     (stat("/mkinitrd.config", &sts) == -1)))
+		ret_val = INITRAMFS_NOT_USED;
+	else
+		ret_val = INITRAMFS_IS_IN_USE;
+
+	return ret_val;
+}

Re: [PATCH 01/14] Stop array reshape when mounted initramfs is detected

[NeilBrown]

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

On Fri, 16 Sep 2011 13:53:54 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> During remounting file system from initramfs to real one,
> mdadm is not able to continue reshape procedure due to lost
> filesystem context.
> To avoid this mdadm detects mounted initramfs and performs
> critical section restore only. Array is set for later
> reshape continuation.
> 
> Reshape continuation will be possible to invoking manually,
> using '--continue' option.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c  |   25 +++++++++++++++++++++++--
>  mdadm.h |    4 ++++
>  util.c  |   22 ++++++++++++++++++++++
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 17d14b6..420177c 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -635,10 +635,21 @@ static int subarray_set_num(char *container, struct mdinfo *sra, char *name, int
>  	return rc;
>  }
>  
> +/* possible return values:
> + * 0 : success
> + * -1: error
> + * INITRAMFS_IS_IN_USE: continue reshape later
> + */
>  int start_reshape(struct mdinfo *sra, int already_running)
>  {
>  	int err;
> -	sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
> +
> +	/* do not block array as we not continue reshape this time
> +	 */
> +	if (initramfs_is_in_use() == INITRAMFS_NOT_USED)
> +		sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
> +	else
> +		sysfs_set_num(sra, NULL, "suspend_lo", 0);
>  	err = sysfs_set_num(sra, NULL, "suspend_hi", 0);
>  	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo", 0);
>  	if (!already_running)
> @@ -2190,6 +2201,14 @@ started:
>  	}
>  	if (restart)
>  		sysfs_set_str(sra, NULL, "array_state", "active");
> +	if (initramfs_is_in_use() == INITRAMFS_IS_IN_USE) {
> +		free(fdlist);
> +		free(offsets);
> +		sysfs_free(sra);
> +		fprintf(stderr, Name ": Reshape has to be continued "
> +			"when root fileststem will be mounted\n");
> +		return 1;
> +	}
>  
>  	/* Now we just need to kick off the reshape and watch, while
>  	 * handling backups of the data...
> @@ -2357,7 +2376,9 @@ int reshape_container(char *container, char *devname,
>  		unfreeze(st);
>  		return 1;
>  	default: /* parent */
> -		printf(Name ": multi-array reshape continues in background\n");
> +		if (initramfs_is_in_use() == INITRAMFS_NOT_USED)
> +			printf(Name ": multi-array reshape continues "
> +			       "in background\n");
>  		return 0;
>  	case 0: /* child */
>  		break;
> diff --git a/mdadm.h b/mdadm.h
> index d616966..9035e67 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1161,6 +1161,10 @@ extern char *human_size(long long bytes);
>  extern char *human_size_brief(long long bytes);
>  extern void print_r10_layout(int layout);
>  
> +extern int initramfs_is_in_use(void);
> +#define INITRAMFS_IS_IN_USE		1
> +#define INITRAMFS_NOT_USED		0
> +
>  #define NoMdDev (1<<23)
>  extern int find_free_devnum(int use_partitions);
>  
> diff --git a/util.c b/util.c
> index 0ea7e0d..7186f3f 100644
> --- a/util.c
> +++ b/util.c
> @@ -1768,3 +1768,25 @@ struct mdinfo *container_choose_spares(struct supertype *st,
>  	}
>  	return disks;
>  }
> +
> +int initramfs_is_in_use(void)
> +{
> +	int ret_val;
> +	struct stat sts;
> +
> +	/* /init
> +	 *	common script for many Linux distribution
> +	 * /sysboot
> +	 *	is specific for RH
> +	 * /mkinitrd.config
> +	 *	is specific for SLES
> +	 */
> +	if ((stat("/init", &sts) == -1) ||
> +	    ((stat("/sysroot", &sts) == -1) &&
> +	     (stat("/mkinitrd.config", &sts) == -1)))
> +		ret_val = INITRAMFS_NOT_USED;
> +	else
> +		ret_val = INITRAMFS_IS_IN_USE;
> +
> +	return ret_val;
> +}
>

Hi,
 I meant to look through this today but didn't manage to get to it.

However an easy comment is that this initramfs_is_in_use is not acceptable.
You are measuring symptoms and not the cause.

Unfortunately it isn't really easy to measure "Is someone likely to want to
replace the current root filesystem with a different one soon".

In the first instance I think this should be implemented as a command line
option:  --freeze-reshape maybe.  It is understood with --assemble to mean
that any reshape should not be continued.
It should be possible to get that into the mdadm flags during initrd but not
in the root filesystem.

The second patch looks like it should be combined with the first.

The rest seems OK at a first glance but I'll need to review it more carefully.

Thanks,
NeilBrown

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

RE: [PATCH 01/14] Stop array reshape when mounted initramfs is detected

[Kwolek, Adam]

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Monday, September 19, 2011 12:42 PM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> Subject: Re: [PATCH 01/14] Stop array reshape when mounted initramfs is
> detected
> 
> On Fri, 16 Sep 2011 13:53:54 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > During remounting file system from initramfs to real one, mdadm is not
> > able to continue reshape procedure due to lost filesystem context.
> > To avoid this mdadm detects mounted initramfs and performs critical
> > section restore only. Array is set for later reshape continuation.
> >
> > Reshape continuation will be possible to invoking manually, using
> > '--continue' option.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  Grow.c  |   25 +++++++++++++++++++++++--
> >  mdadm.h |    4 ++++
> >  util.c  |   22 ++++++++++++++++++++++
> >  3 files changed, 49 insertions(+), 2 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 17d14b6..420177c 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -635,10 +635,21 @@ static int subarray_set_num(char *container,
> struct mdinfo *sra, char *name, int
> >  	return rc;
> >  }
> >
> > +/* possible return values:
> > + * 0 : success
> > + * -1: error
> > + * INITRAMFS_IS_IN_USE: continue reshape later  */
> >  int start_reshape(struct mdinfo *sra, int already_running)  {
> >  	int err;
> > -	sysfs_set_num(sra, NULL, "suspend_lo", 0x7FFFFFFFFFFFFFFFULL);
> > +
> > +	/* do not block array as we not continue reshape this time
> > +	 */
> > +	if (initramfs_is_in_use() == INITRAMFS_NOT_USED)
> > +		sysfs_set_num(sra, NULL, "suspend_lo",
> 0x7FFFFFFFFFFFFFFFULL);
> > +	else
> > +		sysfs_set_num(sra, NULL, "suspend_lo", 0);
> >  	err = sysfs_set_num(sra, NULL, "suspend_hi", 0);
> >  	err = err ?: sysfs_set_num(sra, NULL, "suspend_lo", 0);
> >  	if (!already_running)
> > @@ -2190,6 +2201,14 @@ started:
> >  	}
> >  	if (restart)
> >  		sysfs_set_str(sra, NULL, "array_state", "active");
> > +	if (initramfs_is_in_use() == INITRAMFS_IS_IN_USE) {
> > +		free(fdlist);
> > +		free(offsets);
> > +		sysfs_free(sra);
> > +		fprintf(stderr, Name ": Reshape has to be continued "
> > +			"when root fileststem will be mounted\n");
> > +		return 1;
> > +	}
> >
> >  	/* Now we just need to kick off the reshape and watch, while
> >  	 * handling backups of the data...
> > @@ -2357,7 +2376,9 @@ int reshape_container(char *container, char
> *devname,
> >  		unfreeze(st);
> >  		return 1;
> >  	default: /* parent */
> > -		printf(Name ": multi-array reshape continues in
> background\n");
> > +		if (initramfs_is_in_use() == INITRAMFS_NOT_USED)
> > +			printf(Name ": multi-array reshape continues "
> > +			       "in background\n");
> >  		return 0;
> >  	case 0: /* child */
> >  		break;
> > diff --git a/mdadm.h b/mdadm.h
> > index d616966..9035e67 100644
> > --- a/mdadm.h
> > +++ b/mdadm.h
> > @@ -1161,6 +1161,10 @@ extern char *human_size(long long bytes);
> > extern char *human_size_brief(long long bytes);  extern void
> > print_r10_layout(int layout);
> >
> > +extern int initramfs_is_in_use(void);
> > +#define INITRAMFS_IS_IN_USE		1
> > +#define INITRAMFS_NOT_USED		0
> > +
> >  #define NoMdDev (1<<23)
> >  extern int find_free_devnum(int use_partitions);
> >
> > diff --git a/util.c b/util.c
> > index 0ea7e0d..7186f3f 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -1768,3 +1768,25 @@ struct mdinfo *container_choose_spares(struct
> supertype *st,
> >  	}
> >  	return disks;
> >  }
> > +
> > +int initramfs_is_in_use(void)
> > +{
> > +	int ret_val;
> > +	struct stat sts;
> > +
> > +	/* /init
> > +	 *	common script for many Linux distribution
> > +	 * /sysboot
> > +	 *	is specific for RH
> > +	 * /mkinitrd.config
> > +	 *	is specific for SLES
> > +	 */
> > +	if ((stat("/init", &sts) == -1) ||
> > +	    ((stat("/sysroot", &sts) == -1) &&
> > +	     (stat("/mkinitrd.config", &sts) == -1)))
> > +		ret_val = INITRAMFS_NOT_USED;
> > +	else
> > +		ret_val = INITRAMFS_IS_IN_USE;
> > +
> > +	return ret_val;
> > +}
> >
> 
> Hi,
>  I meant to look through this today but didn't manage to get to it.
> 
> However an easy comment is that this initramfs_is_in_use is not
> acceptable.
> You are measuring symptoms and not the cause.
> 
> Unfortunately it isn't really easy to measure "Is someone likely to want
> to replace the current root filesystem with a different one soon".
> 
> In the first instance I think this should be implemented as a command
> line
> option:  --freeze-reshape maybe.  It is understood with --assemble to
> mean that any reshape should not be continued.
> It should be possible to get that into the mdadm flags during initrd but
> not in the root filesystem.
> 
> The second patch looks like it should be combined with the first.
> 
> The rest seems OK at a first glance but I'll need to review it more
> carefully.
> 
> Thanks,
> NeilBrown

Thank you for the first part of the review.

I was worry about file system detection I've made, and you confirms my doubts.
I like your idea about another option for reshape freeze at the early boot stage.
I'll include it in second approach.

I'm waiting for rest of your review.
When I'll get it, I'll prepare updated fix.

BR
Adam

[PATCH 02/14] Stop container reshape while using initramfs

[Adam Kwolek]

When first array in container is set for later reshape mdadm
has exit for remaining container operations. Do not wait for
current reshape finalization as it will be performed manually later.

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

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

diff --git a/Grow.c b/Grow.c
index 420177c..cec32d7 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2435,6 +2435,12 @@ int reshape_container(char *container, char *devname,
 				   content, force, NULL,
 				   backup_file, quiet, 1, restart);
 		close(fd);
+
+		if (initramfs_is_in_use() == INITRAMFS_IS_IN_USE) {
+			sysfs_free(cc);
+			exit(0);
+		}
+
 		restart = 0;
 		if (rv)
 			break;

[PATCH 03/14] FIX: Do not unblock array accidentally

[Adam Kwolek]

When sysfs_set_array() function is called, it tests if array
can be configured using sysfs. Setting metadata_version entry
can accidentally unblock mdmon when array is under reshape.
To avoid this, blocking character '-' is checked and if is is set,
it is used for array test.

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

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

diff --git a/sysfs.c b/sysfs.c
index 2146264..903202d 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -541,7 +541,17 @@ int sysfs_set_array(struct mdinfo *info, int vers)
 	ver[0] = 0;
 	if (info->array.major_version == -1 &&
 	    info->array.minor_version == -2) {
+		char buf[1024];
 		strcat(strcpy(ver, "external:"), info->text_version);
+		if (sysfs_get_str(info, NULL, "metadata_version",
+						  buf, 1024) > 0) {
+			/* Testing string should not affect
+			 * monitor blocking functionality
+			 * Set blocking character if present in sysfs already
+			 */
+			if (buf[9] == '-')
+				ver[9] = '-';
+		}
 
 		if ((vers % 100) < 2 ||
 		    sysfs_set_str(info, NULL, "metadata_version",

Re: [PATCH 03/14] FIX: Do not unblock array accidentally

[NeilBrown]

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

On Fri, 16 Sep 2011 13:54:09 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When sysfs_set_array() function is called, it tests if array
> can be configured using sysfs. Setting metadata_version entry
> can accidentally unblock mdmon when array is under reshape.
> To avoid this, blocking character '-' is checked and if is is set,
> it is used for array test.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  sysfs.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/sysfs.c b/sysfs.c
> index 2146264..903202d 100644
> --- a/sysfs.c
> +++ b/sysfs.c
> @@ -541,7 +541,17 @@ int sysfs_set_array(struct mdinfo *info, int vers)
>  	ver[0] = 0;
>  	if (info->array.major_version == -1 &&
>  	    info->array.minor_version == -2) {
> +		char buf[1024];
>  		strcat(strcpy(ver, "external:"), info->text_version);
> +		if (sysfs_get_str(info, NULL, "metadata_version",
> +						  buf, 1024) > 0) {
> +			/* Testing string should not affect
> +			 * monitor blocking functionality
> +			 * Set blocking character if present in sysfs already
> +			 */
> +			if (buf[9] == '-')
> +				ver[9] = '-';
> +		}
>  
>  		if ((vers % 100) < 2 ||
>  		    sysfs_set_str(info, NULL, "metadata_version",
> 
> --
> 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

I have applied this patch, though I rewrote the comment a bit.

thanks,
NeilBrown


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

[PATCH 04/14] Add continue option to grow command

[Adam Kwolek]

To allow for reshape continuation, 'continue' option is added
to grow command. It is not possible to check, if any existing mdadm
instance is going to take ownership on blocked md array or it is
orphaned/interrupted reshape.
To resolve this problem grow command with continue option,
is allowed when current mdadm instance is only one in the system.

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

 ReadMe.c |    1 +
 mdadm.c  |   17 ++++++++++++++++-
 mdadm.h  |    2 ++
 util.c   |   37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 1 deletions(-)

diff --git a/ReadMe.c b/ReadMe.c
index b658841..f0dc0d9 100644
--- a/ReadMe.c
+++ b/ReadMe.c
@@ -190,6 +190,7 @@ struct option long_options[] = {
     {"backup-file", 1,0, BackupFile},
     {"invalid-backup",0,0,InvalidBackup},
     {"array-size", 1, 0, 'Z'},
+    {"continue", 0, 0, Continue},
 
     /* For Incremental */
     {"rebuild-map", 0, 0, RebuildMapOpt},
diff --git a/mdadm.c b/mdadm.c
index 4b817ab..f30534a 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -74,6 +74,7 @@ int main(int argc, char *argv[])
 	int export = 0;
 	int assume_clean = 0;
 	char *symlinks = NULL;
+	int grow_continue = 0;
 	/* autof indicates whether and how to create device node.
 	 * bottom 3 bits are style.  Rest (when shifted) are number of parts
 	 * 0  - unset
@@ -988,7 +989,21 @@ int main(int argc, char *argv[])
 			}
 			backup_file = optarg;
 			continue;
-
+		case O(GROW, Continue):
+			/* Continuer broken grow
+			 * To be sure that continuation is allowed check
+			 * if there is only one (current) mdadm instance
+			 */
+			grow_continue = count_mdadms();
+			fprintf(stderr, Name ": %i mdadm instance(s) found\n",
+				grow_continue);
+			if (grow_continue != 1) {
+				fprintf(stderr, Name ": Grow continuation "
+					"requires single mdadm instance "
+					"running.\n");
+				exit(2);
+			}
+			continue;
 		case O(ASSEMBLE, InvalidBackup):
 			/* Acknowledge that the backupfile is invalid, but ask
 			 * to continue anyway
diff --git a/mdadm.h b/mdadm.h
index 9035e67..7e3a618 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -313,6 +313,7 @@ enum special_options {
 	RebuildMapOpt,
 	InvalidBackup,
 	UdevRules,
+	Continue,
 };
 
 /* structures read from config file */
@@ -1162,6 +1163,7 @@ extern char *human_size_brief(long long bytes);
 extern void print_r10_layout(int layout);
 
 extern int initramfs_is_in_use(void);
+extern int count_mdadms(void);
 #define INITRAMFS_IS_IN_USE		1
 #define INITRAMFS_NOT_USED		0
 
diff --git a/util.c b/util.c
index 7186f3f..5077c2f 100644
--- a/util.c
+++ b/util.c
@@ -1790,3 +1790,40 @@ int initramfs_is_in_use(void)
 
 	return ret_val;
 }
+
+int count_mdadms(void)
+{
+	int ret_val = 0;
+	DIR *dirp;
+	struct dirent *d_entry;
+	int fd;
+
+	dirp = opendir("/proc");
+	if (!dirp)
+		return -1;
+
+	while ((d_entry = readdir(dirp)) != NULL) {
+		char p[PATH_MAX];
+
+		if (!strcmp(d_entry->d_name, ".") ||
+			!strcmp(d_entry->d_name, "..") ||
+			!strcmp(d_entry->d_name, "self"))
+			continue;
+		errno = 0;
+		fd = (int)strtol(d_entry->d_name, NULL, 10);
+		if (errno)
+			continue;
+		sprintf(p, "/proc/%s/cmdline", d_entry->d_name);
+		fd = open(p, O_RDONLY);
+		if (fd < 0)
+			continue;
+		if (read(fd, p, PATH_MAX) > 0) {
+			if (strcmp(p, "mdadm") == 0)
+				ret_val++;
+		}
+		close(fd);
+	}
+	closedir(dirp);
+
+	return ret_val;
+}

Re: [PATCH 04/14] Add continue option to grow command

[NeilBrown]

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

On Fri, 16 Sep 2011 13:54:17 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> To allow for reshape continuation, 'continue' option is added
> to grow command. It is not possible to check, if any existing mdadm
> instance is going to take ownership on blocked md array or it is
> orphaned/interrupted reshape.
> To resolve this problem grow command with continue option,
> is allowed when current mdadm instance is only one in the system.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>

Checking how many instances of mdadm are running is definitely wrong.
What are you trying to guard against here?

If we do need locking to prevent two processes trying to manage an array at
the same time, I would suggest creating a 'pid' file
- /var/run/mdadm-reshape-mdXXX.pid
or something like that.

NeilBrown


> ---
> 
>  ReadMe.c |    1 +
>  mdadm.c  |   17 ++++++++++++++++-
>  mdadm.h  |    2 ++
>  util.c   |   37 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 56 insertions(+), 1 deletions(-)
> 
> diff --git a/ReadMe.c b/ReadMe.c
> index b658841..f0dc0d9 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -190,6 +190,7 @@ struct option long_options[] = {
>      {"backup-file", 1,0, BackupFile},
>      {"invalid-backup",0,0,InvalidBackup},
>      {"array-size", 1, 0, 'Z'},
> +    {"continue", 0, 0, Continue},
>  
>      /* For Incremental */
>      {"rebuild-map", 0, 0, RebuildMapOpt},
> diff --git a/mdadm.c b/mdadm.c
> index 4b817ab..f30534a 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -74,6 +74,7 @@ int main(int argc, char *argv[])
>  	int export = 0;
>  	int assume_clean = 0;
>  	char *symlinks = NULL;
> +	int grow_continue = 0;
>  	/* autof indicates whether and how to create device node.
>  	 * bottom 3 bits are style.  Rest (when shifted) are number of parts
>  	 * 0  - unset
> @@ -988,7 +989,21 @@ int main(int argc, char *argv[])
>  			}
>  			backup_file = optarg;
>  			continue;
> -
> +		case O(GROW, Continue):
> +			/* Continuer broken grow
> +			 * To be sure that continuation is allowed check
> +			 * if there is only one (current) mdadm instance
> +			 */
> +			grow_continue = count_mdadms();
> +			fprintf(stderr, Name ": %i mdadm instance(s) found\n",
> +				grow_continue);
> +			if (grow_continue != 1) {
> +				fprintf(stderr, Name ": Grow continuation "
> +					"requires single mdadm instance "
> +					"running.\n");
> +				exit(2);
> +			}
> +			continue;
>  		case O(ASSEMBLE, InvalidBackup):
>  			/* Acknowledge that the backupfile is invalid, but ask
>  			 * to continue anyway
> diff --git a/mdadm.h b/mdadm.h
> index 9035e67..7e3a618 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -313,6 +313,7 @@ enum special_options {
>  	RebuildMapOpt,
>  	InvalidBackup,
>  	UdevRules,
> +	Continue,
>  };
>  
>  /* structures read from config file */
> @@ -1162,6 +1163,7 @@ extern char *human_size_brief(long long bytes);
>  extern void print_r10_layout(int layout);
>  
>  extern int initramfs_is_in_use(void);
> +extern int count_mdadms(void);
>  #define INITRAMFS_IS_IN_USE		1
>  #define INITRAMFS_NOT_USED		0
>  
> diff --git a/util.c b/util.c
> index 7186f3f..5077c2f 100644
> --- a/util.c
> +++ b/util.c
> @@ -1790,3 +1790,40 @@ int initramfs_is_in_use(void)
>  
>  	return ret_val;
>  }
> +
> +int count_mdadms(void)
> +{
> +	int ret_val = 0;
> +	DIR *dirp;
> +	struct dirent *d_entry;
> +	int fd;
> +
> +	dirp = opendir("/proc");
> +	if (!dirp)
> +		return -1;
> +
> +	while ((d_entry = readdir(dirp)) != NULL) {
> +		char p[PATH_MAX];
> +
> +		if (!strcmp(d_entry->d_name, ".") ||
> +			!strcmp(d_entry->d_name, "..") ||
> +			!strcmp(d_entry->d_name, "self"))
> +			continue;
> +		errno = 0;
> +		fd = (int)strtol(d_entry->d_name, NULL, 10);
> +		if (errno)
> +			continue;
> +		sprintf(p, "/proc/%s/cmdline", d_entry->d_name);
> +		fd = open(p, O_RDONLY);
> +		if (fd < 0)
> +			continue;
> +		if (read(fd, p, PATH_MAX) > 0) {
> +			if (strcmp(p, "mdadm") == 0)
> +				ret_val++;
> +		}
> +		close(fd);
> +	}
> +	closedir(dirp);
> +
> +	return ret_val;
> +}
> 
> --
> 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


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

RE: [PATCH 04/14] Add continue option to grow command

[Kwolek, Adam]

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, September 21, 2011 4:22 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> Subject: Re: [PATCH 04/14] Add continue option to grow command
> 
> On Fri, 16 Sep 2011 13:54:17 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > To allow for reshape continuation, 'continue' option is added to grow
> > command. It is not possible to check, if any existing mdadm instance
> > is going to take ownership on blocked md array or it is
> > orphaned/interrupted reshape.
> > To resolve this problem grow command with continue option, is allowed
> > when current mdadm instance is only one in the system.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> 
> Checking how many instances of mdadm are running is definitely wrong.
> What are you trying to guard against here?


Double reshape '--continue' command execution.


> 
> If we do need locking to prevent two processes trying to manage an array at
> the same time, I would suggest creating a 'pid' file
> - /var/run/mdadm-reshape-mdXXX.pid
> or something like that.


OK, I'll think about this.

BR
Adam

> 
> NeilBrown
> 
> 
> > ---
> >
> >  ReadMe.c |    1 +
> >  mdadm.c  |   17 ++++++++++++++++-
> >  mdadm.h  |    2 ++
> >  util.c   |   37 +++++++++++++++++++++++++++++++++++++
> >  4 files changed, 56 insertions(+), 1 deletions(-)
> >
> > diff --git a/ReadMe.c b/ReadMe.c
> > index b658841..f0dc0d9 100644
> > --- a/ReadMe.c
> > +++ b/ReadMe.c
> > @@ -190,6 +190,7 @@ struct option long_options[] = {
> >      {"backup-file", 1,0, BackupFile},
> >      {"invalid-backup",0,0,InvalidBackup},
> >      {"array-size", 1, 0, 'Z'},
> > +    {"continue", 0, 0, Continue},
> >
> >      /* For Incremental */
> >      {"rebuild-map", 0, 0, RebuildMapOpt}, diff --git a/mdadm.c
> > b/mdadm.c index 4b817ab..f30534a 100644
> > --- a/mdadm.c
> > +++ b/mdadm.c
> > @@ -74,6 +74,7 @@ int main(int argc, char *argv[])
> >  	int export = 0;
> >  	int assume_clean = 0;
> >  	char *symlinks = NULL;
> > +	int grow_continue = 0;
> >  	/* autof indicates whether and how to create device node.
> >  	 * bottom 3 bits are style.  Rest (when shifted) are number of parts
> >  	 * 0  - unset
> > @@ -988,7 +989,21 @@ int main(int argc, char *argv[])
> >  			}
> >  			backup_file = optarg;
> >  			continue;
> > -
> > +		case O(GROW, Continue):
> > +			/* Continuer broken grow
> > +			 * To be sure that continuation is allowed check
> > +			 * if there is only one (current) mdadm instance
> > +			 */
> > +			grow_continue = count_mdadms();
> > +			fprintf(stderr, Name ": %i mdadm instance(s)
> found\n",
> > +				grow_continue);
> > +			if (grow_continue != 1) {
> > +				fprintf(stderr, Name ": Grow continuation "
> > +					"requires single mdadm instance "
> > +					"running.\n");
> > +				exit(2);
> > +			}
> > +			continue;
> >  		case O(ASSEMBLE, InvalidBackup):
> >  			/* Acknowledge that the backupfile is invalid, but ask
> >  			 * to continue anyway
> > diff --git a/mdadm.h b/mdadm.h
> > index 9035e67..7e3a618 100644
> > --- a/mdadm.h
> > +++ b/mdadm.h
> > @@ -313,6 +313,7 @@ enum special_options {
> >  	RebuildMapOpt,
> >  	InvalidBackup,
> >  	UdevRules,
> > +	Continue,
> >  };
> >
> >  /* structures read from config file */ @@ -1162,6 +1163,7 @@ extern
> > char *human_size_brief(long long bytes);  extern void
> > print_r10_layout(int layout);
> >
> >  extern int initramfs_is_in_use(void);
> > +extern int count_mdadms(void);
> >  #define INITRAMFS_IS_IN_USE		1
> >  #define INITRAMFS_NOT_USED		0
> >
> > diff --git a/util.c b/util.c
> > index 7186f3f..5077c2f 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -1790,3 +1790,40 @@ int initramfs_is_in_use(void)
> >
> >  	return ret_val;
> >  }
> > +
> > +int count_mdadms(void)
> > +{
> > +	int ret_val = 0;
> > +	DIR *dirp;
> > +	struct dirent *d_entry;
> > +	int fd;
> > +
> > +	dirp = opendir("/proc");
> > +	if (!dirp)
> > +		return -1;
> > +
> > +	while ((d_entry = readdir(dirp)) != NULL) {
> > +		char p[PATH_MAX];
> > +
> > +		if (!strcmp(d_entry->d_name, ".") ||
> > +			!strcmp(d_entry->d_name, "..") ||
> > +			!strcmp(d_entry->d_name, "self"))
> > +			continue;
> > +		errno = 0;
> > +		fd = (int)strtol(d_entry->d_name, NULL, 10);
> > +		if (errno)
> > +			continue;
> > +		sprintf(p, "/proc/%s/cmdline", d_entry->d_name);
> > +		fd = open(p, O_RDONLY);
> > +		if (fd < 0)
> > +			continue;
> > +		if (read(fd, p, PATH_MAX) > 0) {
> > +			if (strcmp(p, "mdadm") == 0)
> > +				ret_val++;
> > +		}
> > +		close(fd);
> > +	}
> > +	closedir(dirp);
> > +
> > +	return ret_val;
> > +}
> >
> > --
> > 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

[PATCH 05/14] Add Grow_continue_command

[Adam Kwolek]

Add function that will be executed in grow-continue case
for external metadata. It checks if any reshape is in progress.
If it finds reshape in progress, Grow_continue() function
is called as during boot time for reshape continuation.

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

 Grow.c  |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mdadm.c |    7 +++-
 mdadm.h |    2 +
 3 files changed, 121 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index cec32d7..2b2cb5a 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3578,6 +3578,119 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist, int cnt
 	return 1;
 }
 
+int Grow_continue_command(char *devname, int fd,
+			  char *backup_file, int verbose)
+{
+	int ret_val = 0;
+	struct supertype *st = NULL;
+	struct mdinfo *content = NULL;
+	char *subarray = NULL;
+	struct mdinfo *cc = NULL;
+	struct mdstat_ent *mdstat = NULL;
+	char container[20];
+	int cfd = -1;
+	int fd2 = -1;
+
+	dprintf("Grow continue from command line called for %s\n",
+		devname);
+
+	st = super_by_fd(fd, &subarray);
+	if (!st) {
+		fprintf(stderr,
+			Name ": Unable to determine metadata format for %s\n",
+			devname);
+		return 1;
+	}
+	dprintf("Grow continue is run for ");
+	if (st->ss->external == 0) {
+		dprintf("native array (%s)\n", devname);
+		/* FIXME : temporary just exit
+		 * for native metadata case
+		 */
+		st->ss->free_super(st);
+		free(subarray);
+		exit(0);
+	} else {
+		int container_dev;
+
+		if (subarray) {
+			dprintf("subarray (%s)\n", subarray);
+			container_dev = st->container_dev;
+			cfd = open_dev_excl(st->container_dev);
+		} else {
+			container_dev = st->devnum;
+			close(fd);
+			cfd = open_dev_excl(st->devnum);
+			dprintf("container (%i)\n", container_dev);
+			fd = cfd;
+		}
+		if (cfd < 0) {
+			fprintf(stderr, Name ": Unable to open container "
+				"for %s\n", devname);
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
+		fmt_devname(container, container_dev);
+
+		/* find in container array under reshape
+		 */
+		ret_val = st->ss->load_container(st, cfd, NULL);
+		if (ret_val) {
+			fprintf(stderr,
+				Name ": Cannot read superblock for %s\n",
+				devname);
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
+
+		cc = st->ss->container_content(st, NULL);
+		for (content = cc; content ; content = content->next) {
+			char *array;
+
+			if (content->reshape_active == 0)
+				continue;
+
+			array = strchr(content->text_version+1, '/')+1;
+			mdstat = mdstat_by_subdev(array, container_dev);
+			if (!mdstat)
+				continue;
+			break;
+		}
+		if (!content) {
+			fprintf(stderr,
+				Name ": Unable to determine reshaped "
+				"array for %s\n", devname);
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
+		fd2 = open_dev(mdstat->devnum);
+		if (fd2 < 0) {
+			fprintf(stderr, Name ": cannot open (md%i)\n",
+				mdstat->devnum);
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
+
+		sysfs_init(content, fd2, mdstat->devnum);
+	}
+
+	/* continue reshape
+	 */
+	ret_val = Grow_continue(fd, st, content, backup_file);
+
+Grow_continue_command_exit:
+	if (fd2 > -1)
+		close(fd2);
+	if (cfd > -1)
+		close(cfd);
+	st->ss->free_super(st);
+	free_mdstat(mdstat);
+	sysfs_free(cc);
+	free(subarray);
+
+	return ret_val;
+}
+
 int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
 		  char *backup_file)
 {
diff --git a/mdadm.c b/mdadm.c
index f30534a..3bd4ebe 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1649,7 +1649,12 @@ int main(int argc, char *argv[])
 				delay = DEFAULT_BITMAP_DELAY;
 			rv = Grow_addbitmap(devlist->devname, mdfd, bitmap_file,
 					    bitmap_chunk, delay, write_behind, force);
-		} else if (size >= 0 || raiddisks != 0 || layout_str != NULL
+		} else if (grow_continue) {
+			rv = Grow_continue_command(devlist->devname,
+						   mdfd, backup_file,
+						   verbose);
+			break;
+		} else	if (size >= 0 || raiddisks != 0 || layout_str != NULL
 			   || chunk != 0 || level != UnSet) {
 			rv = Grow_reshape(devlist->devname, mdfd, quiet, backup_file,
 					  size, level, layout_str, chunk, raiddisks,
diff --git a/mdadm.h b/mdadm.h
index 7e3a618..7761db6 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1024,6 +1024,8 @@ extern int Grow_restart(struct supertype *st, struct mdinfo *info,
 			int *fdlist, int cnt, char *backup_file, int verbose);
 extern int Grow_continue(int mdfd, struct supertype *st,
 			 struct mdinfo *info, char *backup_file);
+extern int Grow_continue_command(char *devname, int fd,
+				 char *backup_file, int verbose);
 
 extern int Assemble(struct supertype *st, char *mddev,
 		    struct mddev_ident *ident,

[PATCH 06/14] Check and run mdmon

[Adam Kwolek]

Reshape can be run for external metadata for monitored array/container
only. In case when array/container is not monitored run mdmon for it.

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

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

diff --git a/Grow.c b/Grow.c
index 2b2cb5a..4d2c361 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3672,6 +3672,21 @@ int Grow_continue_command(char *devname, int fd,
 		}
 
 		sysfs_init(content, fd2, mdstat->devnum);
+
+		/* start mdmon in case it is not run
+		 */
+		if (!mdmon_running(container_dev))
+			start_mdmon(container_dev);
+		ping_monitor(container);
+
+		if (mdmon_running(container_dev))
+			st->update_tail = &st->updates;
+		else {
+			fprintf(stderr, Name ":  No mdmon found. "
+				"Grow cannot continue.\n");
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
 	}
 
 	/* continue reshape

[PATCH 07/14] FIX: Memory leak during Assembly

[Adam Kwolek]

For fdlist pointer allocated in assemble_container_content() function,
free() is never called. This patch fixes this memory leak.

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

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

diff --git a/Assemble.c b/Assemble.c
index 25cfec1..66d2ee4 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1584,6 +1584,7 @@ int assemble_container_content(struct supertype *st, int mdfd,
 				if (fdlist[spare] >= 0)
 					close(fdlist[spare]);
 			}
+			free(fdlist);
 			if (err) {
 				fprintf(stderr, Name ": Failed to restore critical"
 					" section for reshape - sorry.\n");

Re: [PATCH 07/14] FIX: Memory leak during Assembly

[NeilBrown]

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

On Fri, 16 Sep 2011 13:54:40 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> For fdlist pointer allocated in assemble_container_content() function,
> free() is never called. This patch fixes this memory leak.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Assemble.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 25cfec1..66d2ee4 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1584,6 +1584,7 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  				if (fdlist[spare] >= 0)
>  					close(fdlist[spare]);
>  			}
> +			free(fdlist);
>  			if (err) {
>  				fprintf(stderr, Name ": Failed to restore critical"
>  					" section for reshape - sorry.\n");
> 
> --
> 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


I've applied this fix, thanks.

NeilBrown


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

[PATCH 08/14] Check if reshape can be restarted

[Adam Kwolek]

When reshape was invoked during initramfs start-up stage
array is pushed in to reshape state already, so read only
state cannot be set again. To verify if reshape continuation should
be allowed sync_action sysfs key is tested and number of existing mdadm
instances is checked again.

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

 Grow.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Grow.c b/Grow.c
index 4d2c361..b331287 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3714,8 +3714,23 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
 	int err;
 
 	err = sysfs_set_str(info, NULL, "array_state", "readonly");
-	if (err)
-		return err;
+	if (err) {
+		/* check if array is set in to reshape stare,
+		 * and current mdadm instance is the only one
+		 */
+		if ((sysfs_get_str(info, NULL, "sync_action",
+				   buf, 40) == 8) &&
+		    (count_mdadms() == 1)) {
+			dprintf("Warning: Grow_continue() cannot set array in"
+				"to read only state [sync_action is %s].",
+				buf);
+			if (strncmp(buf, "reshape", 7))
+				return err;
+			dprintf("Allow to continue.\n");
+		} else
+			return err;
+	}
+
 	if (st->ss->external) {
 		fmt_devname(buf, st->container_dev);
 		container = buf;

Re: [PATCH 08/14] Check if reshape can be restarted

[NeilBrown]

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

On Fri, 16 Sep 2011 13:54:48 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When reshape was invoked during initramfs start-up stage
> array is pushed in to reshape state already, so read only
> state cannot be set again. To verify if reshape continuation should
> be allowed sync_action sysfs key is tested and number of existing mdadm
> instances is checked again.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>

Again, it is not at all clear to me what you are trying to protect against
here...

The "set_str ... readonly" is going to need to be changed.

In the normal case, this is where the array is activated, but it is activated
read-only to ensure no reshape automatically starts.

In the --continue case it is already running and reshape is suspended, so
there is no need to set it to readonly when Grow_continue is called from
Grow_continue_command.

NeilBrown


> ---
> 
>  Grow.c |   19 +++++++++++++++++--
>  1 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 4d2c361..b331287 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3714,8 +3714,23 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
>  	int err;
>  
>  	err = sysfs_set_str(info, NULL, "array_state", "readonly");
> -	if (err)
> -		return err;
> +	if (err) {
> +		/* check if array is set in to reshape stare,
> +		 * and current mdadm instance is the only one
> +		 */
> +		if ((sysfs_get_str(info, NULL, "sync_action",
> +				   buf, 40) == 8) &&
> +		    (count_mdadms() == 1)) {
> +			dprintf("Warning: Grow_continue() cannot set array in"
> +				"to read only state [sync_action is %s].",
> +				buf);
> +			if (strncmp(buf, "reshape", 7))
> +				return err;
> +			dprintf("Allow to continue.\n");
> +		} else
> +			return err;
> +	}
> +
>  	if (st->ss->external) {
>  		fmt_devname(buf, st->container_dev);
>  		container = buf;
> 
> --
> 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


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

RE: [PATCH 08/14] Check if reshape can be restarted

[Kwolek, Adam]

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Wednesday, September 21, 2011 4:28 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> Subject: Re: [PATCH 08/14] Check if reshape can be restarted
> 
> On Fri, 16 Sep 2011 13:54:48 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > When reshape was invoked during initramfs start-up stage array is
> > pushed in to reshape state already, so read only state cannot be set
> > again. To verify if reshape continuation should be allowed sync_action
> > sysfs key is tested and number of existing mdadm instances is checked
> > again.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> 
> Again, it is not at all clear to me what you are trying to protect against here...
> 
> The "set_str ... readonly" is going to need to be changed.
> 
> In the normal case, this is where the array is activated, but it is activated
> read-only to ensure no reshape automatically starts.
> 
> In the --continue case it is already running and reshape is suspended, so
> there is no need to set it to readonly when Grow_continue is called from
> Grow_continue_command.


This is exactly the case.

We have 2 continuation possibilities,  when Grow_continue() is called: 

1. Assembly when  array is set to readonly state and this activates it.

2. grow --continue. Reshape is continued and array is already in reshape state,
     It is not possible to set readonly state again, this is why sync_action is checked
     For "reshape" to allow continuation for run.
    Conjunction with count_mdadms() /the single mdadm instance/ guards any potential race conditions.
       
BR
Adam

> 
> NeilBrown
> 
> 
> > ---
> >
> >  Grow.c |   19 +++++++++++++++++--
> >  1 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 4d2c361..b331287 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -3714,8 +3714,23 @@ int Grow_continue(int mdfd, struct supertype
> *st, struct mdinfo *info,
> >  	int err;
> >
> >  	err = sysfs_set_str(info, NULL, "array_state", "readonly");
> > -	if (err)
> > -		return err;
> > +	if (err) {
> > +		/* check if array is set in to reshape stare,
> > +		 * and current mdadm instance is the only one
> > +		 */
> > +		if ((sysfs_get_str(info, NULL, "sync_action",
> > +				   buf, 40) == 8) &&
> > +		    (count_mdadms() == 1)) {
> > +			dprintf("Warning: Grow_continue() cannot set array
> in"
> > +				"to read only state [sync_action is %s].",
> > +				buf);
> > +			if (strncmp(buf, "reshape", 7))
> > +				return err;
> > +			dprintf("Allow to continue.\n");
> > +		} else
> > +			return err;
> > +	}
> > +
> >  	if (st->ss->external) {
> >  		fmt_devname(buf, st->container_dev);
> >  		container = buf;
> >
> > --
> > 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

[PATCH 09/14] Move restore backup code to function

[Adam Kwolek]

Reshape backup should be able to be restored during reshape continuation
also. To reuse already existing code it is moved to function.

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

 Assemble.c |   40 +++------------------------------------
 Grow.c     |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mdadm.h    |    6 ++++++
 3 files changed, 71 insertions(+), 37 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index 66d2ee4..6188fb5 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1555,44 +1555,10 @@ int assemble_container_content(struct supertype *st, int mdfd,
 
 		if (content->reshape_active) {
 			int spare = content->array.raid_disks + expansion;
-			int i;
-			int *fdlist = malloc(sizeof(int) *
-					     (working + expansion
-					      + content->array.raid_disks));
-			for (i=0; i<spare; i++)
-				fdlist[i] = -1;
-			for (dev = content->devs; dev; dev = dev->next) {
-				char buf[20];
-				int fd;
-				sprintf(buf, "%d:%d",
-					dev->disk.major,
-					dev->disk.minor);
-				fd = dev_open(buf, O_RDWR);
-
-				if (dev->disk.raid_disk >= 0)
-					fdlist[dev->disk.raid_disk] = fd;
-				else
-					fdlist[spare++] = fd;
-			}
-			if (st->ss->external && st->ss->recover_backup)
-				err = st->ss->recover_backup(st, content);
-			else
-				err = Grow_restart(st, content, fdlist, spare,
-						   backup_file, verbose > 0);
-			while (spare > 0) {
-				spare--;
-				if (fdlist[spare] >= 0)
-					close(fdlist[spare]);
-			}
-			free(fdlist);
-			if (err) {
-				fprintf(stderr, Name ": Failed to restore critical"
-					" section for reshape - sorry.\n");
-				if (!backup_file)
-					fprintf(stderr, Name ":  Possibly you need"
-						" to specify a --backup-file\n");
+			if (restore_backup(st, content,
+					   working,
+					   spare, backup_file, verbose))
 				return 1;
-			}
 
 			err = Grow_continue(mdfd, st, content, backup_file);
 		} else switch(content->array.level) {
diff --git a/Grow.c b/Grow.c
index b331287..0b96f7a 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3578,6 +3578,68 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist, int cnt
 	return 1;
 }
 
+int restore_backup(struct supertype *st,
+		   struct mdinfo *content,
+		   int working_disks,
+		   int spares,
+		   char *backup_file,
+		   int verbose)
+{
+	int i;
+	int *fdlist;
+	struct mdinfo *dev;
+	int err;
+	int disk_count = working_disks + spares;
+	int next_spare = working_disks;
+
+	dprintf("Called restore_backup()\n");
+	fdlist = malloc(sizeof(int) * disk_count);
+	if (fdlist == NULL) {
+		fprintf(stderr,
+			Name ": cannot allocate memory for disk list\n");
+		return 1;
+	}
+	for (i = 0; i < disk_count; i++)
+		fdlist[i] = -1;
+	for (dev = content->devs; dev; dev = dev->next) {
+		char buf[22];
+		int fd;
+		sprintf(buf, "%d:%d",
+			dev->disk.major,
+			dev->disk.minor);
+		fd = dev_open(buf, O_RDWR);
+
+		if (dev->disk.raid_disk >= 0)
+			fdlist[dev->disk.raid_disk] = fd;
+		else
+			fdlist[next_spare++] = fd;
+	}
+
+	if (st->ss->external && st->ss->recover_backup)
+		err = st->ss->recover_backup(st, content);
+	else
+		err = Grow_restart(st, content, fdlist, spares,
+				   backup_file, verbose > 0);
+
+	while (disk_count > 0) {
+		disk_count--;
+		if (fdlist[disk_count] >= 0)
+			close(fdlist[disk_count]);
+	}
+	free(fdlist);
+	if (err) {
+		fprintf(stderr, Name ": Failed to restore critical"
+			" section for reshape - sorry.\n");
+		if (!backup_file)
+			fprintf(stderr, Name ":  Possibly you need"
+				" to specify a --backup-file\n");
+		return 1;
+	}
+
+	dprintf("restore_backup() returns status OK.\n");
+	return 0;
+}
+
 int Grow_continue_command(char *devname, int fd,
 			  char *backup_file, int verbose)
 {
diff --git a/mdadm.h b/mdadm.h
index 7761db6..6165979 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1026,6 +1026,12 @@ extern int Grow_continue(int mdfd, struct supertype *st,
 			 struct mdinfo *info, char *backup_file);
 extern int Grow_continue_command(char *devname, int fd,
 				 char *backup_file, int verbose);
+extern int restore_backup(struct supertype *st,
+			  struct mdinfo *content,
+			  int working_disks,
+			  int spares,
+			  char *backup_file,
+			  int verbose);
 
 extern int Assemble(struct supertype *st, char *mddev,
 		    struct mddev_ident *ident,

Re: [PATCH 09/14] Move restore backup code to function

[NeilBrown]

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

On Fri, 16 Sep 2011 13:54:55 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Reshape backup should be able to be restored during reshape continuation
> also. To reuse already existing code it is moved to function.

I have applied this, but I had to fix it up a bit first.
'spare' is not the same as 'spares' - it is closer to 'next_spare'.

Thanks,
NeilBrown

> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Assemble.c |   40 +++------------------------------------
>  Grow.c     |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  mdadm.h    |    6 ++++++
>  3 files changed, 71 insertions(+), 37 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 66d2ee4..6188fb5 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1555,44 +1555,10 @@ int assemble_container_content(struct supertype *st, int mdfd,
>  
>  		if (content->reshape_active) {
>  			int spare = content->array.raid_disks + expansion;
> -			int i;
> -			int *fdlist = malloc(sizeof(int) *
> -					     (working + expansion
> -					      + content->array.raid_disks));
> -			for (i=0; i<spare; i++)
> -				fdlist[i] = -1;
> -			for (dev = content->devs; dev; dev = dev->next) {
> -				char buf[20];
> -				int fd;
> -				sprintf(buf, "%d:%d",
> -					dev->disk.major,
> -					dev->disk.minor);
> -				fd = dev_open(buf, O_RDWR);
> -
> -				if (dev->disk.raid_disk >= 0)
> -					fdlist[dev->disk.raid_disk] = fd;
> -				else
> -					fdlist[spare++] = fd;
> -			}
> -			if (st->ss->external && st->ss->recover_backup)
> -				err = st->ss->recover_backup(st, content);
> -			else
> -				err = Grow_restart(st, content, fdlist, spare,
> -						   backup_file, verbose > 0);
> -			while (spare > 0) {
> -				spare--;
> -				if (fdlist[spare] >= 0)
> -					close(fdlist[spare]);
> -			}
> -			free(fdlist);
> -			if (err) {
> -				fprintf(stderr, Name ": Failed to restore critical"
> -					" section for reshape - sorry.\n");
> -				if (!backup_file)
> -					fprintf(stderr, Name ":  Possibly you need"
> -						" to specify a --backup-file\n");
> +			if (restore_backup(st, content,
> +					   working,
> +					   spare, backup_file, verbose))
>  				return 1;
> -			}
>  
>  			err = Grow_continue(mdfd, st, content, backup_file);
>  		} else switch(content->array.level) {
> diff --git a/Grow.c b/Grow.c
> index b331287..0b96f7a 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3578,6 +3578,68 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist, int cnt
>  	return 1;
>  }
>  
> +int restore_backup(struct supertype *st,
> +		   struct mdinfo *content,
> +		   int working_disks,
> +		   int spares,
> +		   char *backup_file,
> +		   int verbose)
> +{
> +	int i;
> +	int *fdlist;
> +	struct mdinfo *dev;
> +	int err;
> +	int disk_count = working_disks + spares;
> +	int next_spare = working_disks;
> +
> +	dprintf("Called restore_backup()\n");
> +	fdlist = malloc(sizeof(int) * disk_count);
> +	if (fdlist == NULL) {
> +		fprintf(stderr,
> +			Name ": cannot allocate memory for disk list\n");
> +		return 1;
> +	}
> +	for (i = 0; i < disk_count; i++)
> +		fdlist[i] = -1;
> +	for (dev = content->devs; dev; dev = dev->next) {
> +		char buf[22];
> +		int fd;
> +		sprintf(buf, "%d:%d",
> +			dev->disk.major,
> +			dev->disk.minor);
> +		fd = dev_open(buf, O_RDWR);
> +
> +		if (dev->disk.raid_disk >= 0)
> +			fdlist[dev->disk.raid_disk] = fd;
> +		else
> +			fdlist[next_spare++] = fd;
> +	}
> +
> +	if (st->ss->external && st->ss->recover_backup)
> +		err = st->ss->recover_backup(st, content);
> +	else
> +		err = Grow_restart(st, content, fdlist, spares,
> +				   backup_file, verbose > 0);
> +
> +	while (disk_count > 0) {
> +		disk_count--;
> +		if (fdlist[disk_count] >= 0)
> +			close(fdlist[disk_count]);
> +	}
> +	free(fdlist);
> +	if (err) {
> +		fprintf(stderr, Name ": Failed to restore critical"
> +			" section for reshape - sorry.\n");
> +		if (!backup_file)
> +			fprintf(stderr, Name ":  Possibly you need"
> +				" to specify a --backup-file\n");
> +		return 1;
> +	}
> +
> +	dprintf("restore_backup() returns status OK.\n");
> +	return 0;
> +}
> +
>  int Grow_continue_command(char *devname, int fd,
>  			  char *backup_file, int verbose)
>  {
> diff --git a/mdadm.h b/mdadm.h
> index 7761db6..6165979 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1026,6 +1026,12 @@ extern int Grow_continue(int mdfd, struct supertype *st,
>  			 struct mdinfo *info, char *backup_file);
>  extern int Grow_continue_command(char *devname, int fd,
>  				 char *backup_file, int verbose);
> +extern int restore_backup(struct supertype *st,
> +			  struct mdinfo *content,
> +			  int working_disks,
> +			  int spares,
> +			  char *backup_file,
> +			  int verbose);
>  
>  extern int Assemble(struct supertype *st, char *mddev,
>  		    struct mddev_ident *ident,
> 
> --
> 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


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

[PATCH 10/14] Perform restore backup for reshape continuation

[Adam Kwolek]

It can happen that reshape is broken by different reason than reboot.
this means that on reshape continuation start we cannot be sure
that critical section has been restored already.

Restore data from checkpoint before reshape continuation to avoid
data corruption.

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

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

diff --git a/Grow.c b/Grow.c
index 0b96f7a..da8b72d 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3652,6 +3652,9 @@ int Grow_continue_command(char *devname, int fd,
 	char container[20];
 	int cfd = -1;
 	int fd2 = -1;
+	int spares;
+	int working_disks;
+	struct mdinfo *dev;
 
 	dprintf("Grow continue from command line called for %s\n",
 		devname);
@@ -3753,7 +3756,20 @@ int Grow_continue_command(char *devname, int fd,
 
 	/* continue reshape
 	 */
-	ret_val = Grow_continue(fd, st, content, backup_file);
+	dev = content->devs;
+	spares = 0;
+	working_disks = 0;
+	while (dev) {
+		if (dev->disk.raid_disk == -1)
+			spares++;
+		else
+			working_disks++;
+		dev = dev->next;
+	}
+	ret_val = restore_backup(st, content, working_disks, spares,
+				 backup_file, verbose);
+	if (!ret_val)
+		ret_val = Grow_continue(fd, st, content, backup_file);
 
 Grow_continue_command_exit:
 	if (fd2 > -1)

Re: [PATCH 10/14] Perform restore backup for reshape continuation

[NeilBrown]

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

On Fri, 16 Sep 2011 13:55:03 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> It can happen that reshape is broken by different reason than reboot.
> this means that on reshape continuation start we cannot be sure
> that critical section has been restored already.

I don't think this is true.

If the array is running, then the critical section must have been restored.
Anything else is simply wrong.

What scenario are you thinking of here?

Thanks,
NeilBrown


> 
> Restore data from checkpoint before reshape continuation to avoid
> data corruption.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 0b96f7a..da8b72d 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3652,6 +3652,9 @@ int Grow_continue_command(char *devname, int fd,
>  	char container[20];
>  	int cfd = -1;
>  	int fd2 = -1;
> +	int spares;
> +	int working_disks;
> +	struct mdinfo *dev;
>  
>  	dprintf("Grow continue from command line called for %s\n",
>  		devname);
> @@ -3753,7 +3756,20 @@ int Grow_continue_command(char *devname, int fd,
>  
>  	/* continue reshape
>  	 */
> -	ret_val = Grow_continue(fd, st, content, backup_file);
> +	dev = content->devs;
> +	spares = 0;
> +	working_disks = 0;
> +	while (dev) {
> +		if (dev->disk.raid_disk == -1)
> +			spares++;
> +		else
> +			working_disks++;
> +		dev = dev->next;
> +	}
> +	ret_val = restore_backup(st, content, working_disks, spares,
> +				 backup_file, verbose);
> +	if (!ret_val)
> +		ret_val = Grow_continue(fd, st, content, backup_file);
>  
>  Grow_continue_command_exit:
>  	if (fd2 > -1)


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

RE: [PATCH 10/14] Perform restore backup for reshape continuation

[Kwolek, Adam]

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, September 21, 2011 4:30 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> Subject: Re: [PATCH 10/14] Perform restore backup for reshape continuation
> 
> On Fri, 16 Sep 2011 13:55:03 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > It can happen that reshape is broken by different reason than reboot.
> > this means that on reshape continuation start we cannot be sure that
> > critical section has been restored already.
> 
> I don't think this is true.
> 
> If the array is running, then the critical section must have been restored.
> Anything else is simply wrong.
> 
> What scenario are you thinking of here?

You are right. If array is assembled backup is restored already.

BR
Adam

> 
> Thanks,
> NeilBrown
> 
> 
> >
> > Restore data from checkpoint before reshape continuation to avoid data
> > corruption.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  Grow.c |   18 +++++++++++++++++-
> >  1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 0b96f7a..da8b72d 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -3652,6 +3652,9 @@ int Grow_continue_command(char *devname,
> int fd,
> >  	char container[20];
> >  	int cfd = -1;
> >  	int fd2 = -1;
> > +	int spares;
> > +	int working_disks;
> > +	struct mdinfo *dev;
> >
> >  	dprintf("Grow continue from command line called for %s\n",
> >  		devname);
> > @@ -3753,7 +3756,20 @@ int Grow_continue_command(char *devname,
> int
> > fd,
> >
> >  	/* continue reshape
> >  	 */
> > -	ret_val = Grow_continue(fd, st, content, backup_file);
> > +	dev = content->devs;
> > +	spares = 0;
> > +	working_disks = 0;
> > +	while (dev) {
> > +		if (dev->disk.raid_disk == -1)
> > +			spares++;
> > +		else
> > +			working_disks++;
> > +		dev = dev->next;
> > +	}
> > +	ret_val = restore_backup(st, content, working_disks, spares,
> > +				 backup_file, verbose);
> > +	if (!ret_val)
> > +		ret_val = Grow_continue(fd, st, content, backup_file);
> >
> >  Grow_continue_command_exit:
> >  	if (fd2 > -1)

[PATCH 11/14] Add possibility to restart reshape for native metadata

[Adam Kwolek]

When mdadm services native metadata, check-pointing functions
can lost file system context in the same way as for external metadata.

This patch adds native metadata continuation support.

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

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

diff --git a/Grow.c b/Grow.c
index da8b72d..fa22382 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3646,6 +3646,7 @@ int Grow_continue_command(char *devname, int fd,
 	int ret_val = 0;
 	struct supertype *st = NULL;
 	struct mdinfo *content = NULL;
+	struct mdinfo array;
 	char *subarray = NULL;
 	struct mdinfo *cc = NULL;
 	struct mdstat_ent *mdstat = NULL;
@@ -3669,12 +3670,14 @@ int Grow_continue_command(char *devname, int fd,
 	dprintf("Grow continue is run for ");
 	if (st->ss->external == 0) {
 		dprintf("native array (%s)\n", devname);
-		/* FIXME : temporary just exit
-		 * for native metadata case
-		 */
-		st->ss->free_super(st);
-		free(subarray);
-		exit(0);
+		if (ioctl(fd, GET_ARRAY_INFO, &array) < 0) {
+			fprintf(stderr, Name ": %s is not an active md array -"
+				" aborting\n", devname);
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
+		content = &array;
+		sysfs_init(content, fd, st->devnum);
 	} else {
 		int container_dev;
 

[PATCH 12/14] Early reshape backup verification

[Adam Kwolek]

Ever reshape needs to backup its critical section. For this purposes
backup file or external metadata mechanisms can be used.

Add verification if backup file or metadata mechanisms can have chance
to restore data form backup.
For native metadata existence of passed (from command line) backup file
is checked. For external metadta manage_reshape() and recover_backup()
are tested for this purposes.

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

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

diff --git a/Grow.c b/Grow.c
index fa22382..d54b8a8 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3667,6 +3667,18 @@ int Grow_continue_command(char *devname, int fd,
 			devname);
 		return 1;
 	}
+
+	/* request backup file
+	 * when no metadta specific backup action is defined
+	 */
+	if ((backup_file == NULL) &&
+	    ((st->ss->manage_reshape == NULL) ||
+	     (st->ss->recover_backup == NULL))) {
+		fprintf(stderr, Name ": Please provide backup file "
+			"for reshape continuation.\n");
+		return 1;
+	}
+
 	dprintf("Grow continue is run for ");
 	if (st->ss->external == 0) {
 		dprintf("native array (%s)\n", devname);

Re: [PATCH 12/14] Early reshape backup verification

[NeilBrown]

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

On Fri, 16 Sep 2011 13:55:18 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Ever reshape needs to backup its critical section. For this purposes
> backup file or external metadata mechanisms can be used.

This is not correct.
For internal metadata the critical section can be stored on a spare device,
and this is preferred where possible.

So it is not the case that you must have a backup_file for non-external
data.

NeilBrown


> 
> Add verification if backup file or metadata mechanisms can have chance
> to restore data form backup.
> For native metadata existence of passed (from command line) backup file
> is checked. For external metadta manage_reshape() and recover_backup()
> are tested for this purposes.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index fa22382..d54b8a8 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3667,6 +3667,18 @@ int Grow_continue_command(char *devname, int fd,
>  			devname);
>  		return 1;
>  	}
> +
> +	/* request backup file
> +	 * when no metadta specific backup action is defined
> +	 */
> +	if ((backup_file == NULL) &&
> +	    ((st->ss->manage_reshape == NULL) ||
> +	     (st->ss->recover_backup == NULL))) {
> +		fprintf(stderr, Name ": Please provide backup file "
> +			"for reshape continuation.\n");
> +		return 1;
> +	}
> +
>  	dprintf("Grow continue is run for ");
>  	if (st->ss->external == 0) {
>  		dprintf("native array (%s)\n", devname);
> 
> --
> 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


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

RE: [PATCH 12/14] Early reshape backup verification

[Kwolek, Adam]

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Wednesday, September 21, 2011 4:31 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> Subject: Re: [PATCH 12/14] Early reshape backup verification
> 
> On Fri, 16 Sep 2011 13:55:18 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > Ever reshape needs to backup its critical section. For this purposes
> > backup file or external metadata mechanisms can be used.
> 
> This is not correct.

You are right.

BR
Adam


> For internal metadata the critical section can be stored on a spare device, and
> this is preferred where possible.
> 
> So it is not the case that you must have a backup_file for non-external data.
> 
> NeilBrown
> 
> 
> >
> > Add verification if backup file or metadata mechanisms can have chance
> > to restore data form backup.
> > For native metadata existence of passed (from command line) backup
> > file is checked. For external metadta manage_reshape() and
> > recover_backup() are tested for this purposes.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  Grow.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index fa22382..d54b8a8 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -3667,6 +3667,18 @@ int Grow_continue_command(char *devname,
> int fd,
> >  			devname);
> >  		return 1;
> >  	}
> > +
> > +	/* request backup file
> > +	 * when no metadta specific backup action is defined
> > +	 */
> > +	if ((backup_file == NULL) &&
> > +	    ((st->ss->manage_reshape == NULL) ||
> > +	     (st->ss->recover_backup == NULL))) {
> > +		fprintf(stderr, Name ": Please provide backup file "
> > +			"for reshape continuation.\n");
> > +		return 1;
> > +	}
> > +
> >  	dprintf("Grow continue is run for ");
> >  	if (st->ss->external == 0) {
> >  		dprintf("native array (%s)\n", devname);
> >
> > --
> > 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

[PATCH 13/14] Check if md allows to control reshape

[Adam Kwolek]

It can happen that there is no mdadm in memory and 'max' was already
set to sync_max. Such array cannot be put under check pointing control
again.

Verify such situation and refuse to control array in such condition.

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

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

diff --git a/Grow.c b/Grow.c
index d54b8a8..04e6679 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3656,6 +3656,7 @@ int Grow_continue_command(char *devname, int fd,
 	int spares;
 	int working_disks;
 	struct mdinfo *dev;
+	char buf[40];
 
 	dprintf("Grow continue from command line called for %s\n",
 		devname);
@@ -3769,6 +3770,24 @@ int Grow_continue_command(char *devname, int fd,
 		}
 	}
 
+	/* verify that array under reshape is stopped
+	 */
+	ret_val = sysfs_get_str(content, NULL, "sync_max", buf, 40);
+	if (ret_val <= 0) {
+		fprintf(stderr, Name
+			": cannot open verify reshape progress for %s (%i)\n",
+			content->sys_name, ret_val);
+		ret_val = 1;
+		goto Grow_continue_command_exit;
+	}
+	dprintf(Name ": Read sync_max sysfs entry is: %s\n", buf);
+	if (strncmp(buf, "max", 3) == 0) {
+		fprintf(stderr, Name ": md is not allowed to finish reshape "
+			"wihout mdadm assistance.\n");
+		ret_val = 1;
+		goto Grow_continue_command_exit;
+	}
+
 	/* continue reshape
 	 */
 	dev = content->devs;

Re: [PATCH 13/14] Check if md allows to control reshape

[NeilBrown]

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

On Fri, 16 Sep 2011 13:55:25 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:

> It can happen that there is no mdadm in memory and 'max' was already
> set to sync_max. Such array cannot be put under check pointing control
> again.

Again I don't understand what you are trying to guard against.
If resync_max is 'max', then the kernel is allowed to fun reshape to
completion without any help from mdadm.  If mdadm needs to monitor things it
always sets resync_max to a smaller value.

Confused.

NeilBrown


> 
> Verify such situation and refuse to control array in such condition.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index d54b8a8..04e6679 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -3656,6 +3656,7 @@ int Grow_continue_command(char *devname, int fd,
>  	int spares;
>  	int working_disks;
>  	struct mdinfo *dev;
> +	char buf[40];
>  
>  	dprintf("Grow continue from command line called for %s\n",
>  		devname);
> @@ -3769,6 +3770,24 @@ int Grow_continue_command(char *devname, int fd,
>  		}
>  	}
>  
> +	/* verify that array under reshape is stopped
> +	 */
> +	ret_val = sysfs_get_str(content, NULL, "sync_max", buf, 40);
> +	if (ret_val <= 0) {
> +		fprintf(stderr, Name
> +			": cannot open verify reshape progress for %s (%i)\n",
> +			content->sys_name, ret_val);
> +		ret_val = 1;
> +		goto Grow_continue_command_exit;
> +	}
> +	dprintf(Name ": Read sync_max sysfs entry is: %s\n", buf);
> +	if (strncmp(buf, "max", 3) == 0) {
> +		fprintf(stderr, Name ": md is not allowed to finish reshape "
> +			"wihout mdadm assistance.\n");
> +		ret_val = 1;
> +		goto Grow_continue_command_exit;
> +	}
> +
>  	/* continue reshape
>  	 */
>  	dev = content->devs;
> 
> --
> 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


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

RE: [PATCH 13/14] Check if md allows to control reshape

[Kwolek, Adam]

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, September 21, 2011 4:34 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> Subject: Re: [PATCH 13/14] Check if md allows to control reshape
> 
> On Fri, 16 Sep 2011 13:55:25 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > It can happen that there is no mdadm in memory and 'max' was already
> > set to sync_max. Such array cannot be put under check pointing control
> > again.
> 
> Again I don't understand what you are trying to guard against.
> If resync_max is 'max', then the kernel is allowed to fun reshape to
> completion without any help from mdadm.  If mdadm needs to monitor
> things it always sets resync_max to a smaller value.
> 
> Confused.

In my opinion running reshape continuation on array that is allowed to freely reshape to the end
is race condition (another mdadm or user via sysfs manage this array already).
We can tell nothing about restart point (read from metadata) when md runs with reshape already.
I think we should not allow to attach mdadm to such reshape process.

I think this guard is required.

BR
Adam 

> 
> NeilBrown
> 
> 
> >
> > Verify such situation and refuse to control array in such condition.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  Grow.c |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index d54b8a8..04e6679 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -3656,6 +3656,7 @@ int Grow_continue_command(char *devname,
> int fd,
> >  	int spares;
> >  	int working_disks;
> >  	struct mdinfo *dev;
> > +	char buf[40];
> >
> >  	dprintf("Grow continue from command line called for %s\n",
> >  		devname);
> > @@ -3769,6 +3770,24 @@ int Grow_continue_command(char *devname,
> int fd,
> >  		}
> >  	}
> >
> > +	/* verify that array under reshape is stopped
> > +	 */
> > +	ret_val = sysfs_get_str(content, NULL, "sync_max", buf, 40);
> > +	if (ret_val <= 0) {
> > +		fprintf(stderr, Name
> > +			": cannot open verify reshape progress for %s
> (%i)\n",
> > +			content->sys_name, ret_val);
> > +		ret_val = 1;
> > +		goto Grow_continue_command_exit;
> > +	}
> > +	dprintf(Name ": Read sync_max sysfs entry is: %s\n", buf);
> > +	if (strncmp(buf, "max", 3) == 0) {
> > +		fprintf(stderr, Name ": md is not allowed to finish reshape "
> > +			"wihout mdadm assistance.\n");
> > +		ret_val = 1;
> > +		goto Grow_continue_command_exit;
> > +	}
> > +
> >  	/* continue reshape
> >  	 */
> >  	dev = content->devs;
> >
> > --
> > 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 13/14] Check if md allows to control reshape

[NeilBrown]

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

On Wed, 21 Sep 2011 07:31:48 +0000 "Kwolek, Adam" <adam.kwolek@intel.com>
wrote:

> 
> 
> > -----Original Message-----
> > From: NeilBrown [mailto:neilb@suse.de]
> > Sent: Wednesday, September 21, 2011 4:34 AM
> > To: Kwolek, Adam
> > Cc: linux-raid@vger.kernel.org; Ciechanowski, Ed; Labun, Marcin
> > Subject: Re: [PATCH 13/14] Check if md allows to control reshape
> > 
> > On Fri, 16 Sep 2011 13:55:25 +0200 Adam Kwolek <adam.kwolek@intel.com>
> > wrote:
> > 
> > > It can happen that there is no mdadm in memory and 'max' was already
> > > set to sync_max. Such array cannot be put under check pointing control
> > > again.
> > 
> > Again I don't understand what you are trying to guard against.
> > If resync_max is 'max', then the kernel is allowed to fun reshape to
> > completion without any help from mdadm.  If mdadm needs to monitor
> > things it always sets resync_max to a smaller value.
> > 
> > Confused.
> 
> In my opinion running reshape continuation on array that is allowed to freely reshape to the end
> is race condition (another mdadm or user via sysfs manage this array already).
> We can tell nothing about restart point (read from metadata) when md runs with reshape already.
> I think we should not allow to attach mdadm to such reshape process.
> 
> I think this guard is required.
> 

I see what your point is now.  I was just a bit confused by the message that
is printed which seems to say something different.

If we need to guard something here, we need to guard against a lot more than
just sync_max == "max".  We need to guard against another mdadm process
monitoring the array (the extra .pid file I mentioned) and also if sync_max
has any value larger than what we would expect from the metadata.

So a test that rejects an attempt to continue reshape on an array where
sync_max is not the correct value makes sense.  Just testing for "max"
doesn't.

Thanks,
NeilBrown

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

[PATCH 14/14] Manual update for --continue option

[Adam Kwolek]

Patch adds to mdadm man the following information:

--continue
  This  is  needed when --grow operation is interrupted and it is not
  restarted automatically during array assembly or array cannot
  be disassembled and assembled again. Option --continue has to be used
  together with -G , ( --grow ) command and device that it should be
  executed on.  All parameters required for reshape continuation will
  be read from array metadata.   If  initial  --grow  command  had
  required --backup-file= option to be set, continuation option will
  require to have exactly the same backup file pointed to also.

  Any other parameter passed together with --continue option will
  be ignored.

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

 mdadm.8.in |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/mdadm.8.in b/mdadm.8.in
index e22fde4..e677be4 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -733,6 +733,30 @@ The file must be stored on a separate device, not on the RAID array
 being reshaped.
 
 .TP
+.BR \-\-continue
+This is needed when
+.B \-\-grow
+operation is interrupted and it is not restarted automatically during array
+assembly or array cannot be disassembled and assembled again. Option
+.BR \-\-continue
+has to be used together with
+.BR \-G
+, (
+.BR \-\-grow
+) command and device that it should be executed on.
+All parameters required for reshape continuation will be read from array metadata.
+If initial
+.BR \-\-grow
+command had required
+.BR \-\-backup\-file=
+option to be set, continuation option will require to have exactly the same
+backup file pointed to also.
+.IP
+Any other parameter passed together with
+.BR \-\-continue
+option will be ignored.
+
+.TP
 .BR \-N ", " \-\-name=
 Set a
 .B name