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