[PATCH 00/21] IMSM Checkpointing Bug Fix Series

[PATCH 00/21] IMSM Checkpointing Bug Fix Series

am 08.06.2011 18:09:33 von adam.kwolek

The following series fixes problems found in IMSM's checkpointing.
It contains rework based on Neil's comments to previous/initial checkpointing
series and tt should be applied on neil_master branch (on the top
of previous checkpointing patches).

BR
Adam


---

Adam Kwolek (21):
MAN: Man update for check-pointing
imsm: Optimize expansion speed when no backup is required
imsm: FIX: Remove timeout from wait_for_reshape_imsm()
imsm: FIX: wait_for_reshape_imsm() cleanup
imsm: FIX: Do not continue reshape when backup exists
FIX: Move buffer to next location
imsm: FIX: Remove unused variables and code
imsm: FIX: Move reshape_progress forward
imsm: FIX: Detect failed devices during recover_backup_imsm()
imsm: FIX: Use metadata information for restore_stripes() and save_stripes()
imsm: FIX: Remove unused parameter from save_backup_imsm() interface
imsm: FIX: Do not use pba_of_lba0 for copy position calculation
imsm: FIX: Do not verify unused parameters
imsm: FIX: Calculate backup location based on metadata information
imsm: FIX: Use macros to data access
imsm: FIX: Check layout for level migration
imsm: FIX: Max position could not be rounded to MB
imsm: FIX: Detect migration end during migration record saving
imsm: FIX: Verify if migration record is loaded correctly
imsm: FIX: Opened handle is not closed
imsm: FIX: Cannot create volume


mdadm.8.in | 9 ++
restripe.c | 6 +
super-intel.c | 237 ++++++++++++++++++++++++++++++++++-----------------------
3 files changed, 153 insertions(+), 99 deletions(-)

--
Signature
--
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 01/21] imsm: FIX: Cannot create volume

am 08.06.2011 18:09:41 von adam.kwolek

Clearing info structure causes mdadm is not able to create workable volume.

During volume creation info structure passed to getinfo() function
contains some information already and cannot be cleared.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index b8d8b4e..471dbd2 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2075,7 +2075,6 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
unsigned int component_size_alligment;
int map_disks = info->array.raid_disks;

- memset(info, 0, sizeof(*info));
if (prev_map)
map_to_analyse = prev_map;


--
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 02/21] imsm: FIX: Opened handle is not closed

am 08.06.2011 18:09:49 von adam.kwolek

Opened file handle should be closed before function exit.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 471dbd2..01ffcc8 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8550,8 +8550,10 @@ int wait_for_reshape_imsm(struct mdinfo *sra, unsigned long long to_complete,
sysfs_set_str(sra, NULL, "sync_max", "max");
to_complete = MaxSector;
} else {
- if (completed > to_complete)
+ if (completed > to_complete) {
+ close(fd);
return -1;
+ }
if (sysfs_set_num(sra, NULL, "sync_max",
to_complete / ndata) != 0) {
close(fd);

--
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 03/21] imsm: FIX: Verify if migration record is loaded

am 08.06.2011 18:09:58 von adam.kwolek

Migration compatibility can be checked when general migration record
is present.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 01ffcc8..269cb0a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -3038,8 +3038,8 @@ static int load_imsm_mpb(int fd, struct intel_super *super, char *devname)

if (lseek64(fd, dsize - (512 * 2), SEEK_SET) < 0) {
if (devname)
- fprintf(stderr,
- Name ": Cannot seek to anchor block on %s: %s\n",
+ fprintf(stderr, Name
+ ": Cannot seek to anchor block on %s: %s\n",
devname, strerror(errno));
return 1;
}
@@ -3836,16 +3836,17 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname)
}

/* load migration record */
- load_imsm_migr_rec(super, NULL);
-
- /* Check for unsupported migration features */
- if (check_mpb_migr_compatibility(super) != 0) {
- fprintf(stderr, Name ": Unsupported migration detected");
- if (devname)
- fprintf(stderr, " on %s\n", devname);
- else
- fprintf(stderr, " (IMSM).\n");
- return 3;
+ if (load_imsm_migr_rec(super, NULL) == 0) {
+ /* Check for unsupported migration features */
+ if (check_mpb_migr_compatibility(super) != 0) {
+ fprintf(stderr,
+ Name ": Unsupported migration detected");
+ if (devname)
+ fprintf(stderr, " on %s\n", devname);
+ else
+ fprintf(stderr, " (IMSM).\n");
+ return 3;
+ }
}

return 0;
@@ -7763,7 +7764,12 @@ abort:
int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
{
struct intel_super *super = st->sb;
- load_imsm_migr_rec(super, info);
+ if (load_imsm_migr_rec(super, info) != 0) {
+ dprintf("imsm: ERROR: Cannot read migration record "
+ "for checkpoint save.\n");
+ return 1;
+ }
+
if (__le32_to_cpu(super->migr_rec->blocks_per_unit) == 0) {
dprintf("ERROR: blocks_per_unit = 0!!!\n");
return 1;

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 04/21] imsm: FIX: Detect migration end during migration record

am 08.06.2011 18:10:06 von adam.kwolek

Checkpoint should be saved when migration is in progress only.
End of reshape (based on passes status) should be detected and it should
not cause abort of reshape/check-pointing/ operation.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 269cb0a..3afa913 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7760,6 +7760,8 @@ abort:
* Returns:
* 0: success
* 1: failure
+ * 2: failure, means no valid migration record
+ * / no general migration in progress /
************************************************************ ******************/
int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
{
@@ -7771,8 +7773,8 @@ int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
}

if (__le32_to_cpu(super->migr_rec->blocks_per_unit) == 0) {
- dprintf("ERROR: blocks_per_unit = 0!!!\n");
- return 1;
+ dprintf("imsm: no migration in progress.\n");
+ return 2;
}

super->migr_rec->curr_migr_unit =
@@ -8842,7 +8844,9 @@ static int imsm_manage_reshape(

sra->reshape_progress = next_step;

- if (save_checkpoint_imsm(st, sra, UNIT_SRC_NORMAL)) {
+ if (save_checkpoint_imsm(st, sra, UNIT_SRC_NORMAL) == 1) {
+ /* ignore error == 2, this can mean end of reshape here
+ */
dprintf("imsm: Cannot write checkpoint to "
"migration record (UNIT_SRC_NORMAL)\n");
goto abort;

--
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/21] imsm: FIX: Max position could not be rounded to MB

am 08.06.2011 18:10:14 von adam.kwolek

When rounded array size information from metadata is used for number
of migration units calculation it can occurs that result of units
can be smaller (-1) than required due to used (rounded) array size).

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 3afa913..2468968 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7621,10 +7621,7 @@ void init_migr_record_imsm(struct supertype *st, struct imsm_dev *dev,
struct imsm_map *map_dest = get_imsm_map(dev, 0);
struct imsm_map *map_src = get_imsm_map(dev, 1);
unsigned long long num_migr_units;
-
- unsigned long long array_blocks =
- (((unsigned long long)__le32_to_cpu(dev->size_high)) << 32) +
- __le32_to_cpu(dev->size_low);
+ unsigned long long array_blocks;

memset(migr_rec, 0, sizeof(struct migr_record));
migr_rec->family_num = __cpu_to_le32(super->anchor->family_num);
@@ -7640,7 +7637,7 @@ void init_migr_record_imsm(struct supertype *st, struct imsm_dev *dev,
__cpu_to_le32(migr_rec->dest_depth_per_unit * new_data_disks);
migr_rec->dest_depth_per_unit =
__cpu_to_le32(migr_rec->dest_depth_per_unit);
-
+ array_blocks = info->component_size * new_data_disks;
num_migr_units =
array_blocks / __le32_to_cpu(migr_rec->blocks_per_unit);

@@ -8737,10 +8734,7 @@ static int imsm_manage_reshape(
goto abort;
}

- max_position =
- __le32_to_cpu(migr_rec->post_migr_vol_cap) +
- ((unsigned long long)__le32_to_cpu(
- migr_rec->post_migr_vol_cap_hi) << 32);
+ max_position = sra->component_size * ndata;

while (__le32_to_cpu(migr_rec->curr_migr_unit) <
__le32_to_cpu(migr_rec->num_migr_units)) {

--
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 06/21] imsm: FIX: Check layout for level migration

am 08.06.2011 18:10:22 von adam.kwolek

When user doesn't specify raid 5 layout for raid0->rai5 migration,
layout structure member is uninitialized. Earlier it cannot be determined
if it is correct or not.
In metadata handle proper verification is placed.

Signed-off-by: Adam Kwolek
Signed-off-by: Krzysztof Wojcik
---

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

diff --git a/super-intel.c b/super-intel.c
index 2468968..3baea6a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8278,7 +8278,6 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
int chunk;

getinfo_super_imsm_volume(st, &info, NULL);
-
if ((geo->level != info.array.level) &&
(geo->level >= 0) &&
(geo->level != UnSet)) {
@@ -8286,6 +8285,14 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
case 0:
if (geo->level == 5) {
change = CH_MIGRATION;
+ if (geo->layout != ALGORITHM_LEFT_ASYMMETRIC) {
+ fprintf(stderr,
+ Name " Error. Requested Layout "
+ "not supported (left-asymmetric layout "
+ "is supported only)!\n");
+ change = -1;
+ goto analyse_change_exit;
+ }
check_devs = 1;
}
if (geo->level == 10) {

--
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 07/21] imsm: FIX: Use macros to data access

am 08.06.2011 18:10:30 von adam.kwolek

Metadata fields has to be accessed using proper macros.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 3baea6a..fea7a3a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1822,7 +1822,7 @@ static __u64 blocks_per_migr_unit(struct intel_super *super,
migr_chunk = migr_strip_blocks_resync(dev);
disks = imsm_num_data_members(dev, 0);
blocks_per_unit = stripes_per_unit * migr_chunk * disks;
- stripe = __le32_to_cpu(map->blocks_per_strip) * disks;
+ stripe = __le16_to_cpu(map->blocks_per_strip) * disks;
segment = blocks_per_unit / stripe;
block_rel = blocks_per_unit - segment * stripe;
parity_depth = parity_segment_depth(dev);
@@ -8716,7 +8716,7 @@ static int imsm_manage_reshape(
ndata = imsm_num_data_members(dev, 0);
odata = imsm_num_data_members(dev, 1);

- chunk = map_src->blocks_per_strip * 512;
+ chunk = __le16_to_cpu(map_src->blocks_per_strip) * 512;
old_data_stripe_length = odata * chunk;

migr_rec = super->migr_rec;
@@ -8765,7 +8765,8 @@ static int imsm_manage_reshape(
if ((current_position + next_step) > max_position)
next_step = max_position - current_position;

- start = (map_src->pba_of_lba0 + dev->reserved_blocks +
+ start = (__le32_to_cpu(map_src->pba_of_lba0) +
+ __le32_to_cpu(dev->reserved_blocks) +
current_position) * 512;

/* allign reading start to old geometry */

--
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 08/21] imsm: FIX: Calculate backup location based on metadata

am 08.06.2011 18:10:38 von adam.kwolek

Use metadata information to calculate backup write offset.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index fea7a3a..0d46132 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7850,7 +7850,7 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)

write_offset = ((unsigned long long)
__le32_to_cpu(migr_rec->dest_1st_member_lba) +
- info->data_offset) * 512;
+ __le32_to_cpu(map_dest->pba_of_lba0)) * 512;

unit_len = __le32_to_cpu(migr_rec->dest_depth_per_unit) * 512;
if (posix_memalign((void **)&buf, 512, unit_len) != 0)

--
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/21] imsm: FIX: Do not verify unused parameters

am 08.06.2011 18:10:46 von adam.kwolek

Parameters that are not used by imsm_manage_reshape() should not cause
failure of this function.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 0d46132..437975f 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8689,7 +8689,7 @@ static int imsm_manage_reshape(
unsigned long long start_buf_shift; /* [bytes] */
int degraded = 0;

- if (!fds || !offsets || !destfd || !destoffsets || !sra)
+ if (!fds || !offsets || !sra)
goto abort;

/* Find volume during the reshape */

--
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 10/21] imsm: FIX: Do not use pba_of_lba0 for copy position

am 08.06.2011 18:10:54 von adam.kwolek

imsm_manage_reshape() should not shift start copy position.
This offset is passed to manage reshape function /and it is used/
as input parameter in offsets table already.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 437975f..b22c7df 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8765,9 +8765,7 @@ static int imsm_manage_reshape(
if ((current_position + next_step) > max_position)
next_step = max_position - current_position;

- start = (__le32_to_cpu(map_src->pba_of_lba0) +
- __le32_to_cpu(dev->reserved_blocks) +
- current_position) * 512;
+ start = current_position * 512;

/* allign reading start to old geometry */
start_buf_shift = start % old_data_stripe_length;

--
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 11/21] imsm: FIX: Remove unused parameter from

am 08.06.2011 18:11:02 von adam.kwolek

new_data parameter is not used in save_backup_imsm().
It is removed from function interface.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index b22c7df..708b51d 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7677,9 +7677,9 @@ void init_migr_record_imsm(struct supertype *st, struct imsm_dev *dev,
* and to write it to the Copy Area.
* Parameters:
* st : supertype information
+ * dev : imsm device that backup is saved for
* info : general array info
* buf : input buffer
- * write_offset : address of data to backup
* length : length of data to backup (blocks_per_unit)
* Returns:
* 0 : success
@@ -7689,7 +7689,6 @@ int save_backup_imsm(struct supertype *st,
struct imsm_dev *dev,
struct mdinfo *info,
void *buf,
- int new_data,
int length)
{
int rv = -1;
@@ -8811,8 +8810,7 @@ static int imsm_manage_reshape(
* in backup general migration area
*/
if (save_backup_imsm(st, dev, sra,
- buf + start_buf_shift,
- ndata, copy_length)) {
+ buf + start_buf_shift, copy_length)) {
dprintf("imsm: Cannot save stripes to "
"target devices\n");
goto abort;

--
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 12/21] imsm: FIX: Use metadata information for

am 08.06.2011 18:11:10 von adam.kwolek

For raid0 reshape imsm uses degraded raid4 for this operation.
Using real raid level (raid0) for stripe calculation causes no need
for parity calculation and can speed up reshape process.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 708b51d..26083c3 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7698,6 +7698,8 @@ int save_backup_imsm(struct supertype *st,
int i;
struct imsm_map *map_dest = get_imsm_map(dev, 0);
int new_disks = map_dest->num_members;
+ int dest_layout = 0;
+ int dest_chunk;

targets = malloc(new_disks * sizeof(int));
if (!targets)
@@ -7716,15 +7718,19 @@ int save_backup_imsm(struct supertype *st,
if (open_backup_targets(info, new_disks, targets))
goto abort;

+ if (map_dest->raid_level != 0)
+ dest_layout = ALGORITHM_LEFT_ASYMMETRIC;
+ dest_chunk = __le16_to_cpu(map_dest->blocks_per_strip) * 512;
+
if (restore_stripes(targets, /* list of dest devices */
target_offsets, /* migration record offsets */
new_disks,
- info->new_chunk,
- info->new_level,
- info->new_layout,
- -1, /* source backup file descriptor */
- 0, /* input buf offset
- * always 0 buf is already offset */
+ dest_chunk,
+ map_dest->raid_level,
+ dest_layout,
+ -1, /* source backup file descriptor */
+ 0, /* input buf offset
+ * always 0 buf is already offseted */
0,
length,
buf) != 0) {
@@ -8687,6 +8693,7 @@ static int imsm_manage_reshape(
unsigned long long start; /* [bytes] */
unsigned long long start_buf_shift; /* [bytes] */
int degraded = 0;
+ int source_layout = 0;

if (!fds || !offsets || !sra)
goto abort;
@@ -8741,6 +8748,8 @@ static int imsm_manage_reshape(
}

max_position = sra->component_size * ndata;
+ if (map_src->raid_level != 0)
+ source_layout = ALGORITHM_LEFT_ASYMMETRIC;

while (__le32_to_cpu(migr_rec->curr_migr_unit) <
__le32_to_cpu(migr_rec->num_migr_units)) {
@@ -8797,8 +8806,8 @@ static int imsm_manage_reshape(
start_buf_shift, next_step_filler);

if (save_stripes(fds, offsets, map_src->num_members,
- chunk, sra->array.level,
- sra->array.layout, 0, NULL, start_src,
+ chunk, map_src->raid_level,
+ source_layout, 0, NULL, start_src,
copy_length +
next_step_filler + start_buf_shift,
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 13/21] imsm: FIX: Detect failed devices during

am 08.06.2011 18:11:18 von adam.kwolek

Detect in recover_backup_imsm() if not opened disks number is smaller
than allowed degradation for given raid level. This allows for reshape restart
on degraded array.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 26083c3..c19ffac 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7826,6 +7826,8 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
unsigned long num_migr_units = __le32_to_cpu(migr_rec->num_migr_units);
int ascending = __le32_to_cpu(migr_rec->ascending_migr);
char buffer[20];
+ int skipped_disks = 0;
+ int max_degradation;

err = sysfs_get_str(info, NULL, "array_state", (char *)buffer, 20);
if (err < 1)
@@ -7849,6 +7851,7 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)

map_dest = get_imsm_map(id->dev, 0);
new_disks = map_dest->num_members;
+ max_degradation = new_disks - imsm_num_data_members(id->dev, 0);

read_offset = (unsigned long long)
__le32_to_cpu(migr_rec->ckpt_area_pba) * 512;
@@ -7867,6 +7870,10 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
open_backup_targets(info, new_disks, targets);

for (i = 0; i < new_disks; i++) {
+ if (targets[i] < 0) {
+ skipped_disks++;
+ continue;
+ }
if (lseek64(targets[i], read_offset, SEEK_SET) < 0) {
fprintf(stderr,
Name ": Cannot seek to block: %s\n",
@@ -7893,6 +7900,13 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
}
}

+ if (skipped_disks > max_degradation) {
+ fprintf(stderr,
+ Name ": Cannot restore data from backup."
+ " Too many failed disks\n");
+ goto abort;
+ }
+
if (ascending && curr_migr_unit < (num_migr_units-1))
curr_migr_unit++;


--
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 14/21] imsm: FIX: Move reshape_progress forward

am 08.06.2011 18:11:26 von adam.kwolek

When array under reshape is assembled, reshape position used in sysfs_set_array()
should be set to position after recovered from backup area.
This avoids data corruption due to reshape the same array area again.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index c19ffac..55829cf 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2194,6 +2194,13 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
unsigned long long array_blocks;
int used_disks;

+ if (__le32_to_cpu(migr_rec->ascending_migr) &&
+ (units <
+ (__le32_to_cpu(migr_rec->num_migr_units)-1)) &&
+ (super->migr_rec->rec_status ==
+ __cpu_to_le32(UNIT_SRC_IN_CP_AREA)))
+ units++;
+
info->reshape_progress = blocks_per_unit * units;

dprintf("IMSM: General Migration checkpoint : %llu "
@@ -7824,7 +7831,6 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
int retval = 1;
unsigned long curr_migr_unit = __le32_to_cpu(migr_rec->curr_migr_unit);
unsigned long num_migr_units = __le32_to_cpu(migr_rec->num_migr_units);
- int ascending = __le32_to_cpu(migr_rec->ascending_migr);
char buffer[20];
int skipped_disks = 0;
int max_degradation;
@@ -7907,16 +7913,13 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
goto abort;
}

- if (ascending && curr_migr_unit < (num_migr_units-1))
- curr_migr_unit++;
-
- migr_rec->curr_migr_unit = __le32_to_cpu(curr_migr_unit);
- super->migr_rec->rec_status = __cpu_to_le32(UNIT_SRC_NORMAL);
- if (write_imsm_migr_rec(st) == 0) {
- __u64 blocks_per_unit = blocks_per_migr_unit(super, id->dev);
- info->reshape_progress = curr_migr_unit * blocks_per_unit;
+ if (save_checkpoint_imsm(st, info, UNIT_SRC_NORMAL)) {
+ /* ignore error == 2, this can mean end of reshape here
+ */
+ dprintf("imsm: Cannot write checkpoint to "
+ "migration record (UNIT_SRC_NORMAL) during restart\n");
+ } else
retval = 0;
- }

abort:
if (targets) {

--
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 15/21] imsm: FIX: Remove unused variables and code

am 08.06.2011 18:11:34 von adam.kwolek

Unused variables and code can be removed from imsm_manage_reshape()

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 55829cf..2dd73c0 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8695,7 +8695,7 @@ static int imsm_manage_reshape(
struct intel_super *super = st->sb;
struct intel_dev *dv = NULL;
struct imsm_dev *dev = NULL;
- struct imsm_map *map_src, *map_dest;
+ struct imsm_map *map_src;
int migr_vol_qan = 0;
int ndata, odata; /* [bytes] */
int chunk; /* [bytes] */
@@ -8705,7 +8705,6 @@ static int imsm_manage_reshape(
unsigned long long max_position; /* array size [bytes] */
unsigned long long next_step; /* [blocks]/[bytes] */
unsigned long long old_data_stripe_length;
- unsigned long long new_data_stripe_length;
unsigned long long start_src; /* [bytes] */
unsigned long long start; /* [bytes] */
unsigned long long start_buf_shift; /* [bytes] */
@@ -8734,7 +8733,6 @@ static int imsm_manage_reshape(
map_src = get_imsm_map(dev, 1);
if (map_src == NULL)
goto abort;
- map_dest = get_imsm_map(dev, 0);

ndata = imsm_num_data_members(dev, 0);
odata = imsm_num_data_members(dev, 1);
@@ -8744,11 +8742,6 @@ static int imsm_manage_reshape(

migr_rec = super->migr_rec;

- /* [bytes] */
- sra->new_chunk = __le16_to_cpu(map_dest->blocks_per_strip) * 512;
- sra->new_level = map_dest->raid_level;
- new_data_stripe_length = sra->new_chunk * ndata;
-
/* initialize migration record for start condition */
if (sra->reshape_progress == 0)
init_migr_record_imsm(st, dev, sra);
@@ -8847,11 +8840,6 @@ static int imsm_manage_reshape(
"migration record (UNIT_SRC_IN_CP_AREA)\n");
goto abort;
}
- /* decrease backup_blocks */
- if (backup_blocks > (unsigned long)next_step)
- backup_blocks -= next_step;
- else
- backup_blocks = 0;
}
/* When data backed up, checkpoint stored,
* kick the kernel to reshape unit of data

--
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 16/21] FIX: Move buffer to next location

am 08.06.2011 18:11:42 von adam.kwolek

When no output file is given save_stripes() should collect amount of stripes
in passed buffer. Currently all stripes are saved in the same area in passed
buffer. This causes that last stripe is returned on buffer begin only.
Increase buffer (buf) pointer when save_stripes() is about switch to next
stripe operation. This allows for proper buffer filling as input parameter
length directs.

Signed-off-by: Adam Kwolek
Signed-off-by: Krzysztof Wojcik
---

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

diff --git a/restripe.c b/restripe.c
index 1c42b60..79c695d 100644
--- a/restripe.c
+++ b/restripe.c
@@ -652,10 +652,14 @@ int save_stripes(int *source, unsigned long long *offsets,
fdisk[0], fdisk[1], bufs);
}
}
- if (dest)
+ if (dest) {
for (i = 0; i < nwrites; i++)
if (write(dest[i], buf, len) != len)
return -1;
+ } else {
+ /* build next stripe in buffer */
+ buf += len;
+ }
length -= len;
start += len;
}

--
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 17/21] imsm: FIX: Do not continue reshape when backup exists

am 08.06.2011 18:11:50 von adam.kwolek

When backup exists in copy area reshape cannot be continued.
In such situation, array is in unstable state.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 2dd73c0..25e706f 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8745,6 +8745,13 @@ static int imsm_manage_reshape(
/* initialize migration record for start condition */
if (sra->reshape_progress == 0)
init_migr_record_imsm(st, dev, sra);
+ else {
+ if (__le32_to_cpu(migr_rec->rec_status) != UNIT_SRC_NORMAL) {
+ dprintf("imsm: cannot restart migration when data "
+ "are present in copy area.\n");
+ goto abort;
+ }
+ }

/* size for data */
buf_size = __le32_to_cpu(migr_rec->blocks_per_unit) * 512;

--
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 18/21] imsm: FIX: wait_for_reshape_imsm() cleanup

am 08.06.2011 18:11:58 von adam.kwolek

This function needs to be corrected.
It should check sysfs operations status and it should not interpret
0 reshape position special meaning.

Unused input parameter is removed also.

Signed-off-by: Adam Kwolek
---

super-intel.c | 54 +++++++++++++++++++++++++++++++++---------------------
1 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 25e706f..8806339 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8560,39 +8560,50 @@ exit_imsm_reshape_super:
* reshape process reach new position
* Parameters:
* sra : general array info
- * to_complete : new sync_max position
* ndata : number of disks in new array's layout
* Returns:
* 0 : success,
* 1 : there is no reshape in progress,
* -1 : fail
************************************************************ ******************/
-int wait_for_reshape_imsm(struct mdinfo *sra, unsigned long long to_complete,
- int ndata)
+int wait_for_reshape_imsm(struct mdinfo *sra, int ndata)
{
int fd = sysfs_get_fd(sra, NULL, "reshape_position");
unsigned long long completed;
+ /* to_complete : new sync_max position */
+ unsigned long long to_complete = sra->reshape_progress;
+ unsigned long long position_to_set = to_complete / ndata;

struct timeval timeout;

- if (fd < 0)
+ if (fd < 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot open reshape_position\n");
return 1;
+ }

- sysfs_fd_get_ll(fd, &completed);
+ if (sysfs_fd_get_ll(fd, &completed) < 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot read reshape_position (no reshape in progres)\n");
+ close(fd);
+ return 0;
+ }

- if (to_complete == 0) {/* reshape till the end of array */
- sysfs_set_str(sra, NULL, "sync_max", "max");
- to_complete = MaxSector;
- } else {
- if (completed > to_complete) {
- close(fd);
- return -1;
- }
- if (sysfs_set_num(sra, NULL, "sync_max",
- to_complete / ndata) != 0) {
- close(fd);
- return -1;
- }
+ if (completed > to_complete) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "wrong next position to set %llu (%llu)\n",
+ to_complete, completed);
+ close(fd);
+ return -1;
+ }
+ dprintf("Position set: %llu\n", position_to_set);
+ if (sysfs_set_num(sra, NULL, "sync_max",
+ position_to_set) != 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot set reshape position to %llu\n",
+ position_to_set);
+ close(fd);
+ return -1;
}

/* FIXME should not need a timeout at all */
@@ -8605,6 +8616,8 @@ int wait_for_reshape_imsm(struct mdinfo *sra, unsigned long long to_complete,
FD_SET(fd, &rfds);
select(fd+1, NULL, NULL, &rfds, &timeout);
if (sysfs_fd_get_ll(fd, &completed) < 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot read reshape_position (in loop)\n");
close(fd);
return 1;
}
@@ -8854,15 +8867,14 @@ static int imsm_manage_reshape(
next_step = next_step + sra->reshape_progress;
sysfs_set_num(sra, NULL, "suspend_lo", sra->reshape_progress);
sysfs_set_num(sra, NULL, "suspend_hi", next_step);
+ sra->reshape_progress = next_step;

/* wait until reshape finish */
- if (wait_for_reshape_imsm(sra, next_step, ndata) < 0) {
+ if (wait_for_reshape_imsm(sra, ndata) < 0) {
dprintf("wait_for_reshape_imsm returned error!\n");
goto abort;
}

- sra->reshape_progress = next_step;
-
if (save_checkpoint_imsm(st, sra, UNIT_SRC_NORMAL) == 1) {
/* ignore error == 2, this can mean end of reshape here
*/

--
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 19/21] imsm: FIX: Remove timeout from wait_for_reshape_imsm()

am 08.06.2011 18:12:06 von adam.kwolek

Timeout should not be used for select function in wait_for_reshape_imsm().

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 8806339..fae1218 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8574,8 +8574,6 @@ int wait_for_reshape_imsm(struct mdinfo *sra, int ndata)
unsigned long long to_complete = sra->reshape_progress;
unsigned long long position_to_set = to_complete / ndata;

- struct timeval timeout;
-
if (fd < 0) {
dprintf("imsm: wait_for_reshape_imsm() "
"cannot open reshape_position\n");
@@ -8606,25 +8604,22 @@ int wait_for_reshape_imsm(struct mdinfo *sra, int ndata)
return -1;
}

- /* FIXME should not need a timeout at all */
- timeout.tv_sec = 30;
- timeout.tv_usec = 0;
do {
char action[20];
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
- select(fd+1, NULL, NULL, &rfds, &timeout);
+ select(fd+1, &rfds, NULL, NULL, NULL);
+ if (sysfs_get_str(sra, NULL, "sync_action",
+ action, 20) > 0 &&
+ strncmp(action, "reshape", 7) != 0)
+ break;
if (sysfs_fd_get_ll(fd, &completed) < 0) {
dprintf("imsm: wait_for_reshape_imsm() "
"cannot read reshape_position (in loop)\n");
close(fd);
return 1;
}
- if (sysfs_get_str(sra, NULL, "sync_action",
- action, 20) > 0 &&
- strncmp(action, "reshape", 7) != 0)
- break;
} while (completed < to_complete);
close(fd);
return 0;

--
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 20/21] imsm: Optimize expansion speed when no backup is

am 08.06.2011 18:12:14 von adam.kwolek

When no reshape backup is required (e.g. OLCE after critical section),
check-pointing can use bigger steps than backup space allows for.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index fae1218..71a1189 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8855,11 +8855,19 @@ static int imsm_manage_reshape(
"migration record (UNIT_SRC_IN_CP_AREA)\n");
goto abort;
}
+ } else {
+ /* set next step to use whole border area */
+ border /= next_step;
+ if (border > 1)
+ next_step *= border;
}
/* When data backed up, checkpoint stored,
* kick the kernel to reshape unit of data
*/
next_step = next_step + sra->reshape_progress;
+ /* limit next step to array max position */
+ if (next_step > max_position)
+ next_step = max_position;
sysfs_set_num(sra, NULL, "suspend_lo", sra->reshape_progress);
sysfs_set_num(sra, NULL, "suspend_hi", next_step);
sra->reshape_progress = next_step;

--
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 21/21] MAN: Man update for check-pointing

am 08.06.2011 18:12:22 von adam.kwolek

Signed-off-by: Adam Kwolek
---

mdadm.8.in | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/mdadm.8.in b/mdadm.8.in
index e1d5651..f549dbf 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -2225,8 +2225,8 @@ succeed.
This is for the following reasons:

.IP 1.
-Intel's native IMSM check-pointing is not fully implemented yet.
-This causes IMSM incompatibility during the grow process: an array
+Intel's native IMSM check-pointing is not fully tested yet.
+This can causes IMSM incompatibility during the grow process: an array
which is growing cannot roam between Microsoft Windows(R) and Linux
systems.

@@ -2234,6 +2234,11 @@ systems.
Interrupting a grow operation is not recommended, because it
has not been fully tested for Intel's IMSM container format yet.

+.PP
+Note: Intel's native checkpointing doesn't use
+.B --backup-file
+option and it is transparent for assembly feature.
+
.SS SIZE CHANGES
Normally when an array is built the "size" is taken from the smallest
of the drives. If all the small drives in an arrays are, one at a

--
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 01/21] imsm: FIX: Cannot create volume

am 09.06.2011 04:42:18 von NeilBrown

On Wed, 08 Jun 2011 18:09:41 +0200 Adam Kwolek wrote:

> Clearing info structure causes mdadm is not able to create workable volume.
>
> During volume creation info structure passed to getinfo() function
> contains some information already and cannot be cleared.
>
> Signed-off-by: Adam Kwolek
> ---
>
> super-intel.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index b8d8b4e..471dbd2 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -2075,7 +2075,6 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
> unsigned int component_size_alligment;
> int map_disks = info->array.raid_disks;
>
> - memset(info, 0, sizeof(*info));
> if (prev_map)
> map_to_analyse = prev_map;
>
>
> --
> 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'm sorry, but this is just a very silly patch.

In many caches the 'info' structure is completely uninitialised, so it really
does make sense to initialise it, which is why I added that.
Just removing it *must* be wrong.

It would be much more helpful if you had explained what the "some
information" was.

Maybe it is the '->next' field that container_content_imsm() sets before
calling getinfo_super_imsm_volume()?? Well that getinfo_super_imsm_volume
doesn't use that field, so we can just move the assignment afterwards.

or maybe ... getinfo_super_imsm_volume uses info->disk.raid_disk, but no
caller ever sets that up that I can see, so that code is plainly wrong
(though ddf makes the same mistake so that is probably my fault).
I've 'fixed' them both to just report on the 'first' disk as that is no worse
than what they currently do.

I that doesn't fix your problem, please explain exactly what is being cleared
that shouldn't be.

NeilBrown

commit 9894ec0d64a9faab719d016bbbf5fbc842757df6
Author: NeilBrown
Date: Thu Jun 9 12:42:02 2011 +1000

Fix some fall-out from recent memset-zero for getinfo_super

container_content_imsm was setting info->next before calling
getinfo_super_imsm_container which now zeros everything.
So move that assignment to afterwards.

So both imsm and ddf were assuming info->disk.raid_disk means
something but it doesn't. So fix those.

Signed-off-by: NeilBrown

diff --git a/super-ddf.c b/super-ddf.c
index 21a917e..3fba2eb 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1429,9 +1429,7 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
info->component_size = __be64_to_cpu(vc->conf.blocks);
}

- for (dl = ddf->dlist; dl ; dl = dl->next)
- if (dl->raiddisk == info->disk.raid_disk)
- break;
+ dl = ddf->dlist;
info->disk.major = 0;
info->disk.minor = 0;
if (dl) {
diff --git a/super-intel.c b/super-intel.c
index b8d8b4e..6fed9eb 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2079,9 +2079,8 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
if (prev_map)
map_to_analyse = prev_map;

- for (dl = super->disks; dl; dl = dl->next)
- if (dl->raiddisk == info->disk.raid_disk)
- break;
+ dl = super->disks;
+
info->container_member = super->current_vol;
info->array.raid_disks = map->num_members;
info->array.level = get_imsm_raid_level(map_to_analyse);
@@ -5446,11 +5445,10 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
sizeof(*this));
break;
}
- memset(this, 0, sizeof(*this));
- this->next = rest;

super->current_vol = i;
getinfo_super_imsm_volume(st, this, NULL);
+ this->next = rest;
for (slot = 0 ; slot < map->num_members; slot++) {
unsigned long long recovery_start;
struct mdinfo *info_d;

--
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 00/21] IMSM Checkpointing Bug Fix Series

am 09.06.2011 05:03:29 von NeilBrown

On Wed, 08 Jun 2011 18:09:33 +0200 Adam Kwolek wrote:

> The following series fixes problems found in IMSM's checkpointing.
> It contains rework based on Neil's comments to previous/initial checkpointing
> series and tt should be applied on neil_master branch (on the top
> of previous checkpointing patches).
>
> BR
> Adam
>
>
> ---
>
> Adam Kwolek (21):
> MAN: Man update for check-pointing
> imsm: Optimize expansion speed when no backup is required
> imsm: FIX: Remove timeout from wait_for_reshape_imsm()
> imsm: FIX: wait_for_reshape_imsm() cleanup
> imsm: FIX: Do not continue reshape when backup exists
> FIX: Move buffer to next location
> imsm: FIX: Remove unused variables and code
> imsm: FIX: Move reshape_progress forward
> imsm: FIX: Detect failed devices during recover_backup_imsm()
> imsm: FIX: Use metadata information for restore_stripes() and save_stripes()
> imsm: FIX: Remove unused parameter from save_backup_imsm() interface
> imsm: FIX: Do not use pba_of_lba0 for copy position calculation
> imsm: FIX: Do not verify unused parameters
> imsm: FIX: Calculate backup location based on metadata information
> imsm: FIX: Use macros to data access
> imsm: FIX: Check layout for level migration
> imsm: FIX: Max position could not be rounded to MB
> imsm: FIX: Detect migration end during migration record saving
> imsm: FIX: Verify if migration record is loaded correctly
> imsm: FIX: Opened handle is not closed
> imsm: FIX: Cannot create volume
>
>
> mdadm.8.in | 9 ++
> restripe.c | 6 +
> super-intel.c | 237 ++++++++++++++++++++++++++++++++++-----------------------
> 3 files changed, 153 insertions(+), 99 deletions(-)
>

Thanks.
Apart from the first one which I have already commented on, these look good.
I have applied them all, thanks.

Please confirm that it all still works for you and let me know if you have
any other changes pending that I should know about.

Thanks,
NeilBrown

--
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 01/21] imsm: FIX: Cannot create volume

am 09.06.2011 08:40:45 von adam.kwolek

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Thursday, June 09, 2011 4:42 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 01/21] imsm: FIX: Cannot create volume
>
> On Wed, 08 Jun 2011 18:09:41 +0200 Adam Kwolek
> wrote:
>
> > Clearing info structure causes mdadm is not able to create workable
> volume.
> >
> > During volume creation info structure passed to getinfo() function
> > contains some information already and cannot be cleared.
> >
> > Signed-off-by: Adam Kwolek
> > ---
> >
> > super-intel.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index b8d8b4e..471dbd2 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -2075,7 +2075,6 @@ static void getinfo_super_imsm_volume(struct
> supertype *st, struct mdinfo *info,
> > unsigned int component_size_alligment;
> > int map_disks = info->array.raid_disks;
> >
> > - memset(info, 0, sizeof(*info));
> > if (prev_map)
> > map_to_analyse = prev_map;
> >
> >
> > --
> > 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'm sorry, but this is just a very silly patch.


You are right in your suspicions about root cause. It is link problem using 'next' field.
(I can see this problem at this moment)

I'll review it and I'll make it less silly ;)

BR
Adam

>
> In many caches the 'info' structure is completely uninitialised, so it
> really
> does make sense to initialise it, which is why I added that.
> Just removing it *must* be wrong.
>
> It would be much more helpful if you had explained what the "some
> information" was.
>
> Maybe it is the '->next' field that container_content_imsm() sets before
> calling getinfo_super_imsm_volume()?? Well that
> getinfo_super_imsm_volume
> doesn't use that field, so we can just move the assignment afterwards.
>
> or maybe ... getinfo_super_imsm_volume uses info->disk.raid_disk, but no
> caller ever sets that up that I can see, so that code is plainly wrong
> (though ddf makes the same mistake so that is probably my fault).
> I've 'fixed' them both to just report on the 'first' disk as that is no
> worse
> than what they currently do.
>
> I that doesn't fix your problem, please explain exactly what is being
> cleared
> that shouldn't be.
>
> NeilBrown
>
> commit 9894ec0d64a9faab719d016bbbf5fbc842757df6
> Author: NeilBrown
> Date: Thu Jun 9 12:42:02 2011 +1000
>
> Fix some fall-out from recent memset-zero for getinfo_super
>
> container_content_imsm was setting info->next before calling
> getinfo_super_imsm_container which now zeros everything.
> So move that assignment to afterwards.
>
> So both imsm and ddf were assuming info->disk.raid_disk means
> something but it doesn't. So fix those.
>
> Signed-off-by: NeilBrown
>
> diff --git a/super-ddf.c b/super-ddf.c
> index 21a917e..3fba2eb 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -1429,9 +1429,7 @@ static void getinfo_super_ddf_bvd(struct supertype
> *st, struct mdinfo *info, cha
> info->component_size = __be64_to_cpu(vc->conf.blocks);
> }
>
> - for (dl = ddf->dlist; dl ; dl = dl->next)
> - if (dl->raiddisk == info->disk.raid_disk)
> - break;
> + dl = ddf->dlist;
> info->disk.major = 0;
> info->disk.minor = 0;
> if (dl) {
> diff --git a/super-intel.c b/super-intel.c
> index b8d8b4e..6fed9eb 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -2079,9 +2079,8 @@ static void getinfo_super_imsm_volume(struct
> supertype *st, struct mdinfo *info,
> if (prev_map)
> map_to_analyse = prev_map;
>
> - for (dl = super->disks; dl; dl = dl->next)
> - if (dl->raiddisk == info->disk.raid_disk)
> - break;
> + dl = super->disks;
> +
> info->container_member = super->current_vol;
> info->array.raid_disks = map->num_members;
> info->array.level = get_imsm_raid_level(map_to_analyse);
> @@ -5446,11 +5445,10 @@ static struct mdinfo
> *container_content_imsm(struct supertype *st, char *subarra
> sizeof(*this));
> break;
> }
> - memset(this, 0, sizeof(*this));
> - this->next = rest;
>
> super->current_vol = i;
> getinfo_super_imsm_volume(st, this, NULL);
> + this->next = rest;
> for (slot = 0 ; slot < map->num_members; slot++) {
> unsigned long long recovery_start;
> struct mdinfo *info_d;

--
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