Simplify -seek Command and Reduce Required Arguments (#34)

This commit improves seek file handling by adding metadata
for the "Number of Requests" at the beginning of the seek file,
eliminating the need to read the entire file to retrieve this value.
The program now prioritizes the block size and request size specified
in the seek file, replacing any values provided via the command line
when the `-seek load` option is used.

Additionally, this commit fixes a segmentation fault issue that
occurred when no options were provided with the `-seek` command.
Comments in the `xdd_func` array have also been updated to match the
current function setup. A test script has been added to ensure the
program correctly sets and maintains the block size, request size,
and "Number of Requests" values during file loading.

Signed-off-by: Cade Gore <[email protected]>

src/client/parse_func.c

@@ -3509,7 +3509,10 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 		args_index += args;  /* skip past the "target <taget number>" */
 	}
 	/* At this point "args_index" is an index to the seek "option" argument */
-	if (strcmp(argv[args_index], "save") == 0) { /* save the seek information in a file */
+	if (argv[args_index] == NULL) {
+		fprintf(stderr, "%s: ERROR: No seek option specified. Please give a valid seek option.\n", xgp->progname);
+		return(-1);
+	} else if (strncmp(argv[args_index], "save", 5) == 0) { /* save the seek information in a file */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3528,7 +3531,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "load") == 0) { /* load seek list from "filename" */
+	} else if (strncmp(argv[args_index], "load", 5) == 0) { /* load seek list from "filename" */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3547,7 +3550,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "disthist") == 0) { /*  Print a Distance Histogram */
+	} else if (strncmp(argv[args_index], "disthist", 9) == 0) { /*  Print a Distance Histogram */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3566,7 +3569,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "seekhist") == 0) { /* Print a Seek Histogram */
+	} else if (strncmp(argv[args_index], "seekhist", 9) == 0) { /* Print a Seek Histogram */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3585,7 +3588,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "sequential") == 0) { /*  Sequential seek list option */
+	} else if (strncmp(argv[args_index], "sequential", 11) == 0) { /*  Sequential seek list option */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3604,7 +3607,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+1);
-	} else if (strcmp(argv[args_index], "random") == 0) { /*  Random seek list option */
+	} else if (strncmp(argv[args_index], "random", 7) == 0) { /*  Random seek list option */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3623,7 +3626,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+1);
-	} else if (strcmp(argv[args_index], "stagger") == 0) { /*  Staggered seek list option */
+	} else if (strncmp(argv[args_index], "stagger", 8) == 0) { /*  Staggered seek list option */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3644,7 +3647,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "interleave") == 0) { /* set the interleave for sequential seek locations */
+	} else if (strncmp(argv[args_index], "interleave", 11) == 0) { /* set the interleave for sequential seek locations */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3663,7 +3666,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "none") == 0) { /* no seeking at all */
+	} else if (strncmp(argv[args_index], "none", 5) == 0) { /* no seeking at all */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3682,7 +3685,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+1);
-	} else if (strcmp(argv[args_index], "range") == 0) { /* set the range of seek locations */
+	} else if (strncmp(argv[args_index], "range", 6) == 0) { /* set the range of seek locations */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);
@@ -3699,7 +3702,7 @@ xddfunc_seek(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flags)
 			}
 		}
 		return(args_index+2);
-	} else if (strcmp(argv[args_index], "seed") == 0) { /* set the seed for random seek locations */
+	} else if (strncmp(argv[args_index], "seed", 5) == 0) { /* set the seed for random seek locations */
 		if (target_number >= 0) {  /* set option for specific target */
 			tdp = xdd_get_target_datap(planp, target_number, argv[0]);
 			if (tdp == NULL) return(-1);

src/client/parse_table.c

@@ -23,7 +23,7 @@
 //            struct xdd_func {
 //	              char    *func_name;     /* name of the function */
 //	              char    *func_alt;      /* Alternate name of the function */
-//                int     (*func_ptr)(int32_t argc, char *argv[], uint32_t flags, uint32_t flags);      /* pointer to the function */
+//                int     (*func_ptr)(int32_t argc, char *argv[], uint32_t flags);      /* pointer to the function */
 //                int     argc;           /* number of arguments */
 //                char    *help;          /* help string */
 //                char    *ext_help[5];   /* Extented help strings */

src/common/access_pattern.c

@@ -198,6 +198,8 @@ xdd_init_seek_list(target_data_t *tdp) {
                                 (rw_op_index * ((tdp->td_reqsize*sp->seek_interleave) + gap));
 			/* end of generating a sequential seek */
 
+			/* Now lets fill in the block sizes to transfer */
+			sp->seeks[rw_index].blocksize = tdp->td_block_size;
 			/* Now lets fill in the request sizes to transfer */
 			sp->seeks[rw_index].reqsize = tdp->td_reqsize;
 			/* Now lets fill in the appropriate operation */
@@ -350,11 +352,12 @@ xdd_save_seek_list(target_data_t *tdp) {
 		}
 		average = (uint64_t)(total / sp->seek_total_ops);
 		/* Print the seek list into the specified file */
-		fprintf(tmp,"# Longest seek=%llu, Shortest seek=%llu, Average seek distance=%llu\n",
+		fprintf(tmp,"# Longest seek=%llu, Shortest seek=%llu, Average seek distance=%llu, Number of Requests=%ld\n",
 			(unsigned long long)longest, 
 			(unsigned long long)shortest, 
-			(unsigned long long)average);
-		fprintf(tmp,"#Ordinal Location Reqsize Operation Time1 Time2\n"); 
+			(unsigned long long)average,
+			(long int)tdp->td_numreqs);
+		fprintf(tmp,"#Ordinal Location Blocksize Reqsize Operation Time1 Time2\n"); 
 		for (i = 0; i < sp->seek_total_ops; i++) {
 			if (sp->seeks[i].operation == SO_OP_READ) 
 				opc = "r";
@@ -366,17 +369,19 @@ xdd_save_seek_list(target_data_t *tdp) {
 				opc = "u";
 
 			if (tdp->td_seekhdr.seek_options & SO_SEEK_NONE) {
-				fprintf(tmp,"%010d %012llu %d %s %016llu %016llu\n",
+				fprintf(tmp,"%010d %012llu %d %d %s %016llu %016llu\n",
 					i,
 					(unsigned long long)sp->seeks[0].block_location, 
+					sp->seeks[0].blocksize, 
 					sp->seeks[0].reqsize, 
 					opc, 
 					(unsigned long long)(sp->seeks[i].time1),
 					(unsigned long long)(sp->seeks[i].time2));
 			} else {
-				fprintf(tmp,"%010d %012llu %d %s %016llu %016llu\n",
+				fprintf(tmp,"%010d %012llu %d %d %s %016llu %016llu\n",
 					i,
 					(unsigned long long)sp->seeks[i].block_location, 
+					sp->seeks[i].blocksize, 
 					sp->seeks[i].reqsize, 
 					opc, 
 					(unsigned long long)(sp->seeks[i].time1),
@@ -463,13 +468,14 @@ xdd_load_seek_list(target_data_t *tdp) {
 	char 		*tp;  		/* token pointer */
 	int32_t 	ordinal; 	/* ordinal number of the seek */
 	uint64_t 	loc;  		/* location */
+	int32_t     blocksz;    /* Block Size */
 	int32_t 	reqsz; 		// Request Size
 	nclk_t		t1,t2; 		/* time1 and time2 */
 	int32_t 	reqsz_high; 	/* highest request size*/
 	char 		rw;  		/* read or write operation */
 	char 		*status; 	/* status of the fgets */
 	struct seekhdr	*sp;
-	char 		line[1024]; 	/* one line of characters */
+	char 		line[LINE_LENGTH]; 	/* one line of characters */
 
 
 	sp = &tdp->td_seekhdr;
@@ -484,7 +490,7 @@ xdd_load_seek_list(target_data_t *tdp) {
 	status = line;
 	i = 0;
 	reqsz_high = 0;
-	while (status != NULL) {
+	while (status != NULL && i < tdp->td_numreqs) {
 		status = fgets(line, sizeof(line), loadfp);
 		if (status == NULL ) continue;
 		tp = line;
@@ -493,26 +499,34 @@ xdd_load_seek_list(target_data_t *tdp) {
 		/* Check for comment line */
 		if (*tp == COMMENT) continue;
 		/* Must be a seek line */
-		sscanf(line,"%d %llu %d %c %llu %llu", 
+		if (sscanf(line,"%d %llu %d %d %c %llu %llu", 
 			&ordinal,
 			(unsigned long long *)(&loc),
+			&blocksz,
 			&reqsz,
 			&rw,
 			(unsigned long long *)(&t1),
-			(unsigned long long *)(&t2));
+			(unsigned long long *)(&t2)) != 7) {
+				fprintf(xgp->errout, "%s: Error: Cannot parse seek load file %s\n",
+						xgp->progname, sp->seek_loadfile);
+				return(-1);
+		}
 		sp->seeks[i].block_location = loc;
 		if ((rw == 'w') || (rw == 'W')) 
 			sp->seeks[i].operation = SO_OP_WRITE;
 		else if ((rw == 'n') || (rw == 'N')) 
 			sp->seeks[i].operation = SO_OP_NOOP; /* NOOP */
 		else sp->seeks[i].operation = SO_OP_READ; /* READ */
+		sp->seeks[i].blocksize = blocksz;
 		sp->seeks[i].reqsize = reqsz;
 		sp->seeks[i].time1 = t1;
 		sp->seeks[i].time2 = t2;
 		if (reqsz > reqsz_high) reqsz_high = reqsz;
 		i++;
 	}
 	sp->seek_iosize = reqsz_high * tdp->td_block_size;
+	/* close the load file */
+	fclose(loadfp);
 	return(0);
 } /* end of xdd_load_seek_list() */
 

src/common/access_pattern.h

@@ -20,7 +20,8 @@
 /** A single seek entry */
 struct seek_entries {
 	int32_t operation; /**< read or write */
-	int32_t reqsize; /**< Size of data transfer in blocks */
+	int32_t blocksize; /**< Size of data transfer in blocks */
+	int32_t reqsize; /**< Number of blocks in the request */
 	uint64_t block_location; /**< Starting location in blocks */
 	nclk_t time1;  /**< Relative time in nano seconds that this operation should start */
 	nclk_t time2;  /**< not yet implemented */

src/common/target_data.c

@@ -120,6 +120,128 @@ xdd_init_new_target_data(target_data_t *tdp, int32_t n) {
 
 } /* end of xdd_init_new_target_data() */
 
+/*----------------------------------------------------------------------------*/
+/* xdd_parse_header() - This function reads the header of the seek load file.
+ */
+static int32_t
+xdd_parse_header(char **line, size_t *length, FILE *loadfp) {
+	if (getline(line, length, loadfp) == -1) {
+		fprintf(xgp->errout, "%s: ERROR: Failed to read the header of the seek load file\n",
+				xgp->progname);
+		return -1;
+	}
+	return 0;
+} // End of xdd_parse_header()
+
+/*----------------------------------------------------------------------------*/
+/* xdd_parse_value_from_line() - This function parses the value from the first
+ * line of the seek load file.
+ */
+static int32_t
+xdd_parse_value_from_line(char *line, const char *pattern, long int *location, const char *filename) {
+	char *line_str = strstr(line, pattern);
+	if (line_str == NULL) {
+		fprintf(xgp->errout, "%s: ERROR: %s is not found in the first line of %s\n",
+				xgp->progname, pattern, filename);
+		return(-1);
+    }
+
+	if (sscanf(line_str, "%*[^=]=%ld", location) != 1) {
+		fprintf(xgp->errout, "%s: ERROR: Cannot parse %s from the first line of %s\n",
+				xgp->progname, pattern, filename);
+		return(-1);
+	}
+
+	return 0;
+} /* end of xdd_parse_value_from_line */
+
+/*----------------------------------------------------------------------------*/
+/* xdd_set_bs_reqsize_numreqs_from_loadfile() - This function reads a target's
+ * seek load file and sets the block size, request size (override the values
+ * from command line) and numreqs if td_numreqs and td_bytes are not provided.
+ */
+static int32_t
+xdd_set_bs_reqsize_numreqs_from_loadfile(target_data_t *tdp) {
+	FILE	*loadfp;			/* Load File Point */
+	char	*line = NULL;		/* one line of characters */
+	size_t	length = 0;			/* length of the line */
+	int 	return_value = 0;
+	int 	block_size = 0;
+	int 	req_size = 0;
+	int64_t numreqs = 0;
+	struct seekhdr	*sp;
+
+	sp = &tdp->td_seekhdr;
+	/* Open the load file */
+	loadfp = fopen(sp->seek_loadfile, "r");
+	if (loadfp == NULL) {
+		fprintf(xgp->errout, "%s: ERROR: Cannot open seek load file %s\n",
+				xgp->progname, sp->seek_loadfile);	
+		return_value = -1;
+		goto close_file_and_return;
+	}
+
+	/* Read the first line */
+	if (xdd_parse_header(&line, &length, loadfp) == -1) {
+		return_value = -1;
+		goto close_file_and_return;
+	}
+
+	/* Get Number of Requests from the file */
+	if (xdd_parse_value_from_line(line, "Number of Requests", &(numreqs), sp->seek_loadfile) == -1) {
+		return_value = -1;
+		goto close_file_and_return;
+	}
+
+	/* If tdp->td_numreqs and tdp->td_bytes is not set OR if tdp->numreqs is greater than */
+	/* the number of requests in the file, set tdp->td_numreqs to the number of requests in the file */
+	if ((!tdp->td_numreqs && !tdp->td_bytes) || tdp->td_numreqs > numreqs) {
+        if (tdp->td_numreqs > numreqs) {
+            fprintf(xgp->errout, "INFO: Number of requests overridden by value from file: %ld\n",
+                    numreqs);
+        }
+        tdp->td_numreqs = numreqs;
+    }
+
+	/* Read the second line */
+	if (xdd_parse_header(&line, &length, loadfp) == -1) {
+		return_value = -1;
+		goto close_file_and_return;
+	}
+
+	/* Read the third line to extract block size and request size */
+	if (xdd_parse_header(&line, &length, loadfp) == -1) {
+		return_value = -1;
+		goto close_file_and_return;
+	}
+
+	if (sscanf(line, "%*d %*u %d %d", &block_size, &req_size) != 2) {
+		fprintf(xgp->errout, "%s: ERROR: Cannot parse block size and request size from the data line of %s\n",
+				xgp->progname, sp->seek_loadfile);
+		return_value = -1;
+		goto close_file_and_return;
+	}
+
+	/* Replace the values from command line to what's in the file */
+	if (block_size > 0) {
+		tdp->td_block_size = block_size;
+	}
+	if (req_size > 0) {
+		tdp->td_reqsize = req_size;
+	}
+
+	free(line);
+	/* close the load file */
+	fclose(loadfp);
+	return return_value;
+
+close_file_and_return:
+	if (loadfp) {
+		fclose(loadfp);
+	}
+	return return_value;	
+} // End of xdd_set_bs_reqsize_numreqs_from_loadfile()
+
 /*----------------------------------------------------------------------------*/
 /* xdd_build_target_data_substructure_calculate_xfer_info() - Will calculate the number of data
  * transfers to perform as well as the total number of bytes for the specified
@@ -295,6 +417,13 @@ xdd_build_target_data_substructure(xdd_plan_t* planp) {
 		if (tdp->td_target_options & TO_ENDTOEND) {
 			xdd_build_target_data_substructure_e2e(planp, tdp);
 		}
+
+		// if a load file is given, it will override the blocksize and reqsize from the command line
+		// it will also check if numreqs is specified, if not, it will set numreqs from the load file
+		if (tdp->td_seekhdr.seek_loadfile) {
+			xdd_set_bs_reqsize_numreqs_from_loadfile(tdp);
+		}
+
 		// Calcualte the data transfer information - number of ops, bytes, starting offset, ...etc.
 		xdd_calculate_xfer_info(tdp);
 

src/common/xint_common.h

@@ -34,6 +34,7 @@
 #define MAX_TARGETS 8192
 #define MAX_TARGET_NAME_LENGTH 2048
 #define MAXSEM 1
+#define LINE_LENGTH 1024
 
 /* program defines */
 #define COMMENT '#'

tests/functional/CMakeLists.txt

@@ -9,6 +9,7 @@ set(FUNCTIONAL
     test_xdd_heartbeat_output.sh
     test_xdd_heartbeat_target.sh
     test_xdd_heartbeat_tod.sh
+    test_xdd_load_file.sh
     test_xdd_lockstep1.sh
     test_xdd_lockstep2.sh
     test_xdd_pretruncate.sh