From a71e892baa615787b74dd3e55a00b4c35b8e15f2 Mon Sep 17 00:00:00 2001 From: Brian Atkinson Date: Mon, 18 Mar 2024 16:21:52 -0600 Subject: [PATCH] Updating how we file datapatterns work Previously when the option -datapattern file/wholefile was used, XDD would open the file for every target and keep it open till the run was over. This is really unnecessary as the file only needs to be opened to read the datapattern in to set the datapattern in the target. Once that has happen, the file can immediately be closed. This is a good idea as there are limits to how many files can be opened in Linux (ulimit). XDD should not tie up the number of avialable open file descriptors just to set a datapattern over an entire XDD run. The code has been updated to open the file, set the target data_pattern buffer, and then immediately close the file. I have added a new functional test case test_xdd_file_datapattern.sh to test out that these this modification did not break the expected functionality of -datapattern file/wholefile. Signed-off-by: Brian Atkinson --- src/base/target_cleanup.c | 1 - src/client/parse_func.c | 18 ++++--- src/common/xint_td.h | 1 - tests/functional/CMakeLists.txt | 1 + tests/functional/test_xdd_file_datapattern.sh | 52 +++++++++++++++++++ 5 files changed, 64 insertions(+), 9 deletions(-) create mode 100755 tests/functional/test_xdd_file_datapattern.sh diff --git a/src/base/target_cleanup.c b/src/base/target_cleanup.c index d9f82e45..56d43418 100644 --- a/src/base/target_cleanup.c +++ b/src/base/target_cleanup.c @@ -69,7 +69,6 @@ xdd_target_thread_cleanup(target_data_t *tdp) { /* If the file target opend a file for the data pattern we must close * the files and clean up the data_pattern buffer */ - close(tdp->dpp_fd); free(tdp->td_dpp->data_pattern); } free(tdp->td_dpp); diff --git a/src/client/parse_func.c b/src/client/parse_func.c index 045afe9e..afb989b6 100644 --- a/src/client/parse_func.c +++ b/src/client/parse_func.c @@ -555,6 +555,8 @@ xddfunc_datapattern(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flag } } else if (strcmp(pattern_type, "file") == 0 || strcmp(pattern_type, "wholefile") == 0) { retval++; + int dp_fd; + if (argc <= args+2) { fprintf(xgp->errout, "%s: not enough arguments specified for the option '-datapattern'\n", xgp->progname); return(0); @@ -583,15 +585,16 @@ xddfunc_datapattern(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flag (char *)argv[args+2]); return(0); } - tdp->dpp_fd = open(tdp->td_dpp->data_pattern_filename, O_RDONLY); - if (tdp->dpp_fd < 0) { + dp_fd = open(tdp->td_dpp->data_pattern_filename, O_RDONLY); + if (dp_fd < 0) { fprintf(xgp->errout, "%s: could not open %s\n", xgp->progname, tdp->td_dpp->data_pattern_filename); return(0); } tdp->td_dpp->data_pattern = (unsigned char *)malloc(sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1)); memset(tdp->td_dpp->data_pattern, '\0', sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1)); - pread(tdp->dpp_fd, tdp->td_dpp->data_pattern, tdp->td_dpp->data_pattern_length, 0); - } else {// Put this option into all Targets + pread(dp_fd, tdp->td_dpp->data_pattern, tdp->td_dpp->data_pattern_length, 0); + close(dp_fd); + } else {// Put this option into all Targets if (flags & XDD_PARSE_PHASE2) { tdp = planp->target_datap[0]; i = 0; @@ -609,15 +612,16 @@ xddfunc_datapattern(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flag (char *)argv[args+2]); return(0); } - tdp->dpp_fd = open(tdp->td_dpp->data_pattern_filename, O_RDONLY); - if (tdp->dpp_fd < 0) { + dp_fd = open(tdp->td_dpp->data_pattern_filename, O_RDONLY); + if (dp_fd < 0) { fprintf(xgp->errout, "%s: could not open %s\n", xgp->progname, tdp->td_dpp->data_pattern_filename); return(0); } tdp->td_dpp->data_pattern_length = pattern_length; tdp->td_dpp->data_pattern = (unsigned char *)malloc(sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1)); memset(tdp->td_dpp->data_pattern, '\0', sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1)); - pread(tdp->dpp_fd, tdp->td_dpp->data_pattern, tdp->td_dpp->data_pattern_length, 0); + pread(dp_fd, tdp->td_dpp->data_pattern, tdp->td_dpp->data_pattern_length, 0); + close(dp_fd); i++; tdp = planp->target_datap[i]; } diff --git a/src/common/xint_td.h b/src/common/xint_td.h index 5e6fd268..27d66c92 100644 --- a/src/common/xint_td.h +++ b/src/common/xint_td.h @@ -156,7 +156,6 @@ struct xint_target_data { struct xint_e2e *td_e2ep; // Pointer to the e2e struct when needed struct xint_extended_stats *td_esp; // Extended Stats Structure Pointer struct xint_triggers *td_trigp; // Triggers Structure Pointer - int dpp_fd; // File descriptor for file data pattern struct xint_data_pattern *td_dpp; // Data Pattern Structure Pointer struct xint_raw *td_rawp; // RAW Data Structure Pointer struct lockstep *td_lsp; // Pointer to the lockstep structure used by the lockstep option diff --git a/tests/functional/CMakeLists.txt b/tests/functional/CMakeLists.txt index 1ae1e8be..3de6c6ba 100644 --- a/tests/functional/CMakeLists.txt +++ b/tests/functional/CMakeLists.txt @@ -2,6 +2,7 @@ set(FUNCTIONAL test_xdd_createnewfiles.sh test_xdd_createnewfiles2.sh test_xdd_dryrun.sh + test_xdd_file_datapattern.sh test_xdd_heartbeat_byte.sh test_xdd_heartbeat_elapsed.sh test_xdd_heartbeat_lf.sh diff --git a/tests/functional/test_xdd_file_datapattern.sh b/tests/functional/test_xdd_file_datapattern.sh new file mode 100755 index 00000000..234decb2 --- /dev/null +++ b/tests/functional/test_xdd_file_datapattern.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# +# Acceptance test for XDD. +# +# Validate the funtionality of -datapattern file/wholefile option by creating a file, setting the datapattern +# from that file and then verifying the contents match +# +# Description - terminates XDD after a given amount of seconds have passed +# +# Get absolute path to script +SCRIPT=${BASH_SOURCE[0]} +SCRIPTPATH=$(dirname "${SCRIPT}") + +# Source the test configuration environment +source "${SCRIPTPATH}"/../test_config +source "${SCRIPTPATH}"/../common.sh + +# Perform pre-test +initialize_test +test_dir="${XDDTEST_LOCAL_MOUNT}/${TESTNAME}" +log_file="$(get_log_file)" + +input_file=$test_dir/data1.dat +output_file=$test_dir/data2.dat + +# Create datapattern input file using dd +dd if=/dev/urandom of="${input_file}" bs=4k count=2 + +# Write out file using xdd with -datapattern file +"${XDDTEST_XDD_EXE}" -op write -reqsize 8 -numreqs 1 -datapattern file "${input_file}" -targets 1 "${output_file}" -qd 1 -passes 1 +# Verify the contents of the file match +diff "${input_file}" "${output_file}" +if [[ $? -ne 0 ]] +then + rm "${input_file}" "${output_file}" + finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match." +fi + +rm "${output_file}" +# Write out file using xdd with -datapattern wholefile +"${XDDTEST_XDD_EXE}" -op write -reqsize 8 -numreqs 1 -datapattern wholefile "${input_file}" -targets 1 "${output_file}" -qd 1 -passes 1 +# Verify the contents of the file match +diff "${input_file}" "${output_file}" +if [[ $? -ne 0 ]] +then + rm "${input_file}" "${output_file}" + finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match." +fi +rm "${input_file}" "${output_file}" + +# Test passed +finalize_test 0