LVM RAID: a couple more fixes
PATCH 1/1 LVM RAID: a couple more fixes
Patch includes:
1) add new function 'raid_remove_top_layer' which will be useful to other conversion functions later (also cleans up code)
2) Add error messages if raid_[extract|add]_images fails
3) DO NOT activate sub_lv's between a suspend/resume
4) Add function prototypes to prevent compiler warnings when compiling with '--with-raid=shared'
Also, can someone provide insight on removing the 'sleep(1)'? Without it, I get "device in use: cannot deactivate" error messages - even though it is /not/ in-use.
brassow
lib/metadata/raid_manip.c +85 -43
@@ -123,6 +123,63 @@ static int raid_in_sync(struct logical_v }/* +* raid_remove_top_layer +* @lv +* @removal_list +* +* Remove top layer of RAID LV in order to convert to linear. +* This function makes no on-disk changes. The residual LVs +* returned in 'removal_list' must be freed by the caller. +* +* Returns: 1 on succes, 0 on failure +*/ +static int raid_remove_top_layer (struct logical_volume *lv, +struct dm_list *removal_list) +{ +struct lv_list *lvl_array, *lvl; +struct lv_segment *seg =first_seg (lv); + +if (!seg_is_mirrored (seg)) { +log_error (INTERNAL_ERROR +"Unable to remove RAID layer from segment type %s" , + seg->segtype->name); +return 0 ; + } + +if (seg->area_count !=1 ) { +log_error (INTERNAL_ERROR +"Unable to remove RAID layer when there" +" is more than one sub-lv" ); +return 0 ; + } + + lvl_array =dm_pool_alloc (lv->vg->vgmem,2 *sizeof (*lvl)); +if (!lvl_array) { +log_error ("Memory allocation failed." ); +return 0 ; + } + +/* Add last metadata area to removal_list */ + lvl_array[0 ].lv =seg_metalv (seg,0 ); +lv_set_visible (seg_metalv (seg,0 )); +remove_seg_from_segs_using_this_lv (seg_metalv (seg,0 ), seg); +seg_metatype (seg,0 ) =AREA_UNASSIGNED ; +dm_list_add (removal_list, &(lvl_array[0 ].list)); + +/* Remove RAID layer and add residual LV to removal_list*/ +seg_lv (seg,0 )->status &= ~RAID_IMAGE ; +lv_set_visible (seg_lv (seg,0 )); + lvl_array[1 ].lv =seg_lv (seg,0 ); +dm_list_add (removal_list, &(lvl_array[1 ].list)); + +if (!remove_layer_from_lv (lv,seg_lv (seg,0 ))) + return_0; + + lv->status &= ~(MIRRORED |RAID ); +return 1 ; +} + +/* * _shift_and_rename_image_components * @seg: Top-level RAID segment *
@@ -373,11 +430,10 @@ static int raid_extract_images(struct loint lv_raid_change_image_count (struct logical_volume *lv,uint32_t new_count,struct dm_list *pvs) { -int r;uint32_t old_count =lv_raid_image_count (lv);struct lv_segment *seg =first_seg (lv);struct dm_list removal_list; -struct lv_list *lvl_array, *lvl; +struct lv_list *lvl;dm_list_init (&removal_list);
@@ -392,34 +448,25 @@ int lv_raid_change_image_count(struct loreturn 1 ; } -if (old_count > new_count) - r =raid_extract_images (lv, new_count, pvs,1 , - &removal_list, &removal_list); -else - r =raid_add_images (lv, new_count, pvs); -if (!r) -return 0 ; +if (old_count > new_count) { +if (!raid_extract_images (lv, new_count, pvs,1 , + &removal_list, &removal_list)) { +log_error ("Failed to extract images from %s/%s" , + lv->vg->name, lv->name); +return 0 ; + } + }else { +if (!raid_add_images (lv, new_count, pvs)) { +log_error ("Failed to add images to %s/%s" , + lv->vg->name, lv->name); +return 0 ; + } + }/* Convert to linear? */ -if (new_count ==1 ) { -/* Add last metadata area to removal_list */ - lvl_array =dm_pool_alloc (lv->vg->cmd->mem,2 *sizeof (*lvl)); -if (!lvl_array) - return_0; - lvl_array[0 ].lv =seg_metalv (seg,0 ); -remove_seg_from_segs_using_this_lv (seg_metalv (seg,0 ), seg); -seg_metatype (seg,0 ) =AREA_UNASSIGNED ; -dm_list_add (&removal_list, &(lvl_array[0 ].list)); - -/* Remove RAID layer */ -seg_lv (seg,0 )->status &= ~RAID_IMAGE ; -lv_set_visible (seg_lv (seg,0 )); - lvl_array[1 ].lv =seg_lv (seg,0 ); -dm_list_add (&removal_list, &(lvl_array[1 ].list)); - -if (!remove_layer_from_lv (lv,seg_lv (seg,0 ))) - return_0; - lv->status &= ~(MIRRORED |RAID ); +if ((new_count ==1 ) && !raid_remove_top_layer (lv, &removal_list)) { +log_error ("Failed to remove RAID layer after linear conversion" ); +return 0 ; }if (!vg_write (lv->vg)) {
@@ -441,22 +488,9 @@ int lv_raid_change_image_count(struct lo }/* -* Bring extracted LVs into existance, so there are no -* conflicts for the main RAID device's resume +* Resume original LV +* This also resumes all other sub-lvs (including the extracted) */ -if (!dm_list_empty (&removal_list)) { - dm_list_iterate_items(lvl, &removal_list) { -/* If top RAID was EX, use EX */ -if (lv_is_active_exclusive_locally (lv)) { -if (!activate_lv_excl (lv->vg->cmd, lvl->lv)) - return_0; - }else { -if (!activate_lv (lv->vg->cmd, lvl->lv)) - return_0; - } - } - } -if (!resume_lv (lv->vg->cmd, lv)) {log_error ("Failed to resume %s/%s after committing changes" , lv->vg->name, lv->name);
@@ -465,9 +499,17 @@ int lv_raid_change_image_count(struct lo/* * Eliminate the extracted LVs +* The extracted devices do not take on their new names +* (i.e. *_extracted) unless we suspend/resume them. It +* is not strictly necessary, so we skip that step. */ if (!dm_list_empty (&removal_list)) { dm_list_iterate_items(lvl, &removal_list) { +/* +* FIXME: Remove 'sleep' that is necessary to prevent +* "device in use" errors - udev not fast enough? +*/ +sleep (1 );if (!deactivate_lv (lv->vg->cmd, lvl->lv)) return_0;if (!lv_remove (lvl->lv))
lib/raid/raid.c +14 -0
@@ -349,6 +349,20 @@ static struct segment_type *init_raid_sereturn segtype; } +#ifndef RAID_INTERNAL /* Shared */ +struct segment_type *init_raid1_segtype (struct cmd_context *cmd); +struct segment_type *init_raid4_segtype (struct cmd_context *cmd); +struct segment_type *init_raid5_segtype (struct cmd_context *cmd); +struct segment_type *init_raid5_la_segtype (struct cmd_context *cmd); +struct segment_type *init_raid5_ra_segtype (struct cmd_context *cmd); +struct segment_type *init_raid5_ls_segtype (struct cmd_context *cmd); +struct segment_type *init_raid5_rs_segtype (struct cmd_context *cmd); +struct segment_type *init_raid6_segtype (struct cmd_context *cmd); +struct segment_type *init_raid6_zr_segtype (struct cmd_context *cmd); +struct segment_type *init_raid6_nr_segtype (struct cmd_context *cmd); +struct segment_type *init_raid6_nc_segtype (struct cmd_context *cmd); +#endif +struct segment_type *init_raid1_segtype (struct cmd_context *cmd) {struct segment_type *segtype;
PATCH 1/1 LVM RAID: a couple more fixes
Dne 12.8.2011 19:10, Jonathan Brassow napsal(a):
Quoted 23 lines
Patch includes:
1) add new function 'raid_remove_top_layer' which will be useful to other conversion functions later (also cleans up code)
2) Add error messages if raid_[extract|add]_images fails
3) DO NOT activate sub_lv's between a suspend/resume
4) Add function prototypes to prevent compiler warnings when compiling with '--with-raid=shared'Also, can someone provide insight on removing the 'sleep(1)'? Without it, I get "device in use: cannot deactivate" error messages - even though it is /not/ in-use.
brassow
lib/metadata/raid_manip.c +3
-0@@ -465,9 +499,17 @@ int lv_raid_change_image_count(struct lo/* * Eliminate the extracted LVs +* The extracted devices do not take on their new names +* (i.e. *_extracted) unless we suspend/resume them. It +* is not strictly necessary, so we skip that step. */
lib/metadata/raid_manip.c int lv_raid_change_image_count(struct lo · line 515
Quoted 10 lines
lib/metadata/raid_manip.c +5
-0if (!dm_list_empty (&removal_list)) { dm_list_iterate_items(lvl, &removal_list) { +/* +* FIXME: Remove 'sleep' that is necessary to prevent +* "device in use" errors - udev not fast enough? +*/ +sleep (1 );if (!deactivate_lv (lv->vg->cmd, lvl->lv)) return_0;if (!lv_remove (lvl->lv))
I've not checked the rest of this patch - just noticed this very strange sleep in the code.
You need to take a look at how vgchange is working.
Essentially you need to split this loop in 2 parts - first deactivate all you can within one 'udev transaction' - then you need to 'synchronize with udev (sync_local_dev_names() - i.e. see set_lv() in lv_manip.c)
Once udev is finished you may start to lv_remove() devices - this would give optimal performance. If you would do that in one loop - there is huge udev latency penalty - thought you may just try it for test it's really working ;)
Zdenek
PATCH 1/1 LVM RAID: a couple more fixes
In reply to Jonathan BrassowQuoted 7 lines · Jonathan Brassow
Patch includes:
1) add new function 'raid_remove_top_layer' which will be useful to other conversion functions later (also cleans up code)
2) Add error messages if raid_[extract|add]_images fails
3) DO NOT activate sub_lv's between a suspend/resume
4) Add function prototypes to prevent compiler warnings when compiling with '--with-raid=shared'
I've checked in all but #3, which I've attached as a smaller patch with kabi's suggested change. I've tested the suggestion and it works (much better than the sleep :).
brassow
lib/metadata/raid_manip.c +3 -15
@@ -488,22 +488,9 @@ int lv_raid_change_image_count(struct lo }/* -* Bring extracted LVs into existance, so there are no -* conflicts for the main RAID device's resume +* Resume original LV +* This also resumes all other sub-lvs (including the extracted) */ -if (!dm_list_empty (&removal_list)) { - dm_list_iterate_items(lvl, &removal_list) { -/* If top RAID was EX, use EX */ -if (lv_is_active_exclusive_locally (lv)) { -if (!activate_lv_excl (lv->vg->cmd, lvl->lv)) - return_0; - }else { -if (!activate_lv (lv->vg->cmd, lvl->lv)) - return_0; - } - } - } -if (!resume_lv (lv->vg->cmd, lv)) {log_error ("Failed to resume %s/%s after committing changes" , lv->vg->name, lv->name);
@@ -513,6 +500,7 @@ int lv_raid_change_image_count(struct lo/* * Eliminate the extracted LVs */ +sync_local_dev_names (lv->vg->cmd);if (!dm_list_empty (&removal_list)) { dm_list_iterate_items(lvl, &removal_list) {if (!deactivate_lv (lv->vg->cmd, lvl->lv))