* [PATCH 0/4] raid0<->raid10 takeover for imsm metadata
@ 2011-01-17 15:42 Krzysztof Wojcik
2011-01-17 15:42 ` [PATCH 1/4] Add raid10 -> raid0 takeover support Krzysztof Wojcik
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Krzysztof Wojcik @ 2011-01-17 15:42 UTC (permalink / raw)
To: neilb
Cc: linux-raid, wojciech.neubauer, adam.kwolek, dan.j.williams,
ed.ciechanowski
The following series implements raid0 <-> raid10 takeover support
for imsm metadata format.
It includes corrections according last code review notes.
---
Krzysztof Wojcik (4):
Add raid10 -> raid0 takeover support
raid0->raid10 takeover- create metadata update
raid0->raid10 takeover- allocate memory for added disks
raid0->raid10 takeover- process metadata update
Grow.c | 1
super-intel.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 201 insertions(+), 1 deletions(-)
--
Krzysztof Wojcik
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/4] Add raid10 -> raid0 takeover support 2011-01-17 15:42 [PATCH 0/4] raid0<->raid10 takeover for imsm metadata Krzysztof Wojcik @ 2011-01-17 15:42 ` Krzysztof Wojcik 2011-01-26 0:58 ` Neil Brown 2011-01-17 15:42 ` [PATCH 2/4] raid0->raid10 takeover- create metadata update Krzysztof Wojcik ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Krzysztof Wojcik @ 2011-01-17 15:42 UTC (permalink / raw) To: neilb Cc: linux-raid, wojciech.neubauer, adam.kwolek, dan.j.williams, ed.ciechanowski The patch introduces takeover form level 10 to level 0 for imsm metadata. This patch contains procedures connected with preparing and applying metadata update during 10 -> 0 takeover. When performing takeover 10->0 mdmon should update the external metadata (due to disk slot and level changes). To achieve that mdadm calls reshape_super() and prepare the "update_takeover" metadata update type. Prepared update is processed by mdmon in process_update(). Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> --- Grow.c | 1 + super-intel.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 1 deletions(-) diff --git a/Grow.c b/Grow.c index ad13d6e..c4125ea 100644 --- a/Grow.c +++ b/Grow.c @@ -1463,6 +1463,7 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file, rv = 1; goto release; } + ping_monitor(container); } info.array = array; diff --git a/super-intel.c b/super-intel.c index 334e0f4..86b365d 100644 --- a/super-intel.c +++ b/super-intel.c @@ -299,6 +299,7 @@ enum imsm_update_type { update_rename_array, update_add_remove_disk, update_reshape_container_disks, + update_takeover }; struct imsm_update_activate_spare { @@ -319,6 +320,15 @@ struct geo_params { int raid_disks; }; +enum takeover_direction { + R10_TO_R0, + R0_TO_R10 +}; +struct imsm_update_takeover { + enum imsm_update_type type; + int subarray; + enum takeover_direction direction; +}; struct imsm_update_reshape { enum imsm_update_type type; @@ -5802,6 +5812,56 @@ update_reshape_exit: return ret_val; } +static int apply_takeover_update(struct imsm_update_takeover *u, + struct intel_super *super) +{ + struct imsm_dev *dev = NULL; + struct imsm_map *map; + struct dl *dm, *du; + struct intel_dev *dv; + + for (dv = super->devlist; dv; dv = dv->next) + if (dv->index == (unsigned int)u->subarray) { + dev = dv->dev; + break; + } + + if (dev == NULL) + return 0; + + map = get_imsm_map(dev, 0); + + if (u->direction == R10_TO_R0) { + /* iterate through devices to mark removed disks as spare */ + for (dm = super->disks; dm; dm = dm->next) { + if (dm->disk.status & FAILED_DISK) { + int idx = dm->index; + /* update indexes on the disk list */ + for (du = super->disks; du; du = du->next) + if (du->index > idx) + du->index--; + /* mark as spare disk */ + dm->disk.status = SPARE_DISK; + dm->index = -1; + } + } + + /* update map */ + map->num_members = map->num_members / 2; + map->map_state = IMSM_T_STATE_NORMAL; + map->num_domains = 1; + map->raid_level = 0; + map->failed_disk_num = -1; + } + + /* update disk order table */ + for (du = super->disks; du; du = du->next) + if (du->index >= 0) + set_imsm_ord_tbl_ent(map, du->index, du->index); + + return 1; +} + static void imsm_process_update(struct supertype *st, struct metadata_update *update) { @@ -5844,6 +5904,13 @@ static void imsm_process_update(struct supertype *st, mpb = super->anchor; switch (type) { + case update_takeover: { + struct imsm_update_takeover *u = (void *)update->buf; + if (apply_takeover_update(u, super)) + super->updates_pending++; + break; + } + case update_reshape_container_disks: { struct imsm_update_reshape *u = (void *)update->buf; if (apply_reshape_container_disks_update( @@ -6691,6 +6758,35 @@ analyse_change_exit: return change; } +int imsm_takeover(struct supertype *st, struct geo_params *geo) +{ + struct intel_super *super = st->sb; + struct imsm_update_takeover *u; + + u = malloc(sizeof(struct imsm_update_takeover)); + if (u == NULL) + return 1; + + u->type = update_takeover; + u->subarray = super->current_vol; + + /* 10->0 transition */ + if (geo->level == 0) + u->direction = R10_TO_R0; + + /* update metadata locally */ + imsm_update_metadata_locally(st, u, + sizeof(struct imsm_update_takeover)); + /* and possibly remotely */ + if (st->update_tail) + append_metadata_update(st, u, + sizeof(struct imsm_update_takeover)); + else + free(u); + + return 0; +} + static int imsm_reshape_super(struct supertype *st, long long size, int level, int layout, int chunksize, int raid_disks, char *backup, char *dev, int verbose) @@ -6772,7 +6868,7 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level, change = imsm_analyze_change(st, &geo); switch (change) { case CH_TAKEOVER: - ret_val = 0; + ret_val = imsm_takeover(st, &geo); break; case CH_CHUNK_MIGR: ret_val = 0; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] Add raid10 -> raid0 takeover support 2011-01-17 15:42 ` [PATCH 1/4] Add raid10 -> raid0 takeover support Krzysztof Wojcik @ 2011-01-26 0:58 ` Neil Brown 2011-01-26 11:34 ` Wojcik, Krzysztof 0 siblings, 1 reply; 8+ messages in thread From: Neil Brown @ 2011-01-26 0:58 UTC (permalink / raw) To: Krzysztof Wojcik Cc: linux-raid, wojciech.neubauer, adam.kwolek, dan.j.williams, ed.ciechanowski On Mon, 17 Jan 2011 16:42:43 +0100 Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote: > The patch introduces takeover form level 10 to level 0 for imsm > metadata. This patch contains procedures connected with preparing > and applying metadata update during 10 -> 0 takeover. > When performing takeover 10->0 mdmon should update the external > metadata (due to disk slot and level changes). > To achieve that mdadm calls reshape_super() and prepare > the "update_takeover" metadata update type. > Prepared update is processed by mdmon in process_update(). I've applied this, but I'm not sure that I like it. I don't think mdadm needs to send a metadata update for 10->0 conversion. It should: - fail all the devices that aren't wanted - remove them - mdmon notices and updates the metadata - mdadm changes the level to RAID0 - mdmon notices and updates the metadata. Currently mdmon doesn't monitor 'level', but it could and probably should. However in the interest of forward progress I won't insist on that for now. Other problems: > > Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> > --- > Grow.c | 1 + > super-intel.c | 98 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files > changed, 98 insertions(+), 1 deletions(-) > > diff --git a/Grow.c b/Grow.c > index ad13d6e..c4125ea 100644 > --- a/Grow.c > +++ b/Grow.c > @@ -1463,6 +1463,7 @@ int Grow_reshape(char *devname, int fd, int > quiet, char *backup_file, rv = 1; > goto release; > } > + ping_monitor(container); Why is this here??? EVERY change should be justified in the notes at the top!!!!!!! > } > > info.array = array; > diff --git a/super-intel.c b/super-intel.c > index 334e0f4..86b365d 100644 > --- a/super-intel.c > +++ b/super-intel.c > @@ -299,6 +299,7 @@ enum imsm_update_type { > update_rename_array, > update_add_remove_disk, > update_reshape_container_disks, > + update_takeover > }; > > struct imsm_update_activate_spare { > @@ -319,6 +320,15 @@ struct geo_params { > int raid_disks; > }; > > +enum takeover_direction { > + R10_TO_R0, > + R0_TO_R10 > +}; > +struct imsm_update_takeover { > + enum imsm_update_type type; > + int subarray; > + enum takeover_direction direction; > +}; > > struct imsm_update_reshape { > enum imsm_update_type type; > @@ -5802,6 +5812,56 @@ update_reshape_exit: > return ret_val; > } > > +static int apply_takeover_update(struct imsm_update_takeover *u, > + struct intel_super *super) > +{ > + struct imsm_dev *dev = NULL; > + struct imsm_map *map; > + struct dl *dm, *du; > + struct intel_dev *dv; > + > + for (dv = super->devlist; dv; dv = dv->next) > + if (dv->index == (unsigned int)u->subarray) { > + dev = dv->dev; > + break; > + } > + > + if (dev == NULL) > + return 0; > + > + map = get_imsm_map(dev, 0); > + > + if (u->direction == R10_TO_R0) { > + /* iterate through devices to mark removed disks as > spare */ > + for (dm = super->disks; dm; dm = dm->next) { > + if (dm->disk.status & FAILED_DISK) { > + int idx = dm->index; > + /* update indexes on the disk list */ > + for (du = super->disks; du; du = > du->next) > + if (du->index > idx) > + du->index--; > + /* mark as spare disk */ > + dm->disk.status = SPARE_DISK; > + dm->index = -1; This looks like it is probably wrong, but I'm not exactly sure what it is trying to do so I cannot be sure. In particular the changes to 'index' seem to be based on some assumption that are not necessarily true. Could you please explain exactly what this is trying to achieve? > + } > + } > + > + /* update map */ > + map->num_members = map->num_members / 2; > + map->map_state = IMSM_T_STATE_NORMAL; > + map->num_domains = 1; I'm confused by this 'num_domains' thing. Could you please explain it for me. The term "parity domains" is meaningless to me in this context. Thanks, NeilBrown > + map->raid_level = 0; > + map->failed_disk_num = -1; > + } > + > + /* update disk order table */ > + for (du = super->disks; du; du = du->next) > + if (du->index >= 0) > + set_imsm_ord_tbl_ent(map, du->index, > du->index); + > + return 1; > +} > + > static void imsm_process_update(struct supertype *st, > struct metadata_update *update) > { > @@ -5844,6 +5904,13 @@ static void imsm_process_update(struct > supertype *st, mpb = super->anchor; > > switch (type) { > + case update_takeover: { > + struct imsm_update_takeover *u = (void *)update->buf; > + if (apply_takeover_update(u, super)) > + super->updates_pending++; > + break; > + } > + > case update_reshape_container_disks: { > struct imsm_update_reshape *u = (void *)update->buf; > if (apply_reshape_container_disks_update( > @@ -6691,6 +6758,35 @@ analyse_change_exit: > return change; > } > > +int imsm_takeover(struct supertype *st, struct geo_params *geo) > +{ > + struct intel_super *super = st->sb; > + struct imsm_update_takeover *u; > + > + u = malloc(sizeof(struct imsm_update_takeover)); > + if (u == NULL) > + return 1; > + > + u->type = update_takeover; > + u->subarray = super->current_vol; > + > + /* 10->0 transition */ > + if (geo->level == 0) > + u->direction = R10_TO_R0; > + > + /* update metadata locally */ > + imsm_update_metadata_locally(st, u, > + sizeof(struct > imsm_update_takeover)); > + /* and possibly remotely */ > + if (st->update_tail) > + append_metadata_update(st, u, > + sizeof(struct > imsm_update_takeover)); > + else > + free(u); > + > + return 0; > +} > + > static int imsm_reshape_super(struct supertype *st, long long size, > int level, int layout, int chunksize, int raid_disks, > char *backup, char *dev, int verbose) > @@ -6772,7 +6868,7 @@ static int imsm_reshape_super(struct supertype > *st, long long size, int level, change = imsm_analyze_change(st, > &geo); switch (change) { > case CH_TAKEOVER: > - ret_val = 0; > + ret_val = imsm_takeover(st, &geo); > break; > case CH_CHUNK_MIGR: > ret_val = 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/4] Add raid10 -> raid0 takeover support 2011-01-26 0:58 ` Neil Brown @ 2011-01-26 11:34 ` Wojcik, Krzysztof 2011-01-27 3:23 ` Neil Brown 0 siblings, 1 reply; 8+ messages in thread From: Wojcik, Krzysztof @ 2011-01-26 11:34 UTC (permalink / raw) To: Neil Brown Cc: linux-raid, Neubauer, Wojciech, Kwolek, Adam, Williams, Dan J, Ciechanowski, Ed > -----Original Message----- > From: Neil Brown [mailto:neilb@suse.de] > Sent: Wednesday, January 26, 2011 1:59 AM > To: Wojcik, Krzysztof > Cc: linux-raid@vger.kernel.org; Neubauer, Wojciech; Kwolek, Adam; > Williams, Dan J; Ciechanowski, Ed > Subject: Re: [PATCH 1/4] Add raid10 -> raid0 takeover support > > On Mon, 17 Jan 2011 16:42:43 +0100 > Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote: > > > The patch introduces takeover form level 10 to level 0 for imsm > > metadata. This patch contains procedures connected with preparing > > and applying metadata update during 10 -> 0 takeover. > > When performing takeover 10->0 mdmon should update the external > > metadata (due to disk slot and level changes). > > To achieve that mdadm calls reshape_super() and prepare > > the "update_takeover" metadata update type. > > Prepared update is processed by mdmon in process_update(). > > I've applied this, but I'm not sure that I like it. > > I don't think mdadm needs to send a metadata update for 10->0 > conversion. > It should: > - fail all the devices that aren't wanted > - remove them > - mdmon notices and updates the metadata > - mdadm changes the level to RAID0 > - mdmon notices and updates the metadata. > > Currently mdmon doesn't monitor 'level', but it could and probably > should. > However in the interest of forward progress I won't insist on that for > now. Ok. Thanks for patch applying. In the future we can consider to change > > Other problems: > > > > > Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> > > --- > > Grow.c | 1 + > > super-intel.c | 98 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files > > changed, 98 insertions(+), 1 deletions(-) > > > > diff --git a/Grow.c b/Grow.c > > index ad13d6e..c4125ea 100644 > > --- a/Grow.c > > +++ b/Grow.c > > @@ -1463,6 +1463,7 @@ int Grow_reshape(char *devname, int fd, int > > quiet, char *backup_file, rv = 1; > > goto release; > > } > > + ping_monitor(container); > > Why is this here??? EVERY change should be justified in the notes at > the top!!!!!!! > > > > } > > > > info.array = array; > > diff --git a/super-intel.c b/super-intel.c > > index 334e0f4..86b365d 100644 > > --- a/super-intel.c > > +++ b/super-intel.c > > @@ -299,6 +299,7 @@ enum imsm_update_type { > > update_rename_array, > > update_add_remove_disk, > > update_reshape_container_disks, > > + update_takeover > > }; > > > > struct imsm_update_activate_spare { > > @@ -319,6 +320,15 @@ struct geo_params { > > int raid_disks; > > }; > > > > +enum takeover_direction { > > + R10_TO_R0, > > + R0_TO_R10 > > +}; > > +struct imsm_update_takeover { > > + enum imsm_update_type type; > > + int subarray; > > + enum takeover_direction direction; > > +}; > > > > struct imsm_update_reshape { > > enum imsm_update_type type; > > @@ -5802,6 +5812,56 @@ update_reshape_exit: > > return ret_val; > > } > > > > +static int apply_takeover_update(struct imsm_update_takeover *u, > > + struct intel_super *super) > > +{ > > + struct imsm_dev *dev = NULL; > > + struct imsm_map *map; > > + struct dl *dm, *du; > > + struct intel_dev *dv; > > + > > + for (dv = super->devlist; dv; dv = dv->next) > > + if (dv->index == (unsigned int)u->subarray) { > > + dev = dv->dev; > > + break; > > + } > > + > > + if (dev == NULL) > > + return 0; > > + > > + map = get_imsm_map(dev, 0); > > + > > + if (u->direction == R10_TO_R0) { > > + /* iterate through devices to mark removed disks as > > spare */ > > + for (dm = super->disks; dm; dm = dm->next) { > > + if (dm->disk.status & FAILED_DISK) { > > + int idx = dm->index; > > + /* update indexes on the disk list */ > > + for (du = super->disks; du; du = > > du->next) > > + if (du->index > idx) > > + du->index--; > > + /* mark as spare disk */ > > + dm->disk.status = SPARE_DISK; > > + dm->index = -1; > > This looks like it is probably wrong, but I'm not exactly sure what it > is trying to do so I cannot be sure. > In particular the changes to 'index' seem to be based on some > assumption that are not necessarily true. > Could you please explain exactly what this is trying to achieve? When we remove disks from array, indexes of existing disks have to be updated to remove gaps between disks- if greater than removed disk index it should be decreased Example: 1. Raid10: Disk 1: index 0 Disk 2: index 1 Disk 3: index 2 Disk 4: index 3 2. Disk 2 and 4 have been removed RAID10: Disk 1: index 0 Disk 2(removed): index 1 Disk 3: index 2 Disk 4(removed): index 3 3. Level has been changed to "0" RAID0: Disk 1: index 0 Disk 2: index -1 Disk 3: index 1 Disk 4: index -1 4. At the end we have following layout: RAID0: Disk 1: index 0 Disk 2: index 1 > > > + } > > + } > > + > > + /* update map */ > > + map->num_members = map->num_members / 2; > > + map->map_state = IMSM_T_STATE_NORMAL; > > + map->num_domains = 1; > > I'm confused by this 'num_domains' thing. Could you please explain > it for me. The term "parity domains" is meaningless to me in this > context. "num_domains" is used to distinguish level 10 and level 1: For level 1: Level= 1 Num_domains= 1 For level 10: Level= 1 Num_domains= 2 For other levels (0, 5) num_domains= 1. > > Thanks, > NeilBrown > > > > > + map->raid_level = 0; > > + map->failed_disk_num = -1; > > + } > > + > > + /* update disk order table */ > > + for (du = super->disks; du; du = du->next) > > + if (du->index >= 0) > > + set_imsm_ord_tbl_ent(map, du->index, > > du->index); + > > + return 1; > > +} > > + > > static void imsm_process_update(struct supertype *st, > > struct metadata_update *update) > > { > > @@ -5844,6 +5904,13 @@ static void imsm_process_update(struct > > supertype *st, mpb = super->anchor; > > > > switch (type) { > > + case update_takeover: { > > + struct imsm_update_takeover *u = (void *)update->buf; > > + if (apply_takeover_update(u, super)) > > + super->updates_pending++; > > + break; > > + } > > + > > case update_reshape_container_disks: { > > struct imsm_update_reshape *u = (void *)update->buf; > > if (apply_reshape_container_disks_update( > > @@ -6691,6 +6758,35 @@ analyse_change_exit: > > return change; > > } > > > > +int imsm_takeover(struct supertype *st, struct geo_params *geo) > > +{ > > + struct intel_super *super = st->sb; > > + struct imsm_update_takeover *u; > > + > > + u = malloc(sizeof(struct imsm_update_takeover)); > > + if (u == NULL) > > + return 1; > > + > > + u->type = update_takeover; > > + u->subarray = super->current_vol; > > + > > + /* 10->0 transition */ > > + if (geo->level == 0) > > + u->direction = R10_TO_R0; > > + > > + /* update metadata locally */ > > + imsm_update_metadata_locally(st, u, > > + sizeof(struct > > imsm_update_takeover)); > > + /* and possibly remotely */ > > + if (st->update_tail) > > + append_metadata_update(st, u, > > + sizeof(struct > > imsm_update_takeover)); > > + else > > + free(u); > > + > > + return 0; > > +} > > + > > static int imsm_reshape_super(struct supertype *st, long long size, > > int level, int layout, int chunksize, int raid_disks, > > char *backup, char *dev, int verbose) > > @@ -6772,7 +6868,7 @@ static int imsm_reshape_super(struct supertype > > *st, long long size, int level, change = imsm_analyze_change(st, > > &geo); switch (change) { > > case CH_TAKEOVER: > > - ret_val = 0; > > + ret_val = imsm_takeover(st, &geo); > > break; > > case CH_CHUNK_MIGR: > > ret_val = 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] Add raid10 -> raid0 takeover support 2011-01-26 11:34 ` Wojcik, Krzysztof @ 2011-01-27 3:23 ` Neil Brown 0 siblings, 0 replies; 8+ messages in thread From: Neil Brown @ 2011-01-27 3:23 UTC (permalink / raw) To: Wojcik, Krzysztof Cc: linux-raid, Neubauer, Wojciech, Kwolek, Adam, Williams, Dan J, Ciechanowski, Ed On Wed, 26 Jan 2011 11:34:08 +0000 "Wojcik, Krzysztof" <krzysztof.wojcik@intel.com> wrote: > > > > -----Original Message----- > > From: Neil Brown [mailto:neilb@suse.de] > > Sent: Wednesday, January 26, 2011 1:59 AM > > To: Wojcik, Krzysztof > > Cc: linux-raid@vger.kernel.org; Neubauer, Wojciech; Kwolek, Adam; > > Williams, Dan J; Ciechanowski, Ed > > Subject: Re: [PATCH 1/4] Add raid10 -> raid0 takeover support > > > > On Mon, 17 Jan 2011 16:42:43 +0100 > > Krzysztof Wojcik <krzysztof.wojcik@intel.com> wrote: > > > > > The patch introduces takeover form level 10 to level 0 for imsm > > > metadata. This patch contains procedures connected with preparing > > > and applying metadata update during 10 -> 0 takeover. > > > When performing takeover 10->0 mdmon should update the external > > > metadata (due to disk slot and level changes). > > > To achieve that mdadm calls reshape_super() and prepare > > > the "update_takeover" metadata update type. > > > Prepared update is processed by mdmon in process_update(). > > > > I've applied this, but I'm not sure that I like it. > > > > I don't think mdadm needs to send a metadata update for 10->0 > > conversion. > > It should: > > - fail all the devices that aren't wanted > > - remove them > > - mdmon notices and updates the metadata > > - mdadm changes the level to RAID0 > > - mdmon notices and updates the metadata. > > > > Currently mdmon doesn't monitor 'level', but it could and probably > > should. > > However in the interest of forward progress I won't insist on that > > for now. > Ok. Thanks for patch applying. > In the future we can consider to change > > > > Other problems: > > > > > > > > Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> > > > --- > > > Grow.c | 1 + > > > super-intel.c | 98 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files > > > changed, 98 insertions(+), 1 deletions(-) > > > > > > diff --git a/Grow.c b/Grow.c > > > index ad13d6e..c4125ea 100644 > > > --- a/Grow.c > > > +++ b/Grow.c > > > @@ -1463,6 +1463,7 @@ int Grow_reshape(char *devname, int fd, int > > > quiet, char *backup_file, rv = 1; > > > goto release; > > > } > > > + ping_monitor(container); > > > > Why is this here??? EVERY change should be justified in the notes at > > the top!!!!!!! > > > > > > > } > > > > > > info.array = array; > > > diff --git a/super-intel.c b/super-intel.c > > > index 334e0f4..86b365d 100644 > > > --- a/super-intel.c > > > +++ b/super-intel.c > > > @@ -299,6 +299,7 @@ enum imsm_update_type { > > > update_rename_array, > > > update_add_remove_disk, > > > update_reshape_container_disks, > > > + update_takeover > > > }; > > > > > > struct imsm_update_activate_spare { > > > @@ -319,6 +320,15 @@ struct geo_params { > > > int raid_disks; > > > }; > > > > > > +enum takeover_direction { > > > + R10_TO_R0, > > > + R0_TO_R10 > > > +}; > > > +struct imsm_update_takeover { > > > + enum imsm_update_type type; > > > + int subarray; > > > + enum takeover_direction direction; > > > +}; > > > > > > struct imsm_update_reshape { > > > enum imsm_update_type type; > > > @@ -5802,6 +5812,56 @@ update_reshape_exit: > > > return ret_val; > > > } > > > > > > +static int apply_takeover_update(struct imsm_update_takeover *u, > > > + struct intel_super *super) > > > +{ > > > + struct imsm_dev *dev = NULL; > > > + struct imsm_map *map; > > > + struct dl *dm, *du; > > > + struct intel_dev *dv; > > > + > > > + for (dv = super->devlist; dv; dv = dv->next) > > > + if (dv->index == (unsigned int)u->subarray) { > > > + dev = dv->dev; > > > + break; > > > + } > > > + > > > + if (dev == NULL) > > > + return 0; > > > + > > > + map = get_imsm_map(dev, 0); > > > + > > > + if (u->direction == R10_TO_R0) { > > > + /* iterate through devices to mark removed disks > > > as spare */ > > > + for (dm = super->disks; dm; dm = dm->next) { > > > + if (dm->disk.status & FAILED_DISK) { > > > + int idx = dm->index; > > > + /* update indexes on the disk > > > list */ > > > + for (du = super->disks; du; du = > > > du->next) > > > + if (du->index > idx) > > > + du->index--; > > > + /* mark as spare disk */ > > > + dm->disk.status = SPARE_DISK; > > > + dm->index = -1; > > > > This looks like it is probably wrong, but I'm not exactly sure what > > it is trying to do so I cannot be sure. > > In particular the changes to 'index' seem to be based on some > > assumption that are not necessarily true. > > Could you please explain exactly what this is trying to achieve? > When we remove disks from array, indexes of existing disks have to be > updated to remove gaps between disks- if greater than removed disk > index it should be decreased Example: 1. Raid10: > Disk 1: index 0 > Disk 2: index 1 > Disk 3: index 2 > Disk 4: index 3 > 2. Disk 2 and 4 have been removed > RAID10: > Disk 1: index 0 > Disk 2(removed): index 1 > Disk 3: index 2 > Disk 4(removed): index 3 > 3. Level has been changed to "0" > RAID0: > Disk 1: index 0 > Disk 2: index -1 > Disk 3: index 1 > Disk 4: index -1 > 4. At the end we have following layout: > RAID0: > Disk 1: index 0 > Disk 2: index 1 And does this work if the devices are in a different order, or if some are missing? It would make more sense to divide-by-2 the indexes of the devices that you want to keep. > > > > > > > + } > > > + } > > > + > > > + /* update map */ > > > + map->num_members = map->num_members / 2; > > > + map->map_state = IMSM_T_STATE_NORMAL; > > > + map->num_domains = 1; > > > > I'm confused by this 'num_domains' thing. Could you please explain > > it for me. The term "parity domains" is meaningless to me in this > > context. > "num_domains" is used to distinguish level 10 and level 1: > For level 1: > Level= 1 > Num_domains= 1 > For level 10: > Level= 1 > Num_domains= 2 > > For other levels (0, 5) num_domains= 1. It would be great if you could include that in the code somewhere as a comment, as the numbers seem to be entirely arbitrary and not follow any clear pattern. Thanks. NeilBrown > > > > > Thanks, > > NeilBrown > > > > > > > > > + map->raid_level = 0; > > > + map->failed_disk_num = -1; > > > + } > > > + > > > + /* update disk order table */ > > > + for (du = super->disks; du; du = du->next) > > > + if (du->index >= 0) > > > + set_imsm_ord_tbl_ent(map, du->index, > > > du->index); + > > > + return 1; > > > +} > > > + > > > static void imsm_process_update(struct supertype *st, > > > struct metadata_update *update) > > > { > > > @@ -5844,6 +5904,13 @@ static void imsm_process_update(struct > > > supertype *st, mpb = super->anchor; > > > > > > switch (type) { > > > + case update_takeover: { > > > + struct imsm_update_takeover *u = (void > > > *)update->buf; > > > + if (apply_takeover_update(u, super)) > > > + super->updates_pending++; > > > + break; > > > + } > > > + > > > case update_reshape_container_disks: { > > > struct imsm_update_reshape *u = (void > > > *)update->buf; if (apply_reshape_container_disks_update( > > > @@ -6691,6 +6758,35 @@ analyse_change_exit: > > > return change; > > > } > > > > > > +int imsm_takeover(struct supertype *st, struct geo_params *geo) > > > +{ > > > + struct intel_super *super = st->sb; > > > + struct imsm_update_takeover *u; > > > + > > > + u = malloc(sizeof(struct imsm_update_takeover)); > > > + if (u == NULL) > > > + return 1; > > > + > > > + u->type = update_takeover; > > > + u->subarray = super->current_vol; > > > + > > > + /* 10->0 transition */ > > > + if (geo->level == 0) > > > + u->direction = R10_TO_R0; > > > + > > > + /* update metadata locally */ > > > + imsm_update_metadata_locally(st, u, > > > + sizeof(struct > > > imsm_update_takeover)); > > > + /* and possibly remotely */ > > > + if (st->update_tail) > > > + append_metadata_update(st, u, > > > + sizeof(struct > > > imsm_update_takeover)); > > > + else > > > + free(u); > > > + > > > + return 0; > > > +} > > > + > > > static int imsm_reshape_super(struct supertype *st, long long > > > size, int level, int layout, int chunksize, int raid_disks, > > > char *backup, char *dev, int > > > verbose) @@ -6772,7 +6868,7 @@ static int > > > imsm_reshape_super(struct supertype *st, long long size, int > > > level, change = imsm_analyze_change(st, &geo); switch (change) { > > > case CH_TAKEOVER: > > > - ret_val = 0; > > > + ret_val = imsm_takeover(st, > > > &geo); break; > > > case CH_CHUNK_MIGR: > > > ret_val = 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] raid0->raid10 takeover- create metadata update 2011-01-17 15:42 [PATCH 0/4] raid0<->raid10 takeover for imsm metadata Krzysztof Wojcik 2011-01-17 15:42 ` [PATCH 1/4] Add raid10 -> raid0 takeover support Krzysztof Wojcik @ 2011-01-17 15:42 ` Krzysztof Wojcik 2011-01-17 15:43 ` [PATCH 3/4] raid0->raid10 takeover- allocate memory for added disks Krzysztof Wojcik 2011-01-17 15:43 ` [PATCH 4/4] raid0->raid10 takeover- process metadata update Krzysztof Wojcik 3 siblings, 0 replies; 8+ messages in thread From: Krzysztof Wojcik @ 2011-01-17 15:42 UTC (permalink / raw) To: neilb Cc: linux-raid, wojciech.neubauer, adam.kwolek, dan.j.williams, ed.ciechanowski Create metadata update for raid0 -> raid10 takeover. Because we have no mdmon running for raid0 we have to update metadata using local update mechanism Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> --- super-intel.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/super-intel.c b/super-intel.c index 86b365d..3ba9962 100644 --- a/super-intel.c +++ b/super-intel.c @@ -6774,6 +6774,10 @@ int imsm_takeover(struct supertype *st, struct geo_params *geo) if (geo->level == 0) u->direction = R10_TO_R0; + /* 0->10 transition */ + if (geo->level == 10) + u->direction = R0_TO_R10; + /* update metadata locally */ imsm_update_metadata_locally(st, u, sizeof(struct imsm_update_takeover)); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] raid0->raid10 takeover- allocate memory for added disks 2011-01-17 15:42 [PATCH 0/4] raid0<->raid10 takeover for imsm metadata Krzysztof Wojcik 2011-01-17 15:42 ` [PATCH 1/4] Add raid10 -> raid0 takeover support Krzysztof Wojcik 2011-01-17 15:42 ` [PATCH 2/4] raid0->raid10 takeover- create metadata update Krzysztof Wojcik @ 2011-01-17 15:43 ` Krzysztof Wojcik 2011-01-17 15:43 ` [PATCH 4/4] raid0->raid10 takeover- process metadata update Krzysztof Wojcik 3 siblings, 0 replies; 8+ messages in thread From: Krzysztof Wojcik @ 2011-01-17 15:43 UTC (permalink / raw) To: neilb Cc: linux-raid, wojciech.neubauer, adam.kwolek, dan.j.williams, ed.ciechanowski Allocate memory will be used in process_update. For raid0->raid10 takeover operation number of disks doubles so we should allocate memory for additional disks and one imsm_dev structure with extended order table. Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> --- super-intel.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 48 insertions(+), 0 deletions(-) diff --git a/super-intel.c b/super-intel.c index 3ba9962..3b2a273 100644 --- a/super-intel.c +++ b/super-intel.c @@ -6231,6 +6231,54 @@ static void imsm_prepare_update(struct supertype *st, size_t len = 0; switch (type) { + case update_takeover: { + struct imsm_update_takeover *u = (void *)update->buf; + if (u->direction == R0_TO_R10) { + void **tail = (void **)&update->space_list; + struct imsm_dev *dev = get_imsm_dev(super, u->subarray); + struct imsm_map *map = get_imsm_map(dev, 0); + int num_members = map->num_members; + void *space; + int size, i; + int err = 0; + /* allocate memory for added disks */ + for (i = 0; i < num_members; i++) { + size = sizeof(struct dl); + space = malloc(size); + if (!space) { + err++; + goto update_takeover_exit; + } + *tail = space; + tail = space; + *tail = NULL; + } + /* allocate memory for new device */ + size = sizeof_imsm_dev(super->devlist->dev, 0) + + (num_members * sizeof(__u32)); + space = malloc(size); + if (!space) { + err++; + goto update_takeover_exit; + } + *tail = space; + tail = space; + *tail = NULL; +update_takeover_exit: + if (!err) { + len = disks_to_mpb_size(num_members * 2); + } else { + /* if allocation didn't success, free buffer */ + while (update->space_list) { + void **sp = update->space_list; + update->space_list = *sp; + free(sp); + } + } + } + + break; + } case update_reshape_container_disks: { /* Every raid device in the container is about to * gain some more devices, and we will enter a ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] raid0->raid10 takeover- process metadata update 2011-01-17 15:42 [PATCH 0/4] raid0<->raid10 takeover for imsm metadata Krzysztof Wojcik ` (2 preceding siblings ...) 2011-01-17 15:43 ` [PATCH 3/4] raid0->raid10 takeover- allocate memory for added disks Krzysztof Wojcik @ 2011-01-17 15:43 ` Krzysztof Wojcik 3 siblings, 0 replies; 8+ messages in thread From: Krzysztof Wojcik @ 2011-01-17 15:43 UTC (permalink / raw) To: neilb Cc: linux-raid, wojciech.neubauer, adam.kwolek, dan.j.williams, ed.ciechanowski Implementation of raid0->raid10 takeover metadata update at process_update level. - We are using memory prievously alocated in prepare_update to create two dummy disks will be inserted in the metadata and new imsm_dev structure with expanded disk order table. - Update indexes in disk list - Update metadata map - Update disk order table Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com> --- super-intel.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 55 insertions(+), 4 deletions(-) diff --git a/super-intel.c b/super-intel.c index 3b2a273..e284a4a 100644 --- a/super-intel.c +++ b/super-intel.c @@ -5813,12 +5813,15 @@ update_reshape_exit: } static int apply_takeover_update(struct imsm_update_takeover *u, - struct intel_super *super) + struct intel_super *super, + void ***space_list) { struct imsm_dev *dev = NULL; + struct intel_dev *dv; + struct imsm_dev *dev_new; struct imsm_map *map; struct dl *dm, *du; - struct intel_dev *dv; + int i; for (dv = super->devlist; dv; dv = dv->next) if (dv->index == (unsigned int)u->subarray) { @@ -5845,7 +5848,6 @@ static int apply_takeover_update(struct imsm_update_takeover *u, dm->index = -1; } } - /* update map */ map->num_members = map->num_members / 2; map->map_state = IMSM_T_STATE_NORMAL; @@ -5854,10 +5856,59 @@ static int apply_takeover_update(struct imsm_update_takeover *u, map->failed_disk_num = -1; } + if (u->direction == R0_TO_R10) { + void **space; + /* update slots in current disk list */ + for (dm = super->disks; dm; dm = dm->next) { + if (dm->index >= 0) + dm->index *= 2; + } + /* create new *missing* disks */ + for (i = 0; i < map->num_members; i++) { + space = *space_list; + if (!space) + continue; + *space_list = *space; + du = (void *)space; + memcpy(du, super->disks, sizeof(*du)); + du->disk.status = FAILED_DISK; + du->disk.scsi_id = 0; + du->fd = -1; + du->minor = 0; + du->major = 0; + du->index = (i * 2) + 1; + sprintf((char *)du->disk.serial, + " MISSING_%d", du->index); + sprintf((char *)du->serial, + "MISSING_%d", du->index); + du->next = super->missing; + super->missing = du; + } + /* create new dev and map */ + space = *space_list; + if (!space) + return 0; + *space_list = *space; + dev_new = (void *)space; + memcpy(dev_new, dev, sizeof(*dev)); + /* update new map */ + map = get_imsm_map(dev_new, 0); + map->failed_disk_num = map->num_members; + map->num_members = map->num_members * 2; + map->map_state = IMSM_T_STATE_NORMAL; + map->num_domains = 2; + map->raid_level = 1; + /* replace dev<->dev_new */ + dv->dev = dev_new; + } /* update disk order table */ for (du = super->disks; du; du = du->next) if (du->index >= 0) set_imsm_ord_tbl_ent(map, du->index, du->index); + for (du = super->missing; du; du = du->next) + if (du->index >= 0) + set_imsm_ord_tbl_ent(map, du->index, + du->index | IMSM_ORD_REBUILD); return 1; } @@ -5906,7 +5957,7 @@ static void imsm_process_update(struct supertype *st, switch (type) { case update_takeover: { struct imsm_update_takeover *u = (void *)update->buf; - if (apply_takeover_update(u, super)) + if (apply_takeover_update(u, super, &update->space_list)) super->updates_pending++; break; } ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-01-27 3:23 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-01-17 15:42 [PATCH 0/4] raid0<->raid10 takeover for imsm metadata Krzysztof Wojcik 2011-01-17 15:42 ` [PATCH 1/4] Add raid10 -> raid0 takeover support Krzysztof Wojcik 2011-01-26 0:58 ` Neil Brown 2011-01-26 11:34 ` Wojcik, Krzysztof 2011-01-27 3:23 ` Neil Brown 2011-01-17 15:42 ` [PATCH 2/4] raid0->raid10 takeover- create metadata update Krzysztof Wojcik 2011-01-17 15:43 ` [PATCH 3/4] raid0->raid10 takeover- allocate memory for added disks Krzysztof Wojcik 2011-01-17 15:43 ` [PATCH 4/4] raid0->raid10 takeover- process metadata update Krzysztof Wojcik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.