chromeos-config: move the retry logic outwards for creating loop devices
In CL:2320013, the initialization of cros_config was moved earlier
into the boot, blocking boot-services so that services like the
fingerprint firmware updater can use cros_config.
However, this exposed another race condition in cros_config: during
early boot, even if /dev/loop-control can be opened and we get a loop
file number, the device may still be busy, either by use from another
process or by the kernel.
Unfortunately, in this situation, just like with /dev/loop-control,
the only option is to retry the operation until we are able to
complete it: there is no option to block on this file until it's
ready.
We percolate the existing retry mechanism outwards to surround the
entire loop setup, and check for busy errors to do the retry.
NOTE: this change needs cherry-picked to M85... I appreciate any
prioritization in the review you are able to do.
BUG=b:162749841,b:162720284,b:162541620,b:162719399
TEST=on eve, 30/30 reboots has working cros_config (compared to
failure with only 2-3 reboots before this change)
Change-Id: I1eada39c2ed8614a1cb4d4c85bc06cd9fe3fa345
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2335706
Tested-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-by: Yuval Peress <peress@chromium.org>
Commit-Queue: Jack Rosenthal <jrosenth@chromium.org>
diff --git a/chromeos-config/libcros_config/configfs.cc b/chromeos-config/libcros_config/configfs.cc
index 34b75af..7c9552d 100644
--- a/chromeos-config/libcros_config/configfs.cc
+++ b/chromeos-config/libcros_config/configfs.cc
@@ -51,43 +51,15 @@
return true;
}
-bool SetupLoopDevice(const base::FilePath& backing_file,
- base::FilePath* loop_file_out) {
+static bool TrySetupLoopDevice(const base::FilePath& backing_file,
+ base::FilePath* loop_file_out) {
const char loop_control_file[] = "/dev/loop-control";
- const int loop_control_total_retries = 10;
- const int loop_control_retry_wait_ms = 10;
- int loop_control_retries = loop_control_total_retries;
- base::ScopedFD loop_control_fd;
- // We want to retry opening the loop control file a number of times
- // in case another process is using it.
- while (true) {
- loop_control_fd.reset(open(loop_control_file, O_RDWR));
- if (loop_control_fd.is_valid()) {
- break;
- } else {
- switch (errno) {
- // We may get any of these errors if we need to retry.
- case EBUSY:
- case EACCES:
- case ENOENT:
- if (loop_control_retries > 0) {
- --loop_control_retries;
- base::PlatformThread::Sleep(
- base::TimeDelta::FromMilliseconds(loop_control_retry_wait_ms));
- continue;
- }
- CROS_CONFIG_LOG(ERROR)
- << "Max retries exceeded when opening " << loop_control_file
- << " (tried " << loop_control_total_retries
- << " times): " << logging::SystemErrorCodeToString(errno);
- return false;
- default:
- CROS_CONFIG_LOG(ERROR)
- << "Error opening loop control device " << loop_control_file
- << ": " << logging::SystemErrorCodeToString(errno);
- return false;
- }
- }
+ base::ScopedFD loop_control_fd(open(loop_control_file, O_RDWR));
+ if (!loop_control_fd.is_valid()) {
+ CROS_CONFIG_LOG(ERROR) << "Error opening loop control file "
+ << loop_control_file << ": "
+ << logging::SystemErrorCodeToString(errno);
+ return false;
}
int device_number = ioctl(loop_control_fd.get(), LOOP_CTL_GET_FREE);
@@ -132,6 +104,48 @@
return true;
}
+// During early boot, a number of resources can be busy
+// (/dev/loop-control or /dev/loopN) due to utilization by other
+// processes on the system. We wrap the setup of the loop device, and
+// when we encounter an error that might indicate a device is busy, we
+// will retry a number of times.
+bool SetupLoopDevice(const base::FilePath& backing_file,
+ base::FilePath* loop_file_out) {
+ const int total_retries = 25;
+ const int retry_wait_ms = 10;
+ int current_try = 0;
+
+ while (true) {
+ if (TrySetupLoopDevice(backing_file, loop_file_out)) {
+ return true;
+ }
+
+ CROS_CONFIG_LOG(ERROR) << "TRY " << current_try << "/" << total_retries
+ << ": Setting up loop device ("
+ << logging::SystemErrorCodeToString(errno) << ")";
+
+ switch (errno) {
+ // We may get any of these errors if we need to retry.
+ case EBUSY:
+ case EACCES:
+ case ENOENT:
+ if (current_try < total_retries) {
+ ++current_try;
+ CROS_CONFIG_LOG(ERROR) << "Retrying in " << retry_wait_ms << " ms";
+ base::PlatformThread::Sleep(
+ base::TimeDelta::FromMilliseconds(retry_wait_ms));
+ continue;
+ }
+ CROS_CONFIG_LOG(ERROR) << "Max retries exceeded";
+ return false;
+ default:
+ CROS_CONFIG_LOG(ERROR)
+ << "No more retries, this does not look like a busy resource";
+ return false;
+ }
+ }
+}
+
bool Mount(const base::FilePath& source,
const base::FilePath& target,
const char* filesystemtype,
