[PATCH 0/2] Release 3.2 summary

[PATCH 0/2] Release 3.2 summary

[Krzysztof Wojcik]

Neil,

Intel plans to include following features in mdadm-3.2 release:
- Array Auto Rebuild,
- Online Capacity Expansion,
- Takeover raid10->raid0,
- Takeover raid0->raid10,
- Takeover raid1->raid0

They are almost incorporated into your repository and tested.
Some of patches are waiting for your acceptation and some will be send
today.
We're getting closer to the end of release and I would like to summarize
and explain what exactly needs to be done.

To finish our features following patches have to be appyied:
mdadm:
- FIX: Meet SET_ARRAY_INFO ioctl requirements (this series)
- FIX: Remove disks in mdmon for external metadata (this series)
- WORKAROUND: mdadm hangs during reshape (PART #2)
- FIX: delta_disk can have UnSet value
- imsm: FIX: spare cannot be added (very imprtand fix- blocks many tests)
- FIX: start_reshape status should be checked
- FIX: Container can be left frozen
- FIX: Array after takeover has to be frozen
- imsm: FIX: do not allow for container operation for the same disks number
- imsm: Update metadata for second array
kernel:
- Add raid1->raid0 takeover support
- md/raid5: FIX: reshape on degraded devices has wrong configuration
- md: do not write resync checkpoint, if max_sector has beeen reached
- unblock the creation of an external metadata RAID if native one exists

Could you review patches above and eventually apply them, please?

Besides "Assemble: allow to assemble spares on their own" patch has to be
discused and final solution has to be found.

After all patches will be applyied we expect all planned featues will be
working and we can finish the 3.2 release. Of course we will continue our
testing after official release and new defects may be discovered.

And question for you. When do you plan to announce official 3.2 release?
Can we assume that this will be at the beginning of next week?

---

Krzysztof Wojcik (2):
      FIX: Remove disks in mdmon for external metadata
      FIX: Meet SET_ARRAY_INFO ioctl requirements


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

-- 
Regards
Krzysztof Wojcik

[PATCH 1/2] FIX: Remove disks in mdmon for external metadata

[Krzysztof Wojcik]

For raid10 -> raid0 takeover operation we should reject disks
in mirror by marking them as 'failed' and then remove them from
array by writing "remove" to disk state.

For external metadata second action is executed by mdmon.
According the description in monitor.c:175 when monitor detect
"faulty" in device state, it blocks the device, mark it as failed
in metadata, unblocks the device and finally writes "remove"
to device state.
For external case writing "remove" to device state in mdadm
is not necessary and harmful.
It may cause following issues:
1. "remove" operation for external case in mdadm is not finish
with successful result because monitor may block the device or disk
has been already removed by monitor.
2. If disk is removed by mdadm earlier than mdmon catch "failed" state,
metadata is not properly updated- is not marked as failed.

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

diff --git a/Grow.c b/Grow.c
index c7e40c0..8c6ff23 100644
--- a/Grow.c
+++ b/Grow.c
@@ -719,7 +719,9 @@ int remove_disks_for_takeover(struct supertype *st,
 
 		sysfs_set_str(sra, sd, "state", "faulty");
 		sysfs_set_str(sra, sd, "slot", "none");
-		sysfs_set_str(sra, sd, "state", "remove");
+		/* for external metadata disks should be removed in mdmon */
+		if (!st->ss->external)
+			sysfs_set_str(sra, sd, "state", "remove");
 		sd->disk.state |= (1<<MD_DISK_REMOVED);
 		sd->disk.state &= ~(1<<MD_DISK_SYNC);
 		sd->next = sra->devs;

[PATCH 2/2] FIX: Meet SET_ARRAY_INFO ioctl requirements

[Krzysztof Wojcik]

Problem has been observed when raid10<->raid0 takeover operation
is executed.
In code updating layout, raid_disks and chunk_size for non-restriping
operations in reshape array functions SET_ARRAY_INFO ioctl call was
not succeeded.
Takeover process finish execution with error, mdadm shows message:
"mdadm: failed to set disks"

Cause is not meeting SET_ARRAY_INFO ioctl requirements:
- only one parameter may be changed at one time
- level of current array info and new info should be the same

Patch introduces solution for this issue.
At the beginning of discussed code we read current information
about array and then compare them with new values should be set.
If particular value is different (and should be set),
we are overwrite only this one in array info and then call ioctl.

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

diff --git a/Grow.c b/Grow.c
index 8c6ff23..194cc3e 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1714,35 +1714,44 @@ static int reshape_array(char *container, int fd, char *devname,
 		/* No restriping needed, but we might need to impose
 		 * some more changes: layout, raid_disks, chunk_size
 		 */
+		/* read current array info */
+		if (ioctl(fd, GET_ARRAY_INFO, &array) != 0) {
+			dprintf("Canot get array information.\n");
+			goto release;
+		}
+		/* compare current array info with new values and if
+		 * it is different update them to new */
 		if (info->new_layout != UnSet &&
-		    info->new_layout != info->array.layout) {
-			info->array.layout = info->new_layout;
-			if (ioctl(fd, SET_ARRAY_INFO, &info->array) != 0) {
+		    info->new_layout != array.layout) {
+			array.layout = info->new_layout;
+			if (ioctl(fd, SET_ARRAY_INFO, &array) != 0) {
 				fprintf(stderr, Name ": failed to set new layout\n");
 				goto release;
 			} else if (!quiet)
 				printf("layout for %s set to %d\n",
-				       devname, info->array.layout);
+				       devname, array.layout);
 		}
 		if (info->delta_disks != UnSet &&
-		    info->delta_disks != 0) {
-			info->array.raid_disks += info->delta_disks;
-			if (ioctl(fd, SET_ARRAY_INFO, &info->array) != 0) {
+		    info->delta_disks != 0 &&
+		    array.raid_disks != (info->array.raid_disks + info->delta_disks)) {
+			array.raid_disks += info->delta_disks;
+			if (ioctl(fd, SET_ARRAY_INFO, &array) != 0) {
 				fprintf(stderr, Name ": failed to set raid disks\n");
 				goto release;
-			} else if (!quiet)
+			} else if (!quiet) {
 				printf("raid_disks for %s set to %d\n",
-				       devname, info->array.raid_disks);
+				       devname, array.raid_disks);
+			}
 		}
 		if (info->new_chunk != 0 &&
-		    info->new_chunk != info->array.chunk_size) {
+		    info->new_chunk != array.chunk_size) {
 			if (sysfs_set_num(info, NULL,
 					  "chunk_size", info->new_chunk) != 0) {
 				fprintf(stderr, Name ": failed to set chunk size\n");
 				goto release;
 			} else if (!quiet)
 				printf("chunk size for %s set to %d\n",
-				       devname, info->array.chunk_size);
+				       devname, array.chunk_size);
 		}
 		unfreeze(st);
 		return 0;

Re: [PATCH 0/2] Release 3.2 summary

[Neil Brown]

On Thu, 27 Jan 2011 17:46:42 +0100
Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote:

> Neil,
> 
> Intel plans to include following features in mdadm-3.2 release:
> - Array Auto Rebuild,
> - Online Capacity Expansion,
> - Takeover raid10->raid0,
> - Takeover raid0->raid10,
> - Takeover raid1->raid0
> 
> They are almost incorporated into your repository and tested.
> Some of patches are waiting for your acceptation and some will be send
> today.
> We're getting closer to the end of release and I would like to
> summarize and explain what exactly needs to be done.
> 
> To finish our features following patches have to be appyied:
> mdadm:
> - FIX: Meet SET_ARRAY_INFO ioctl requirements (this series)
> - FIX: Remove disks in mdmon for external metadata (this series)
> - WORKAROUND: mdadm hangs during reshape (PART #2)
> - FIX: delta_disk can have UnSet value
> - imsm: FIX: spare cannot be added (very imprtand fix- blocks many
> tests)
> - FIX: start_reshape status should be checked
> - FIX: Container can be left frozen
> - FIX: Array after takeover has to be frozen
> - imsm: FIX: do not allow for container operation for the same disks
> number
> - imsm: Update metadata for second array
> kernel:
> - Add raid1->raid0 takeover support
> - md/raid5: FIX: reshape on degraded devices has wrong configuration
> - md: do not write resync checkpoint, if max_sector has beeen reached
> - unblock the creation of an external metadata RAID if native one
> exists
> 
> Could you review patches above and eventually apply them, please?
> 
> Besides "Assemble: allow to assemble spares on their own" patch has
> to be discused and final solution has to be found.
> 
> After all patches will be applyied we expect all planned featues will
> be working and we can finish the 3.2 release. Of course we will
> continue our testing after official release and new defects may be
> discovered.
> 
> And question for you. When do you plan to announce official 3.2
> release? Can we assume that this will be at the beginning of next
> week?

I think I have applied and pushed out all of the patches in my inbox
that I can agree with or easily fix.  There are a few that raise real
issues but that I am not happy with the approach.  And some that I
don't agree with at all.  But as you say, most have been resolved.

One issue that hasn't been addressed much at all is restarting an array
that is in the middle of a reshape.  I want to at least get that
working for native arrays before I release 3.2.  Getting that working
for external metadata will probably have to wait for 3.2.1.

I very much hope to release 3.2 on Monday.  I will announce it as a
'developer' release - it mostly works but needs lots of testing before
anyone depends on it.

My minimum requirement for 3.2 is that the mdadm test suite passes.

Thanks,
NeilBrown

> 
> ---
> 
> Krzysztof Wojcik (2):
>       FIX: Remove disks in mdmon for external metadata
>       FIX: Meet SET_ARRAY_INFO ioctl requirements
> 
> 
>  Grow.c |   35 +++++++++++++++++++++++------------
>  1 files changed, 23 insertions(+), 12 deletions(-)
> 

RE: [PATCH 0/2] Release 3.2 summary

[Wojcik, Krzysztof]

Neil,

Thank you for applying last patches.
It seems that only two patches left to complete features planed for 3.2 release:
- imsm: Update metadata for second array
- imsm: FIX: not all disks are released in free_imsm_disks()

Please, apply them if they are meet your requirements.

After the 3.2 release we plan to submit to you next mdadm's features.
On Monday I will send you a draft plan our next steps related to mdadm (what and at what time we plan to submit)
I hope that this will improve our cooperation.

I would also mention that we expect that the kernel patches will be applied.

Regards
Krzysztof

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of Neil Brown
> Sent: Friday, January 28, 2011 2:26 AM
> To: Wojcik, Krzysztof
> Cc: linux-raid@vger.kernel.org; Neubauer, Wojciech; Kwolek, Adam;
> Williams, Dan J; Ciechanowski, Ed
> Subject: Re: [PATCH 0/2] Release 3.2 summary
> 
> On Thu, 27 Jan 2011 17:46:42 +0100
> Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote:
> 
> > Neil,
> >
> > Intel plans to include following features in mdadm-3.2 release:
> > - Array Auto Rebuild,
> > - Online Capacity Expansion,
> > - Takeover raid10->raid0,
> > - Takeover raid0->raid10,
> > - Takeover raid1->raid0
> >
> > They are almost incorporated into your repository and tested.
> > Some of patches are waiting for your acceptation and some will be
> send
> > today.
> > We're getting closer to the end of release and I would like to
> > summarize and explain what exactly needs to be done.
> >
> > To finish our features following patches have to be appyied:
> > mdadm:
> > - FIX: Meet SET_ARRAY_INFO ioctl requirements (this series)
> > - FIX: Remove disks in mdmon for external metadata (this series)
> > - WORKAROUND: mdadm hangs during reshape (PART #2)
> > - FIX: delta_disk can have UnSet value
> > - imsm: FIX: spare cannot be added (very imprtand fix- blocks many
> > tests)
> > - FIX: start_reshape status should be checked
> > - FIX: Container can be left frozen
> > - FIX: Array after takeover has to be frozen
> > - imsm: FIX: do not allow for container operation for the same disks
> > number
> > - imsm: Update metadata for second array
> > kernel:
> > - Add raid1->raid0 takeover support
> > - md/raid5: FIX: reshape on degraded devices has wrong configuration
> > - md: do not write resync checkpoint, if max_sector has beeen reached
> > - unblock the creation of an external metadata RAID if native one
> > exists
> >
> > Could you review patches above and eventually apply them, please?
> >
> > Besides "Assemble: allow to assemble spares on their own" patch has
> > to be discused and final solution has to be found.
> >
> > After all patches will be applyied we expect all planned featues will
> > be working and we can finish the 3.2 release. Of course we will
> > continue our testing after official release and new defects may be
> > discovered.
> >
> > And question for you. When do you plan to announce official 3.2
> > release? Can we assume that this will be at the beginning of next
> > week?
> 
> I think I have applied and pushed out all of the patches in my inbox
> that I can agree with or easily fix.  There are a few that raise real
> issues but that I am not happy with the approach.  And some that I
> don't agree with at all.  But as you say, most have been resolved.
> 
> One issue that hasn't been addressed much at all is restarting an array
> that is in the middle of a reshape.  I want to at least get that
> working for native arrays before I release 3.2.  Getting that working
> for external metadata will probably have to wait for 3.2.1.
> 
> I very much hope to release 3.2 on Monday.  I will announce it as a
> 'developer' release - it mostly works but needs lots of testing before
> anyone depends on it.
> 
> My minimum requirement for 3.2 is that the mdadm test suite passes.
> 
> Thanks,
> NeilBrown
> 
> >
> > ---
> >
> > Krzysztof Wojcik (2):
> >       FIX: Remove disks in mdmon for external metadata
> >       FIX: Meet SET_ARRAY_INFO ioctl requirements
> >
> >
> >  Grow.c |   35 +++++++++++++++++++++++------------
> >  1 files changed, 23 insertions(+), 12 deletions(-)
> >
> 
> --
> 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 0/2] Release 3.2 summary

[NeilBrown]

On Sat, 29 Jan 2011 17:52:45 +0000 "Wojcik, Krzysztof"
<krzysztof.wojcik@intel.com> wrote:

> Neil,
> 
> Thank you for applying last patches.
> It seems that only two patches left to complete features planed for 3.2 release:
> - imsm: Update metadata for second array
> - imsm: FIX: not all disks are released in free_imsm_disks()
> 
> Please, apply them if they are meet your requirements.
> 
> After the 3.2 release we plan to submit to you next mdadm's features.

This is probably not a good idea.  I don't want to add new features to mdadm
until I am sure that the current ones are stable - and I am not.

I need to see lots of testing of the current features - both for IMSM
metadata and for native metadata, including restart of an array that is
undergoing reshape etc.
I also need to do quite a bit of code review.  Reviewing each patch as it
goes in is of course important, but it is quite possible to miss things in
that process.  The recent changes evolved quite a bit as they were being
developed, so I am far from confident that the final form really is right.

> On Monday I will send you a draft plan our next steps related to mdadm (what and at what time we plan to submit)
> I hope that this will improve our cooperation.

I'll be happy to look over your draft plan and comment on it, but I don't
expect to be reviewing/accepting any patches for new functionality at least
until 3.2.1 is out which will be primarily bug fixes.

NeilBrown



> 
> I would also mention that we expect that the kernel patches will be applied.
> 
> Regards
> Krzysztof
> 

Re: [PATCH 0/2] Release 3.2 summary

[NeilBrown]

On Sat, 29 Jan 2011 17:52:45 +0000 "Wojcik, Krzysztof"
<krzysztof.wojcik@intel.com> wrote:

> Neil,
> 
> Thank you for applying last patches.
> It seems that only two patches left to complete features planed for 3.2 release:
> - imsm: Update metadata for second array
> - imsm: FIX: not all disks are released in free_imsm_disks()
> 
> Please, apply them if they are meet your requirements.

All the acceptable patches are now in my devel-3.2 branch

   git://neil.brown.name/mdadm devel-3.2

It still doesn't pass all of my self-tests, though it is getting closer.
Hopefully it will pass them all tomorrow.

> 
> I would also mention that we expect that the kernel patches will be applied.

All the acceptable patches are now in my for-next branch:

    git://neil.brown.name/md  for-next

Others have been replied to.

NeilBrown