Skip to content

Commit 418f020

Browse files
ochomenosochomoisesFigueroa
authored andcommitted
Recursive protection to _take_dump() (#412)
* added recursive check for DRIVE_DUMP error * Test commit for multithreading with pthread * Added join() to the pthread and change lock() for unlock() * Removed the destroy() functions for the mutex and setted the mutex to be global * Accepted new error messaages * Added recusivity error message * Fixed identation issues * Removed the unnecessary thead safety * added mutex for thread protection * test commit for mutex * test commit for mutex * test commit for mutex * test commit for mutex * test commit for mutex * test commit for mutex * test commit for mutex * test commit for mutex * test commit for mutex * mutex added with function to start it * mutex added with function to start it * Added recursive counter to sg_data structure and removed the mutex * A sum operator is changed to a binary operator OR _parse_logPage * Changed datatype from uint16_t to uint8_t from cast * Wrong variable call fixed. * Added null check to recursive_counter * Implemented some optimizations to _parse_logPage and changed _take_dump to work with priv->recursive_counter * Identation fixed. * Changed the conditions flow to the original version * changed datatype to recursive_counter --------- Co-authored-by: moisesFigueroa <figueroa.moises@ibm.com>
1 parent 46465a5 commit 418f020

File tree

3 files changed

+20
-7
lines changed

3 files changed

+20
-7
lines changed

messages/tape_linux_sg/root.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ root:table {
100100
30258W:string { "Cannot retrieve drive dump: failed to read buffer (%d)." }
101101
30259W:string { "Cannot retrieve drive dump: failed to write to dump file (%d)." }
102102
30260W:string { "Cannot retrieve drive dump: wrote %d bytes out, expected %d." }
103+
30261W:string { "Cannot retrieve drive dump: failed to write to communicate with drive. Tried (%d) times." }
103104
30261I:string { "Taking drive dump in buffer." }
104105
30262I:string { "Forcing drive dump." }
105106
30263I:string { "%s returns %s (%d) %s." }
@@ -136,6 +137,7 @@ root:table {
136137
30294I:string { "Setting up timeout values from %s." }
137138
30295I:string { "Have unstable TUR response, start over (Cur = %d, Prev = %d)." }
138139
30296I:string { "Capturing a stable TUR at line %d." }
140+
30297W:string { "Cannot retrieve drive dump: failed to communicate with drive. Tried (%d) times." }
139141

140142
30392D:string { "Backend %s %s." }
141143
30393D:string { "Backend %s: %d %s." }

src/tape_drivers/linux/sg/sg_tape.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ struct sg_global_data global_data;
103103
#define TU_DEFAULT_TIMEOUT (60)
104104
#define MAX_RETRY (100)
105105

106+
#define MAX_TAKE_DUMP_ATTEMPTS (10)
107+
106108
/* Forward references (For keep function order to struct tape_ops) */
107109
int sg_readpos(void *device, struct tc_position *pos);
108110
int sg_locate(void *device, struct tc_position dest, struct tc_position *pos);
@@ -118,19 +120,17 @@ static inline int _parse_logPage(const unsigned char *logdata,
118120
const uint16_t param, uint32_t *param_size,
119121
unsigned char *buf, const size_t bufsize)
120122
{
121-
uint16_t page_len, param_code, param_len;
122-
uint32_t i;
123+
const uint16_t page_len = ((uint16_t)logdata[2] << 8) | (uint16_t)logdata[3];
124+
uint16_t param_code, param_len;
125+
uint32_t i = LOG_PAGE_HEADER_SIZE;
123126
uint32_t ret = -EDEV_INTERNAL_ERROR;
124127

125-
page_len = ((uint16_t)logdata[2] << 8) + (uint16_t)logdata[3];
126-
i = LOG_PAGE_HEADER_SIZE;
127-
128128
while(i < page_len)
129129
{
130-
param_code = ((uint16_t)logdata[i] << 8) + (uint16_t)logdata[i+1];
130+
param_code = ((uint16_t)logdata[i] << 8) | (uint16_t)logdata[i+1];
131131
param_len = (uint16_t)logdata[i + LOG_PAGE_PARAMSIZE_OFFSET];
132132

133-
if(param_code == param)
133+
if (param_code == param)
134134
{
135135
*param_size = param_len;
136136
if(bufsize < param_len){
@@ -143,6 +143,7 @@ static inline int _parse_logPage(const unsigned char *logdata,
143143
break;
144144
}
145145
}
146+
146147
i += param_len + LOG_PAGE_PARAM_OFFSET;
147148
}
148149

@@ -392,6 +393,13 @@ static int _take_dump(struct sg_data *priv, bool capture_unforced)
392393
time_t now;
393394
struct tm *tm_now;
394395

396+
/* To check if the function became recursive */
397+
if (priv->recursive_counter > MAX_TAKE_DUMP_ATTEMPTS) {
398+
ltfsmsg(LTFS_WARN, 30297W, priv->recursive_counter);
399+
return 0;
400+
}
401+
priv->recursive_counter++;
402+
395403
if (priv->vendor != VENDOR_IBM)
396404
return 0;
397405

@@ -426,6 +434,8 @@ static int _take_dump(struct sg_data *priv, bool capture_unforced)
426434

427435
ltfs_profiler_add_entry(priv->profiler, NULL, TAPEBEND_REQ_EXIT(REQ_TC_TAKEDUMPDRV));
428436

437+
priv->recursive_counter = 0;
438+
429439
return 0;
430440
}
431441

src/tape_drivers/linux/sg/sg_tape.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ struct sg_data {
8282
struct timeout_tape *timeouts; /**< Timeout table */
8383
struct tc_drive_info info; /**< Drive information */
8484
FILE* profiler; /**< The file pointer for profiler */
85+
int recursive_counter; /**< Recursive counter for take dump */
8586
};
8687

8788
struct sg_global_data {

0 commit comments

Comments
 (0)