main - vgimportdevices: fix locking when creating devices file

main - vgimportdevices: fix locking when creating devices file

[David Teigland]

Gitweb:        https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=0eebd9d7802c724ee71b6ebb80940ea6007f9c7a
Commit:        0eebd9d7802c724ee71b6ebb80940ea6007f9c7a
Parent:        3c49a2e43ccfbad720a3134484c7870a14b1135b
Author:        David Teigland <teigland@redhat.com>
AuthorDate:    Tue Aug 30 14:40:48 2022 -0500
Committer:     David Teigland <teigland@redhat.com>
CommitterDate: Tue Aug 30 14:52:00 2022 -0500

vgimportdevices: fix locking when creating devices file

Take the devices file lock before creating a new devices file.
(Was missed by the change to preemptively create the devices
file prior to setup_devices(), which was done to improve the
error path.)
---
 lib/device/dev-cache.c  |  7 +++----
 lib/device/device_id.c  |  1 +
 tools/vgimportdevices.c | 10 ++++++++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 193eb7585..85f9b499a 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -1937,10 +1937,9 @@ int setup_devices(struct cmd_context *cmd)
 
 	if (!file_exists) {
 		/*
-		 * pvcreate/vgcreate/vgimportdevices/lvmdevices-add create
-		 * a new devices file here if it doesn't exist.
-		 * They have the create_edit_devices_file flag set.
-		 * First they create/lock-ex the devices file lockfile.
+		 * pvcreate/vgcreate create a new devices file here if it
+		 * doesn't exist.  They have create_edit_devices_file=1.
+		 * First create/lock-ex the devices file lockfile.
 		 * Other commands will not use a devices file if none exists.
 		 */
 		lock_mode = LOCK_EX;
diff --git a/lib/device/device_id.c b/lib/device/device_id.c
index 6c70f110c..a0a626533 100644
--- a/lib/device/device_id.c
+++ b/lib/device/device_id.c
@@ -2468,6 +2468,7 @@ static int _lock_devices_file(struct cmd_context *cmd, int mode, int nonblock, i
 
 	if (_devices_file_locked == mode) {
 		/* can happen when a command holds an ex lock and does an update in device_ids_validate */
+		/* can happen when vgimportdevices calls this directly, followed later by setup_devices */
 		if (held)
 			*held = 1;
 		return 1;
diff --git a/tools/vgimportdevices.c b/tools/vgimportdevices.c
index 9ade1b9e4..23c2718ff 100644
--- a/tools/vgimportdevices.c
+++ b/tools/vgimportdevices.c
@@ -132,8 +132,10 @@ int vgimportdevices(struct cmd_context *cmd, int argc, char **argv)
 		return ECMD_FAILED;
 
 	/*
-	 * Prepare devices file preemptively because the error path for this
-	 * case from process_each is not as clean.
+	 * Prepare/create devices file preemptively because the error path for
+	 * this case from process_each/setup_devices is not as clean.
+	 * This means that when setup_devices is called, it the devices
+	 * file steps will be redundant, and need to handle being repeated.
 	 */
 	if (!setup_devices_file(cmd)) {
 		log_error("Failed to set up devices file.");
@@ -143,6 +145,10 @@ int vgimportdevices(struct cmd_context *cmd, int argc, char **argv)
 		log_error("Devices file not enabled.");
 		return ECMD_FAILED;
 	}
+	if (!lock_devices_file(cmd, LOCK_EX)) {
+		log_error("Failed to lock the devices file.");
+		return ECMD_FAILED;
+	}
 	if (!devices_file_exists(cmd)) {
 	       	if (!devices_file_touch(cmd)) {
 			log_error("Failed to create devices file.");