[PATCH v2 0/7] Seven fio patches

[PATCH v2 0/7] Seven fio patches

[Bart Van Assche]

Hi Jens,

The following changes are included in this patch series:
- Declare local functions 'static' in zbd.c.
- Make the zbd test scripts more robust.
- Use snprintf() instead of strncpy() + explicit null termination.
- Rework the approach for suppressing gcc 9 address of packed member warnings.
- Add a 'fulltest' target that runs the zbd tests.

Compared to v1, two new patches have been added and the patch for making
Travis run the ZBD tests has been left out. That patch had been prepared
considerable time ago and I have not yet figured out how to load the null_blk
kernel module today in a Travis environment.

Please consider these patches for the official fio git repository.

Thanks,

Bart.

Bart Van Assche (7):
  zbd: Declare local functions 'static'
  zbd: Improve robustness of unit tests
  Optimize the code that copies strings
  Refine packed annotations in stat.h
  Verify the absence of holes in struct jobs_eta at compile time
  Revert "eta: Fix compiler warning"
  Makefile: Add 'fulltest' target

 Makefile                            | 17 +++++++++
 cconv.c                             |  7 ++--
 client.c                            |  5 +--
 diskutil.c                          |  9 +++--
 engines/net.c                       |  6 ++--
 engines/sg.c                        |  4 +--
 eta.c                               | 11 +++---
 filesetup.c                         |  6 ++--
 fio.h                               |  2 ++
 gclient.c                           |  4 +--
 init.c                              | 19 +++-------
 ioengines.c                         |  3 +-
 options.c                           |  3 +-
 parse.c                             |  6 ++--
 server.c                            | 26 ++++++--------
 stat.c                              | 15 ++++----
 stat.h                              | 54 ++++++++++++++++-------------
 t/zbd/run-tests-against-zoned-nullb |  2 +-
 t/zbd/test-zbd-support              |  4 +--
 verify.c                            |  3 +-
 zbd.c                               |  6 ++--
 21 files changed, 106 insertions(+), 106 deletions(-)

-- 
2.22.0

[PATCH v2 1/7] zbd: Declare local functions 'static'

[Bart Van Assche]

This patch fixes two sparse warnings.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 zbd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/zbd.c b/zbd.c
index d7e91e37e010..2383c57dfc90 100644
--- a/zbd.c
+++ b/zbd.c
@@ -481,7 +481,7 @@ out:
  *
  * Returns 0 upon success and a negative error code upon failure.
  */
-int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
+static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
 {
 	enum blk_zoned_model zbd_model;
 	int ret = 0;
@@ -933,8 +933,8 @@ static void zbd_close_zone(struct thread_data *td, const struct fio_file *f,
  * a multiple of the fio block size. The caller must neither hold z->mutex
  * nor f->zbd_info->mutex. Returns with z->mutex held upon success.
  */
-struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
-					       struct io_u *io_u)
+static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
+						      struct io_u *io_u)
 {
 	const uint32_t min_bs = td->o.min_bs[io_u->ddir];
 	const struct fio_file *f = io_u->file;
-- 
2.22.0

[PATCH v2 2/7] zbd: Improve robustness of unit tests

[Bart Van Assche]

Give up if creation of the null_blk instance fails.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 t/zbd/run-tests-against-zoned-nullb | 2 +-
 t/zbd/test-zbd-support              | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/zbd/run-tests-against-zoned-nullb b/t/zbd/run-tests-against-zoned-nullb
index 9336716d4762..0952011c576e 100755
--- a/t/zbd/run-tests-against-zoned-nullb
+++ b/t/zbd/run-tests-against-zoned-nullb
@@ -24,6 +24,6 @@ modprobe null_blk nr_devices=0 &&
     echo 4096 > blocksize &&
     echo 1024 > size &&
     echo 1 > memory_backed &&
-    echo 1 > power
+    echo 1 > power || exit $?
 
 "${scriptdir}"/test-zbd-support "$@" /dev/nullb0
diff --git a/t/zbd/test-zbd-support b/t/zbd/test-zbd-support
index 6fb48ef091cf..ed54a0aa21fd 100755
--- a/t/zbd/test-zbd-support
+++ b/t/zbd/test-zbd-support
@@ -772,8 +772,8 @@ source "$(dirname "$0")/functions" || exit $?
 dev=$1
 realdev=$(readlink -f "$dev")
 basename=$(basename "$realdev")
-major=$((0x$(stat -L -c '%t' "$realdev")))
-minor=$((0x$(stat -L -c '%T' "$realdev")))
+major=$((0x$(stat -L -c '%t' "$realdev"))) || exit $?
+minor=$((0x$(stat -L -c '%T' "$realdev"))) || exit $?
 disk_size=$(($(<"/sys/dev/block/$major:$minor/size")*512))
 # When the target is a partition device, get basename of its holder device to
 # access sysfs path of the holder device
-- 
2.22.0

[PATCH v2 3/7] Optimize the code that copies strings

[Bart Van Assche]

Using strncpy() to copy strings is suboptimal because strncpy writes a
bunch of additional unnecessary null bytes. Use snprintf() instead of
strncpy(). An additional advantage of snprintf() is that it guarantees
that the output string is '\0'-terminated.

This patch is an improvement for commit 32e31c8c5f7b ("Fix string copy
compilation warnings").

Cc: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 cconv.c       |  7 +++----
 client.c      |  5 +++--
 diskutil.c    |  9 ++++-----
 engines/net.c |  6 ++----
 engines/sg.c  |  4 ++--
 filesetup.c   |  6 ++----
 gclient.c     |  4 ++--
 init.c        | 19 +++++--------------
 ioengines.c   |  3 +--
 options.c     |  3 +--
 parse.c       |  6 ++----
 server.c      | 26 +++++++++++---------------
 stat.c        | 15 ++++++++-------
 verify.c      |  3 +--
 14 files changed, 47 insertions(+), 69 deletions(-)

diff --git a/cconv.c b/cconv.c
index 50e45c63a636..0e6572462788 100644
--- a/cconv.c
+++ b/cconv.c
@@ -13,10 +13,9 @@ static void string_to_cpu(char **dst, const uint8_t *src)
 
 static void __string_to_net(uint8_t *dst, const char *src, size_t dst_size)
 {
-	if (src) {
-		dst[dst_size - 1] = '\0';
-		strncpy((char *) dst, src, dst_size - 1);
-	} else
+	if (src)
+		snprintf((char *) dst, dst_size, "%s", src);
+	else
 		dst[0] = '\0';
 }
 
diff --git a/client.c b/client.c
index 43cfbd43b9d3..e0047af06a3e 100644
--- a/client.c
+++ b/client.c
@@ -520,7 +520,7 @@ static void probe_client(struct fio_client *client)
 
 	sname = server_name(client, buf, sizeof(buf));
 	memset(pdu.server, 0, sizeof(pdu.server));
-	strncpy((char *) pdu.server, sname, sizeof(pdu.server) - 1);
+	snprintf((char *) pdu.server, sizeof(pdu.server), "%s", sname);
 
 	fio_net_send_cmd(client->fd, FIO_NET_CMD_PROBE, &pdu, sizeof(pdu), &tag, &client->cmd_list);
 }
@@ -574,7 +574,8 @@ static int fio_client_connect_sock(struct fio_client *client)
 
 	memset(addr, 0, sizeof(*addr));
 	addr->sun_family = AF_UNIX;
-	strncpy(addr->sun_path, client->hostname, sizeof(addr->sun_path) - 1);
+	snprintf(addr->sun_path, sizeof(addr->sun_path), "%s",
+		 client->hostname);
 
 	fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (fd < 0) {
diff --git a/diskutil.c b/diskutil.c
index 7be4c022431e..f074401501ba 100644
--- a/diskutil.c
+++ b/diskutil.c
@@ -181,8 +181,7 @@ static int get_device_numbers(char *file_name, int *maj, int *min)
 		/*
 		 * must be a file, open "." in that path
 		 */
-		tempname[PATH_MAX - 1] = '\0';
-		strncpy(tempname, file_name, PATH_MAX - 1);
+		snprintf(tempname, ARRAY_SIZE(tempname), "%s", file_name);
 		p = dirname(tempname);
 		if (stat(p, &st)) {
 			perror("disk util stat");
@@ -314,7 +313,8 @@ static struct disk_util *disk_util_add(struct thread_data *td, int majdev,
 		sfree(du);
 		return NULL;
 	}
-	strncpy((char *) du->dus.name, basename(path), FIO_DU_NAME_SZ - 1);
+	snprintf((char *) du->dus.name, ARRAY_SIZE(du->dus.name), "%s",
+		 basename(path));
 	du->sysfs_root = strdup(path);
 	du->major = majdev;
 	du->minor = mindev;
@@ -435,8 +435,7 @@ static struct disk_util *__init_per_file_disk_util(struct thread_data *td,
 			log_err("unknown sysfs layout\n");
 			return NULL;
 		}
-		tmp[PATH_MAX - 1] = '\0';
-		strncpy(tmp, p, PATH_MAX - 1);
+		snprintf(tmp, ARRAY_SIZE(tmp), "%s", p);
 		sprintf(path, "%s", tmp);
 	}
 
diff --git a/engines/net.c b/engines/net.c
index ca6fb344b897..91f25774690a 100644
--- a/engines/net.c
+++ b/engines/net.c
@@ -1105,8 +1105,7 @@ static int fio_netio_setup_connect_unix(struct thread_data *td,
 	struct sockaddr_un *soun = &nd->addr_un;
 
 	soun->sun_family = AF_UNIX;
-	memset(soun->sun_path, 0, sizeof(soun->sun_path));
-	strncpy(soun->sun_path, path, sizeof(soun->sun_path) - 1);
+	snprintf(soun->sun_path, sizeof(soun->sun_path), "%s", path);
 	return 0;
 }
 
@@ -1135,9 +1134,8 @@ static int fio_netio_setup_listen_unix(struct thread_data *td, const char *path)
 
 	mode = umask(000);
 
-	memset(addr, 0, sizeof(*addr));
 	addr->sun_family = AF_UNIX;
-	strncpy(addr->sun_path, path, sizeof(addr->sun_path) - 1);
+	snprintf(addr->sun_path, sizeof(addr->sun_path), "%s", path);
 	unlink(path);
 
 	len = sizeof(addr->sun_family) + strlen(path) + 1;
diff --git a/engines/sg.c b/engines/sg.c
index c46b9abaec87..a1a6de4ce248 100644
--- a/engines/sg.c
+++ b/engines/sg.c
@@ -1181,8 +1181,8 @@ static char *fio_sgio_errdetails(struct io_u *io_u)
 	}
 
 	if (!(hdr->info & SG_INFO_CHECK) && !strlen(msg))
-		strncpy(msg, "SG Driver did not report a Host, Driver or Device check",
-			MAXERRDETAIL - 1);
+		snprintf(msg, MAXERRDETAIL, "%s",
+			 "SG Driver did not report a Host, Driver or Device check");
 
 	return msg;
 }
diff --git a/filesetup.c b/filesetup.c
index 17fa31fb3cd4..57eca1bf5bf0 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -805,8 +805,7 @@ static unsigned long long get_fs_free_counts(struct thread_data *td)
 		} else if (f->filetype != FIO_TYPE_FILE)
 			continue;
 
-		buf[255] = '\0';
-		strncpy(buf, f->file_name, 255);
+		snprintf(buf, ARRAY_SIZE(buf), "%s", f->file_name);
 
 		if (stat(buf, &sb) < 0) {
 			if (errno != ENOENT)
@@ -829,8 +828,7 @@ static unsigned long long get_fs_free_counts(struct thread_data *td)
 			continue;
 
 		fm = calloc(1, sizeof(*fm));
-		strncpy(fm->__base, buf, sizeof(fm->__base));
-		fm->__base[255] = '\0'; 
+		snprintf(fm->__base, ARRAY_SIZE(fm->__base), "%s", buf);
 		fm->base = basename(fm->__base);
 		fm->key = sb.st_dev;
 		flist_add(&fm->list, &list);
diff --git a/gclient.c b/gclient.c
index 04275a1384c2..64324177ef1a 100644
--- a/gclient.c
+++ b/gclient.c
@@ -318,7 +318,7 @@ static void gfio_update_thread_status(struct gui_entry *ge,
 	static char message[100];
 	const char *m = message;
 
-	strncpy(message, status_message, sizeof(message) - 1);
+	snprintf(message, sizeof(message), "%s", status_message);
 	gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ge->thread_status_pb), m);
 	gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(ge->thread_status_pb), perc / 100.0);
 	gtk_widget_queue_draw(ge->ui->window);
@@ -330,7 +330,7 @@ static void gfio_update_thread_status_all(struct gui *ui, char *status_message,
 	static char message[100];
 	const char *m = message;
 
-	strncpy(message, status_message, sizeof(message) - 1);
+	strncpy(message, sizeof(message), "%s", status_message);
 	gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ui->thread_status_pb), m);
 	gtk_progress_bar_set_fraction(GTK_PROGRESS_BAR(ui->thread_status_pb), perc / 100.0);
 	gtk_widget_queue_draw(ui->window);
diff --git a/init.c b/init.c
index c9f6198ea63a..63f2168eabcb 100644
--- a/init.c
+++ b/init.c
@@ -1273,8 +1273,7 @@ static char *make_filename(char *buf, size_t buf_size,struct thread_options *o,
 	for (f = &fpre_keywords[0]; f->keyword; f++)
 		f->strlen = strlen(f->keyword);
 
-	buf[buf_size - 1] = '\0';
-	strncpy(buf, o->filename_format, buf_size - 1);
+	snprintf(buf, buf_size, "%s", o->filename_format);
 
 	memset(copy, 0, sizeof(copy));
 	for (f = &fpre_keywords[0]; f->keyword; f++) {
@@ -1353,7 +1352,7 @@ static char *make_filename(char *buf, size_t buf_size,struct thread_options *o,
 			if (post_start)
 				strncpy(dst, buf + post_start, dst_left);
 
-			strncpy(buf, copy, buf_size - 1);
+			snprintf(buf, buf_size, "%s", copy);
 		} while (1);
 	}
 
@@ -2029,20 +2028,12 @@ static int __parse_jobs_ini(struct thread_data *td,
 				 */
 				if (access(filename, F_OK) &&
 				    (ts = strrchr(file, '/'))) {
-					int len = ts - file +
-						strlen(filename) + 2;
-
-					if (!(full_fn = calloc(1, len))) {
+					if (asprintf(&full_fn, "%.*s%s",
+						 (int)(ts - file + 1), file,
+						 filename) < 0) {
 						ret = ENOMEM;
 						break;
 					}
-
-					strncpy(full_fn,
-						file, (ts - file) + 1);
-					strncpy(full_fn + (ts - file) + 1,
-						filename,
-						len - (ts - file) - 1);
-					full_fn[len - 1] = 0;
 					filename = full_fn;
 				}
 
diff --git a/ioengines.c b/ioengines.c
index aa4ccd2755c9..40fa75c382b4 100644
--- a/ioengines.c
+++ b/ioengines.c
@@ -125,8 +125,7 @@ static struct ioengine_ops *__load_ioengine(const char *name)
 {
 	char engine[64];
 
-	engine[sizeof(engine) - 1] = '\0';
-	strncpy(engine, name, sizeof(engine) - 1);
+	snprintf(engine, sizeof(engine), "%s", name);
 
 	/*
 	 * linux libaio has alias names, so convert to what we want
diff --git a/options.c b/options.c
index f4c9bedf377f..447f231e7148 100644
--- a/options.c
+++ b/options.c
@@ -4902,8 +4902,7 @@ char *fio_option_dup_subs(const char *opt)
 		return NULL;
 	}
 
-	in[OPT_LEN_MAX] = '\0';
-	strncpy(in, opt, OPT_LEN_MAX);
+	snprintf(in, sizeof(in), "%s", opt);
 
 	while (*inptr && nchr > 0) {
 		if (inptr[0] == '$' && inptr[1] == '{') {
diff --git a/parse.c b/parse.c
index a7d4516e4702..c4fd4626df9f 100644
--- a/parse.c
+++ b/parse.c
@@ -602,8 +602,7 @@ static int __handle_option(const struct fio_option *o, const char *ptr,
 		if (!is_time && o->is_time)
 			is_time = o->is_time;
 
-		tmp[sizeof(tmp) - 1] = '\0';
-		strncpy(tmp, ptr, sizeof(tmp) - 1);
+		snprintf(tmp, sizeof(tmp), "%s", ptr);
 		p = strchr(tmp, ',');
 		if (p)
 			*p = '\0';
@@ -829,8 +828,7 @@ static int __handle_option(const struct fio_option *o, const char *ptr,
 		char tmp[128];
 		char *p1, *p2;
 
-		tmp[sizeof(tmp) - 1] = '\0';
-		strncpy(tmp, ptr, sizeof(tmp) - 1);
+		snprintf(tmp, sizeof(tmp), "%s", ptr);
 
 		/* Handle bsrange with separate read,write values: */
 		p1 = strchr(tmp, ',');
diff --git a/server.c b/server.c
index 23e549a590a7..e7846227f2b1 100644
--- a/server.c
+++ b/server.c
@@ -865,7 +865,8 @@ static int handle_probe_cmd(struct fio_net_cmd *cmd)
 	strcpy(me, (char *) pdu->server);
 
 	gethostname((char *) probe.hostname, sizeof(probe.hostname));
-	strncpy((char *) probe.fio_version, fio_version_string, sizeof(probe.fio_version) - 1);
+	snprintf((char *) probe.fio_version, sizeof(probe.fio_version), "%s",
+		 fio_version_string);
 
 	/*
 	 * If the client supports compression and we do too, then enable it
@@ -1470,12 +1471,10 @@ void fio_server_send_ts(struct thread_stat *ts, struct group_run_stats *rs)
 
 	memset(&p, 0, sizeof(p));
 
-	strncpy(p.ts.name, ts->name, FIO_JOBNAME_SIZE);
-	p.ts.name[FIO_JOBNAME_SIZE - 1] = '\0';
-	strncpy(p.ts.verror, ts->verror, FIO_VERROR_SIZE);
-	p.ts.verror[FIO_VERROR_SIZE - 1] = '\0';
-	strncpy(p.ts.description, ts->description, FIO_JOBDESC_SIZE);
-	p.ts.description[FIO_JOBDESC_SIZE - 1] = '\0';
+	snprintf(p.ts.name, sizeof(p.ts.name), "%s", ts->name);
+	snprintf(p.ts.verror, sizeof(p.ts.verror), "%s", ts->verror);
+	snprintf(p.ts.description, sizeof(p.ts.description), "%s",
+		 ts->description);
 
 	p.ts.error		= cpu_to_le32(ts->error);
 	p.ts.thread_number	= cpu_to_le32(ts->thread_number);
@@ -1666,8 +1665,7 @@ static void convert_dus(struct disk_util_stat *dst, struct disk_util_stat *src)
 {
 	int i;
 
-	dst->name[FIO_DU_NAME_SZ - 1] = '\0';
-	strncpy((char *) dst->name, (char *) src->name, FIO_DU_NAME_SZ - 1);
+	snprintf((char *) dst->name, sizeof(dst->name), "%s", src->name);
 
 	for (i = 0; i < 2; i++) {
 		dst->s.ios[i]		= cpu_to_le64(src->s.ios[i]);
@@ -1977,8 +1975,7 @@ int fio_send_iolog(struct thread_data *td, struct io_log *log, const char *name)
 	else
 		pdu.compressed = 0;
 
-	strncpy((char *) pdu.name, name, FIO_NET_NAME_MAX);
-	pdu.name[FIO_NET_NAME_MAX - 1] = '\0';
+	snprintf((char *) pdu.name, sizeof(pdu.name), "%s", name);
 
 	/*
 	 * We can't do this for a pre-compressed log, but for that case,
@@ -2195,9 +2192,8 @@ static int fio_init_server_sock(void)
 
 	mode = umask(000);
 
-	memset(&addr, 0, sizeof(addr));
 	addr.sun_family = AF_UNIX;
-	strncpy(addr.sun_path, bind_sock, sizeof(addr.sun_path) - 1);
+	snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", bind_sock);
 
 	len = sizeof(addr.sun_family) + strlen(bind_sock) + 1;
 
@@ -2247,9 +2243,9 @@ static int fio_init_server_connection(void)
 		if (p)
 			strcat(p, port);
 		else
-			strncpy(bind_str, port, sizeof(bind_str) - 1);
+			snprintf(bind_str, sizeof(bind_str), "%s", port);
 	} else
-		strncpy(bind_str, bind_sock, sizeof(bind_str) - 1);
+		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
 
 	log_info("fio: server listening on %s\n", bind_str);
 
diff --git a/stat.c b/stat.c
index bf87917c2956..33637900df62 100644
--- a/stat.c
+++ b/stat.c
@@ -1828,10 +1828,11 @@ void __show_run_stats(void)
 			/*
 			 * These are per-group shared already
 			 */
-			strncpy(ts->name, td->o.name, FIO_JOBNAME_SIZE - 1);
+			snprintf(ts->name, sizeof(ts->name), "%s", td->o.name);
 			if (td->o.description)
-				strncpy(ts->description, td->o.description,
-						FIO_JOBDESC_SIZE - 1);
+				snprintf(ts->description,
+					 sizeof(ts->description), "%s",
+					 td->o.description);
 			else
 				memset(ts->description, 0, FIO_JOBDESC_SIZE);
 
@@ -1868,12 +1869,12 @@ void __show_run_stats(void)
 			if (!td->error && td->o.continue_on_error &&
 			    td->first_error) {
 				ts->error = td->first_error;
-				ts->verror[sizeof(ts->verror) - 1] = '\0';
-				strncpy(ts->verror, td->verror, sizeof(ts->verror) - 1);
+				snprintf(ts->verror, sizeof(ts->verror), "%s",
+					 td->verror);
 			} else  if (td->error) {
 				ts->error = td->error;
-				ts->verror[sizeof(ts->verror) - 1] = '\0';
-				strncpy(ts->verror, td->verror, sizeof(ts->verror) - 1);
+				snprintf(ts->verror, sizeof(ts->verror), "%s",
+					 td->verror);
 			}
 		}
 
diff --git a/verify.c b/verify.c
index f79ab43aab6b..48ba051da3ad 100644
--- a/verify.c
+++ b/verify.c
@@ -1635,8 +1635,7 @@ struct all_io_list *get_all_io_list(int save_mask, size_t *sz)
 			s->rand.state32.s[3] = 0;
 			s->rand.use64 = 0;
 		}
-		s->name[sizeof(s->name) - 1] = '\0';
-		strncpy((char *) s->name, td->o.name, sizeof(s->name) - 1);
+		snprintf((char *) s->name, sizeof(s->name), "%s", td->o.name);
 		next = io_list_next(s);
 	}
 
-- 
2.22.0

[PATCH v2 4/7] Refine packed annotations in stat.h

[Bart Van Assche]

Instead of declaring the whole structure packed, only declare non-aligned
members packed. This patch is an alternative way to fix the following gcc 9
compiler warnings:

eta.c: In function 'calc_thread_status':
eta.c:510:7: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  510 |     je->rate);
      |     ~~^~~~~~
eta.c:522:66: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  522 |  calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je->rate);
      |                                                                ~~^~~~~~
eta.c:523:64: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  523 |  calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je->iops);
      |

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 stat.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/stat.h b/stat.h
index e9551381ce7b..c6353c70ae08 100644
--- a/stat.h
+++ b/stat.h
@@ -260,10 +260,11 @@ struct jobs_eta {
 
 	uint64_t m_rate[DDIR_RWDIR_CNT], t_rate[DDIR_RWDIR_CNT];
 	uint64_t rate[DDIR_RWDIR_CNT];
-	uint32_t m_iops[DDIR_RWDIR_CNT], t_iops[DDIR_RWDIR_CNT];
-	uint32_t iops[DDIR_RWDIR_CNT];
-	uint64_t elapsed_sec;
-	uint64_t eta_sec;
+	uint32_t m_iops[DDIR_RWDIR_CNT] __attribute__((packed));
+	uint32_t t_iops[DDIR_RWDIR_CNT] __attribute__((packed));
+	uint32_t iops[DDIR_RWDIR_CNT] __attribute__((packed));
+	uint64_t elapsed_sec __attribute__((packed));
+	uint64_t eta_sec __attribute__((packed));
 	uint32_t is_pow2;
 	uint32_t unit_base;
 
@@ -276,7 +277,7 @@ struct jobs_eta {
 	 */
 	uint32_t nr_threads;
 	uint8_t run_str[];
-} __attribute__((packed));
+};
 
 struct io_u_plat_entry {
 	struct flist_head list;
-- 
2.22.0

[PATCH v2 5/7] Verify the absence of holes in struct jobs_eta at compile time

[Bart Van Assche]

This helps to verify the correctness of a later patch that will modify
this structure.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 eta.c  |  2 ++
 fio.h  |  2 ++
 stat.h | 55 +++++++++++++++++++++++++++++--------------------------
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/eta.c b/eta.c
index 647a1bdd8eed..4095a257d206 100644
--- a/eta.c
+++ b/eta.c
@@ -736,6 +736,8 @@ void print_thread_status(void)
 
 void print_status_init(int thr_number)
 {
+	BUILD_BUG_ON(sizeof(struct jobs_eta) != sizeof(struct jobs_eta_packed));
+
 	DRD_IGNORE_VAR(__run_str);
 	__run_str[thr_number] = 'P';
 	update_condensed_str(__run_str, run_str);
diff --git a/fio.h b/fio.h
index 2094d30b863e..4bf96fc1cd62 100644
--- a/fio.h
+++ b/fio.h
@@ -66,6 +66,8 @@
 #include <cuda.h>
 #endif
 
+#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+
 struct fio_sem;
 
 /*
diff --git a/stat.h b/stat.h
index c6353c70ae08..c209ab6c7a96 100644
--- a/stat.h
+++ b/stat.h
@@ -251,33 +251,36 @@ struct thread_stat {
 	uint64_t cachemiss;
 } __attribute__((packed));
 
-struct jobs_eta {
-	uint32_t nr_running;
-	uint32_t nr_ramp;
-
-	uint32_t nr_pending;
-	uint32_t nr_setting_up;
-
-	uint64_t m_rate[DDIR_RWDIR_CNT], t_rate[DDIR_RWDIR_CNT];
-	uint64_t rate[DDIR_RWDIR_CNT];
-	uint32_t m_iops[DDIR_RWDIR_CNT] __attribute__((packed));
-	uint32_t t_iops[DDIR_RWDIR_CNT] __attribute__((packed));
-	uint32_t iops[DDIR_RWDIR_CNT] __attribute__((packed));
-	uint64_t elapsed_sec __attribute__((packed));
-	uint64_t eta_sec __attribute__((packed));
-	uint32_t is_pow2;
-	uint32_t unit_base;
-
-	uint32_t sig_figs;
-
-	uint32_t files_open;
+#define JOBS_ETA {							\
+	uint32_t nr_running;						\
+	uint32_t nr_ramp;						\
+									\
+	uint32_t nr_pending;						\
+	uint32_t nr_setting_up;						\
+									\
+	uint64_t m_rate[DDIR_RWDIR_CNT], t_rate[DDIR_RWDIR_CNT];	\
+	uint64_t rate[DDIR_RWDIR_CNT];					\
+	uint32_t m_iops[DDIR_RWDIR_CNT] __attribute__((packed));	\
+	uint32_t t_iops[DDIR_RWDIR_CNT] __attribute__((packed));	\
+	uint32_t iops[DDIR_RWDIR_CNT] __attribute__((packed));		\
+	uint64_t elapsed_sec __attribute__((packed));			\
+	uint64_t eta_sec __attribute__((packed));			\
+	uint32_t is_pow2;						\
+	uint32_t unit_base;						\
+									\
+	uint32_t sig_figs;						\
+									\
+	uint32_t files_open;						\
+									\
+	/*								\
+	 * Network 'copy' of run_str[]					\
+	 */								\
+	uint32_t nr_threads;						\
+	uint8_t run_str[];						\
+}
 
-	/*
-	 * Network 'copy' of run_str[]
-	 */
-	uint32_t nr_threads;
-	uint8_t run_str[];
-};
+struct jobs_eta JOBS_ETA;
+struct jobs_eta_packed JOBS_ETA __attribute__((packed));
 
 struct io_u_plat_entry {
 	struct flist_head list;
-- 
2.22.0

[PATCH v2 6/7] Revert "eta: Fix compiler warning"

[Bart Van Assche]

This reverts commit df0ca15ce2ffac0df3e9dcf4bfe4121333c4aa27. Due to
the previous patch it is no longer necessary to confuse the compiler
about accesses of the 'rate' and 'iops' members in struct jobs_eta.

Cc: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 eta.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/eta.c b/eta.c
index 4095a257d206..3d09a9229d93 100644
--- a/eta.c
+++ b/eta.c
@@ -392,9 +392,6 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 	static unsigned long long disp_io_iops[DDIR_RWDIR_CNT];
 	static struct timespec rate_prev_time, disp_prev_time;
 
-	void *je_rate = (void *) je->rate;
-	void *je_iops = (void *) je->iops;
-
 	if (!force) {
 		if (!(output_format & FIO_OUTPUT_NORMAL) &&
 		    f_out == stdout)
@@ -510,7 +507,7 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 
 	if (write_bw_log && rate_time > bw_avg_time && !in_ramp_time(td)) {
 		calc_rate(unified_rw_rep, rate_time, io_bytes, rate_io_bytes,
-				je_rate);
+				je->rate);
 		memcpy(&rate_prev_time, &now, sizeof(now));
 		add_agg_sample(sample_val(je->rate[DDIR_READ]), DDIR_READ, 0);
 		add_agg_sample(sample_val(je->rate[DDIR_WRITE]), DDIR_WRITE, 0);
@@ -522,8 +519,8 @@ bool calc_thread_status(struct jobs_eta *je, int force)
 	if (!force && !eta_time_within_slack(disp_time))
 		return false;
 
-	calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je_rate);
-	calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je_iops);
+	calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je->rate);
+	calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je->iops);
 
 	memcpy(&disp_prev_time, &now, sizeof(now));
 
-- 
2.22.0

[PATCH v2 7/7] Makefile: Add 'fulltest' target

[Bart Van Assche]

Make it easier to run the zoned block device tests.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 Makefile | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Makefile b/Makefile
index fe02bf1df86f..7c21ef8316f7 100644
--- a/Makefile
+++ b/Makefile
@@ -531,6 +531,21 @@ doc: tools/plot/fio2gnuplot.1
 test: fio
 	./fio --minimal --thread --exitall_on_error --runtime=1s --name=nulltest --ioengine=null --rw=randrw --iodepth=2 --norandommap --random_generator=tausworthe64 --size=16T --name=verifyfstest --filename=fiotestfile.tmp --unlink=1 --rw=write --verify=crc32c --verify_state_save=0 --size=16K
 
+fulltest:
+	sudo modprobe null_blk &&				 	\
+	if [ ! -e /usr/include/libzbc/zbc.h ]; then			\
+	  git clone https://github.com/hgst/libzbc &&		 	\
+	  (cd libzbc &&						 	\
+	   ./autogen.sh &&					 	\
+	   ./configure --prefix=/usr &&				 	\
+	   make -j &&						 	\
+	   sudo make install)						\
+	fi &&					 			\
+	sudo t/zbd/run-tests-against-regular-nullb &&		 	\
+	if [ -e /sys/module/null_blk/parameters/zoned ]; then		\
+		sudo t/zbd/run-tests-against-zoned-nullb;	 	\
+	fi
+
 install: $(PROGS) $(SCRIPTS) tools/plot/fio2gnuplot.1 FORCE
 	$(INSTALL) -m 755 -d $(DESTDIR)$(bindir)
 	$(INSTALL) $(PROGS) $(SCRIPTS) $(DESTDIR)$(bindir)
@@ -541,3 +556,5 @@ install: $(PROGS) $(SCRIPTS) tools/plot/fio2gnuplot.1 FORCE
 	$(INSTALL) -m 644 $(SRCDIR)/tools/hist/fiologparser_hist.py.1 $(DESTDIR)$(mandir)/man1
 	$(INSTALL) -m 755 -d $(DESTDIR)$(sharedir)
 	$(INSTALL) -m 644 $(SRCDIR)/tools/plot/*gpm $(DESTDIR)$(sharedir)/
+
+.PHONY: test fulltest
-- 
2.22.0

Re: [PATCH v2 5/7] Verify the absence of holes in struct jobs_eta at compile time

[Jens Axboe]

On 8/13/19 8:20 PM, Bart Van Assche wrote:
> This helps to verify the correctness of a later patch that will modify
> this structure.

I'm fine with this solution (for packed vs unpacked), but we already
have compiletime_assert() for these kinds of checked. Sorry, forgot
to mention that first time around.

-- 
Jens Axboe