Skip to content
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

Add f3write --keep and f3read --delete parameters #140

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions f3probe.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#define _POSIX_C_SOURCE 200809L
#define _GNU_SOURCE

#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -51,6 +52,8 @@ static struct argp_option options[] = {
"Reset method to use during the probe", 0},
{"time-ops", 't', NULL, 0,
"Time reads, writes, and resets", 0},
{"fix", 'i', NULL, 0,
"Run f3fix if counterfeit flash memory detected", 0},
{ 0 }
};

Expand All @@ -76,6 +79,7 @@ struct args {
int block_order;
int cache_order;
int strict_cache;
bool fix;
AltraMayor marked this conversation as resolved.
Show resolved Hide resolved
};

static error_t parse_opt(int key, char *arg, struct argp_state *state)
Expand Down Expand Up @@ -167,7 +171,11 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
case 't':
args->time_ops = true;
break;


case 'i':
args->fix = true;
break;

case ARGP_KEY_INIT:
args->filename = NULL;
break;
Expand Down Expand Up @@ -373,7 +381,9 @@ static int test_device(struct args *args)
uint64_t read_count, read_time_us;
uint64_t write_count, write_time_us;
uint64_t reset_count, reset_time_us;
uint64_t last_good_sector;
Copy link
Owner

@AltraMayor AltraMayor Jul 28, 2020

Choose a reason for hiding this comment

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

Instead of moving the declaration of last_good_sector here, declare char *fix_cmd = NULL;

More details in other comments.

Copy link
Owner

Choose a reason for hiding this comment

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

You've missed this item.

Copy link
Owner

Choose a reason for hiding this comment

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

To clarify, the declaration of last_good_sector should be moved back to the body of the switch..case.

const char *final_dev_filename;
char *fix_cmd;

AltraMayor marked this conversation as resolved.
Show resolved Hide resolved
dev = args->debug
? create_file_device(args->filename, args->real_size_byte,
Expand Down Expand Up @@ -474,8 +484,9 @@ static int test_device(struct args *args)
case FKTY_LIMBO:
case FKTY_WRAPAROUND:
case FKTY_CHAIN: {
uint64_t last_good_sector = (real_size_byte >> 9) - 1;
last_good_sector = (real_size_byte >> 9) - 1;
assert(block_order >= 9);
assert(asprintf(&fix_cmd, "f3fix --last-sec=%" PRIu64 " %s", last_good_sector, final_dev_filename) > 0);
Copy link
Owner

Choose a reason for hiding this comment

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

Break line to fit within the 80-column limit.

printf("Bad news: The device `%s' is a counterfeit of type %s\n\n"
"You can \"fix\" this device using the following command:\n"
"f3fix --last-sec=%" PRIu64 " %s\n",
Copy link
Owner

Choose a reason for hiding this comment

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

  1. Replace the line "f3fix --last-sec=%" PRIu64 " %s\n", with "%s\n",.
  2. Replace the line last_good_sector, final_dev_filename); below with fix_cmd);.

Expand Down Expand Up @@ -507,6 +518,10 @@ static int test_device(struct args *args)
report_ops("Reset", reset_count, reset_time_us);
}

if (args->fix && fix_cmd) {
system(fix_cmd);
}
Comment on lines +521 to +523
Copy link
Owner

Choose a reason for hiding this comment

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

Style: drop the braces since the body of this if statement is a single line.

free(fix_cmd);
free((void *)final_dev_filename);
return fake_type == FKTY_GOOD ? 0 : 100 + fake_type;
}
Expand Down Expand Up @@ -545,6 +560,7 @@ int main(int argc, char **argv)
.block_order = 0,
.cache_order = -1,
.strict_cache = false,
.fix = false,
};

/* Read parameters. */
Expand Down
20 changes: 18 additions & 2 deletions f3read.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ static struct argp_option options[] = {
"Maximum read rate", 0},
{"show-progress", 'p', "NUM", 0,
"Show progress if NUM is not zero", 0},
{"delete", 'd', NULL, 0,
"Delete corrupted NUM.h2w files", 0},
{ 0 }
};

Expand All @@ -48,6 +50,7 @@ struct args {
long end_at;
long max_read_rate;
int show_progress;
bool delete;
Copy link
Owner

Choose a reason for hiding this comment

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

Align indentation with other fields.

Copy link
Owner

Choose a reason for hiding this comment

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

You've missed this item.

const char *dev_path;
};

Expand Down Expand Up @@ -85,6 +88,10 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
args->show_progress = !!arg_to_long(state, arg);
break;

case 'd':
args->delete = true;
break;

case ARGP_KEY_INIT:
args->dev_path = NULL;
break;
Expand Down Expand Up @@ -340,7 +347,7 @@ static inline void pr_avg_speed(double speed)
}

static void iterate_files(const char *path, const long *files,
long start_at, long end_at, long max_read_rate, int progress)
long start_at, long end_at, long max_read_rate, int progress, bool delete)
{
uint64_t tot_ok, tot_corrupted, tot_changed, tot_overwritten, tot_size;
int and_read_all = 1;
Expand Down Expand Up @@ -379,6 +386,14 @@ static void iterate_files(const char *path, const long *files,
tot_overwritten += stats.secs_overwritten;
tot_size += stats.bytes_read;
and_read_all = and_read_all && stats.read_all;
if (delete && (stats.secs_corrupted || tot_changed || tot_overwritten)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Please replace tot_changed with stats.secs_changed and tot_overwritten with stats.secs_overwritten. My fault, again.

Break the line as needed to fit within the 80-column limit.

const char *filename;
char *full_fn = full_fn_from_number(&filename, path, *files);
assert(full_fn);
if (unlink(full_fn))
err(errno, "Can't remove file %s\n", full_fn);
free(full_fn);
}
files++;
}
assert(!gettimeofday(&t2, NULL));
Expand Down Expand Up @@ -428,6 +443,7 @@ int main(int argc, char **argv)
.max_read_rate = 0,
/* If stdout isn't a terminal, supress progress. */
.show_progress = isatty(STDOUT_FILENO),
.delete = false,
};

/* Read parameters. */
Expand All @@ -437,7 +453,7 @@ int main(int argc, char **argv)
files = ls_my_files(args.dev_path, args.start_at, args.end_at);

iterate_files(args.dev_path, files, args.start_at, args.end_at,
args.max_read_rate, args.show_progress);
args.max_read_rate, args.show_progress, args.delete);
free((void *)files);
return 0;
}
31 changes: 24 additions & 7 deletions f3write.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ static struct argp_option options[] = {
"Maximum write rate", 0},
{"show-progress", 'p', "NUM", 0,
"Show progress if NUM is not zero", 0},
{"keep", 'k', 0, 0,
Copy link
Owner

Choose a reason for hiding this comment

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

Replace the first 0 after 'k' with NULL.

"Keep existing NUM.h2w files instead of removing them", 0},
{ 0 }
};

Expand All @@ -47,6 +49,7 @@ struct args {
long end_at;
long max_write_rate;
int show_progress;
bool keep;
const char *dev_path;
};

Expand Down Expand Up @@ -84,6 +87,10 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
args->show_progress = !!arg_to_long(state, arg);
break;

case 'k':
args->keep = true;
break;

case ARGP_KEY_INIT:
args->dev_path = NULL;
break;
Expand Down Expand Up @@ -170,7 +177,7 @@ static int write_chunk(int fd, size_t chunk_size, uint64_t *poffset)

/* Return true when disk is full. */
static int create_and_fill_file(const char *path, long number, size_t size,
int *phas_suggested_max_write_rate, struct flow *fw)
int *phas_suggested_max_write_rate, struct flow *fw, bool keep)
{
char *full_fn;
const char *filename;
Expand All @@ -184,16 +191,24 @@ static int create_and_fill_file(const char *path, long number, size_t size,
/* Create the file. */
full_fn = full_fn_from_number(&filename, path, number);
assert(full_fn);

Copy link
Owner

Choose a reason for hiding this comment

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

Style: drop this new blank line.

printf("Creating file %s ... ", filename);
fflush(stdout);
fd = open(full_fn, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR);
fd = open(full_fn, O_CREAT | O_WRONLY | O_EXCL, S_IRUSR | S_IWUSR);
if (fd < 0) {
if (errno == ENOSPC) {
printf("No space left.\n");
free(full_fn);
return true;
}
err(errno, "Can't create file %s", full_fn);
if (errno == EEXIST) {
brianpow marked this conversation as resolved.
Show resolved Hide resolved
if (keep) {
printf("Skipped.\n");
free(full_fn);
return false;
}
err(errno, "Unexpectedly found file %s, but option --keep is not in use", full_fn);
Copy link
Owner

Choose a reason for hiding this comment

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

Could replace the but with and? My fault, sorry.

}
}
assert(fd >= 0);

Expand Down Expand Up @@ -273,7 +288,7 @@ static int flush_chunk(const struct flow *fw, int fd)
}

static int fill_fs(const char *path, long start_at, long end_at,
long max_write_rate, int progress)
long max_write_rate, int progress, bool keep)
{
uint64_t free_space;
struct flow fw;
Expand Down Expand Up @@ -310,7 +325,7 @@ static int fill_fs(const char *path, long start_at, long end_at,
assert(!gettimeofday(&t1, NULL));
for (i = start_at; i <= end_at; i++)
if (create_and_fill_file(path, i, GIGABYTES,
&has_suggested_max_write_rate, &fw))
&has_suggested_max_write_rate, &fw, keep))
break;
assert(!gettimeofday(&t2, NULL));

Expand Down Expand Up @@ -360,6 +375,7 @@ int main(int argc, char **argv)
.start_at = 0,
.end_at = LONG_MAX - 1,
.max_write_rate = 0,
.keep = false,
/* If stdout isn't a terminal, supress progress. */
.show_progress = isatty(STDOUT_FILENO),
};
Expand All @@ -368,8 +384,9 @@ int main(int argc, char **argv)
argp_parse(&argp, argc, argv, 0, NULL, &args);
print_header(stdout, "write");

unlink_old_files(args.dev_path, args.start_at, args.end_at);
if(!args.keep)
unlink_old_files(args.dev_path, args.start_at, args.end_at);

return fill_fs(args.dev_path, args.start_at, args.end_at,
args.max_write_rate, args.show_progress);
args.max_write_rate, args.show_progress, args.keep);
}