lvm2-commits February 2016

lvm2-commits@lists.fedorahosted.org

8 participants
174 discussions

master - pvremove: use common toollib processing code
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=04d34da706a596b7… Commit: 04d34da706a596b7c1d7c5376fb42aa99374fc51 Parent: 30c69b3f8ac6c2a4c8b1a47b1482ac4b6a7b8d2e Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 16 15:00:50 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 pvremove: use common toollib processing code Use the new pvcreate_each_device() function from toollib. --- lib/metadata/metadata-exported.h | 2 + tools/commands.h | 2 +- tools/pvremove.c | 48 +++++-- tools/toollib.c | 282 ++++++++++++++++++++++++++++++++------ 4 files changed, 276 insertions(+), 58 deletions(-) diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 008aef2..9c6abd7 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -591,9 +591,11 @@ struct pvcreate_each_params { struct dm_list arg_process; /* pvcreate_device, used for processing */ struct dm_list arg_confirm; /* pvcreate_device, used for processing */ struct dm_list arg_create; /* pvcreate_device, used for pvcreate */ + struct dm_list arg_remove; /* pvcreate_device, used for pvremove */ struct dm_list arg_fail; /* pvcreate_device, failed to create */ struct dm_list pvs; /* pv_list, created and usable for vgcreate/vgextend */ const char *orphan_vg_name; + unsigned is_remove : 1; /* is removing PVs, not creating */ unsigned preserve_existing : 1; unsigned check_failed : 1; }; diff --git a/tools/commands.h b/tools/commands.h index fcd2c24..25784bf 100644 --- a/tools/commands.h +++ b/tools/commands.h @@ -911,7 +911,7 @@ xx(lvpoll, xx(pvremove, "Remove LVM label(s) from physical volume(s)", - 0, + ENABLE_ALL_DEVS, "pvremove\n" "\t[--commandprofile ProfileName]\n" "\t[-d|--debug]\n" diff --git a/tools/pvremove.c b/tools/pvremove.c index 1d42c6e..1628ecc 100644 --- a/tools/pvremove.c +++ b/tools/pvremove.c @@ -17,33 +17,51 @@ int pvremove(struct cmd_context *cmd, int argc, char **argv) { - int i; - unsigned force_count; - unsigned prompt; - struct dm_list pv_names; + struct processing_handle *handle; + struct pvcreate_each_params pp; + int ret; if (!argc) { log_error("Please enter a physical volume path"); return EINVALID_CMD_LINE; } - force_count = arg_count(cmd, force_ARG); - prompt = arg_count(cmd, yes_ARG); + pvcreate_each_params_set_defaults(&pp); - dm_list_init(&pv_names); + pp.is_remove = 1; + pp.force = arg_count(cmd, force_ARG); + pp.yes = arg_count(cmd, yes_ARG); + pp.pv_count = argc; + pp.pv_names = argv; - /* Needed to change the set of orphan PVs. */ + /* + * Needed to change the set of orphan PVs. + * (disable afterward to prevent process_each_pv from doing + * a shared global lock since it's already acquired it ex.) + */ if (!lockd_gl(cmd, "ex", 0)) return_ECMD_FAILED; + cmd->lockd_gl_disable = 1; - for (i = 0; i < argc; i++) { - dm_unescape_colons_and_at_signs(argv[i], NULL, NULL); - if (!str_list_add(cmd->mem, &pv_names, argv[i])) - return_ECMD_FAILED; + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + return ECMD_FAILED; } - if (!pvremove_many(cmd, &pv_names, force_count, prompt)) - return_ECMD_FAILED; + /* + * pvremove uses the same toollib function as pvcreate, + * but sets "is_remove" which changes the check function, + * and the actual create vs remove step. + */ + + if (!pvcreate_each_device(cmd, handle, &pp)) + ret = ECMD_FAILED; + else { + /* pvcreate_each_device returns with orphans locked */ + unlock_vg(cmd, VG_ORPHANS); + ret = ECMD_PROCESSED; + } - return ECMD_PROCESSED; + destroy_processing_handle(cmd, handle); + return ret; } diff --git a/tools/toollib.c b/tools/toollib.c index ec0335d..dc025e4 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -3452,6 +3452,7 @@ void pvcreate_each_params_set_defaults(struct pvcreate_each_params *pp) dm_list_init(&pp->arg_process); dm_list_init(&pp->arg_confirm); dm_list_init(&pp->arg_create); + dm_list_init(&pp->arg_remove); dm_list_init(&pp->arg_fail); dm_list_init(&pp->pvs); } @@ -3566,6 +3567,7 @@ int pvcreate_each_params_from_args(struct cmd_context *cmd, struct pvcreate_each enum { PROMPT_PVCREATE_PV_IN_VG = 1, + PROMPT_PVREMOVE_PV_IN_VG = 2, }; enum { @@ -3603,45 +3605,78 @@ struct pvcreate_device { unsigned is_used_unknown_pv : 1; /* device is a PV used in an unknown VG */ }; +/* + * If a PV is in a VG, and pvcreate or pvremove is run on it: + * + * pvcreate|pvremove -f : fails + * pvcreate|pvremove -y : fails + * pvcreate|pvremove -f -y : fails + * pvcreate|pvremove -ff : get y/n prompt + * pvcreate|pvremove -ff -y : succeeds + * + * FIXME: there are a lot of various phrasings used depending on the + * command and specific case. Find some similar way to phrase these. + */ + static void _check_pvcreate_prompt(struct cmd_context *cmd, struct pvcreate_each_params *pp, struct pvcreate_prompt *prompt, int ask) { + const char *vgname = prompt->vg_name ? prompt->vg_name : "<unknown>"; + const char *pvname = prompt->pv_name; + + /* The VG name can be unknown when the PV is used but metadata is not available */ + if (prompt->type == PROMPT_PVCREATE_PV_IN_VG) { if (pp->force != DONT_PROMPT_OVERRIDE) { prompt->answer = PROMPT_ANSWER_NO; - /* FIXME: use similar error messages for these cases */ - if (prompt->vg_name_unknown) { - log_error("PV '%s' is marked as belonging to a VG but its metadata is missing.", - prompt->pv_name); - log_error("Can't initialize PV '%s' without -ff.", prompt->pv_name); + log_error("PV '%s' is marked as belonging to a VG but its metadata is missing.", pvname); + log_error("Can't initialize PV '%s' without -ff.", pvname); } else if (!strcmp(command_name(cmd), "pvcreate")) { - log_error("Can't initialize physical volume \"%s\" of volume group \"%s\" without -ff", - prompt->pv_name, prompt->vg_name); + log_error("Can't initialize physical volume \"%s\" of volume group \"%s\" without -ff", pvname, vgname); } else { - log_error("Physical volume '%s' is already in volume group '%s'", - prompt->pv_name, prompt->vg_name); - log_error("Unable to add physical volume '%s' to volume group '%s'", - prompt->pv_name, prompt->vg_name); + log_error("Physical volume '%s' is already in volume group '%s'", pvname, vgname); + log_error("Unable to add physical volume '%s' to volume group '%s'", pvname, vgname); } } else if (pp->yes) { prompt->answer = PROMPT_ANSWER_YES; } else if (ask) { - const char *vgname = prompt->vg_name ? prompt->vg_name : "<unknown>"; + if (yes_no_prompt("Really INITIALIZE physical volume \"%s\" of volume group \"%s\" [y/n]? ", pvname, vgname) == 'n') { + prompt->answer = PROMPT_ANSWER_NO; + log_error("%s: physical volume not initialized", pvname); + } else { + prompt->answer = PROMPT_ANSWER_YES; + log_warn("WARNING: Forcing physical volume creation on %s of volume group \"%s\"", pvname, vgname); + } + } - if (yes_no_prompt("Really INITIALIZE physical volume \"%s\" of volume group \"%s\" [y/n]? ", - prompt->pv_name, vgname) == 'n') { + } else if (prompt->type == PROMPT_PVREMOVE_PV_IN_VG) { + if (pp->force != DONT_PROMPT_OVERRIDE) { + prompt->answer = PROMPT_ANSWER_NO; + + if (prompt->vg_name_unknown) + log_error("PV %s belongs to a VG but its metadata is missing.", pvname); + else + log_error("PV %s belongs to Volume Group %s so please use vgreduce first.", pvname, vgname); + log_error("(If you are certain you need pvremove, then confirm by using --force twice.)"); + } else if (pp->yes) { + log_warn("WARNING: PV %s belongs to Volume Group %s", pvname, vgname); + prompt->answer = PROMPT_ANSWER_YES; + } else if (ask) { + log_warn("WARNING: PV %s belongs to Volume Group %s", pvname, vgname); + if (yes_no_prompt("Really WIPE LABELS from physical volume \"%s\" of volume group \"%s\" [y/n]? ", pvname, vgname) == 'n') { prompt->answer = PROMPT_ANSWER_NO; - log_error("%s: physical volume not initialized", prompt->pv_name); + log_error("%s: physical volume label not removed", pvname); } else { prompt->answer = PROMPT_ANSWER_YES; - log_warn("WARNING: Forcing physical volume creation on %s of volume group \"%s\"", - prompt->pv_name, vgname); } } + + if ((prompt->answer == PROMPT_ANSWER_YES) && (pp->force == DONT_PROMPT_OVERRIDE)) + log_warn("WARNING: Wiping physical volume label from %s of volume group \"%s\"", pvname, vgname); } } @@ -3687,10 +3722,10 @@ static struct pvcreate_device *_pvcreate_list_find_name(struct dm_list *devices, * and the arg will be reported as not found. */ -static int _pvcreate_check1_single(struct cmd_context *cmd, - struct volume_group *vg, - struct physical_volume *pv, - struct processing_handle *handle) +static int _pvcreate_check_single(struct cmd_context *cmd, + struct volume_group *vg, + struct physical_volume *pv, + struct processing_handle *handle) { struct pvcreate_each_params *pp = (struct pvcreate_each_params *) handle->custom_handle; struct pvcreate_device *pd; @@ -3830,10 +3865,10 @@ static int _pvcreate_check1_single(struct cmd_context *cmd, * prompts, then it will not be found during this check. */ -static int _pvcreate_check2_single(struct cmd_context *cmd, - struct volume_group *vg, - struct physical_volume *pv, - struct processing_handle *handle) +static int _pv_confirm_single(struct cmd_context *cmd, + struct volume_group *vg, + struct physical_volume *pv, + struct processing_handle *handle) { struct pvcreate_each_params *pp = (struct pvcreate_each_params *) handle->custom_handle; struct pvcreate_device *pd; @@ -3849,7 +3884,7 @@ static int _pvcreate_check2_single(struct cmd_context *cmd, if (!found) return 1; - /* Repeat the same from check1. */ + /* Repeat the same from check_single. */ if (!dev_test_excl(pv->dev)) { /* FIXME Detect whether device-mapper itself is still using it */ log_error("Can't open %s exclusively. Mounted filesystem?", @@ -3866,12 +3901,12 @@ static int _pvcreate_check2_single(struct cmd_context *cmd, /* Device is a PV used in a VG. */ if (pd->is_orphan_pv || pd->is_not_pv || pd->is_used_unknown_pv) { - /* In check1 it was an orphan or unused. */ + /* In check_single it was an orphan or unused. */ goto fail; } if (pd->is_vg_pv && pd->vg_name && strcmp(pd->vg_name, vg->name)) { - /* In check1 it was in a different VG. */ + /* In check_single it was in a different VG. */ goto fail; } @@ -3879,17 +3914,17 @@ static int _pvcreate_check2_single(struct cmd_context *cmd, /* Device is an orphan PV. */ if (pd->is_not_pv) { - /* In check1 it was not a PV. */ + /* In check_single it was not a PV. */ goto fail; } if (pd->is_vg_pv) { - /* In check1 it was in a VG. */ + /* In check_single it was in a VG. */ goto fail; } if (is_used_pv(pv) != pd->is_used_unknown_pv) { - /* In check1 it was different. */ + /* In check_single it was different. */ goto fail; } @@ -3897,26 +3932,162 @@ static int _pvcreate_check2_single(struct cmd_context *cmd, /* Device is not a PV. */ if (pd->is_orphan_pv || pd->is_used_unknown_pv) { - /* In check1 it was an orphan PV. */ + /* In check_single it was an orphan PV. */ goto fail; } if (pd->is_vg_pv) { - /* In check1 it was in a VG. */ + /* In check_single it was in a VG. */ goto fail; } } - /* Device is unchanged from check1. */ + /* Device is unchanged from check_single. */ dm_list_move(&pp->arg_process, &pd->list); return 1; fail: - log_error("Cannot use device %s: it changed during create.", pd->name); + log_error("Cannot use device %s: it changed during prompt.", pd->name); dm_list_move(&pp->arg_fail, &pd->list); return 1; } +static int _pvremove_check_single(struct cmd_context *cmd, + struct volume_group *vg, + struct physical_volume *pv, + struct processing_handle *handle) +{ + struct pvcreate_each_params *pp = (struct pvcreate_each_params *) handle->custom_handle; + struct pvcreate_device *pd; + struct pvcreate_prompt *prompt; + struct label *label; + struct device *dev; + int found = 0; + + if (!pv->dev) + return 1; + + /* + * Check if one of the command args in arg_devices + * matches this device. + */ + dm_list_iterate_items(pd, &pp->arg_devices) { + dev = dev_cache_get(pd->name, cmd->full_filter); + if (dev != pv->dev) + continue; + + pd->dev = pv->dev; + if (pv->dev->pvid[0]) + strncpy(pd->pvid, pv->dev->pvid, ID_LEN); + found = 1; + break; + } + + if (!found) + return 1; + + log_debug("Checking device %s for pvremove %.32s", + pv_dev_name(pv), pv->dev->pvid[0] ? pv->dev->pvid : ""); + + /* + * This test will fail if the device belongs to an MD array. + */ + if (!dev_test_excl(pv->dev)) { + /* FIXME Detect whether device-mapper itself is still using it */ + log_error("Can't open %s exclusively. Mounted filesystem?", + pv_dev_name(pv)); + dm_list_move(&pp->arg_fail, &pd->list); + return 1; + } + + /* + * Is there a pv here already? + * If not, this is an error unless you used -f. + */ + if (!label_read(pd->dev, &label, 0)) { + if (pp->force) { + dm_list_move(&pp->arg_process, &pd->list); + return 1; + } else { + log_error("No PV label found on %s.", pd->name); + dm_list_move(&pp->arg_fail, &pd->list); + return 1; + } + } + + /* + * What kind of device is this: an orphan PV, an uninitialized/unused + * device, a PV used in a VG. + */ + + if (vg && !is_orphan_vg(vg->name)) { + /* Device is a PV used in a VG. */ + log_debug("Found pvremove arg %s: pv is used in %s", pd->name, vg->name); + pd->is_vg_pv = 1; + pd->vg_name = dm_pool_strdup(cmd->mem, vg->name); + + } else if (vg && is_orphan_vg(vg->name)) { + if (is_used_pv(pv)) { + /* Device is used in an unknown VG. */ + log_debug("Found pvremove arg %s: pv is used in unknown VG", pd->name); + pd->is_used_unknown_pv = 1; + } else { + /* Device is an orphan PV. */ + log_debug("Found pvremove arg %s: pv is orphan in %s", pd->name, vg->name); + pd->is_orphan_pv = 1; + } + + if (!strcmp(vg->name, FMT_LVM1_ORPHAN_VG_NAME)) + pp->orphan_vg_name = FMT_LVM1_ORPHAN_VG_NAME; + else + pp->orphan_vg_name = FMT_TEXT_ORPHAN_VG_NAME; + } else { + log_debug("Found pvremove arg %s: device is not a pv", pd->name); + /* Device is not a PV. */ + pd->is_not_pv = 1; + } + + if (pd->is_not_pv) { + pd->dev = pv->dev; + log_error("No PV found on device %s", pd->name); + dm_list_move(&pp->arg_fail, &pd->list); + return 1; + } + + /* + * pvremove is being run on this device, and it's not a PV, + * or is an orphan PV. Neither case requires a prompt. + */ + if (pd->is_orphan_pv) { + pd->dev = pv->dev; + dm_list_move(&pp->arg_process, &pd->list); + return 1; + } + + /* + * pvremove is being run on this device, but the device is in a VG. + * A prompt or force option is required to use it. + */ + + if (!(prompt = dm_pool_zalloc(cmd->mem, sizeof(*prompt)))) { + log_error("prompt alloc failed"); + pp->check_failed = 1; + return 0; + } + prompt->dev = pd->dev; + prompt->pv_name = dm_pool_strdup(cmd->mem, pd->name); + if (pd->is_used_unknown_pv) + prompt->vg_name_unknown = 1; + else + prompt->vg_name = dm_pool_strdup(cmd->mem, vg->name); + prompt->type = PROMPT_PVREMOVE_PV_IN_VG; + dm_list_add(&pp->prompts, &prompt->list); + + pd->dev = pv->dev; + dm_list_move(&pp->arg_process, &pd->list); + return 1; +} + /* * This can be used by pvcreate, vgcreate and vgextend to create PVs. The * callers need to set up the pvcreate_each_params structure based on command @@ -4026,7 +4197,8 @@ int pvcreate_each_device(struct cmd_context *cmd, * If it's added to arg_process but needs a prompt or force option, then * a corresponding prompt entry is added to pp->prompts. */ - process_each_pv(cmd, 0, NULL, NULL, 1, 0, handle, _pvcreate_check1_single); + process_each_pv(cmd, 0, NULL, NULL, 1, 0, handle, + pp->is_remove ? _pvremove_check_single : _pvcreate_check_single); /* * A fatal error was found while checking. @@ -4056,7 +4228,7 @@ int pvcreate_each_device(struct cmd_context *cmd, * The command cannot continue if there are no devices to create. */ if (dm_list_empty(&pp->arg_process)) { - log_error("No devices found."); + log_debug("No devices to process."); goto_bad; } @@ -4157,7 +4329,7 @@ int pvcreate_each_device(struct cmd_context *cmd, dm_list_splice(&pp->arg_confirm, &pp->arg_process); - process_each_pv(cmd, 0, NULL, NULL, 1, 0, handle, _pvcreate_check2_single); + process_each_pv(cmd, 0, NULL, NULL, 1, 0, handle, _pv_confirm_single); dm_list_iterate_items(pd, &pp->arg_confirm) log_error("Device %s not found (or ignored by filtering).", pd->name); @@ -4171,7 +4343,7 @@ int pvcreate_each_device(struct cmd_context *cmd, goto_bad; if (dm_list_empty(&pp->arg_process)) { - log_error("No devices found."); + log_debug("No devices to process."); goto_bad; } @@ -4186,7 +4358,10 @@ do_command: dm_list_move(&pp->arg_process, &pd->list); } - dm_list_splice(&pp->arg_create, &pp->arg_process); + if (pp->is_remove) + dm_list_splice(&pp->arg_remove, &pp->arg_process); + else + dm_list_splice(&pp->arg_create, &pp->arg_process); /* * Wipe signatures on devices being created. @@ -4334,8 +4509,31 @@ do_command: dm_list_add(&pp->pvs, &pvl->list); } - dm_list_iterate_items(pvl, &pp->pvs) - log_debug("pv command succeeded for %s", pv_dev_name(pvl->pv)); + /* + * Remove PVs from devices for pvremove. + */ + dm_list_iterate_items_safe(pd, pd2, &pp->arg_remove) { + struct lvmcache_info *info; + + if (!label_remove(pd->dev)) { + log_error("Failed to wipe existing label(s) on %s", pd->name); + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + + info = lvmcache_info_from_pvid(pd->pvid, 0); + if (info) + lvmcache_del(info); + + if (!lvmetad_pv_gone_by_dev(pd->dev, NULL)) { + log_error("Failed to remove PV %s from lvmetad", pd->name); + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + + log_print_unless_silent("Labels on physical volume \"%s\" successfully wiped", + pd->name); + } dm_list_iterate_items(pd, &pp->arg_fail) log_debug("pv command failed for %s", pd->name);

1 0

master - toollib: remove unsed pvcreate params function
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=30c69b3f8ac6c2a4… Commit: 30c69b3f8ac6c2a4c8b1a47b1482ac4b6a7b8d2e Parent: 0cd7d2332c4f6b5f6b71e6fe38a7678074c34bcc Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Jan 26 12:04:37 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 toollib: remove unsed pvcreate params function which has been replaced by an equivalent pvcreate_each params function. --- tools/toollib.c | 115 ------------------------------------------------------- tools/toollib.h | 2 - 2 files changed, 0 insertions(+), 117 deletions(-) diff --git a/tools/toollib.c b/tools/toollib.c index 88cbb51..ec0335d 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -1141,121 +1141,6 @@ void lv_spawn_background_polling(struct cmd_context *cmd, } } -/* - * Intial sanity checking of non-recovery related command-line arguments. - * - * Output arguments: - * pp: structure allocated by caller, fields written / validated here - */ -int pvcreate_params_validate(struct cmd_context *cmd, int argc, - struct pvcreate_params *pp) -{ - if (!argc) { - log_error("Please enter a physical volume path."); - return 0; - } - - pp->yes = arg_count(cmd, yes_ARG); - pp->force = (force_t) arg_count(cmd, force_ARG); - - if (arg_int_value(cmd, labelsector_ARG, 0) >= LABEL_SCAN_SECTORS) { - log_error("labelsector must be less than %lu.", - LABEL_SCAN_SECTORS); - return 0; - } else { - pp->labelsector = arg_int64_value(cmd, labelsector_ARG, - DEFAULT_LABELSECTOR); - } - - if (!(cmd->fmt->features & FMT_MDAS) && - (arg_count(cmd, pvmetadatacopies_ARG) || - arg_count(cmd, metadatasize_ARG) || - arg_count(cmd, dataalignment_ARG) || - arg_count(cmd, dataalignmentoffset_ARG))) { - log_error("Metadata and data alignment parameters only " - "apply to text format."); - return 0; - } - - if (!(cmd->fmt->features & FMT_BAS) && - arg_count(cmd, bootloaderareasize_ARG)) { - log_error("Bootloader area parameters only " - "apply to text format."); - return 0; - } - - if (arg_count(cmd, metadataignore_ARG)) - pp->metadataignore = arg_int_value(cmd, metadataignore_ARG, - DEFAULT_PVMETADATAIGNORE); - else - pp->metadataignore = find_config_tree_bool(cmd, metadata_pvmetadataignore_CFG, NULL); - - if (arg_count(cmd, pvmetadatacopies_ARG) && - !arg_int_value(cmd, pvmetadatacopies_ARG, -1) && - pp->metadataignore) { - log_error("metadataignore only applies to metadatacopies > 0"); - return 0; - } - - pp->zero = arg_int_value(cmd, zero_ARG, 1); - - if (arg_sign_value(cmd, dataalignment_ARG, SIGN_NONE) == SIGN_MINUS) { - log_error("Physical volume data alignment may not be negative."); - return 0; - } - pp->data_alignment = arg_uint64_value(cmd, dataalignment_ARG, UINT64_C(0)); - - if (pp->data_alignment > UINT32_MAX) { - log_error("Physical volume data alignment is too big."); - return 0; - } - - if (arg_sign_value(cmd, dataalignmentoffset_ARG, SIGN_NONE) == SIGN_MINUS) { - log_error("Physical volume data alignment offset may not be negative"); - return 0; - } - pp->data_alignment_offset = arg_uint64_value(cmd, dataalignmentoffset_ARG, UINT64_C(0)); - - if (pp->data_alignment_offset > UINT32_MAX) { - log_error("Physical volume data alignment offset is too big."); - return 0; - } - - if ((pp->data_alignment + pp->data_alignment_offset) && - (pp->rp.pe_start != PV_PE_START_CALC)) { - if ((pp->data_alignment ? pp->rp.pe_start % pp->data_alignment : pp->rp.pe_start) != pp->data_alignment_offset) { - log_warn("WARNING: Ignoring data alignment %s" - " incompatible with restored pe_start value %s)", - display_size(cmd, pp->data_alignment + pp->data_alignment_offset), - display_size(cmd, pp->rp.pe_start)); - pp->data_alignment = 0; - pp->data_alignment_offset = 0; - } - } - - if (arg_sign_value(cmd, metadatasize_ARG, SIGN_NONE) == SIGN_MINUS) { - log_error("Metadata size may not be negative."); - return 0; - } - - if (arg_sign_value(cmd, bootloaderareasize_ARG, SIGN_NONE) == SIGN_MINUS) { - log_error("Bootloader area size may not be negative."); - return 0; - } - - pp->pvmetadatasize = arg_uint64_value(cmd, metadatasize_ARG, UINT64_C(0)); - if (!pp->pvmetadatasize) - pp->pvmetadatasize = find_config_tree_int(cmd, metadata_pvmetadatasize_CFG, NULL); - - pp->pvmetadatacopies = arg_int_value(cmd, pvmetadatacopies_ARG, -1); - if (pp->pvmetadatacopies < 0) - pp->pvmetadatacopies = find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL); - - pp->rp.ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, pp->rp.ba_size); - - return 1; -} - int get_activation_monitoring_mode(struct cmd_context *cmd, int *monitoring_mode) { diff --git a/tools/toollib.h b/tools/toollib.h index 0ef9e92..c092c74 100644 --- a/tools/toollib.h +++ b/tools/toollib.h @@ -177,8 +177,6 @@ int lv_refresh(struct cmd_context *cmd, struct logical_volume *lv); int vg_refresh_visible(struct cmd_context *cmd, struct volume_group *vg); void lv_spawn_background_polling(struct cmd_context *cmd, struct logical_volume *lv); -int pvcreate_params_validate(struct cmd_context *cmd, int argc, - struct pvcreate_params *pp); int get_activation_monitoring_mode(struct cmd_context *cmd, int *monitoring_mode);

1 0

master - liblvm: replace pvcreate_single with pvcreate_vol
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=0cd7d2332c4f6b5f… Commit: 0cd7d2332c4f6b5f6b71e6fe38a7678074c34bcc Parent: a9940bd3c928598d1f15a60503c146046e55a357 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Jan 26 11:50:11 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 liblvm: replace pvcreate_single with pvcreate_vol And remove the pvcreate_single wrapper. --- lib/metadata/metadata-exported.h | 2 -- lib/metadata/pv_manip.c | 21 --------------------- liblvm/lvm_pv.c | 12 ++++++++++-- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 0a45397..008aef2 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -647,8 +647,6 @@ struct lvresize_params { const char *ac_type; }; -int pvcreate_single(struct cmd_context *cmd, const char *pv_name, - struct pvcreate_params *pp); void pvcreate_params_set_defaults(struct pvcreate_params *pp); /* diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c index 4a0992b..f54111c 100644 --- a/lib/metadata/pv_manip.c +++ b/lib/metadata/pv_manip.c @@ -864,24 +864,3 @@ out: return ret; } - -int pvcreate_single(struct cmd_context *cmd, const char *pv_name, - struct pvcreate_params *pp) -{ - int r = 0; - - if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) { - log_error("Can't get lock for orphan PVs"); - return 0; - } - - if (!(pvcreate_vol(cmd, pv_name, pp, 1))) - goto_out; - - r = 1; - -out: - unlock_vg(cmd, VG_ORPHANS); - - return r; -} diff --git a/liblvm/lvm_pv.c b/liblvm/lvm_pv.c index 9b979f3..9bc7153 100644 --- a/liblvm/lvm_pv.c +++ b/liblvm/lvm_pv.c @@ -419,6 +419,7 @@ int lvm_pv_params_set_property(pv_create_params_t params, const char *name, static int _pv_create(pv_create_params_t params) { struct cmd_context *cmd = (struct cmd_context *)params->libh; + int rc = 0; if (params->pv_p.size) { if (params->pv_p.size % SECTOR_SIZE) { @@ -428,9 +429,16 @@ static int _pv_create(pv_create_params_t params) params->pv_p.size = params->pv_p.size >> SECTOR_SHIFT; } - if (!pvcreate_single(cmd, params->pv_name, &params->pv_p)) + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) { + log_errno(EINVAL, "Can't get lock for orphan PVs"); return -1; - return 0; + } + + if (!(pvcreate_vol(cmd, params->pv_name, &params->pv_p, 1))) + rc = -1; + + unlock_vg(cmd, VG_ORPHANS); + return rc; } int lvm_pv_create(lvm_t libh, const char *pv_name, uint64_t size)

1 0

master - vgcreate: use the common toollib pv create
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=a9940bd3c928598d… Commit: a9940bd3c928598d1f15a60503c146046e55a357 Parent: 749a7cecf895264bb8b924e2cc091d13cb111965 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Jan 26 11:34:59 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 vgcreate: use the common toollib pv create Use the new pvcreate_each_device() function from toollib, previously added for pvcreate, in place of the old pvcreate_vol(). This also requires shifting the location where the lock is acquired for the new VG name. The lock for the new VG is supposed to be acquired before pvcreate. This means splitting the vg_lock_newname() out of vg_create(), and calling vg_lock_newname() directly before pvcreate, and then calling the remainder of vg_create() after pvcreate. The new function vg_lock_and_create() now does vg_lock_newname() + vg_create(), like the previous version of vg_create(). The lock on the new VG name is released before the pvcreate and reacquired after the pvcreate because pvcreate needs to reset lvmcache, which doesn't work when locks are held. An exception could likely be made for the new VG name lock, which would allow vgcreate to hold the new VG name lock across the pvcreate step. --- lib/metadata/metadata-exported.h | 1 + lib/metadata/metadata.c | 33 ++++++++------- liblvm/lvm_vg.c | 2 +- tools/vgcreate.c | 83 ++++++++++++++++++++++++++++--------- tools/vgsplit.c | 6 +- 5 files changed, 86 insertions(+), 39 deletions(-) diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 87342d8..0a45397 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -752,6 +752,7 @@ uint32_t pv_list_extents_free(const struct dm_list *pvh); int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name); int vg_validate(struct volume_group *vg); struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name); +struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name); int vg_remove_mdas(struct volume_group *vg); int vg_remove_check(struct volume_group *vg); void vg_remove_pvs(struct volume_group *vg); diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 92a9e82..20caab9 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -1087,6 +1087,24 @@ int vg_has_unknown_segments(const struct volume_group *vg) return 0; } +struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name) +{ + uint32_t rc; + + if (!validate_name(vg_name)) { + log_error("Invalid vg name %s", vg_name); + /* FIXME: use _vg_make_handle() w/proper error code */ + return NULL; + } + + rc = vg_lock_newname(cmd, vg_name); + if (rc != SUCCESS) + /* NOTE: let caller decide - this may be check for existence */ + return _vg_make_handle(cmd, NULL, rc); + + return vg_create(cmd, vg_name); +} + /* * Create a VG with default parameters. * Returns: @@ -1103,21 +1121,6 @@ struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name) .context.vg_ref.vg_name = vg_name }; struct format_instance *fid; - uint32_t rc; - - if (!validate_name(vg_name)) { - log_error("Invalid vg name %s", vg_name); - /* FIXME: use _vg_make_handle() w/proper error code */ - return NULL; - } - - rc = vg_lock_newname(cmd, vg_name); - if (rc != SUCCESS) - /* NOTE: let caller decide - this may be check for existence */ - return _vg_make_handle(cmd, NULL, rc); - - /* Strip dev_dir if present */ - vg_name = strip_dir(vg_name, cmd->dev_dir); if (!(vg = alloc_vg("vg_create", cmd, vg_name))) goto_bad; diff --git a/liblvm/lvm_vg.c b/liblvm/lvm_vg.c index 80f51ad..7e48df0 100644 --- a/liblvm/lvm_vg.c +++ b/liblvm/lvm_vg.c @@ -54,7 +54,7 @@ vg_t lvm_vg_create(lvm_t libh, const char *vg_name) struct volume_group *vg = NULL; struct saved_env e = store_user_env((struct cmd_context *)libh); - vg = vg_create((struct cmd_context *)libh, vg_name); + vg = vg_lock_and_create((struct cmd_context *)libh, vg_name); /* FIXME: error handling is still TBD */ if (vg_read_error(vg)) { release_vg(vg); diff --git a/tools/vgcreate.c b/tools/vgcreate.c index ac53538..456e207 100644 --- a/tools/vgcreate.c +++ b/tools/vgcreate.c @@ -17,14 +17,16 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) { + struct processing_handle *handle; + struct pvcreate_each_params pp; struct vgcreate_params vp_new; struct vgcreate_params vp_def; struct volume_group *vg; const char *tag; const char *clustered_message = ""; char *vg_name; - struct pvcreate_params pp; struct arg_value_group_list *current_group; + uint32_t rc; if (!argc) { log_error("Please provide volume group name and " @@ -36,10 +38,19 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) argc--; argv++; - pvcreate_params_set_defaults(&pp); - if (!pvcreate_params_validate(cmd, argc, &pp)) { + pvcreate_each_params_set_defaults(&pp); + + if (!pvcreate_each_params_from_args(cmd, &pp)) return EINVALID_CMD_LINE; - } + + pp.pv_count = argc; + pp.pv_names = argv; + + /* Don't create a new PV on top of an existing PV like pvcreate does. */ + pp.preserve_existing = 1; + + /* pvcreate within vgcreate cannot be forced. */ + pp.force = 0; if (!vgcreate_params_set_defaults(cmd, &vp_def, NULL)) return EINVALID_CMD_LINE; @@ -48,28 +59,64 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; if (!vgcreate_params_validate(cmd, &vp_new)) - return EINVALID_CMD_LINE; + return EINVALID_CMD_LINE; /* * Needed to change the global VG namespace, * and to change the set of orphan PVs. */ if (!lockd_gl_create(cmd, "ex", vp_new.lock_type)) - return ECMD_FAILED; + return_ECMD_FAILED; + cmd->lockd_gl_disable = 1; lvmcache_seed_infos_from_lvmetad(cmd); - /* Create the new VG */ - vg = vg_create(cmd, vp_new.vg_name); - if (vg_read_error(vg)) { - if (vg_read_error(vg) == FAILED_EXIST) + /* + * Check if the VG name already exists. This should be done before + * creating PVs on any of the devices. + */ + if ((rc = vg_lock_newname(cmd, vp_new.vg_name)) != SUCCESS) { + if (rc == FAILED_EXIST) log_error("A volume group called %s already exists.", vp_new.vg_name); else log_error("Can't get lock for %s.", vp_new.vg_name); - release_vg(vg); return ECMD_FAILED; } + /* + * FIXME: we have to unlock/relock the new VG name around the pvcreate + * step because pvcreate needs to destroy lvmcache, which doesn't allow + * any locks to be held. There shouldn't be any reason to require this + * VG lock to be released, so the lvmcache destroy rule about locks + * seems to be unwarranted here. + */ + unlock_vg(cmd, vp_new.vg_name); + + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + return ECMD_FAILED; + } + + if (!pvcreate_each_device(cmd, handle, &pp)) { + destroy_processing_handle(cmd, handle); + return_ECMD_FAILED; + } + + /* Relock the new VG name, see comment above. */ + if (!lock_vol(cmd, vp_new.vg_name, LCK_VG_WRITE, NULL)) { + destroy_processing_handle(cmd, handle); + return_ECMD_FAILED; + } + + /* + * pvcreate_each_device returns with the VG_ORPHANS write lock held, + * which was used to do pvcreate. Now to create the VG using those + * PVs, the VG lock will be taken (with the orphan lock already held.) + */ + + if (!(vg = vg_create(cmd, vp_new.vg_name))) + goto_bad; + if (vg->fid->fmt->features & FMT_CONFIG_PROFILE) vg->profile = vg->cmd->profile_params->global_metadata_profile; @@ -80,15 +127,10 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) !vg_set_clustered(vg, vp_new.clustered) || !vg_set_system_id(vg, vp_new.system_id) || !vg_set_mda_copies(vg, vp_new.vgmetadatacopies)) - goto bad_orphan; - - if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) { - log_error("Can't get lock for orphan PVs"); - goto bad_orphan; - } + goto_bad; /* attach the pv's */ - if (!vg_extend(vg, argc, (const char* const*)argv, &pp)) + if (!vg_extend_each_pv(vg, &pp)) goto_bad; if (vp_new.max_lv != vg->max_lv) @@ -176,12 +218,13 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) } out: release_vg(vg); + destroy_processing_handle(cmd, handle); return ECMD_PROCESSED; bad: + unlock_vg(cmd, vp_new.vg_name); unlock_vg(cmd, VG_ORPHANS); -bad_orphan: release_vg(vg); - unlock_vg(cmd, vp_new.vg_name); + destroy_processing_handle(cmd, handle); return ECMD_FAILED; } diff --git a/tools/vgsplit.c b/tools/vgsplit.c index 540fd1a..8335570 100644 --- a/tools/vgsplit.c +++ b/tools/vgsplit.c @@ -413,7 +413,7 @@ static struct volume_group *_vgsplit_to(struct cmd_context *cmd, * we obtained a WRITE lock and could not find the vgname in the * system. Thus, the split will be into a new VG. */ - vg_to = vg_create(cmd, vg_name_to); + vg_to = vg_lock_and_create(cmd, vg_name_to); if (vg_read_error(vg_to) == FAILED_LOCKING) { log_error("Can't get lock for %s", vg_name_to); release_vg(vg_to); @@ -526,8 +526,8 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv) return_ECMD_FAILED; /* * Set metadata format of original VG. - * NOTE: We must set the format before calling vg_create() - * since vg_create() calls the per-format constructor. + * NOTE: We must set the format before calling vg_lock_and_create() + * since vg_lock_and_create() calls the per-format constructor. */ cmd->fmt = vg_from->fid->fmt;

1 0

master - vgextend: use the common toollib pv create
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=749a7cecf895264b… Commit: 749a7cecf895264bb8b924e2cc091d13cb111965 Parent: 6a1c22aadf26bfa60782537968d1a03b535cd555 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Mon Feb 15 14:28:09 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 vgextend: use the common toollib pv create --- tools/vgextend.c | 89 +++++++++++++++++++++++++++++++++-------------------- 1 files changed, 55 insertions(+), 34 deletions(-) diff --git a/tools/vgextend.c b/tools/vgextend.c index d22f426..2780755 100644 --- a/tools/vgextend.c +++ b/tools/vgextend.c @@ -16,9 +16,7 @@ #include "tools.h" struct vgextend_params { - struct pvcreate_params pp; - int pv_count; - const char *const *pv_names; + struct pvcreate_each_params pp; }; static int _restore_pv(struct volume_group *vg, const char *pv_name) @@ -49,14 +47,15 @@ static int _vgextend_restoremissing(struct cmd_context *cmd __attribute__((unuse struct processing_handle *handle) { struct vgextend_params *vp = (struct vgextend_params *) handle->custom_handle; + struct pvcreate_each_params *pp = &vp->pp; int fixed = 0; int i; if (!archive(vg)) return_0; - for (i = 0; i < vp->pv_count; i++) - if (_restore_pv(vg, vp->pv_names[i])) + for (i = 0; i < pp->pv_count; i++) + if (_restore_pv(vg, pp->pv_names[i])) fixed++; if (!fixed) { @@ -78,7 +77,7 @@ static int _vgextend_single(struct cmd_context *cmd, const char *vg_name, struct volume_group *vg, struct processing_handle *handle) { struct vgextend_params *vp = (struct vgextend_params *) handle->custom_handle; - struct pvcreate_params *pp = &vp->pp; + struct pvcreate_each_params *pp = &vp->pp; uint32_t mda_copies; uint32_t mda_used; int ret = ECMD_FAILED; @@ -94,12 +93,7 @@ static int _vgextend_single(struct cmd_context *cmd, const char *vg_name, if (!archive(vg)) return_ECMD_FAILED; - if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) { - log_error("Can't get lock for orphan PVs"); - return ECMD_FAILED; - } - - if (!vg_extend(vg, vp->pv_count, vp->pv_names, pp)) + if (!vg_extend_each_pv(vg, pp)) goto_out; if (arg_count(cmd, metadataignore_ARG)) { @@ -114,7 +108,7 @@ static int _vgextend_single(struct cmd_context *cmd, const char *vg_name, } } - log_verbose("Volume group \"%s\" will be extended by %d new physical volumes", vg_name, vp->pv_count); + log_verbose("Volume group \"%s\" will be extended by %d new physical volumes", vg_name, pp->pv_count); if (!vg_write(vg) || !vg_commit(vg)) goto_out; @@ -123,19 +117,17 @@ static int _vgextend_single(struct cmd_context *cmd, const char *vg_name, log_print_unless_silent("Volume group \"%s\" successfully extended", vg_name); ret = ECMD_PROCESSED; - out: - unlock_vg(cmd, VG_ORPHANS); - return ret; } int vgextend(struct cmd_context *cmd, int argc, char **argv) { + struct processing_handle *handle; struct vgextend_params vp; + struct pvcreate_each_params *pp = &vp.pp; unsigned restoremissing = arg_is_set(cmd, restoremissing_ARG); - struct processing_handle *handle; - const char *one_vgname; + const char *vg_name; int ret; if (!argc) { @@ -144,27 +136,56 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) return EINVALID_CMD_LINE; } - one_vgname = skip_dev_dir(cmd, argv[0], NULL); - if (arg_count(cmd, metadatacopies_ARG)) { log_error("Invalid option --metadatacopies, " "use --pvmetadatacopies instead."); return EINVALID_CMD_LINE; } - pvcreate_params_set_defaults(&vp.pp); - vp.pv_count = argc - 1; - vp.pv_names = (const char* const*)(argv + 1); + vg_name = skip_dev_dir(cmd, argv[0], NULL); + argc--; + argv++; - if (!pvcreate_params_validate(cmd, vp.pv_count, &vp.pp)) - return_EINVALID_CMD_LINE; + pvcreate_each_params_set_defaults(pp); - if (!(handle = init_processing_handle(cmd))) { - log_error("Failed to initialize processing handle."); - return ECMD_FAILED; - } + if (!pvcreate_each_params_from_args(cmd, pp)) + return EINVALID_CMD_LINE; - handle->custom_handle = &vp; + pp->pv_count = argc; + pp->pv_names = argv; + + /* Don't create a new PV on top of an existing PV like pvcreate does. */ + pp->preserve_existing = 1; + + /* pvcreate within vgextend cannot be forced. */ + pp->force = 0; + + /* + * Needed to change the set of orphan PVs. + * (disable afterward to prevent process_each_pv from doing + * a shared global lock since it's already acquired it ex.) + */ + if (!lockd_gl(cmd, "ex", 0)) + return_ECMD_FAILED; + cmd->lockd_gl_disable = 1; + + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + return ECMD_FAILED; + } + + if (!restoremissing) { + if (!pvcreate_each_device(cmd, handle, pp)) { + destroy_processing_handle(cmd, handle); + return_ECMD_FAILED; + } + } + + /* + * pvcreate_each_device returns with the VG_ORPHANS write lock held, + * which was used to do pvcreate. Now to create the VG using those + * PVs, the VG lock will be taken (with the orphan lock already held.) + */ /* * It is always ok to add new PVs to a VG - even if there are @@ -174,15 +195,15 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) */ cmd->handles_missing_pvs = 1; - /* Needed to change the set of orphan PVs. */ - if (!lockd_gl(cmd, "ex", 0)) - return_ECMD_FAILED; + handle->custom_handle = &vp; - ret = process_each_vg(cmd, 0, NULL, one_vgname, + ret = process_each_vg(cmd, 0, NULL, vg_name, READ_FOR_UPDATE, handle, restoremissing ? &_vgextend_restoremissing : &_vgextend_single); destroy_processing_handle(cmd, handle); + if (!restoremissing) + unlock_vg(cmd, VG_ORPHANS); return ret; }

1 0

master - pvcreate: use the common toollib pv create
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=6a1c22aadf26bfa6… Commit: 6a1c22aadf26bfa60782537968d1a03b535cd555 Parent: 71671778ab0b2ae4f9f014b37372939b00e90e1d Author: David Teigland <teigland(a)redhat.com> AuthorDate: Mon Feb 15 14:09:42 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 pvcreate: use the common toollib pv create --- tools/pvcreate.c | 143 ++++++++++++++++++++++++++++++++++------------------- 1 files changed, 92 insertions(+), 51 deletions(-) diff --git a/tools/pvcreate.c b/tools/pvcreate.c index 31657c7..47eb85c 100644 --- a/tools/pvcreate.c +++ b/tools/pvcreate.c @@ -22,13 +22,10 @@ * Output arguments: * pp: structure allocated by caller, fields written / validated here */ -static int pvcreate_restore_params_validate(struct cmd_context *cmd, - int argc, char **argv, - struct pvcreate_params *pp) +static int pvcreate_each_restore_params_from_args(struct cmd_context *cmd, int argc, + struct pvcreate_each_params *pp) { - const char *uuid = NULL; - struct volume_group *vg; - struct pv_list *existing_pvl; + pp->restorefile = arg_str_value(cmd, restorefile_ARG, NULL); if (arg_count(cmd, restorefile_ARG) && !arg_count(cmd, uuidstr_ARG)) { log_error("--uuid is required with --restorefile"); @@ -48,35 +45,10 @@ static int pvcreate_restore_params_validate(struct cmd_context *cmd, return 0; } - if (arg_count(cmd, uuidstr_ARG)) { - uuid = arg_str_value(cmd, uuidstr_ARG, ""); - if (!id_read_format(&pp->rp.id, uuid)) - return 0; - pp->rp.idp = &pp->rp.id; - lvmcache_seed_infos_from_lvmetad(cmd); /* need to check for UUID dups */ - } - - if (arg_count(cmd, restorefile_ARG)) { - pp->rp.restorefile = arg_str_value(cmd, restorefile_ARG, ""); - /* The uuid won't already exist */ - if (!(vg = backup_read_vg(cmd, NULL, pp->rp.restorefile))) { - log_error("Unable to read volume group from %s", - pp->rp.restorefile); + if (arg_count(cmd, uuidstr_ARG)) { + pp->uuid_str = arg_str_value(cmd, uuidstr_ARG, ""); + if (!id_read_format(&pp->id, pp->uuid_str)) return 0; - } - if (!(existing_pvl = find_pv_in_vg_by_uuid(vg, pp->rp.idp))) { - release_vg(vg); - log_error("Can't find uuid %s in backup file %s", - uuid, pp->rp.restorefile); - return 0; - } - pp->rp.ba_start = pv_ba_start(existing_pvl->pv); - pp->rp.ba_size = pv_ba_size(existing_pvl->pv); - pp->rp.pe_start = pv_pe_start(existing_pvl->pv); - pp->rp.extent_size = pv_pe_size(existing_pvl->pv); - pp->rp.extent_count = pv_pe_count(existing_pvl->pv); - - release_vg(vg); } if (arg_sign_value(cmd, physicalvolumesize_ARG, SIGN_NONE) == SIGN_MINUS) { @@ -90,34 +62,103 @@ static int pvcreate_restore_params_validate(struct cmd_context *cmd, return 1; } +static int pvcreate_each_restore_params_from_backup(struct cmd_context *cmd, + struct pvcreate_each_params *pp) +{ + struct volume_group *vg; + struct pv_list *existing_pvl; + + /* + * When restoring a PV, params need to be read from a backup file. + */ + if (!pp->restorefile) + return 1; + + if (!(vg = backup_read_vg(cmd, NULL, pp->restorefile))) { + log_error("Unable to read volume group from %s", pp->restorefile); + return 0; + } + + if (!(existing_pvl = find_pv_in_vg_by_uuid(vg, &pp->id))) { + release_vg(vg); + log_error("Can't find uuid %s in backup file %s", + pp->uuid_str, pp->restorefile); + return 0; + } + + pp->ba_start = pv_ba_start(existing_pvl->pv); + pp->ba_size = pv_ba_size(existing_pvl->pv); + pp->pe_start = pv_pe_start(existing_pvl->pv); + pp->extent_size = pv_pe_size(existing_pvl->pv); + pp->extent_count = pv_pe_count(existing_pvl->pv); + + release_vg(vg); + return 1; +} + int pvcreate(struct cmd_context *cmd, int argc, char **argv) { - int i; - int ret = ECMD_PROCESSED; - struct pvcreate_params pp; + struct processing_handle *handle; + struct pvcreate_each_params pp; + int ret; - /* Needed to change the set of orphan PVs. */ - if (!lockd_gl(cmd, "ex", 0)) - return_ECMD_FAILED; + if (!argc) { + log_error("Please enter a physical volume path."); + return 0; + } - pvcreate_params_set_defaults(&pp); + /* + * Device info needs to be available for reading the VG backup file in + * pvcreate_each_restore_params_from_backup. + */ + lvmcache_seed_infos_from_lvmetad(cmd); + + /* + * Five kinds of pvcreate param values: + * 1. defaults + * 2. recovery-related command line args + * 3. recovery-related args from backup file + * 4. normal command line args + * (this also checks some settings from 2 & 3) + * 5. argc/argv free args specifying devices + */ + + pvcreate_each_params_set_defaults(&pp); + + if (!pvcreate_each_restore_params_from_args(cmd, argc, &pp)) + return EINVALID_CMD_LINE; - if (!pvcreate_restore_params_validate(cmd, argc, argv, &pp)) { + if (!pvcreate_each_restore_params_from_backup(cmd, &pp)) return EINVALID_CMD_LINE; - } - if (!pvcreate_params_validate(cmd, argc, &pp)) { + + if (!pvcreate_each_params_from_args(cmd, &pp)) return EINVALID_CMD_LINE; - } - for (i = 0; i < argc; i++) { - if (sigint_caught()) - return_ECMD_FAILED; + pp.pv_count = argc; + pp.pv_names = argv; - dm_unescape_colons_and_at_signs(argv[i], NULL, NULL); + /* + * Needed to change the set of orphan PVs. + * (disable afterward to prevent process_each_pv from doing + * a shared global lock since it's already acquired it ex.) + */ + if (!lockd_gl(cmd, "ex", 0)) + return_ECMD_FAILED; + cmd->lockd_gl_disable = 1; + + if (!(handle = init_processing_handle(cmd))) { + log_error("Failed to initialize processing handle."); + return ECMD_FAILED; + } - if (!pvcreate_single(cmd, argv[i], &pp)) - ret = ECMD_FAILED; + if (!pvcreate_each_device(cmd, handle, &pp)) + ret = ECMD_FAILED; + else { + /* pvcreate_each_device returns with orphans locked */ + unlock_vg(cmd, VG_ORPHANS); + ret = ECMD_PROCESSED; } + destroy_processing_handle(cmd, handle); return ret; }

1 0

master - toollib: add two phase pv processing code
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=71671778ab0b2ae4… Commit: 71671778ab0b2ae4f9f014b37372939b00e90e1d Parent: ff2267012aea37e08ed1fc2a511f8a0d3f0d145b Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 16 14:15:24 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 toollib: add two phase pv processing code This is common code for handling PV create/remove that can be shared by pvcreate/vgcreate/vgextend/pvremove. This does not change any commands to use the new code. - Pull out the hidden equivalent of process_each_pv into an actual top level process_each_pv. - Pull the prompts to the top level, and do not run any prompts while locks are held. The orphan lock is reacquired after any prompts are done, and the devices being created are checked for any change made while the lock was not held. Previously, pvcreate_vol() was the shared function for creating a PV for pvcreate, vgcreate, vgextend. Now, it will be toollib function pvcreate_each_device(). pvcreate_vol() was called effectively as a helper, from within vgcreate and vgextend code paths. pvcreate_each_device() will be called at the same level as other process_each functions. One of the main problems with pvcreate_vol() is that it included a hidden equivalent of process_each_pv for each device being created: pvcreate_vol() -> _pvcreate_check() -> find_pv_by_name() -> get_pvs() -> get_pvs_internal() -> _get_pvs() -> get_vgids() -> /* equivalent to process_each_pv */ dm_list_iterate_items(vgids) vg = vg_read_internal() dm_list_iterate_items(&vg->pvs) pvcreate_each_device() reorganizes the code so that each-VG-each-PV loop is done once, and uses the standard process_each_pv function at the top level of the function. --- lib/metadata/metadata-exported.h | 57 +++ lib/metadata/metadata.c | 39 ++ tools/commands.h | 6 +- tools/pvchange.c | 2 +- tools/pvdisplay.c | 5 +- tools/pvmove.c | 2 +- tools/pvresize.c | 2 +- tools/pvscan.c | 2 +- tools/reporter.c | 6 +- tools/toollib.c | 996 +++++++++++++++++++++++++++++++++++++- tools/toollib.h | 8 +- tools/tools.h | 2 + tools/vgreduce.c | 2 +- 13 files changed, 1097 insertions(+), 32 deletions(-) diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index fc09e2e..87342d8 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -542,6 +542,62 @@ struct pvcreate_params { struct pvcreate_restorable_params rp; }; +/* + * FIXME: rename this pvcreate_params once the old pvcreate_params is unused. + * This can probably be put in toollib.h. + */ +struct pvcreate_each_params { + /* + * From argc and argv. + */ + char **pv_names; + uint32_t pv_count; + + /* + * From command line args. + */ + int zero; + uint64_t size; + uint64_t data_alignment; + uint64_t data_alignment_offset; + int pvmetadatacopies; + uint64_t pvmetadatasize; + int64_t labelsector; + force_t force; + unsigned yes; + unsigned metadataignore; + + /* + * From recovery-specific command line args. + */ + const char *restorefile; /* NULL if no --restorefile option */ + const char *uuid_str; /* id in printable format, NULL if no id */ + struct id id; + + /* + * From reading VG backup file. + */ + uint64_t ba_start; + uint64_t ba_size; + uint64_t pe_start; + uint32_t extent_count; + uint32_t extent_size; + + /* + * Used for command processing. + */ + struct dm_list prompts; /* pvcreate_prompt */ + struct dm_list arg_devices; /* pvcreate_device, one for each pv_name */ + struct dm_list arg_process; /* pvcreate_device, used for processing */ + struct dm_list arg_confirm; /* pvcreate_device, used for processing */ + struct dm_list arg_create; /* pvcreate_device, used for pvcreate */ + struct dm_list arg_fail; /* pvcreate_device, failed to create */ + struct dm_list pvs; /* pv_list, created and usable for vgcreate/vgextend */ + const char *orphan_vg_name; + unsigned preserve_existing : 1; + unsigned check_failed : 1; +}; + struct lvresize_params { const char *vg_name; /* only-used when VG is not yet opened (in /tools) */ const char *lv_name; @@ -705,6 +761,7 @@ int vg_rename(struct cmd_context *cmd, struct volume_group *vg, const char *new_name); int vg_extend(struct volume_group *vg, int pv_count, const char *const *pv_names, struct pvcreate_params *pp); +int vg_extend_each_pv(struct volume_group *vg, struct pvcreate_each_params *pp); int vg_reduce(struct volume_group *vg, const char *pv_name); int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg, diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index d7a9d81..92a9e82 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -791,6 +791,40 @@ int vg_extend(struct volume_group *vg, int pv_count, const char *const *pv_names return 1; } +int vg_extend_each_pv(struct volume_group *vg, struct pvcreate_each_params *pp) +{ + struct pv_list *pvl; + unsigned int max_phys_block_size = 0; + + log_debug_metadata("Adding PVs to VG %s", vg->name); + + if (_vg_bad_status_bits(vg, RESIZEABLE_VG)) + return_0; + + dm_list_iterate_items(pvl, &pp->pvs) { + log_debug_metadata("Adding PV %s to VG %s", pv_dev_name(pvl->pv), vg->name); + + if (!(check_dev_block_size_for_vg(pvl->pv->dev, + (const struct volume_group *) vg, + &max_phys_block_size))) { + log_error("PV %s has wrong block size", pv_dev_name(pvl->pv)); + return_0; + } + + if (!add_pv_to_vg(vg, pv_dev_name(pvl->pv), pvl->pv, 0)) { + log_error("PV %s cannot be added to VG %s.", + pv_dev_name(pvl->pv), vg->name); + return_0; + } + } + + (void) _check_pv_dev_sizes(vg); + + dm_list_splice(&vg->pv_write_list, &pp->pvs); + + return 1; +} + int vg_reduce(struct volume_group *vg, const char *pv_name) { struct physical_volume *pv; @@ -3839,6 +3873,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, struct cached_vg_fmtdata *vg_fmtdata = NULL; /* Additional format-specific data about the vg */ unsigned use_previous_vg; + log_very_verbose("Reading VG %s %.32s", vgname ?: "<no name>", vgid ?: "<no vgid>"); + if (is_orphan_vg(vgname)) { if (use_precommitted) { log_error(INTERNAL_ERROR "vg_read_internal requires vgname " @@ -5322,6 +5358,9 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha return _vg_make_handle(cmd, vg, FAILED_LOCKING); } + if (already_locked) + log_very_verbose("Locking %s already done", vg_name); + if (is_orphan_vg(vg_name)) status_flags &= ~LVM_WRITE; diff --git a/tools/commands.h b/tools/commands.h index 0748568..fcd2c24 100644 --- a/tools/commands.h +++ b/tools/commands.h @@ -766,7 +766,7 @@ xx(pvck, xx(pvcreate, "Initialize physical volume(s) for use by LVM", - 0, + ENABLE_ALL_DEVS, "pvcreate\n" "\t[--norestorefile]\n" "\t[--restorefile file]\n" @@ -1119,7 +1119,7 @@ xx(vgconvert, xx(vgcreate, "Create a volume group", - 0, + MUST_USE_ALL_ARGS | ENABLE_ALL_DEVS, "vgcreate\n" "\t[-A|--autobackup {y|n}]\n" "\t[--addtag Tag]\n" @@ -1214,7 +1214,7 @@ xx(vgexport, xx(vgextend, "Add physical volumes to a volume group", - 0, + MUST_USE_ALL_ARGS | ENABLE_ALL_DEVS, "vgextend\n" "\t[-A|--autobackup y|n]\n" "\t[--restoremissing]\n" diff --git a/tools/pvchange.c b/tools/pvchange.c index 7f9aaf8..3c12c14 100644 --- a/tools/pvchange.c +++ b/tools/pvchange.c @@ -228,7 +228,7 @@ int pvchange(struct cmd_context *cmd, int argc, char **argv) } } - ret = process_each_pv(cmd, argc, argv, NULL, READ_FOR_UPDATE, handle, _pvchange_single); + ret = process_each_pv(cmd, argc, argv, NULL, 0, READ_FOR_UPDATE, handle, _pvchange_single); if (!argc) unlock_vg(cmd, VG_GLOBAL); diff --git a/tools/pvdisplay.c b/tools/pvdisplay.c index bad2df0..4d8275c 100644 --- a/tools/pvdisplay.c +++ b/tools/pvdisplay.c @@ -107,8 +107,9 @@ int pvdisplay(struct cmd_context *cmd, int argc, char **argv) } } - ret = process_each_pv(cmd, argc, argv, NULL, 0, NULL, - _pvdisplay_single); + ret = process_each_pv(cmd, argc, argv, NULL, + arg_is_set(cmd, all_ARG), 0, + NULL, _pvdisplay_single); if (lock_global) unlock_vg(cmd, VG_GLOBAL); diff --git a/tools/pvmove.c b/tools/pvmove.c index 0b8ca46..b590745 100644 --- a/tools/pvmove.c +++ b/tools/pvmove.c @@ -895,7 +895,7 @@ int pvmove(struct cmd_context *cmd, int argc, char **argv) handle->custom_handle = &pp; - process_each_pv(cmd, 1, &pv_name, NULL, + process_each_pv(cmd, 1, &pv_name, NULL, 0, is_abort ? 0 : READ_FOR_UPDATE, handle, is_abort ? &_pvmove_read_single : &_pvmove_setup_single); diff --git a/tools/pvresize.c b/tools/pvresize.c index e23c954..b79098c 100644 --- a/tools/pvresize.c +++ b/tools/pvresize.c @@ -87,7 +87,7 @@ int pvresize(struct cmd_context *cmd, int argc, char **argv) handle->custom_handle = &params; - ret = process_each_pv(cmd, argc, argv, NULL, READ_FOR_UPDATE, handle, + ret = process_each_pv(cmd, argc, argv, NULL, 0, READ_FOR_UPDATE, handle, _pvresize_single); log_print_unless_silent("%d physical volume(s) resized / %d physical volume(s) " diff --git a/tools/pvscan.c b/tools/pvscan.c index 7d7a96f..b224c30 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -422,7 +422,7 @@ int pvscan(struct cmd_context *cmd, int argc, char **argv) handle->custom_handle = &params; - ret = process_each_pv(cmd, argc, argv, NULL, 0, handle, _pvscan_single); + ret = process_each_pv(cmd, argc, argv, NULL, 0, 0, handle, _pvscan_single); if (!params.pvs_found) log_print_unless_silent("No matching physical volumes found"); diff --git a/tools/reporter.c b/tools/reporter.c index f7fb543..1a24698 100644 --- a/tools/reporter.c +++ b/tools/reporter.c @@ -893,7 +893,8 @@ static int _report(struct cmd_context *cmd, int argc, char **argv, break; case PVS: if (args_are_pvs) - r = process_each_pv(cmd, argc, argv, NULL, 0, + r = process_each_pv(cmd, argc, argv, NULL, + arg_is_set(cmd, all_ARG), 0, &handle, &_pvs_single); else r = process_each_vg(cmd, argc, argv, NULL, 0, @@ -910,7 +911,8 @@ static int _report(struct cmd_context *cmd, int argc, char **argv, break; case PVSEGS: if (args_are_pvs) - r = process_each_pv(cmd, argc, argv, NULL, 0, + r = process_each_pv(cmd, argc, argv, NULL, + arg_is_set(cmd, all_ARG), 0, &handle, lv_info_needed && !lv_segment_status_needed ? &_pvsegs_with_lv_info_single : !lv_info_needed && lv_segment_status_needed ? &_pvsegs_with_lv_status_single : diff --git a/tools/toollib.c b/tools/toollib.c index 22c38a1..88cbb51 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -14,6 +14,8 @@ */ #include "tools.h" +#include "format1.h" +#include "format-text.h" #include <sys/stat.h> #include <signal.h> @@ -1740,8 +1742,6 @@ static int _get_arg_vgnames(struct cmd_context *cmd, int ret_max = ECMD_PROCESSED; const char *vg_name; - log_verbose("Using volume group(s) on command line."); - if (one_vgname) { if (!str_list_add(cmd->mem, arg_vgnames, dm_pool_strdup(cmd->mem, one_vgname))) { @@ -1935,6 +1935,7 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags, int skip; int notfound; int process_all = 0; + int already_locked; /* * If no VG names or tags were supplied, then process all VGs. @@ -1964,6 +1965,8 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags, continue; } + already_locked = lvmcache_vgname_is_locked(vg_name); + vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state); if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) { stack; @@ -1989,7 +1992,7 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags, ret_max = ret; } - if (!vg_read_error(vg)) + if (!vg_read_error(vg) && !already_locked) unlock_vg(cmd, vg_name); endvg: release_vg(vg); @@ -2177,6 +2180,8 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv, int ret_max = ECMD_PROCESSED; int ret; + log_debug("Processing each VG"); + /* Disable error in vg_read so we can print it from ignore_vg. */ cmd->vg_read_print_access_error = 0; @@ -2227,7 +2232,7 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv, * . A VG name is specified which may refer to one * of multiple VGs on the system with that name. */ - log_very_verbose("Get list of VGs on system"); + log_debug("Get list of VGs on system"); if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0)) { ret_max = ECMD_FAILED; @@ -2493,8 +2498,6 @@ static int _get_arg_lvnames(struct cmd_context *cmd, const char *vgname_def; unsigned dev_dir_found; - log_verbose("Using logical volume(s) on command line."); - for (; opt < argc; opt++) { lv_name = argv[opt]; dev_dir_found = 0; @@ -2596,6 +2599,7 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag int ret; int skip; int notfound; + int already_locked; dm_list_iterate_items(vgnl, vgnameids_to_process) { if (sigint_caught()) @@ -2646,6 +2650,8 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag continue; } + already_locked = lvmcache_vgname_is_locked(vg_name); + vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state); if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) { stack; @@ -2662,7 +2668,8 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag if (ret > ret_max) ret_max = ret; - unlock_vg(cmd, vg_name); + if (!already_locked) + unlock_vg(cmd, vg_name); endvg: release_vg(vg); if (!lockd_vg(cmd, vg_name, "un", 0, &lockd_state)) @@ -2747,7 +2754,7 @@ int process_each_lv(struct cmd_context *cmd, int argc, char **argv, uint32_t rea * . A VG name is specified which may refer to one * of multiple VGs on the system with that name. */ - log_very_verbose("Get list of VGs on system"); + log_debug("Get list of VGs on system"); if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0)) { ret_max = ECMD_FAILED; @@ -2809,8 +2816,6 @@ static int _get_arg_pvnames(struct cmd_context *cmd, char *arg_name; int ret_max = ECMD_PROCESSED; - log_verbose("Using physical volume(s) on command line."); - for (; opt < argc; opt++) { arg_name = argv[opt]; @@ -2875,6 +2880,8 @@ static int _get_all_devices(struct cmd_context *cmd, struct dm_list *all_devices struct device_id_list *dil; int r = ECMD_FAILED; + log_debug("Getting list of all devices"); + lvmcache_seed_infos_from_lvmetad(cmd); if (!(iter = dev_iter_create(cmd->full_filter, 1))) { @@ -3000,6 +3007,8 @@ static int _process_device_list(struct cmd_context *cmd, struct dm_list *all_dev int ret_max = ECMD_PROCESSED; int ret = 0; + log_debug("Processing devices that are not PVs"); + /* * Pretend that each device is a PV with dummy values. * FIXME Formalise this extension or find an alternative. @@ -3243,6 +3252,7 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags, int ret; int skip; int notfound; + int already_locked; dm_list_iterate_items(vgnl, all_vgnameids) { if (sigint_caught()) @@ -3258,6 +3268,10 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags, continue; } + log_debug("Processing PVs in VG %s", vg_name); + + already_locked = lvmcache_vgname_is_locked(vg_name); + vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state); if (_ignore_vg(vg, vg_name, NULL, read_flags, &skip, &notfound)) { stack; @@ -3282,7 +3296,7 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags, if (ret > ret_max) ret_max = ret; - if (!skip) + if (!skip && !already_locked) unlock_vg(cmd, vg->name); endvg: release_vg(vg); @@ -3298,9 +3312,8 @@ endvg: } int process_each_pv(struct cmd_context *cmd, - int argc, char **argv, - const char *only_this_vgname, - uint32_t read_flags, + int argc, char **argv, const char *only_this_vgname, + int all_is_set, uint32_t read_flags, struct processing_handle *handle, process_single_pv_fn_t process_single_pv) { @@ -3313,9 +3326,12 @@ int process_each_pv(struct cmd_context *cmd, struct device_id_list *dil; int process_all_pvs; int process_all_devices; + int orphans_locked; int ret_max = ECMD_PROCESSED; int ret; + log_debug("Processing each PV"); + /* * When processing a specific VG name, warn if it's inconsistent and * print an error if it's not found. Otherwise we're processing all @@ -3352,10 +3368,11 @@ int process_each_pv(struct cmd_context *cmd, return ret; } + orphans_locked = lvmcache_vgname_is_locked(VG_ORPHANS); + process_all_pvs = dm_list_empty(&arg_pvnames) && dm_list_empty(&arg_tags); - process_all_devices = process_all_pvs && (cmd->command->flags & ENABLE_ALL_DEVS) && - arg_count(cmd, all_ARG); + process_all_devices = process_all_pvs && (cmd->command->flags & ENABLE_ALL_DEVS) && all_is_set; /* Needed for a current listing of the global VG namespace. */ if (!only_this_vgname && !lockd_gl(cmd, "sh", 0)) @@ -3365,10 +3382,13 @@ int process_each_pv(struct cmd_context *cmd, * This full scan would be done by _get_all_devices() if * it were not done here first. It's called here first * so that get_vgnameids() will look at any new devices. + * When orphans is already locked, these steps are done + * before process_each_pv is called. */ - if (!trust_cache()) { - dev_cache_full_scan(cmd->full_filter); + if (!trust_cache() && !orphans_locked) { + log_debug("Scanning for available devices"); lvmcache_destroy(cmd, 1, 0); + dev_cache_full_scan(cmd->full_filter); } /* @@ -3406,6 +3426,14 @@ int process_each_pv(struct cmd_context *cmd, ret_max = ret; /* + * If the orphans lock was held, there shouldn't be missed devices. If + * there were, we cannot clear the cache while holding the orphans lock + * anyway. + */ + if (orphans_locked) + goto skip_missed; + + /* * Some PVs may have been missed by the first search if another command * moved them at the same time. Repeat the search for only the * specific PVs missed. lvmcache needs clearing for a fresh search. @@ -3452,6 +3480,7 @@ int process_each_pv(struct cmd_context *cmd, } } +skip_missed: dm_list_iterate_items(dil, &arg_devices) { log_error("Failed to find physical volume \"%s\".", dev_name(dil->dev)); ret_max = ECMD_FAILED; @@ -3510,3 +3539,934 @@ int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv, return ECMD_PROCESSED; } + +void pvcreate_each_params_set_defaults(struct pvcreate_each_params *pp) +{ + memset(pp, 0, sizeof(*pp)); + + pp->zero = 1; + pp->size = 0; + pp->data_alignment = UINT64_C(0); + pp->data_alignment_offset = UINT64_C(0); + pp->pvmetadatacopies = DEFAULT_PVMETADATACOPIES; + pp->pvmetadatasize = DEFAULT_PVMETADATASIZE; + pp->labelsector = DEFAULT_LABELSECTOR; + pp->force = PROMPT; + pp->yes = 0; + pp->metadataignore = DEFAULT_PVMETADATAIGNORE; + pp->restorefile = NULL; + pp->uuid_str = NULL; + pp->ba_start = 0; + pp->ba_size = 0; + pp->pe_start = PV_PE_START_CALC; + pp->extent_count = 0; + pp->extent_size = 0; + + dm_list_init(&pp->prompts); + dm_list_init(&pp->arg_devices); + dm_list_init(&pp->arg_process); + dm_list_init(&pp->arg_confirm); + dm_list_init(&pp->arg_create); + dm_list_init(&pp->arg_fail); + dm_list_init(&pp->pvs); +} + +int pvcreate_each_params_from_args(struct cmd_context *cmd, struct pvcreate_each_params *pp) +{ + pp->yes = arg_count(cmd, yes_ARG); + pp->force = (force_t) arg_count(cmd, force_ARG); + + if (arg_int_value(cmd, labelsector_ARG, 0) >= LABEL_SCAN_SECTORS) { + log_error("labelsector must be less than %lu.", + LABEL_SCAN_SECTORS); + return 0; + } else { + pp->labelsector = arg_int64_value(cmd, labelsector_ARG, + DEFAULT_LABELSECTOR); + } + + if (!(cmd->fmt->features & FMT_MDAS) && + (arg_count(cmd, pvmetadatacopies_ARG) || + arg_count(cmd, metadatasize_ARG) || + arg_count(cmd, dataalignment_ARG) || + arg_count(cmd, dataalignmentoffset_ARG))) { + log_error("Metadata and data alignment parameters only " + "apply to text format."); + return 0; + } + + if (!(cmd->fmt->features & FMT_BAS) && + arg_count(cmd, bootloaderareasize_ARG)) { + log_error("Bootloader area parameters only " + "apply to text format."); + return 0; + } + + if (arg_count(cmd, metadataignore_ARG)) + pp->metadataignore = arg_int_value(cmd, metadataignore_ARG, + DEFAULT_PVMETADATAIGNORE); + else + pp->metadataignore = find_config_tree_bool(cmd, metadata_pvmetadataignore_CFG, NULL); + + if (arg_count(cmd, pvmetadatacopies_ARG) && + !arg_int_value(cmd, pvmetadatacopies_ARG, -1) && + pp->metadataignore) { + log_error("metadataignore only applies to metadatacopies > 0"); + return 0; + } + + pp->zero = arg_int_value(cmd, zero_ARG, 1); + + if (arg_sign_value(cmd, dataalignment_ARG, SIGN_NONE) == SIGN_MINUS) { + log_error("Physical volume data alignment may not be negative."); + return 0; + } + pp->data_alignment = arg_uint64_value(cmd, dataalignment_ARG, UINT64_C(0)); + + if (pp->data_alignment > UINT32_MAX) { + log_error("Physical volume data alignment is too big."); + return 0; + } + + if (arg_sign_value(cmd, dataalignmentoffset_ARG, SIGN_NONE) == SIGN_MINUS) { + log_error("Physical volume data alignment offset may not be negative"); + return 0; + } + pp->data_alignment_offset = arg_uint64_value(cmd, dataalignmentoffset_ARG, UINT64_C(0)); + + if (pp->data_alignment_offset > UINT32_MAX) { + log_error("Physical volume data alignment offset is too big."); + return 0; + } + + if ((pp->data_alignment + pp->data_alignment_offset) && + (pp->pe_start != PV_PE_START_CALC)) { + if ((pp->data_alignment ? pp->pe_start % pp->data_alignment : pp->pe_start) != pp->data_alignment_offset) { + log_warn("WARNING: Ignoring data alignment %s" + " incompatible with restored pe_start value %s)", + display_size(cmd, pp->data_alignment + pp->data_alignment_offset), + display_size(cmd, pp->pe_start)); + pp->data_alignment = 0; + pp->data_alignment_offset = 0; + } + } + + if (arg_sign_value(cmd, metadatasize_ARG, SIGN_NONE) == SIGN_MINUS) { + log_error("Metadata size may not be negative."); + return 0; + } + + if (arg_sign_value(cmd, bootloaderareasize_ARG, SIGN_NONE) == SIGN_MINUS) { + log_error("Bootloader area size may not be negative."); + return 0; + } + + pp->pvmetadatasize = arg_uint64_value(cmd, metadatasize_ARG, UINT64_C(0)); + if (!pp->pvmetadatasize) + pp->pvmetadatasize = find_config_tree_int(cmd, metadata_pvmetadatasize_CFG, NULL); + + pp->pvmetadatacopies = arg_int_value(cmd, pvmetadatacopies_ARG, -1); + if (pp->pvmetadatacopies < 0) + pp->pvmetadatacopies = find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL); + + if (pp->pvmetadatacopies > 2) { + log_error("Metadatacopies may only be 0, 1 or 2"); + return 0; + } + + pp->ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, pp->ba_size); + + return 1; +} + +enum { + PROMPT_PVCREATE_PV_IN_VG = 1, +}; + +enum { + PROMPT_ANSWER_NO = 1, + PROMPT_ANSWER_YES = 2 +}; + +/* + * When a prompt entry is created, save any strings or info + * in this struct that are needed for the prompt messages. + * The VG/PV structs are not be available when the prompt + * is run. + */ +struct pvcreate_prompt { + struct dm_list list; + uint32_t type; + const char *pv_name; + const char *vg_name; + struct device *dev; + int answer; + unsigned abort_command : 1; + unsigned vg_name_unknown : 1; +}; + +struct pvcreate_device { + struct dm_list list; + const char *name; + struct device *dev; + char pvid[ID_LEN + 1]; + const char *vg_name; + int wiped; + unsigned is_not_pv : 1; /* device is not a PV */ + unsigned is_orphan_pv : 1; /* device is an orphan PV */ + unsigned is_vg_pv : 1; /* device is a PV used in a VG */ + unsigned is_used_unknown_pv : 1; /* device is a PV used in an unknown VG */ +}; + +static void _check_pvcreate_prompt(struct cmd_context *cmd, + struct pvcreate_each_params *pp, + struct pvcreate_prompt *prompt, + int ask) +{ + if (prompt->type == PROMPT_PVCREATE_PV_IN_VG) { + if (pp->force != DONT_PROMPT_OVERRIDE) { + prompt->answer = PROMPT_ANSWER_NO; + + /* FIXME: use similar error messages for these cases */ + + if (prompt->vg_name_unknown) { + log_error("PV '%s' is marked as belonging to a VG but its metadata is missing.", + prompt->pv_name); + log_error("Can't initialize PV '%s' without -ff.", prompt->pv_name); + } else if (!strcmp(command_name(cmd), "pvcreate")) { + log_error("Can't initialize physical volume \"%s\" of volume group \"%s\" without -ff", + prompt->pv_name, prompt->vg_name); + } else { + log_error("Physical volume '%s' is already in volume group '%s'", + prompt->pv_name, prompt->vg_name); + log_error("Unable to add physical volume '%s' to volume group '%s'", + prompt->pv_name, prompt->vg_name); + } + } else if (pp->yes) { + prompt->answer = PROMPT_ANSWER_YES; + } else if (ask) { + const char *vgname = prompt->vg_name ? prompt->vg_name : "<unknown>"; + + if (yes_no_prompt("Really INITIALIZE physical volume \"%s\" of volume group \"%s\" [y/n]? ", + prompt->pv_name, vgname) == 'n') { + prompt->answer = PROMPT_ANSWER_NO; + log_error("%s: physical volume not initialized", prompt->pv_name); + } else { + prompt->answer = PROMPT_ANSWER_YES; + log_warn("WARNING: Forcing physical volume creation on %s of volume group \"%s\"", + prompt->pv_name, vgname); + } + } + } +} + +static struct pvcreate_device *_pvcreate_list_find_dev(struct dm_list *devices, struct device *dev) +{ + struct pvcreate_device *pd; + + dm_list_iterate_items(pd, devices) { + if (pd->dev == dev) + return pd; + } + + return NULL; +} + +static struct pvcreate_device *_pvcreate_list_find_name(struct dm_list *devices, const char *name) +{ + struct pvcreate_device *pd; + + dm_list_iterate_items(pd, devices) { + if (!strcmp(pd->name, name)) + return pd; + } + + return NULL; +} + +/* + * If this function decides that a arg_devices entry cannot be used, but the + * command might be able to continue without it, then it moves that entry from + * arg_devices to arg_fail. + * + * If this function decides that an arg_devices entry could be used (possibly + * requiring a prompt), then it moves the entry from arg_devices to arg_process. + * + * Any arg_devices entries that are not moved to arg_fail or arg_process were + * not found. The caller will decide if the command can continue if any + * arg_devices entries were not found, or if any were moved to arg_fail. + * + * This check does not need to look at PVs in foreign, shared or clustered VGs. + * If pvcreate/vgcreate/vgextend specifies a device in a + * foreign/shared/clustered VG, that VG will not be processed by this function, + * and the arg will be reported as not found. + */ + +static int _pvcreate_check1_single(struct cmd_context *cmd, + struct volume_group *vg, + struct physical_volume *pv, + struct processing_handle *handle) +{ + struct pvcreate_each_params *pp = (struct pvcreate_each_params *) handle->custom_handle; + struct pvcreate_device *pd; + struct pvcreate_prompt *prompt; + struct device *dev; + int found = 0; + + if (!pv->dev) + return 1; + + /* + * Check if one of the command args in arg_devices + * matches this device. + * + * (Possible optimization: the first time this _single + * function is called, we could iterate through all + * arg_devices entries, do the name to dev lookup + * with dev_cache_get() and set the pd->dev fields. + * Subsequent _single calls would just compare devs + * and not do any dev_cache_get(). This would avoid + * repeating dev_cache_get() for arg_devices entries.) + */ + dm_list_iterate_items(pd, &pp->arg_devices) { + dev = dev_cache_get(pd->name, cmd->full_filter); + if (dev != pv->dev) + continue; + + pd->dev = pv->dev; + if (pv->dev->pvid[0]) + strncpy(pd->pvid, pv->dev->pvid, ID_LEN); + found = 1; + break; + } + + /* + * Check if the uuid specified for the new PV is used by another PV. + */ + if (!found && pv->dev && pp->uuid_str && id_equal(&pv->id, &pp->id)) { + log_error("uuid %s already in use on \"%s\"", pp->uuid_str, pv_dev_name(pv)); + pp->check_failed = 1; + return 0; + } + + if (!found) + return 1; + + log_debug("Checking device %s for pvcreate %.32s", + pv_dev_name(pv), pv->dev->pvid[0] ? pv->dev->pvid : ""); + + /* + * This test will fail if the device belongs to an MD array. + */ + if (!dev_test_excl(pv->dev)) { + /* FIXME Detect whether device-mapper itself is still using it */ + log_error("Can't open %s exclusively. Mounted filesystem?", + pv_dev_name(pv)); + dm_list_move(&pp->arg_fail, &pd->list); + return 1; + } + + /* + * What kind of device is this: an orphan PV, an uninitialized/unused + * device, a PV used in a VG. + */ + + if (vg && !is_orphan_vg(vg->name)) { + /* Device is a PV used in a VG. */ + log_debug("Found pvcreate arg %s: pv is used in %s", pd->name, vg->name); + pd->is_vg_pv = 1; + pd->vg_name = dm_pool_strdup(cmd->mem, vg->name); + + } else if (vg && is_orphan_vg(vg->name)) { + if (is_used_pv(pv)) { + /* Device is used in an unknown VG. */ + log_debug("Found pvcreate arg %s: pv is used in unknown VG", pd->name); + pd->is_used_unknown_pv = 1; + } else { + /* Device is an orphan PV. */ + log_debug("Found pvcreate arg %s: pv is orphan in %s", pd->name, vg->name); + pd->is_orphan_pv = 1; + } + + if (!strcmp(vg->name, FMT_LVM1_ORPHAN_VG_NAME)) + pp->orphan_vg_name = FMT_LVM1_ORPHAN_VG_NAME; + else + pp->orphan_vg_name = FMT_TEXT_ORPHAN_VG_NAME; + } else { + log_debug("Found pvcreate arg %s: device is not a pv", pd->name); + /* Device is not a PV. */ + pd->is_not_pv = 1; + } + + /* + * pvcreate is being run on this device, and it's not a PV, + * or is an orphan PV. Neither case requires a prompt. + */ + if (pd->is_orphan_pv || pd->is_not_pv) { + pd->dev = pv->dev; + dm_list_move(&pp->arg_process, &pd->list); + return 1; + } + + /* + * pvcreate is being run on this device, but the device is already + * a PV in a VG. A prompt or force option is required to use it. + */ + + if (!(prompt = dm_pool_zalloc(cmd->mem, sizeof(*prompt)))) { + log_error("prompt alloc failed"); + pp->check_failed = 1; + return 0; + } + prompt->dev = pd->dev; + prompt->type = PROMPT_PVCREATE_PV_IN_VG; + prompt->pv_name = dm_pool_strdup(cmd->mem, pd->name); + if (pd->is_used_unknown_pv) + prompt->vg_name_unknown = 1; + else + prompt->vg_name = dm_pool_strdup(cmd->mem, vg->name); + dm_list_add(&pp->prompts, &prompt->list); + + pd->dev = pv->dev; + dm_list_move(&pp->arg_process, &pd->list); + return 1; +} + +/* + * This repeats the first check -- devices should be found, and should not have + * changed since the first check. If they were changed/used while the orphans + * lock was not held (during prompting), then they can't be used any more and + * are moved to arg_fail. If they are not found by this loop, that also + * disqualifies them from being used. Each arg_confirm entry that's found and + * is ok, is moved to arg_process. Those not found will remain in arg_confirm. + * + * This check does not need to look in foreign/shared/clustered VGs. If a + * device from arg_confirm was used in a foreign/shared/clustered VG during the + * prompts, then it will not be found during this check. + */ + +static int _pvcreate_check2_single(struct cmd_context *cmd, + struct volume_group *vg, + struct physical_volume *pv, + struct processing_handle *handle) +{ + struct pvcreate_each_params *pp = (struct pvcreate_each_params *) handle->custom_handle; + struct pvcreate_device *pd; + int found = 0; + + dm_list_iterate_items(pd, &pp->arg_confirm) { + if (pd->dev != pv->dev) + continue; + found = 1; + break; + } + + if (!found) + return 1; + + /* Repeat the same from check1. */ + if (!dev_test_excl(pv->dev)) { + /* FIXME Detect whether device-mapper itself is still using it */ + log_error("Can't open %s exclusively. Mounted filesystem?", + pv_dev_name(pv)); + goto fail; + } + + /* + * What kind of device is this: an orphan PV, an uninitialized/unused + * device, a PV used in a VG. + */ + + if (vg && !is_orphan_vg(vg->name)) { + /* Device is a PV used in a VG. */ + + if (pd->is_orphan_pv || pd->is_not_pv || pd->is_used_unknown_pv) { + /* In check1 it was an orphan or unused. */ + goto fail; + } + + if (pd->is_vg_pv && pd->vg_name && strcmp(pd->vg_name, vg->name)) { + /* In check1 it was in a different VG. */ + goto fail; + } + + } else if (is_orphan(pv)) { + /* Device is an orphan PV. */ + + if (pd->is_not_pv) { + /* In check1 it was not a PV. */ + goto fail; + } + + if (pd->is_vg_pv) { + /* In check1 it was in a VG. */ + goto fail; + } + + if (is_used_pv(pv) != pd->is_used_unknown_pv) { + /* In check1 it was different. */ + goto fail; + } + + } else { + /* Device is not a PV. */ + + if (pd->is_orphan_pv || pd->is_used_unknown_pv) { + /* In check1 it was an orphan PV. */ + goto fail; + } + + if (pd->is_vg_pv) { + /* In check1 it was in a VG. */ + goto fail; + } + } + + /* Device is unchanged from check1. */ + dm_list_move(&pp->arg_process, &pd->list); + return 1; + +fail: + log_error("Cannot use device %s: it changed during create.", pd->name); + dm_list_move(&pp->arg_fail, &pd->list); + return 1; +} + +/* + * This can be used by pvcreate, vgcreate and vgextend to create PVs. The + * callers need to set up the pvcreate_each_params structure based on command + * line args. This includes the pv_names field which specifies the devices to + * create PVs on. + * + * This uses process_each_pv() and should be called from a high level in the + * command -- the same level at which other instances of process_each are + * called. + * + * This function returns 0 (failed) if the caller requires all specified + * devices to be created, and any of those devices are not found, or any of + * them cannot be created. + * + * This function returns 1 (success) if the caller requires all specified + * devices to be created, and all are created, or if the caller does not + * require all specified devices to be created and one or more were created. + * + * When this function returns 1 (success), it returns to the caller with the + * VG_ORPHANS write lock held. + */ + +int pvcreate_each_device(struct cmd_context *cmd, + struct processing_handle *handle, + struct pvcreate_each_params *pp) +{ + struct pvcreate_restorable_params rp; + struct pvcreate_device *pd, *pd2; + struct pvcreate_prompt *prompt, *prompt2; + struct physical_volume *pv; + struct volume_group *orphan_vg; + struct dm_list arg_sort; + struct pv_list *pvl; + struct pv_list *vgpvl; + const char *pv_name; + int consistent = 0; + int must_use_all = (cmd->command->flags & MUST_USE_ALL_ARGS); + int found; + int i; + + dm_list_init(&arg_sort); + + handle->custom_handle = pp; + + /* + * Create a list entry for each name arg. + */ + for (i = 0; i < pp->pv_count; i++) { + dm_unescape_colons_and_at_signs(pp->pv_names[i], NULL, NULL); + + pv_name = pp->pv_names[i]; + + if (!(pd = dm_pool_zalloc(cmd->mem, sizeof(*pd)))) { + log_error("alloc failed"); + return 0; + } + + if (!(pd->name = dm_pool_strdup(cmd->mem, pv_name))) { + log_error("strdup failed"); + return 0; + } + + dm_list_add(&pp->arg_devices, &pd->list); + } + + /* + * This function holds the orphans lock while reading VGs to look for + * devices. This means the orphans lock is held while VG locks are + * acquired, which is against lvmcache lock ordering rules, so disable + * the lvmcache lock ordering checks. + */ + lvmcache_lock_ordering(0); + + /* + * Clear the cache before acquiring the orphan lock. (Clearing the + * cache with locks held is an error.) We want the orphan lock + * acquired before process_each_pv. If the orphan lock is not held + * when process_each_pv is called, then process_each_pv clears the + * cache. + */ + lvmcache_destroy(cmd, 1, 0); + + /* + * If no prompts require a user response, this orphan lock is held + * throughout, and pvcreate_each_device() returns with it held so that + * vgcreate/vgextend use the PVs created here to add to a VG. + */ + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) { + log_error("Can't get lock for orphan PVs"); + return 0; + } + + dev_cache_full_scan(cmd->full_filter); + + /* + * Use process_each_pv to search all existing PVs and devices. + * + * This is a slightly different way to use process_each_pv, because the + * command args (arg_devices) are not being processed directly by + * process_each_pv (argc and argv are not passed). Instead, + * process_each_pv is processing all existing PVs and devices, and the + * single function is matching each of those against the command args + * (arg_devices). + * + * If an arg_devices entry is found during process_each_pv, it's moved + * to arg_process if it can be used, or arg_fail if it cannot be used. + * If it's added to arg_process but needs a prompt or force option, then + * a corresponding prompt entry is added to pp->prompts. + */ + process_each_pv(cmd, 0, NULL, NULL, 1, 0, handle, _pvcreate_check1_single); + + /* + * A fatal error was found while checking. + */ + if (pp->check_failed) + goto_bad; + + /* + * Check if all arg_devices were found by process_each_pv. + */ + dm_list_iterate_items(pd, &pp->arg_devices) + log_error("Device %s not found (or ignored by filtering).", pd->name); + + /* + * Can the command continue if some specified devices were not found? + */ + if (!dm_list_empty(&pp->arg_devices) && must_use_all) + goto_bad; + + /* + * Can the command continue if some specified devices cannot be used? + */ + if (!dm_list_empty(&pp->arg_fail) && must_use_all) + goto_bad; + + /* + * The command cannot continue if there are no devices to create. + */ + if (dm_list_empty(&pp->arg_process)) { + log_error("No devices found."); + goto_bad; + } + + /* + * Clear any prompts that have answers without asking the user. + */ + dm_list_iterate_items_safe(prompt, prompt2, &pp->prompts) { + _check_pvcreate_prompt(cmd, pp, prompt, 0); + + switch (prompt->answer) { + case PROMPT_ANSWER_YES: + /* The PV can be used, leave it on arg_process. */ + dm_list_del(&prompt->list); + break; + case PROMPT_ANSWER_NO: + /* The PV cannot be used, remove it from arg_process. */ + if ((pd = _pvcreate_list_find_dev(&pp->arg_process, prompt->dev))) + dm_list_move(&pp->arg_fail, &pd->list); + dm_list_del(&prompt->list); + break; + } + } + + if (!dm_list_empty(&pp->arg_fail) && must_use_all) + goto_bad; + + /* + * If no remaining prompts need a user response, then keep orphans + * locked and go directly to the create steps. + */ + if (dm_list_empty(&pp->prompts)) + goto do_command; + + /* + * Prompts require asking the user, so release the orphans lock, ask + * the questions, reacquire the orphans lock, verify that the PVs were + * not used during the questions, then do the create steps. + */ + unlock_vg(cmd, VG_ORPHANS); + + /* + * Process prompts that require asking the user. The orphans lock is + * not held, so there's no harm in waiting for a user to respond. + */ + dm_list_iterate_items_safe(prompt, prompt2, &pp->prompts) { + _check_pvcreate_prompt(cmd, pp, prompt, 1); + + switch (prompt->answer) { + case PROMPT_ANSWER_YES: + /* The PV can be used, leave it on arg_process. */ + dm_list_del(&prompt->list); + break; + case PROMPT_ANSWER_NO: + /* The PV cannot be used, remove it from arg_process. */ + if ((pd = _pvcreate_list_find_dev(&pp->arg_process, prompt->dev))) + dm_list_move(&pp->arg_fail, &pd->list); + dm_list_del(&prompt->list); + break; + } + + if (!dm_list_empty(&pp->arg_fail) && must_use_all) + goto_out; + + if (sigint_caught()) + goto_out; + + if (prompt->abort_command) + goto_out; + } + + /* + * Clear the cache, reacquire the orphans write lock, then check again + * that the devices can still be used. If the second loop finds them + * changed, or can't find them any more, then they aren't used. + * Clear the cache here before locking orphans, since it won't be + * done by process_each_pv with orphans already locked. + */ + + lvmcache_destroy(cmd, 1, 0); + + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) { + log_error("Can't get lock for orphan PVs"); + goto_out; + } + + /* + * The device args began on the arg_devices list, then the first check + * loop moved those entries to arg_process as they were found. Devices + * not found during the first loop are not being used, and remain on + * arg_devices. + * + * Now, the arg_process entries are moved to arg_confirm, and the second + * check loop moves them back to arg_process as they are found and are + * unchanged. Like the first loop, the second loop moves an entry to + * arg_fail if it cannot be used. After the second loop, any devices + * remaining on arg_confirm were not found and are not used. + */ + + dm_list_splice(&pp->arg_confirm, &pp->arg_process); + + process_each_pv(cmd, 0, NULL, NULL, 1, 0, handle, _pvcreate_check2_single); + + dm_list_iterate_items(pd, &pp->arg_confirm) + log_error("Device %s not found (or ignored by filtering).", pd->name); + + /* Some devices were not found during the second check. */ + if (!dm_list_empty(&pp->arg_confirm) && must_use_all) + goto_bad; + + /* Some devices changed during the second check. */ + if (!dm_list_empty(&pp->arg_fail) && must_use_all) + goto_bad; + + if (dm_list_empty(&pp->arg_process)) { + log_error("No devices found."); + goto_bad; + } + +do_command: + + /* + * Reorder arg_process entries to match the original order of args. + */ + dm_list_splice(&arg_sort, &pp->arg_process); + for (i = 0; i < pp->pv_count; i++) { + if ((pd = _pvcreate_list_find_name(&arg_sort, pp->pv_names[i]))) + dm_list_move(&pp->arg_process, &pd->list); + } + + dm_list_splice(&pp->arg_create, &pp->arg_process); + + /* + * Wipe signatures on devices being created. + */ + dm_list_iterate_items_safe(pd, pd2, &pp->arg_create) { + log_verbose("Wiping signatures on new PV %s", pd->name); + + if (!wipe_known_signatures(cmd, pd->dev, pd->name, TYPE_LVM1_MEMBER | TYPE_LVM2_MEMBER, + 0, pp->yes, pp->force, &pd->wiped)) { + dm_list_move(&pp->arg_fail, &pd->list); + } + + if (sigint_caught()) + goto_bad; + } + + if (!dm_list_empty(&pp->arg_fail) && must_use_all) + goto_bad; + + /* + * Find existing orphan PVs that vgcreate or vgextend want to use. + * "preserve_existing" means that the command wants to use existing PVs + * and not recreate a new PV on top of an existing PV. + */ + if (pp->preserve_existing && pp->orphan_vg_name) { + log_debug("Using existing orphan PVs in %s", pp->orphan_vg_name); + + if (!(orphan_vg = vg_read_internal(cmd, pp->orphan_vg_name, NULL, 0, &consistent))) { + log_error("Cannot read orphans VG %s", pp->orphan_vg_name); + goto_bad; + } + + dm_list_iterate_items_safe(pd, pd2, &pp->arg_create) { + if (!pd->is_orphan_pv) + continue; + + if (!(pvl = dm_pool_alloc(cmd->mem, sizeof(*pvl)))) { + log_error("alloc pvl failed"); + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + + found = 0; + dm_list_iterate_items(vgpvl, &orphan_vg->pvs) { + if (vgpvl->pv->dev == pd->dev) { + found = 1; + break; + } + } + + if (found) { + log_debug("Using existing orphan PV %s", pv_dev_name(vgpvl->pv)); + pvl->pv = vgpvl->pv; + dm_list_add(&pp->pvs, &pvl->list); + } else { + log_error("Failed to find PV %s", pd->name); + dm_list_move(&pp->arg_fail, &pd->list); + } + } + } + + /* + * Create PVs on devices. Either create a new PV on top of an existing + * one (e.g. for pvcreate), or create a new PV on a device that is not + * a PV. + */ + dm_list_iterate_items_safe(pd, pd2, &pp->arg_create) { + /* Using existing orphan PVs is covered above. */ + if (pp->preserve_existing && pd->is_orphan_pv) + continue; + + if (!dm_list_empty(&pp->arg_fail) && must_use_all) + break; + + if (!(pvl = dm_pool_alloc(cmd->mem, sizeof(*pvl)))) { + log_error("alloc pvl failed"); + dm_list_move(&pp->arg_fail, &pd->list); + } + + pv_name = pd->name; + + log_debug("Creating a new PV on %s", pv_name); + + /* FIXME: get rid of rp usage in pv_create to avoid this. */ + memset(&rp, 0, sizeof(rp)); + rp.restorefile = pp->restorefile; + if (pp->uuid_str) { + rp.id = pp->id; + rp.idp = &pp->id; + } + rp.ba_start = pp->ba_start; + rp.ba_size = pp->ba_size; + rp.pe_start = pp->pe_start; + rp.extent_count = pp->extent_count; + rp.extent_size = pp->extent_size; + + if (!(pv = pv_create(cmd, pd->dev, pp->size, pp->data_alignment, + pp->data_alignment_offset, pp->labelsector, + pp->pvmetadatacopies, pp->pvmetadatasize, + pp->metadataignore, &rp))) { + log_error("Failed to setup physical volume \"%s\"", pv_name); + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + + log_verbose("Set up physical volume for \"%s\" with %" PRIu64 + " available sectors", pv_name, pv_size(pv)); + + if (!label_remove(pv->dev)) { + log_error("Failed to wipe existing label on %s", pv_name); + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + + if (pp->zero) { + log_verbose("Zeroing start of device %s", pv_name); + + if (!dev_open_quiet(pv->dev)) { + log_error("%s not opened: device not zeroed", pv_name); + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + + if (!dev_set(pv->dev, UINT64_C(0), (size_t) 2048, 0)) { + log_error("%s not wiped: aborting", pv_name); + if (!dev_close(pv->dev)) + stack; + dm_list_move(&pp->arg_fail, &pd->list); + continue; + } + if (!dev_close(pv->dev)) + stack; + } + + log_verbose("Writing physical volume data to disk \"%s\"", pv_name); + + if (!pv_write(cmd, pv, 0)) { + log_error("Failed to write physical volume \"%s\"", pv_name); + dm_list_move(&pp->arg_fail, &pd->list); + } + + log_print_unless_silent("Physical volume \"%s\" successfully created", pv_name); + + pvl->pv = pv; + dm_list_add(&pp->pvs, &pvl->list); + } + + dm_list_iterate_items(pvl, &pp->pvs) + log_debug("pv command succeeded for %s", pv_dev_name(pvl->pv)); + + dm_list_iterate_items(pd, &pp->arg_fail) + log_debug("pv command failed for %s", pd->name); + + if (!dm_list_empty(&pp->arg_fail)) + goto_bad; + + /* + * Returns with VG_ORPHANS write lock held because vgcreate and + * vgextend want to use the newly created PVs. + */ + return 1; + +bad: + unlock_vg(cmd, VG_ORPHANS); +out: + return 0; +} + diff --git a/tools/toollib.h b/tools/toollib.h index fc502b9..0ef9e92 100644 --- a/tools/toollib.h +++ b/tools/toollib.h @@ -101,8 +101,8 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv, struct processing_handle *handle, process_single_vg_fn_t process_single_vg); -int process_each_pv(struct cmd_context *cmd, int argc, char **argv, - const char *vg_name, uint32_t read_flags, +int process_each_pv(struct cmd_context *cmd, int argc, char **argv, const char *vg_name, + int all_is_set, uint32_t read_flags, struct processing_handle *handle, process_single_pv_fn_t process_single_pv); @@ -152,6 +152,10 @@ const char *extract_vgname(struct cmd_context *cmd, const char *lv_name); const char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name, unsigned *dev_dir_found); +void pvcreate_each_params_set_defaults(struct pvcreate_each_params *pp); +int pvcreate_each_params_from_args(struct cmd_context *cmd, struct pvcreate_each_params *pp); +int pvcreate_each_device(struct cmd_context *cmd, struct processing_handle *handle, struct pvcreate_each_params *pp); + /* * Builds a list of pv's from the names in argv. Used in * lvcreate/extend. diff --git a/tools/tools.h b/tools/tools.h index 979b5e3..5875376 100644 --- a/tools/tools.h +++ b/tools/tools.h @@ -107,6 +107,8 @@ struct arg_value_group_list { #define NO_METADATA_PROCESSING 0x00000040 /* Command wants to scan for new devices and force labels to be read from them all. */ #define REQUIRES_FULL_LABEL_SCAN 0x00000080 +/* Command must use all specified arg names and fail if all cannot be used. */ +#define MUST_USE_ALL_ARGS 0x00000100 /* a register of the lvm commands */ struct command { diff --git a/tools/vgreduce.c b/tools/vgreduce.c index 2efaa7c..41b5b83 100644 --- a/tools/vgreduce.c +++ b/tools/vgreduce.c @@ -188,7 +188,7 @@ int vgreduce(struct cmd_context *cmd, int argc, char **argv) /* FIXME: Pass private struct through to all these functions */ /* and update in batch afterwards? */ return process_each_pv(cmd, argc, argv, vg_name, - READ_FOR_UPDATE, NULL, + 0, READ_FOR_UPDATE, NULL, _vgreduce_single); log_verbose("Finding volume group \"%s\"", vg_name);

1 0

master - vgconvert: refactor to avoid pvcreate code
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=ff2267012aea37e0… Commit: ff2267012aea37e08ed1fc2a511f8a0d3f0d145b Parent: 5dd615c41ee1c6c77a927b5f39bf1554caaa572f Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 16 13:22:50 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 vgconvert: refactor to avoid pvcreate code This uses the vg->pv_write_list in place of the vg->pvs_to_write list, and eliminates the use of pvcreate_params. The label remove and zeroing steps are shifted out of vg_write() to the higher level like pvcreate will do. --- lib/format_text/archiver.c | 80 ++++++++++++++++++++++++++++++-------------- lib/format_text/archiver.h | 9 ++++- tools/vgconvert.c | 31 +++++++++-------- 3 files changed, 79 insertions(+), 41 deletions(-) diff --git a/lib/format_text/archiver.c b/lib/format_text/archiver.c index e52854f..07dece3 100644 --- a/lib/format_text/archiver.c +++ b/lib/format_text/archiver.c @@ -329,11 +329,11 @@ struct volume_group *backup_read_vg(struct cmd_context *cmd, return vg; } -static int _restore_vg_should_write_pv(struct physical_volume *pv, struct pvcreate_params *pp) +static int _restore_vg_should_write_pv(struct physical_volume *pv, int do_pvcreate) { struct lvmcache_info *info; - if (pp) + if (do_pvcreate) return 1; if (!(pv->fmt->features & FMT_PV_FLAGS)) @@ -359,8 +359,15 @@ static int _restore_vg_should_write_pv(struct physical_volume *pv, struct pvcrea /* ORPHAN and VG locks held before calling this */ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, - struct pvcreate_params *pp, int drop_lvmetad) + int drop_lvmetad, + int do_pvcreate, + uint64_t bootloaderareasize, + int pvmetadatacopies, + uint64_t pvmetadatasize, + uint64_t label_sector) + { + struct pvcreate_restorable_params rp = { 0 }; struct dm_list new_pvs; struct pv_list *pvl, *new_pvl; struct physical_volume *existing_pv, *pv; @@ -368,7 +375,6 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, struct format_instance *fid; struct format_instance_ctx fic; int should_write_pv; - struct pv_to_write *pvw; uint32_t tmp_extent_size; /* @@ -377,25 +383,26 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, */ /* Prepare new PVs if needed. */ - if (pp) { + if (do_pvcreate) { dm_list_init(&new_pvs); dm_list_iterate_items(pvl, &vg->pvs) { existing_pv = pvl->pv; - pp->rp.id = existing_pv->id; - pp->rp.idp = &pp->rp.id; - pp->rp.pe_start = pv_pe_start(existing_pv); - pp->rp.extent_count = pv_pe_count(existing_pv); - pp->rp.extent_size = pv_pe_size(existing_pv); + rp.id = existing_pv->id; + rp.idp = &rp.id; + rp.pe_start = pv_pe_start(existing_pv); + rp.extent_count = pv_pe_count(existing_pv); + rp.extent_size = pv_pe_size(existing_pv); + rp.ba_size = bootloaderareasize; /* pe_end = pv_pe_count(existing_pv) * pv_pe_size(existing_pv) + pe_start - 1 */ if (!(pv = pv_create(cmd, pv_dev(existing_pv), 0, 0, 0, - pp->labelsector, - pp->pvmetadatacopies, - pp->pvmetadatasize, - 0, &pp->rp))) { + label_sector, + pvmetadatacopies, + pvmetadatasize, + 0, &rp))) { log_error("Failed to setup physical volume \"%s\".", pv_dev_name(existing_pv)); return 0; @@ -408,6 +415,7 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, pv_dev_name(pvl->pv)); return 0; } + new_pvl->pv = pv; dm_list_add(&new_pvs, &new_pvl->list); @@ -427,7 +435,7 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, return 0; } - if (pp) { + if (do_pvcreate) { log_verbose("Deleting existing metadata for VG %s.", vg->name); if (!vg_remove_mdas(vg)) { cmd->fmt->ops->destroy_instance(fid); @@ -449,23 +457,18 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, /* Add any metadata areas on the PVs */ dm_list_iterate_items(pvl, pvs) { - if ((should_write_pv = _restore_vg_should_write_pv(pvl->pv, pp)) < 0) + if ((should_write_pv = _restore_vg_should_write_pv(pvl->pv, do_pvcreate)) < 0) return_0; if (should_write_pv) { - if (!(pvw = dm_pool_zalloc(vg->vgmem, sizeof(*pvw)))) { + if (!(new_pvl = dm_pool_zalloc(vg->vgmem, sizeof(*new_pvl)))) { log_error("Failed to allocate structure for scheduled " "writing of PV '%s'.", pv_dev_name(pvl->pv)); return 0; } - if (pp) { - pvw->new_pv = 1; - pvw->pp = pp; - } - - pvw->pv = pvl->pv; - dm_list_add(&vg->pvs_to_write, &pvw->list); + new_pvl->pv = pvl->pv; + dm_list_add(&vg->pv_write_list, &new_pvl->list); } /* Add any metadata areas on the PV. */ @@ -480,6 +483,33 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, vg->extent_size = tmp_extent_size; } + if (do_pvcreate) { + dm_list_iterate_items(pvl, &vg->pv_write_list) { + struct device *dev = pv_dev(pvl->pv); + const char *pv_name = dev_name(dev); + + if (!label_remove(dev)) { + log_error("Failed to wipe existing label on %s", pv_name); + return 0; + } + + log_verbose("Zeroing start of device %s", pv_name); + if (!dev_open_quiet(dev)) { + log_error("%s not opened: device not zeroed", pv_name); + return 0; + } + + if (!dev_set(dev, UINT64_C(0), (size_t) 2048, 0)) { + log_error("%s not wiped: aborting", pv_name); + if (!dev_close(dev)) + stack; + return 0; + } + if (!dev_close(dev)) + stack; + } + } + if (!vg_write(vg)) return_0; @@ -531,7 +561,7 @@ int backup_restore_from_file(struct cmd_context *cmd, const char *vg_name, missing_pvs = vg_missing_pv_count(vg); if (missing_pvs == 0) - r = backup_restore_vg(cmd, vg, NULL, 1); + r = backup_restore_vg(cmd, vg, 1, 0, 0, 0, 0, 0); else log_error("Cannot restore Volume Group %s with %i PVs " "marked as missing.", vg->name, missing_pvs); diff --git a/lib/format_text/archiver.h b/lib/format_text/archiver.h index f85e5af..b8e53e3 100644 --- a/lib/format_text/archiver.h +++ b/lib/format_text/archiver.h @@ -51,8 +51,15 @@ int backup_remove(struct cmd_context *cmd, const char *vg_name); struct volume_group *backup_read_vg(struct cmd_context *cmd, const char *vg_name, const char *file); + int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg, - struct pvcreate_params *pp, int drop_lvmetad); + int drop_lvmetad, + int do_pvcreate, + uint64_t bootloaderareasize, + int pvmetadatacopies, + uint64_t pvmetadatasize, + uint64_t label_sector); + int backup_restore_from_file(struct cmd_context *cmd, const char *vg_name, const char *file, int force); int backup_restore(struct cmd_context *cmd, const char *vg_name, int force); diff --git a/tools/vgconvert.c b/tools/vgconvert.c index de21a47..1462d77 100644 --- a/tools/vgconvert.c +++ b/tools/vgconvert.c @@ -19,11 +19,13 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, struct volume_group *vg, struct processing_handle *handle __attribute__((unused))) { - struct pvcreate_params pp; struct logical_volume *lv; struct lv_list *lvl; struct lvinfo info; int active = 0; + int pvmetadatacopies = 0; + uint64_t pvmetadatasize = 0; + uint64_t bootloaderareasize = 0; if (!vg_check_status(vg, LVM_WRITE | EXPORTED_VG)) return_ECMD_FAILED; @@ -34,23 +36,19 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, return ECMD_FAILED; } - pvcreate_params_set_defaults(&pp); - if (cmd->fmt->features & FMT_MDAS) { if (arg_sign_value(cmd, metadatasize_ARG, SIGN_NONE) == SIGN_MINUS) { log_error("Metadata size may not be negative"); return EINVALID_CMD_LINE; } - pp.pvmetadatasize = arg_uint64_value(cmd, metadatasize_ARG, UINT64_C(0)); - if (!pp.pvmetadatasize) - pp.pvmetadatasize = - find_config_tree_int(cmd, metadata_pvmetadatasize_CFG, NULL); + pvmetadatasize = arg_uint64_value(cmd, metadatasize_ARG, UINT64_C(0)); + if (!pvmetadatasize) + pvmetadatasize = find_config_tree_int(cmd, metadata_pvmetadatasize_CFG, NULL); - pp.pvmetadatacopies = arg_int_value(cmd, pvmetadatacopies_ARG, -1); - if (pp.pvmetadatacopies < 0) - pp.pvmetadatacopies = - find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL); + pvmetadatacopies = arg_int_value(cmd, pvmetadatacopies_ARG, -1); + if (pvmetadatacopies < 0) + pvmetadatacopies = find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL); } if (cmd->fmt->features & FMT_BAS) { @@ -59,11 +57,9 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, return EINVALID_CMD_LINE; } - pp.rp.ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, UINT64_C(0)); + bootloaderareasize = arg_uint64_value(cmd, bootloaderareasize_ARG, UINT64_C(0)); } - pp.labelsector = arg_int64_value(cmd, labelsector_ARG, DEFAULT_LABELSECTOR); - if (!vg_check_new_extent_size(cmd->fmt, vg->extent_size)) return_ECMD_FAILED; @@ -131,7 +127,12 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name, log_verbose("Writing metadata for VG %s using format %s", vg_name, cmd->fmt->name); - if (!backup_restore_vg(cmd, vg, &pp, 0)) { + + if (!backup_restore_vg(cmd, vg, 0, 1, + bootloaderareasize, + pvmetadatacopies, + pvmetadatasize, + arg_int64_value(cmd, labelsector_ARG, DEFAULT_LABELSECTOR))) { log_error("Conversion failed for volume group %s.", vg_name); log_error("Use pvcreate and vgcfgrestore to repair from " "archived metadata.");

1 0

master - metadata: use pv_write_list for _check_old_pv_ext_for_vg
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=5dd615c41ee1c6c7… Commit: 5dd615c41ee1c6c77a927b5f39bf1554caaa572f Parent: bafbc72c8c42f2e48b332c54ce00403c95dc74f8 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 16 13:02:00 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 metadata: use pv_write_list for _check_old_pv_ext_for_vg The _check_old_pv_ext_for_vg() function only needs to do pv_write(), so it can use the simpler pv_list structs on the pv_write_list. --- lib/metadata/metadata.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index a5ee865..d7a9d81 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -3084,12 +3084,12 @@ out: return r; } -static int _pv_in_pvs_to_write_list(struct physical_volume *pv, struct volume_group *vg) +static int _pv_in_pv_list(struct physical_volume *pv, struct dm_list *head) { - struct pv_to_write *pvw; + struct pv_list *pvl; - dm_list_iterate_items(pvw, &vg->pvs_to_write) { - if (pvw->pv == pv) + dm_list_iterate_items(pvl, head) { + if (pvl->pv == pv) return 1; } @@ -3102,8 +3102,7 @@ static int _pv_in_pvs_to_write_list(struct physical_volume *pv, struct volume_gr */ static int _check_old_pv_ext_for_vg(struct volume_group *vg) { - struct pv_list *pvl; - struct pv_to_write *pvw; + struct pv_list *pvl, *new_pvl; int pv_needs_rewrite; if (!(vg->fid->fmt->features & FMT_PV_FLAGS)) @@ -3122,15 +3121,15 @@ static int _check_old_pv_ext_for_vg(struct volume_group *vg) /* * Schedule PV for writing only once! */ - if (_pv_in_pvs_to_write_list(pvl->pv, vg)) + if (_pv_in_pv_list(pvl->pv, &vg->pv_write_list)) continue; - if (!(pvw = dm_pool_zalloc(vg->vgmem, sizeof(*pvw)))) { + if (!(new_pvl = dm_pool_zalloc(vg->vgmem, sizeof(*new_pvl)))) { log_error("pv_to_write allocation for '%s' failed", pv_dev_name(pvl->pv)); return 0; } - pvw->pv = pvl->pv; - dm_list_add(&vg->pvs_to_write, &pvw->list); + new_pvl->pv = pvl->pv; + dm_list_add(&vg->pv_write_list, &new_pvl->list); } }

1 0

master - metadata: refactor part of add_pv_to_vg
by David Teigland 25 Feb '16

25 Feb '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=bafbc72c8c42f2e4… Commit: bafbc72c8c42f2e48b332c54ce00403c95dc74f8 Parent: 5e5ad77f5f89bdc8a054d962a89e450900b249e7 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 16 12:49:57 2016 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Thu Feb 25 09:14:09 2016 -0600 metadata: refactor part of add_pv_to_vg This shifts the use of the 'pv_to_write' struct and the 'pvcreate_params' struct to the one caller of add_pv_to_vg, which is made static. --- lib/metadata/metadata.c | 34 ++++++++++++++++------------------ lib/metadata/metadata.h | 4 ---- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index d3d4cab..a5ee865 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -169,18 +169,15 @@ void del_pvl_from_vgs(struct volume_group *vg, struct pv_list *pvl) * @vg - volume group to add to * @pv_name - name of the pv (to be removed) * @pv - physical volume to add to volume group - * @pp - physical volume creation params (OPTIONAL) * * Returns: * 0 - failure * 1 - success * FIXME: remove pv_name - obtain safely from pv */ -int add_pv_to_vg(struct volume_group *vg, const char *pv_name, - struct physical_volume *pv, struct pvcreate_params *pp, - int new_pv) +static int add_pv_to_vg(struct volume_group *vg, const char *pv_name, + struct physical_volume *pv, int new_pv) { - struct pv_to_write *pvw; struct pv_list *pvl; struct format_instance *fid = vg->fid; struct dm_pool *mem = vg->vgmem; @@ -289,18 +286,6 @@ int add_pv_to_vg(struct volume_group *vg, const char *pv_name, break; } - if ((pv->fmt->features & FMT_PV_FLAGS) || - (pv->status & UNLABELLED_PV)) { - if (!(pvw = dm_pool_zalloc(mem, sizeof(*pvw)))) { - log_error("pv_to_write allocation for '%s' failed", pv_name); - return 0; - } - pvw->pv = pv; - pvw->pp = new_pv ? pp : NULL; - pvw->new_pv = new_pv; - dm_list_add(&vg->pvs_to_write, &pvw->list); - } - return 1; } @@ -723,6 +708,7 @@ static int vg_extend_single_pv(struct volume_group *vg, char *pv_name, unsigned int *max_phys_block_size) { struct physical_volume *pv; + struct pv_to_write *pvw; int new_pv = 0; pv = find_pv_by_name(vg->cmd, pv_name, 1, 1); @@ -741,9 +727,21 @@ static int vg_extend_single_pv(struct volume_group *vg, char *pv_name, max_phys_block_size))) goto_bad; - if (!add_pv_to_vg(vg, pv_name, pv, pp, new_pv)) + if (!add_pv_to_vg(vg, pv_name, pv, new_pv)) goto_bad; + if ((pv->fmt->features & FMT_PV_FLAGS) || + (pv->status & UNLABELLED_PV)) { + if (!(pvw = dm_pool_zalloc(vg->vgmem, sizeof(*pvw)))) { + log_error("pv_to_write allocation for '%s' failed", pv_name); + return 0; + } + pvw->pv = pv; + pvw->pp = new_pv ? pp : NULL; + pvw->new_pv = new_pv; + dm_list_add(&vg->pvs_to_write, &pvw->list); + } + return 1; bad: free_pv_fid(pv); diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h index f2b62e4..192f88e 100644 --- a/lib/metadata/metadata.h +++ b/lib/metadata/metadata.h @@ -504,10 +504,6 @@ struct id pv_id(const struct physical_volume *pv); const struct format_type *pv_format_type(const struct physical_volume *pv); struct id pv_vgid(const struct physical_volume *pv); -int add_pv_to_vg(struct volume_group *vg, const char *pv_name, - struct physical_volume *pv, struct pvcreate_params *pp, - int new_pv); - uint64_t find_min_mda_size(struct dm_list *mdas); char *tags_format_and_copy(struct dm_pool *mem, const struct dm_list *tagsl);

1 0

← Newer
1
2
3
4
5
6
7
8
...
18
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits February 2016