diff mbox series

[2/3] Allow acquiring multiple loop devices

Message ID 20200129144913.9889-3-mdoucha@suse.cz
State Superseded
Headers show
Series LVM support scripts for OpenQA | expand

Commit Message

Martin Doucha Jan. 29, 2020, 2:49 p.m. UTC
tst_acquire_device__() currently uses a hardcoded filename so only one loop
device can be used at a time. Allow setting arbitrary temp filename so that
multiple different loop devices can be acquired.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/old/old_device.h   | 15 ++++++++++++++-
 lib/tst_device.c           | 36 +++++++++++++++++++++++-------------
 testcases/lib/tst_device.c | 14 +++++++++-----
 3 files changed, 46 insertions(+), 19 deletions(-)

Comments

Petr Vorel Feb. 3, 2020, 3:24 p.m. UTC | #1
Hi Martin,

> tst_acquire_device__() currently uses a hardcoded filename so only one loop
> device can be used at a time. Allow setting arbitrary temp filename so that
> multiple different loop devices can be acquired.

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Good idea, thanks!

BTW IMHO DEV_FILE should be #define DEV_FILE "test_dev.%d.img", where %d would
be PID to fix clash when tests run in paralel (e.g. mkswap01.sh and df01.sh).
I'll send a patch tomorrow, based on this one.

Kind regards,
Petr
Cyril Hrubis Feb. 7, 2020, 4:45 p.m. UTC | #2
Hi!
> > tst_acquire_device__() currently uses a hardcoded filename so only one loop
> > device can be used at a time. Allow setting arbitrary temp filename so that
> > multiple different loop devices can be acquired.
> 
> > Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Good idea, thanks!
> 
> BTW IMHO DEV_FILE should be #define DEV_FILE "test_dev.%d.img", where %d would
> be PID to fix clash when tests run in paralel (e.g. mkswap01.sh and df01.sh).
> I'll send a patch tomorrow, based on this one.

Huh, do we even attempt to support parallel runs at this point? I doubt
so.
Cyril Hrubis Feb. 7, 2020, 4:49 p.m. UTC | #3
Hi!
This part looks fine.
Petr Vorel Feb. 7, 2020, 5:07 p.m. UTC | #4
Hi Cyril,

> > BTW IMHO DEV_FILE should be #define DEV_FILE "test_dev.%d.img", where %d would
> > be PID to fix clash when tests run in paralel (e.g. mkswap01.sh and df01.sh).
> > I'll send a patch tomorrow, based on this one.

> Huh, do we even attempt to support parallel runs at this point? I doubt
> so.
No, but I'd expect it'd be nice to have this support for runltp-ng. Or not?

Kind regards,
Petr
Cyril Hrubis Feb. 7, 2020, 5:11 p.m. UTC | #5
Hi!
> > > BTW IMHO DEV_FILE should be #define DEV_FILE "test_dev.%d.img", where %d would
> > > be PID to fix clash when tests run in paralel (e.g. mkswap01.sh and df01.sh).
> > > I'll send a patch tomorrow, based on this one.
> 
> > Huh, do we even attempt to support parallel runs at this point? I doubt
> > so.
> No, but I'd expect it'd be nice to have this support for runltp-ng. Or not?

We will fix this, when we get there. Also thinking of it the file that
is backing the device is actually created in the test temporary
directory, which has unique name anyways.
Petr Vorel Feb. 10, 2020, 3:48 p.m. UTC | #6
Hi,

> > > > BTW IMHO DEV_FILE should be #define DEV_FILE "test_dev.%d.img", where %d would
> > > > be PID to fix clash when tests run in paralel (e.g. mkswap01.sh and df01.sh).
> > > > I'll send a patch tomorrow, based on this one.

> > > Huh, do we even attempt to support parallel runs at this point? I doubt
> > > so.
> > No, but I'd expect it'd be nice to have this support for runltp-ng. Or not?

> We will fix this, when we get there. Also thinking of it the file that
> is backing the device is actually created in the test temporary
> directory, which has unique name anyways.
If I remember it correctly, the file was in /tmp directory (the default)
(probably before creating temporary directory and cd into it).
I was really able to crash on tests, when running 2 in paralel.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/include/old/old_device.h b/include/old/old_device.h
index d2f1ecde5..a6e9fea86 100644
--- a/include/old/old_device.h
+++ b/include/old/old_device.h
@@ -52,13 +52,26 @@  static inline const char *tst_acquire_device(void (cleanup_fn)(void))
 	return tst_acquire_device_(cleanup_fn, 0);
 }
 
+/*
+ * Acquire a loop device with specified temp filename. This function allows
+ * you to acquire multiple devices at the same time. LTP_DEV is ignored.
+ * If you call this function directly, use tst_detach_device() to release
+ * the devices. tst_release_device() will not work correctly.
+ *
+ * The return value points to a static buffer and additional calls of
+ * tst_acquire_loop_device() or tst_acquire_device() will overwrite it.
+ */
+const char *tst_acquire_loop_device(unsigned int size, const char *filename);
+
 /*
  * @dev: device path returned by the tst_acquire_device()
  */
 int tst_release_device(const char *dev);
 
 /*
- * @dev: device path returned by the tst_acquire_device()
+ * Cleanup function for tst_acquire_loop_device(). If you have acquired
+ * a device using tst_acquire_device(), use tst_release_device() instead.
+ * @dev: device path returned by the tst_acquire_loop_device()
  */
 int tst_detach_device(const char *dev);
 
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 89b9c96de..ac6806e43 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -228,10 +228,28 @@  int tst_dev_sync(int fd)
 	return syscall(__NR_syncfs, fd);
 }
 
+const char *tst_acquire_loop_device(unsigned int size, const char *filename)
+{
+	unsigned int acq_dev_size = MAX(size, DEV_SIZE_MB);
+
+	if (tst_fill_file(filename, 0, 1024 * 1024, acq_dev_size)) {
+		tst_resm(TWARN | TERRNO, "Failed to create %s", filename);
+		return NULL;
+	}
+
+	if (tst_find_free_loopdev(dev_path, sizeof(dev_path)) == -1)
+		return NULL;
+
+	if (tst_attach_device(dev_path, filename))
+		return NULL;
+
+	return dev_path;
+}
+
 const char *tst_acquire_device__(unsigned int size)
 {
 	int fd;
-	char *dev;
+	const char *dev;
 	struct stat st;
 	unsigned int acq_dev_size;
 	uint64_t ltp_dev_size;
@@ -282,20 +300,12 @@  const char *tst_acquire_device__(unsigned int size)
 				ltp_dev_size, acq_dev_size);
 	}
 
-	if (tst_fill_file(DEV_FILE, 0, 1024 * 1024, acq_dev_size)) {
-		tst_resm(TWARN | TERRNO, "Failed to create " DEV_FILE);
-		return NULL;
-	}
-
-	if (tst_find_free_loopdev(dev_path, sizeof(dev_path)) == -1)
-		return NULL;
+	dev = tst_acquire_loop_device(acq_dev_size, DEV_FILE);
 
-	if (tst_attach_device(dev_path, DEV_FILE))
-		return NULL;
+	if (dev)
+		device_acquired = 1;
 
-	device_acquired = 1;
-
-	return dev_path;
+	return dev;
 }
 
 const char *tst_acquire_device_(void (cleanup_fn)(void), unsigned int size)
diff --git a/testcases/lib/tst_device.c b/testcases/lib/tst_device.c
index a657db30b..2a3ab1222 100644
--- a/testcases/lib/tst_device.c
+++ b/testcases/lib/tst_device.c
@@ -18,7 +18,7 @@  static struct tst_test test = {
 
 static void print_help(void)
 {
-	fprintf(stderr, "\nUsage: tst_device acquire [size]\n");
+	fprintf(stderr, "\nUsage: tst_device acquire [size [filename]]\n");
 	fprintf(stderr, "   or: tst_device release /path/to/device\n\n");
 }
 
@@ -27,10 +27,10 @@  static int acquire_device(int argc, char *argv[])
 	unsigned int size = 0;
 	const char *device;
 
-	if (argc > 3)
+	if (argc > 4)
 		return 1;
 
-	if (argc == 3) {
+	if (argc >= 3) {
 		size = atoi(argv[2]);
 
 		if (!size) {
@@ -40,7 +40,11 @@  static int acquire_device(int argc, char *argv[])
 		}
 	}
 
-	device = tst_acquire_device__(size);
+	if (argc >= 4) {
+		device = tst_acquire_loop_device(size, argv[3]);
+	} else {
+		device = tst_acquire_device__(size);
+	}
 
 	if (!device)
 		return 1;
@@ -61,7 +65,7 @@  static int release_device(int argc, char *argv[])
 		return 1;
 
 	/*
-	 * tst_acquire_device() was called in a different process.
+	 * tst_acquire_[loop_]device() was called in a different process.
 	 * tst_release_device() would think that no device was acquired yet
 	 * and do nothing. Call tst_detach_device() directly to bypass
 	 * the check.