From: Tomohiro Kusumi <tkusumi@tuxera.com>
Date: Thu, 31 Aug 2017 20:13:05 +0000 (+0300)
Subject: fix broken external ioengine option
X-Git-Tag: fio-3.1~26
X-Git-Url: https://git.kernel.dk/?p=fio.git;a=commitdiff_plain;h=6730b40f1f2f9a07ac6b28d39fba293fedb1f239

fix broken external ioengine option

get_engine_name() function added to support external ioengine option
via "external:" prefix in below commits in 2007 has been broken.
 7b395ca5('Prefix external io engine loading with 'external'')
 8a7bd877('Document loading external io engines')

It seems to have been broken since below commit in 2011 which made
__handle_option() strip ':' and after while parsing the option.
 c44b1ff5('Add sub-option support (sort-of) and convert libaio_userspace_reap')

Above change made fio dlopen "external" instead of the path located
after "external:", since the path had already been stripped by
__handle_option() by the time get_engine_name() was called. This commit
fixes it by adding ->ioengine_so_path pointer to keep the path while
parsing using sub-option callback.

Note that removing get_engine_name() doesn't affect non "external:"
ioengine options, as the option parser also strips while parsing.

-- before this commit
 # ./fio --name=xxx --ioengine=external:./engines/skeleton_external.so
 fio: engine external not loadable
 fio: failed to load engine external
 fio: file:ioengines.c:91, func=dlopen, error=external: cannot open shared object file: No such file or directory

-- with this commit
 # ./fio --name=xxx --ioengine=external:./engines/skeleton_external.so
 xxx: (g=0): rw=read, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=engine_name, iodepth=1
 fio-3.0-32-g68f6f
 Starting 1 process
 xxx: you need to specify size=
 fio: pid=0, err=22/file:filesetup.c:973, func=total_file_size, error=Invalid argument

 Run status group 0 (all jobs):

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---

diff --git a/init.c b/init.c
index 625c9371..2da64ba8 100644
--- a/init.c
+++ b/init.c
@@ -912,20 +912,6 @@ static int fixup_options(struct thread_data *td)
 	return ret;
 }
 
-/* External engines are specified by "external:name.o") */
-static const char *get_engine_name(const char *str)
-{
-	char *p = strstr(str, ":");
-
-	if (!p)
-		return str;
-
-	p++;
-	strip_blank_front(&p);
-	strip_blank_end(p);
-	return p;
-}
-
 static void init_rand_file_service(struct thread_data *td)
 {
 	unsigned long nranges = td->o.nr_files << FIO_FSERVICE_SHIFT;
@@ -1057,7 +1043,10 @@ int ioengine_load(struct thread_data *td)
 		free_ioengine(td);
 	}
 
-	engine = get_engine_name(td->o.ioengine);
+	/*
+	 * Use ->ioengine_so_path if an external ioengine is specified.
+	 */
+	engine = td->o.ioengine_so_path ?: td->o.ioengine;
 	td->io_ops = load_ioengine(td, engine);
 	if (!td->io_ops) {
 		log_err("fio: failed to load engine %s\n", engine);
diff --git a/options.c b/options.c
index 443791ab..54fa4eef 100644
--- a/options.c
+++ b/options.c
@@ -1462,6 +1462,39 @@ static int str_write_hist_log_cb(void *data, const char *str)
 	return 0;
 }
 
+/*
+ * str is supposed to be a substring of the strdup'd original string,
+ * and is valid only if it's a regular file path.
+ * This function keeps the pointer to the path as needed later.
+ *
+ * "external:/path/to/so\0" <- original pointer updated with strdup'd
+ * "external\0"             <- above pointer after parsed, i.e. ->ioengine
+ *          "/path/to/so\0" <- str argument, i.e. ->ioengine_so_path
+ */
+static int str_ioengine_external_cb(void *data, const char *str)
+{
+	struct thread_data *td = cb_data_to_td(data);
+	struct stat sb;
+	char *p;
+
+	if (!str) {
+		log_err("fio: null external ioengine path\n");
+		return 1;
+	}
+
+	p = (char *)str; /* str is mutable */
+	strip_blank_front(&p);
+	strip_blank_end(p);
+
+	if (stat(p, &sb) || !S_ISREG(sb.st_mode)) {
+		log_err("fio: invalid external ioengine path \"%s\"\n", p);
+		return 1;
+	}
+
+	td->o.ioengine_so_path = p;
+	return 0;
+}
+
 static int rw_verify(struct fio_option *o, void *data)
 {
 	struct thread_data *td = cb_data_to_td(data);
@@ -1812,6 +1845,7 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 #endif
 			  { .ival = "external",
 			    .help = "Load external engine (append name)",
+			    .cb = str_ioengine_external_cb,
 			  },
 		},
 	},
diff --git a/thread_options.h b/thread_options.h
index 26a3e0e6..fd6576e2 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -53,6 +53,7 @@ struct thread_options {
 	char *filename_format;
 	char *opendir;
 	char *ioengine;
+	char *ioengine_so_path;
 	char *mmapfile;
 	enum td_ddir td_ddir;
 	unsigned int rw_seq;