Skip to content

fix: race condition for device error propagation#620

Open
madjesc wants to merge 1 commit into
LinearTapeFileSystem:release/v2.4.9.0from
madjesc:fix/unified-race-condition
Open

fix: race condition for device error propagation#620
madjesc wants to merge 1 commit into
LinearTapeFileSystem:release/v2.4.9.0from
madjesc:fix/unified-race-condition

Conversation

@madjesc

@madjesc madjesc commented Jun 22, 2026

Copy link
Copy Markdown

fix: file backend NO_SENSE errortype when it should be backwards
fix: unified simple problems

Changelog

Fixed

Race Condition in Device Error Propagation

  • Resolved race condition in error propagation mechanism for device operations
  • Centralized write error handling in iosched.c to prevent inconsistent error states across multiple code paths

Error Type Correction in File Debug Driver

  • Fixed incorrect error type mapping in filedebug_tc.c where NO_SENSE and WRITE_PERM error types were reversed
  • Now correctly returns EDEV_WRITE_PERM when force_errortype is set, and EDEV_NO_SENSE otherwise

Write Permission Error Handling

  • Improved handling of write permission errors by consolidating logic in I/O scheduler layer
  • Removed redundant MAM (Medium Auxiliary Memory) volume lock updates from multiple locations
  • Centralized MAM lock status updates to prevent race conditions during error scenarios

Changed

Code Refactoring

  • Moved _unified_write_index_after_perm function from unified.c to iosched.c as _iosched_write_index_after_perm
  • Removed duplicate error handling code from unified scheduler
  • Simplified error return logic in ltfs_fsops_write function

Error Handling Architecture

  • Consolidated write error handling at the I/O scheduler level
  • Removed scattered MAM volume lock updates from ltfs_write_index function
  • Improved error propagation flow from low-level write operations to higher layers

Added

New Error Checking Macro

  • Added IS_RW_PERM macro in ltfs_error.h for checking read/write permission errors

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have confirmed my fix is effective or that my feature works

@madjesc
madjesc force-pushed the fix/unified-race-condition branch from befb20a to c0aeabf Compare June 22, 2026 16:47
@vandelvan
vandelvan changed the base branch from main to release/v2.4.9.0 July 3, 2026 19:06

@amissael95 amissael95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you efforts and working in this change. Leaving a couple of comments. Please ensure the fix is tested and documented in this PR properly.

Comment thread src/tape_drivers/generic/file/filedebug_tc.c Outdated
Comment thread src/libltfs/iosched.c Outdated
Comment thread src/libltfs/iosched.c
Comment thread src/iosched/unified.c Outdated
@madjesc
madjesc force-pushed the fix/unified-race-condition branch from c0aeabf to 6a00127 Compare July 16, 2026 19:03
Comment thread src/libltfs/ltfs.c
ltfs_mutex_unlock(&vol->device->read_only_flag_mutex);
}

if (update_vollock) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing these lines? Looks like you do not longer want to set the volstat value, do you plan to set this values from somewhere else?

Comment thread src/iosched/unified.c Outdated
fix: NO_SENSE errortype when it should be backwards
fix: unified simple problems
@madjesc
madjesc force-pushed the fix/unified-race-condition branch from 6a00127 to 0f0a343 Compare July 22, 2026 17:46
Comment thread src/iosched/unified.c
*/

#include "libltfs/ltfs.h"
#include "libltfs/ltfs_locking_old.h"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this header necessary? I see deletions only in this file

Comment thread src/libltfs/iosched.c
ret = size;
if (ret < 0) {
if (IS_WRITE_PERM(-ret)) {
mam_lockval vollock = d->matches_name_criteria ? PWE_MAM_IP : PWE_MAM_DP;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the best way to obtain the Partition to lock?
I also see we do not mantain the PWE_MAM_BOTH logic in case a PWE is already present in a partition, this could not lead to problems on analysis?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants