-
Notifications
You must be signed in to change notification settings - Fork 103
fix: race condition for device error propagation #620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/v2.4.9.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,8 +50,11 @@ | |
| ** | ||
| ************************************************************************************* | ||
| */ | ||
| #include "ltfs_error.h" | ||
| #include "ltfs_fuse.h" | ||
| #include "ltfs_fsops_raw.h" | ||
| #include "ltfs.h" | ||
| #include "tape.h" | ||
| #include "iosched.h" | ||
|
|
||
| struct iosched_priv { | ||
|
|
@@ -61,6 +64,53 @@ struct iosched_priv { | |
| void *backend_handle; /**< Backend private data */ | ||
| }; | ||
|
|
||
| int _iosched_write_index_after_perm(int write_ret, struct ltfs_volume *volume) | ||
| { | ||
| int ret = 0; | ||
| struct tc_position err_pos; | ||
| uint64_t last_index_pos = UINT64_MAX; | ||
| unsigned long blocksize; | ||
|
|
||
| if (!IS_WRITE_PERM(-write_ret)) { | ||
| /* Nothing to do for non-medium error */ | ||
| return ret; | ||
| } | ||
|
|
||
| ltfsmsg(LTFS_INFO, 13024I, write_ret); | ||
| blocksize = ltfs_get_blocksize(volume); | ||
|
|
||
| ret = tape_get_first_untransfered_position(volume->device, &err_pos); | ||
| if (ret < 0) { | ||
| ltfsmsg(LTFS_ERR, 13026E, "get error pos", ret); | ||
| return ret; | ||
| } | ||
|
|
||
| /* Check the err_pos is larger than the last index position of the partition */ | ||
| if (err_pos.partition == ltfs_part_id2num(volume->label->partid_ip, volume)) { | ||
| last_index_pos = volume->ip_coh.set_id; | ||
| } else { | ||
| last_index_pos = volume->dp_coh.set_id; | ||
| } | ||
|
|
||
| if (last_index_pos > err_pos.block) { | ||
| ltfsmsg(LTFS_INFO, 13027I, (int)err_pos.partition, | ||
| (unsigned long long)err_pos.block, (unsigned long long)last_index_pos); | ||
| err_pos.block = last_index_pos + 1; | ||
| } | ||
|
|
||
| ltfsmsg(LTFS_INFO, 13025I, (int)err_pos.partition, (unsigned long long)err_pos.block, blocksize); | ||
| ret = ltfs_fsraw_cleanup_extent(volume->index->root, err_pos, blocksize, volume); | ||
| if (ret < 0) { | ||
| ltfsmsg(LTFS_ERR, 13026E, "extent cleanup", ret); | ||
| return ret; | ||
| } | ||
|
|
||
| ret = ltfs_write_index(ltfs_ip_id(volume), SYNC_WRITE_PERM, volume); | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Initialize the I/O scheduler. | ||
| * @param plugin The plugin to take scheduler operations from. | ||
|
|
@@ -230,10 +280,20 @@ ssize_t iosched_write(struct dentry *d, const char *buf, size_t size, off_t offs | |
| CHECK_ARG_NULL(d, -LTFS_NULL_ARG); | ||
|
|
||
| ret = priv->ops->write(d, buf, size, offset, isupdatetime, priv->backend_handle); | ||
| if (ret > 0 && (size_t) ret > size) | ||
| ret = size; | ||
| if (ret < 0) { | ||
| if (IS_WRITE_PERM(-ret)) { | ||
| mam_lockval vollock = d->matches_name_criteria ? PWE_MAM_IP : PWE_MAM_DP; | ||
|
madjesc marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the best way to obtain the Partition to lock? |
||
| int sync_ret = _iosched_write_index_after_perm(ret, vol); | ||
| if (sync_ret < 0) { | ||
| tape_set_cart_volume_lock_status(vol, PWE_MAM); | ||
| return sync_ret; | ||
| } | ||
| tape_set_cart_volume_lock_status(vol, vollock); | ||
| } | ||
| return ret; | ||
| } | ||
|
|
||
| return ret; | ||
| return ret > (ssize_t)size? (ssize_t)size : ret; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,7 @@ | |
|
|
||
| #include "fs.h" | ||
| #include "ltfs.h" | ||
| #include "ltfs_error.h" | ||
| #include "ltfs_internal.h" | ||
| #include "libltfs/ltfslogging.h" | ||
| #include "ltfs_copyright.h" | ||
|
|
@@ -2431,15 +2432,14 @@ size_t ltfs_max_cache_size(struct ltfs_volume *vol) | |
| */ | ||
| int ltfs_write_index(char partition, char *reason, struct ltfs_volume *vol) | ||
| { | ||
| int ret, ret_mam; | ||
| int ret; | ||
| struct tape_offset old_selfptr, old_backptr; | ||
| struct ltfs_timespec modtime_old = { .tv_sec = 0, .tv_nsec = 0 }; | ||
| bool generation_inc = false; | ||
| struct tc_position physical_selfptr, current_position; | ||
| char *cache_path_save = NULL; | ||
| bool write_perm = (strcmp(reason, SYNC_WRITE_PERM) == 0); | ||
| bool update_vollock = false; | ||
| int volstat = -1, new_volstat = 0; | ||
| int volstat = -1; | ||
| char *bc_print = NULL; | ||
| unsigned long long diff; | ||
|
|
||
|
|
@@ -2567,13 +2567,13 @@ int ltfs_write_index(char partition, char *reason, struct ltfs_volume *vol) | |
| return -1; | ||
| } | ||
|
|
||
| /* Prior to writing the index, compare the current location of the head position to the head location | ||
| /* Prior to writing the index, compare the current location of the head position to the head location | ||
| that is kept in the cache of ltfs (physical_selfptr). If they are different return error (-1) */ | ||
| diff = ((unsigned long long)physical_selfptr.block - (unsigned long long)current_position.block); | ||
| if (diff) { | ||
| /* Position mismatch, diff not equal zero */ | ||
| ltfsmsg(LTFS_INFO, 17293E, (unsigned long long)physical_selfptr.block, (unsigned long long)current_position.block); | ||
| return -1; | ||
| return -LTFS_INDEX_INVALID; | ||
| } | ||
|
|
||
| old_selfptr = vol->index->selfptr; | ||
|
|
@@ -2594,9 +2594,6 @@ int ltfs_write_index(char partition, char *reason, struct ltfs_volume *vol) | |
| vol->index->backptr = old_backptr; | ||
| vol->index->selfptr = old_selfptr; | ||
|
|
||
| if (IS_WRITE_PERM(-ret)) | ||
| update_vollock = true; | ||
|
|
||
| goto out_write_perm; | ||
| } | ||
| } | ||
|
|
@@ -2614,9 +2611,6 @@ int ltfs_write_index(char partition, char *reason, struct ltfs_volume *vol) | |
| vol->index->backptr = old_backptr; | ||
| vol->index->selfptr = old_selfptr; | ||
|
|
||
| if (IS_WRITE_PERM(-ret)) | ||
| update_vollock = true; | ||
|
|
||
| goto out_write_perm; | ||
| } | ||
|
|
||
|
|
@@ -2631,9 +2625,6 @@ int ltfs_write_index(char partition, char *reason, struct ltfs_volume *vol) | |
| vol->index->backptr = old_backptr; | ||
| vol->index->selfptr = old_selfptr; | ||
|
|
||
| if (IS_WRITE_PERM(-ret)) | ||
| update_vollock = true; | ||
|
|
||
| goto out_write_perm; | ||
| } | ||
|
|
||
|
|
@@ -2677,23 +2668,6 @@ int ltfs_write_index(char partition, char *reason, struct ltfs_volume *vol) | |
| ltfs_mutex_unlock(&vol->device->read_only_flag_mutex); | ||
| } | ||
|
|
||
| if (update_vollock) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
| if (volstat == PWE_MAM_DP && partition == ltfs_ip_id(vol)) | ||
| new_volstat = PWE_MAM_BOTH; | ||
| else if (volstat == PWE_MAM_IP && partition == ltfs_dp_id(vol)) | ||
| new_volstat = PWE_MAM_BOTH; | ||
| else if (volstat == UNLOCKED_MAM && partition == ltfs_ip_id(vol)) | ||
| new_volstat = PWE_MAM_IP; | ||
| else if (volstat == UNLOCKED_MAM && partition == ltfs_dp_id(vol)) | ||
| new_volstat = PWE_MAM_DP; | ||
|
|
||
| if (new_volstat) { | ||
| ret_mam = tape_set_cart_volume_lock_status(vol, new_volstat); | ||
| if (ret_mam) | ||
| ret = ret_mam; | ||
| } | ||
| } | ||
|
|
||
| return ret; | ||
| } | ||
|
|
||
|
|
@@ -4443,7 +4417,7 @@ static int _ltfs_write_rao_file(char *file_path_org, unsigned char *buf, size_t | |
| ltfsmsg(LTFS_ERR, 10001E, __FILE__); | ||
| return -LTFS_NO_MEMORY; | ||
| } | ||
|
|
||
| arch_open(&fd, path, | ||
| O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, | ||
| SHARE_FLAG_DENYRW, PERMISSION_READWRITE); | ||
|
|
||
There was a problem hiding this comment.
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