diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 1d92e0198..e2d19e899 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -772,7 +772,7 @@ next: * to that VG after a scan. */ -int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid) +static int _label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid, int rw) { struct dm_list devs; struct device_list *devl, *devl2; @@ -804,7 +804,10 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const /* FIXME: should we also rescan unused_duplicate_devs for devs being rescanned here and then repeat resolving the duplicates? */ - label_scan_devs(cmd, cmd->filter, &devs); + if (rw) + label_scan_devs_rw(cmd, cmd->filter, &devs); + else + label_scan_devs(cmd, cmd->filter, &devs); dm_list_iterate_items_safe(devl, devl2, &devs) { dm_list_del(&devl->list); @@ -819,6 +822,16 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const return 1; } +int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid) +{ + return _label_rescan_vg(cmd, vgname, vgid, 0); +} + +int lvmcache_label_rescan_vg_rw(struct cmd_context *cmd, const char *vgname, const char *vgid) +{ + return _label_rescan_vg(cmd, vgname, vgid, 1); +} + /* * Uses label_scan to populate lvmcache with 'vginfo' struct for each VG * and associated 'info' structs for those VGs. Only VG summary information diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 5b78d7afb..e2d967c62 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -71,6 +71,7 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset); int lvmcache_label_scan(struct cmd_context *cmd); int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid); +int lvmcache_label_rescan_vg_rw(struct cmd_context *cmd, const char *vgname, const char *vgid); /* Add/delete a device */ struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid, diff --git a/lib/label/label.c b/lib/label/label.c index a8d87ec16..185e51b0c 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -1118,7 +1118,37 @@ int label_scan_devs(struct cmd_context *cmd, struct dev_filter *f, struct dm_lis _scan_list(cmd, f, devs, NULL); - /* FIXME: this function should probably fail if any devs couldn't be scanned */ + return 1; +} + +/* + * This function is used when the caller plans to write to the devs, so opening + * them RW during rescan avoids needing to close and reopen with WRITE in + * dev_write_bytes. + */ + +int label_scan_devs_rw(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs) +{ + struct device_list *devl; + + if (!scan_bcache) { + if (!_setup_bcache(0)) + return 0; + } + + dm_list_iterate_items(devl, devs) { + if (_in_bcache(devl->dev)) { + bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _scan_dev_close(devl->dev); + } + /* + * With this flag set, _scan_dev_open() done by + * _scan_list() will do open RW + */ + devl->dev->flags |= DEV_BCACHE_WRITE; + } + + _scan_list(cmd, f, devs, NULL); return 1; } diff --git a/lib/label/label.h b/lib/label/label.h index 07bb77d88..f06b7df63 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -104,6 +104,7 @@ extern struct bcache *scan_bcache; int label_scan(struct cmd_context *cmd); int label_scan_devs(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs); +int label_scan_devs_rw(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs); int label_scan_devs_excl(struct dm_list *devs); void label_scan_invalidate(struct device *dev); void label_scan_invalidate_lv(struct cmd_context *cmd, struct logical_volume *lv); diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 9029d3f64..966c88f95 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -184,8 +184,9 @@ #define READ_ALLOW_EXPORTED 0x00020000U #define READ_OK_NOTFOUND 0x00040000U #define READ_WARN_INCONSISTENT 0x00080000U -#define READ_FOR_UPDATE 0x00100000U /* A meta-flag, useful with toollib for_each_* functions. */ -#define PROCESS_SKIP_SCAN 0x00200000U /* skip lvmcache_label_scan in process_each_pv */ +#define READ_FOR_UPDATE 0x00100000U /* command tells vg_read it plans to write the vg */ +#define PROCESS_SKIP_SCAN 0x00200000U /* skip lvmcache_label_scan in process_each_pv */ +#define READ_FOR_ACTIVATE 0x00400000U /* command tells vg_read it plans to activate the vg */ /* vg_read returns these in error_flags */ #define FAILED_NOT_ENABLED 0x00000001U diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 039a7d690..7b0d6ce92 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -4476,7 +4476,8 @@ void vg_write_commit_bad_mdas(struct cmd_context *cmd, struct volume_group *vg) static struct volume_group *_vg_read(struct cmd_context *cmd, const char *vgname, const char *vgid, - unsigned precommitted) + unsigned precommitted, + int writing) { const struct format_type *fmt = cmd->fmt; struct format_instance *fid = NULL; @@ -4530,8 +4531,11 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, * we can also skip the rescan in that case. */ if (!cmd->can_use_one_scan || lvmcache_scan_mismatch(cmd, vgname, vgid)) { - log_debug_metadata("Rescanning devices for %s", vgname); - lvmcache_label_rescan_vg(cmd, vgname, vgid); + log_debug_metadata("Rescanning devices for %s %s", vgname, writing ? "rw" : ""); + if (writing) + lvmcache_label_rescan_vg_rw(cmd, vgname, vgid); + else + lvmcache_label_rescan_vg(cmd, vgname, vgid); } else { log_debug_metadata("Skipped rescanning devices for %s", vgname); } @@ -4752,6 +4756,7 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const int missing_pv_flag = 0; uint32_t failure = 0; int writing = (read_flags & READ_FOR_UPDATE); + int activating = (read_flags & READ_FOR_ACTIVATE); if (is_orphan_vg(vg_name)) { log_very_verbose("Reading orphan VG %s", vg_name); @@ -4766,13 +4771,23 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const return NULL; } - if (!lock_vol(cmd, vg_name, writing ? LCK_VG_WRITE : LCK_VG_READ, NULL)) { + /* + * When a command is reading the VG with the intention of eventually + * writing it, it passes the READ_FOR_UPDATE flag. This causes vg_read + * to acquire an exclusive VG lock, and causes vg_read to do some more + * checks, e.g. that the VG is writable and not exported. It also + * means that when the label scan is repeated on the VG's devices, the + * VG's PVs can be reopened read-write when rescanning in anticipation + * of needing to write to them. + */ + + if (!lock_vol(cmd, vg_name, (writing || activating) ? LCK_VG_WRITE : LCK_VG_READ, NULL)) { log_error("Can't get lock for %s", vg_name); failure |= FAILED_LOCKING; goto_bad; } - if (!(vg = _vg_read(cmd, vg_name, vgid, 0))) { + if (!(vg = _vg_read(cmd, vg_name, vgid, 0, writing))) { /* Some callers don't care if the VG doesn't exist and don't want an error message. */ if (!(read_flags & READ_OK_NOTFOUND)) log_error("Volume group \"%s\" not found", vg_name); @@ -4883,29 +4898,36 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const goto_bad; } - if (writing && !(read_flags & READ_ALLOW_EXPORTED) && vg_is_exported(vg)) { - log_error("Volume group %s is exported", vg->name); - failure |= FAILED_EXPORTED; - goto_bad; - } + /* + * If the command intends to write or activate the VG, there are + * additional restrictions. FIXME: These restrictions should + * probably be checked/applied after vg_read returns. + */ + if (writing || activating) { + if (!(read_flags & READ_ALLOW_EXPORTED) && vg_is_exported(vg)) { + log_error("Volume group %s is exported", vg->name); + failure |= FAILED_EXPORTED; + goto_bad; + } - if (writing && !(vg->status & LVM_WRITE)) { - log_error("Volume group %s is read-only", vg->name); - failure |= FAILED_READ_ONLY; - goto_bad; - } + if (!(vg->status & LVM_WRITE)) { + log_error("Volume group %s is read-only", vg->name); + failure |= FAILED_READ_ONLY; + goto_bad; + } - if (!cmd->handles_missing_pvs && (missing_pv_dev || missing_pv_flag) && writing) { - log_error("Cannot change VG %s while PVs are missing.", vg->name); - log_error("See vgreduce --removemissing and vgextend --restoremissing."); - failure |= FAILED_NOT_ENABLED; - goto_bad; - } + if (!cmd->handles_missing_pvs && (missing_pv_dev || missing_pv_flag)) { + log_error("Cannot change VG %s while PVs are missing.", vg->name); + log_error("See vgreduce --removemissing and vgextend --restoremissing."); + failure |= FAILED_NOT_ENABLED; + goto_bad; + } - if (!cmd->handles_unknown_segments && vg_has_unknown_segments(vg) && writing) { - log_error("Cannot change VG %s with unknown segments in it!", vg->name); - failure |= FAILED_NOT_ENABLED; /* FIXME new failure code here? */ - goto_bad; + if (!cmd->handles_unknown_segments && vg_has_unknown_segments(vg)) { + log_error("Cannot change VG %s with unknown segments in it!", vg->name); + failure |= FAILED_NOT_ENABLED; /* FIXME new failure code here? */ + goto_bad; + } } /* diff --git a/tools/lvchange.c b/tools/lvchange.c index 7bdf99742..e7fb57d1d 100644 --- a/tools/lvchange.c +++ b/tools/lvchange.c @@ -1435,7 +1435,7 @@ int lvchange_activate_cmd(struct cmd_context *cmd, int argc, char **argv) } else /* Component LVs might be active, support easy deactivation */ cmd->process_component_lvs = 1; - ret = process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, + ret = process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_ACTIVATE, NULL, &_lvchange_activate_check, &_lvchange_activate_single); if (ret != ECMD_PROCESSED) diff --git a/tools/pvscan.c b/tools/pvscan.c index d41345fac..12711cb0a 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -900,7 +900,7 @@ static int _pvscan_aa(struct cmd_context *cmd, struct pvscan_aa_params *pp, return ECMD_PROCESSED; } - ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, READ_FOR_UPDATE, 0, handle, _pvscan_aa_single); + ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, READ_FOR_ACTIVATE, 0, handle, _pvscan_aa_single); destroy_processing_handle(cmd, handle); diff --git a/tools/vgchange.c b/tools/vgchange.c index a17f4566f..aad6db32f 100644 --- a/tools/vgchange.c +++ b/tools/vgchange.c @@ -800,8 +800,10 @@ int vgchange(struct cmd_context *cmd, int argc, char **argv) cmd->lockd_vg_enforce_sh = 1; } - if (update || arg_is_set(cmd, activate_ARG)) + if (update) flags |= READ_FOR_UPDATE; + else if (arg_is_set(cmd, activate_ARG)) + flags |= READ_FOR_ACTIVATE; if (!(handle = init_processing_handle(cmd, NULL))) { log_error("Failed to initialize processing handle.");