From 455dbd8444074aa235c284d0e3285410b9096401 Mon Sep 17 00:00:00 2001
From: David Teigland <teigland@redhat.com>
Date: Thu, 21 Feb 2019 15:52:45 -0600
Subject: [PATCH 2/5] pvscan: fix autoactivation from concurrent pvscans
Use a file lock to ensure that only one pvscan will do
initialization of pvs_online, otherwise multiple concurrent
pvscans may all see an empty pvs_online directory and
do initialization.
The pvscan that is doing initialization should also only
attempt to activate complete VGs.
---
tools/pvscan.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 120 insertions(+), 21 deletions(-)
diff --git a/tools/pvscan.c b/tools/pvscan.c
index 2a884df..df46e04 100644
--- a/tools/pvscan.c
+++ b/tools/pvscan.c
@@ -19,6 +19,7 @@
#include "lib/metadata/metadata.h"
#include <dirent.h>
+#include <sys/file.h>
struct pvscan_params {
int new_pvs_found;
@@ -32,10 +33,61 @@ struct pvscan_params {
};
struct pvscan_aa_params {
- int refresh_all;
unsigned int activate_errors;
};
+static const char *_online_file = DEFAULT_RUN_DIR "/pvs_online_lock";
+static int _online_fd = -1;
+
+static int _lock_online(int mode, int nonblock)
+{
+ int fd;
+ int op = mode;
+ int ret;
+
+ if (nonblock)
+ op |= LOCK_NB;
+
+ if (_online_fd != -1) {
+ log_warn("lock_online existing fd %d", _online_fd);
+ return 0;
+ }
+
+ fd = open(_online_file, O_RDWR | S_IRUSR | S_IWUSR);
+ if (fd < 0) {
+ log_debug("lock_online open errno %d", errno);
+ return 0;
+ }
+
+ ret = flock(fd, op);
+ if (!ret) {
+ _online_fd = fd;
+ return 1;
+ }
+
+ if (close(fd))
+ stack;
+ return 0;
+}
+
+static void _unlock_online(void)
+{
+ int ret;
+
+ if (_online_fd == -1) {
+ log_warn("unlock_online no existing fd");
+ return;
+ }
+
+ ret = flock(_online_fd, LOCK_UN);
+ if (ret)
+ log_warn("unlock_online flock errno %d", errno);
+
+ if (close(_online_fd))
+ stack;
+ _online_fd = -1;
+}
+
static int _pvscan_display_pv(struct cmd_context *cmd,
struct physical_volume *pv,
struct pvscan_params *params)
@@ -341,6 +393,20 @@ static void _online_pvid_dir_setup(void)
log_debug("Failed to create %s", _pvs_online_dir);
}
+static void _online_file_setup(void)
+{
+ FILE *fp;
+ struct stat st;
+
+ if (!stat(_online_file, &st))
+ return;
+
+ if (!(fp = fopen(_online_file, "w")))
+ return;
+ if (fclose(fp))
+ stack;
+}
+
static int _online_pvid_files_missing(void)
{
DIR *dir;
@@ -584,12 +650,12 @@ static int _pvscan_aa_single(struct cmd_context *cmd, const char *vg_name,
}
static int _pvscan_aa(struct cmd_context *cmd, struct pvscan_aa_params *pp,
- int all_vgs, struct dm_list *vgnames)
+ struct dm_list *vgnames)
{
struct processing_handle *handle = NULL;
int ret;
- if (!all_vgs && dm_list_empty(vgnames)) {
+ if (dm_list_empty(vgnames)) {
log_debug("No VGs to autoactivate.");
return ECMD_PROCESSED;
}
@@ -601,11 +667,6 @@ static int _pvscan_aa(struct cmd_context *cmd, struct pvscan_aa_params *pp,
handle->custom_handle = pp;
- if (all_vgs) {
- cmd->cname->flags |= ALL_VGS_IS_DEFAULT;
- pp->refresh_all = 1;
- }
-
ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, READ_FOR_UPDATE, 0, handle, _pvscan_aa_single);
destroy_processing_handle(cmd, handle);
@@ -617,7 +678,8 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
{
struct pvscan_aa_params pp = { 0 };
struct dm_list single_devs;
- struct dm_list found_vgnames;
+ struct dm_list vgnames;
+ struct dm_list *complete_vgnames = NULL;
struct device *dev;
struct device_list *devl;
const char *pv_name;
@@ -628,12 +690,14 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
struct arg_value_group_list *current_group;
dev_t devno;
int do_activate = arg_is_set(cmd, activate_ARG);
- int all_vgs = 0;
int add_errors = 0;
int ret = ECMD_PROCESSED;
dm_list_init(&single_devs);
- dm_list_init(&found_vgnames);
+ dm_list_init(&vgnames);
+
+ if (do_activate)
+ complete_vgnames = &vgnames;
if (arg_is_set(cmd, major_ARG) + arg_is_set(cmd, minor_ARG))
devno_args = 1;
@@ -644,6 +708,7 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
}
_online_pvid_dir_setup();
+ _online_file_setup();
if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_READ, NULL)) {
log_error("Unable to obtain global lock.");
@@ -654,19 +719,53 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
* Scan all devices when no args are given.
*/
if (!argc && !devno_args) {
- log_verbose("pvscan all devices.");
+ _lock_online(LOCK_EX, 0);
+ log_verbose("pvscan all devices for requested refresh.");
_online_pvid_files_remove();
- _online_pvscan_all_devs(cmd, NULL, NULL);
- all_vgs = 1;
+ /* identify complete vgs, and only activate those vgs */
+ _online_pvscan_all_devs(cmd, complete_vgnames, NULL);
+ _unlock_online();
goto activate;
}
+ /*
+ * Initialization case:
+ * lock_online ex
+ * if empty
+ * pvscan all
+ * create pvid files
+ * identify complete vgs
+ * unlock_online
+ * activate complete vgs
+ *
+ * Non-initialization case:
+ * lock_online ex
+ * if not empty
+ * unlock_unlock
+ * pvscan devs
+ * create pvid files
+ * identify complete vgs
+ * activate complete vgs
+ *
+ * In the non-init case, a VG with two PVs, where both PVs appear at once
+ * two parallel pvscans for each PV create the pvid files for each PV in
+ * parallel, then both pvscans see the vg has completed, and both pvscans
+ * activate the VG in parallel. The activations should be serialized by
+ * the VG lock.
+ */
+
+ _lock_online(LOCK_EX, 0);
+
if (_online_pvid_files_missing()) {
log_verbose("pvscan all devices to initialize available PVs.");
_online_pvid_files_remove();
- _online_pvscan_all_devs(cmd, NULL, NULL);
- all_vgs = 1;
+ /* identify complete vgs, and only activate those vgs */
+ _online_pvscan_all_devs(cmd, complete_vgnames, NULL);
+ _unlock_online();
goto activate;
+ } else {
+ log_verbose("pvscan only specific devices.");
+ _unlock_online();
}
if (argc || devno_args) {
@@ -743,7 +842,7 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
* Devices that exist and pass the lvmetad filter
* are online.
*/
- if (!_online_pvscan_one(cmd, dev, NULL, &found_vgnames, 0, &pvid_without_metadata))
+ if (!_online_pvscan_one(cmd, dev, NULL, complete_vgnames, 0, &pvid_without_metadata))
add_errors++;
}
}
@@ -794,7 +893,7 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
* Devices that exist and pass the lvmetad filter
* are online.
*/
- if (!_online_pvscan_one(cmd, devl->dev, NULL, &found_vgnames, 0, &pvid_without_metadata))
+ if (!_online_pvscan_one(cmd, devl->dev, NULL, complete_vgnames, 0, &pvid_without_metadata))
add_errors++;
}
}
@@ -809,10 +908,10 @@ activate:
* scan all devs and pick out the complete VG holding this
* device so we can then autoactivate that VG.
*/
- if (!dm_list_empty(&single_devs) && dm_list_empty(&found_vgnames) &&
+ if (!dm_list_empty(&single_devs) && complete_vgnames && dm_list_empty(complete_vgnames) &&
pvid_without_metadata && do_activate) {
log_verbose("pvscan all devices for PV without metadata: %s.", pvid_without_metadata);
- _online_pvscan_all_devs(cmd, &found_vgnames, &single_devs);
+ _online_pvscan_all_devs(cmd, complete_vgnames, &single_devs);
}
/*
@@ -821,7 +920,7 @@ activate:
* list, and we can attempt to autoactivate LVs in the VG.
*/
if (do_activate)
- ret = _pvscan_aa(cmd, &pp, all_vgs, &found_vgnames);
+ ret = _pvscan_aa(cmd, &pp, complete_vgnames);
out:
if (add_errors || pp.activate_errors)
--
1.8.3.1