diff mbox series

[v2] Add tst_dev_block_size utility

Message ID 20220114145227.12707-1-andrea.cervesato@suse.de
State Superseded
Headers show
Series [v2] Add tst_dev_block_size utility | expand

Commit Message

Andrea Cervesato Jan. 14, 2022, 2:52 p.m. UTC
Added tst_dev_block_size function in tst_device.c and updated
c-test-api.txt documentation.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 doc/c-test-api.txt            | 11 ++++
 include/tst_device.h          |  7 +++
 lib/newlib_tests/tst_device.c | 96 +++++++++++++++++++++++++++--------
 lib/tst_device.c              | 15 ++++++
 4 files changed, 109 insertions(+), 20 deletions(-)

Comments

Cyril Hrubis Feb. 1, 2022, 11:52 a.m. UTC | #1
Hi!
> +[source,c]
> +-------------------------------------------------------------------------------
> +#include "tst_test.h"
> +
> +int tst_dev_block_size(const char *path);
> +-------------------------------------------------------------------------------
> +
> +This function returns the size of a single IO block for the specific `path`.
                                        ^
					physical device block size for
					the specific `path`
> +It finds the device where `path` is located and then uses `ioctl` to get a
                                                                   ^
								   Maybe
								   we
								   should
								   add
								   here
								   the
								   ioctl
								   constant
								   i.e.
								   BLKSSZGET
> +single device block size.
   ^
   physical
> +
>  1.16 Formatting a device with a filesystem
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 72c560c02..bf0fb5320 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -112,4 +112,11 @@ void tst_purge_dir(const char *path);
>   */
>  void tst_find_backing_dev(const char *path, char *dev);
>  
> +/*
> + * Returns the size of a single IO block for the specific path
                               ^
			       physical device block size for ...
> + * @path   Path to find the block size
> + * @return Size of the block size
> + */
> +int tst_dev_block_size(const char *path);
> +
>  #endif	/* TST_DEVICE_H__ */
> diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
> index 0bee0a939..3f9a43576 100644
> --- a/lib/newlib_tests/tst_device.c
> +++ b/lib/newlib_tests/tst_device.c
> @@ -1,47 +1,103 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2016 Linux Test Project
> + * Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>   */
> +#define _GNU_SOURCE
>  
>  #include <stdlib.h>
>  #include <sys/mount.h>
>  #include <stdint.h>
> +#include <stdio.h>
> +#include <lapi/loop.h>
> +#include <time.h>
>  
>  #include "tst_test.h"
>  
> -static void do_test(void)
> +#define PAGESIZE 2048
            ^
	    Huh?

The usuall PAGESIZE is 4k, then there are architecture with (optional)
64k pages, but I doubt that I have seen anything with 2k PAGESIZE.

This should have been named DEVBLOCKSIZE instead, naming it PAGESIZE
makes it utterly confusing.

> +#define DEV_MIN_SIZE 300
> +
> +static char *mntpoint;
> +static uint64_t ltp_dev_size;
> +
> +static void setup(void)
>  {
>  	int fd;
> -	const char *dev;
> -	char block_dev[100];
> -	uint64_t ltp_dev_size;
> +	int ret;
>  
> -	dev = tst_device->dev;
> -	if (!dev)
> -		tst_brk(TCONF, "Failed to acquire test device");
> +	ret = asprintf(&mntpoint, "%s/mnt", tst_get_tmpdir());
> +	if (ret < 0)
> +		tst_brk(TBROK, "asprintf failure");
>  
> -	SAFE_MKFS(dev, "ext2", NULL, NULL);
> +	while ((fd = SAFE_OPEN(tst_device->dev, O_RDONLY, 0666)) < 0)
> +		usleep(100);

Why the loop here?

It does not make any sense especially since you call SAFE_OPEN() that
cannot fail.

> -	fd = SAFE_OPEN(dev, O_RDONLY);
>  	SAFE_IOCTL(fd, BLKGETSIZE64, &ltp_dev_size);
> +	SAFE_IOCTL(fd, LOOP_SET_BLOCK_SIZE, PAGESIZE);
>  	SAFE_CLOSE(fd);
>  
> -	if (ltp_dev_size/1024/1024 == 300)
> -		tst_res(TPASS, "Got expected device size");
> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, NULL, NULL);
> +
> +	SAFE_MKDIR(mntpoint, 0777);
> +	SAFE_MOUNT(tst_device->dev, mntpoint, tst_device->fs_type, 0, 0);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (tst_is_mounted(mntpoint))
> +		SAFE_UMOUNT(mntpoint);
> +}
> +
> +static void test_dev_min_size(void)
> +{
> +	uint64_t size;
> +
> +	size = ltp_dev_size / 1024 / 1024;
> +
> +	if (size == DEV_MIN_SIZE)
> +		tst_res(TPASS, "Got expected device size %lu", size);
> +	else
> +		tst_res(TFAIL, "Expected device size is %d but got %lu", DEV_MIN_SIZE, size);
> +}
> +
> +static void test_tst_find_backing_dev(void)
> +{
> +	char block_dev[100];
> +
> +	tst_find_backing_dev(mntpoint, block_dev);
> +
> +	if (!strcmp(tst_device->dev, block_dev))
> +		tst_res(TPASS, "%s belongs to %s block dev", mntpoint, block_dev);
>  	else
> -		tst_res(TFAIL, "Got unexpected device size");
> -
> -	tst_find_backing_dev("/boot", block_dev);
> -	tst_res(TPASS, "/boot belongs to %s block dev", block_dev);
> -	tst_find_backing_dev("/", block_dev);
> -	tst_res(TPASS, "/ belongs to %s block dev", block_dev);
> -	tst_find_backing_dev("/tmp", block_dev);
> -	tst_find_backing_dev("/boot/xuyang", block_dev);
> +		tst_res(TFAIL, "%s should belong to %s, but %s is returned", mntpoint, tst_device->dev, block_dev);
> +}
> +
> +static void test_tst_dev_block_size(void)
> +{
> +	int block_size;
> +
> +	block_size = tst_dev_block_size(mntpoint);
> +
> +	if (block_size == PAGESIZE)
> +		tst_res(TPASS, "%s has %d block size", mntpoint, block_size);
> +	else
> +		tst_res(TFAIL, "%s has %d block size, but expected is %i", mntpoint, block_size, PAGESIZE);
> +}
> +
> +static void do_test(void)
> +{
> +	test_dev_min_size();
> +	test_tst_find_backing_dev();
> +	test_tst_dev_block_size();
>  }
>  
>  static struct tst_test test = {
>  	.needs_root = 1,
>  	.needs_device = 1,
> -	.dev_min_size = 300,
> +	.needs_tmpdir = 1,
> +	.dev_min_size = DEV_MIN_SIZE,
>  	.test_all = do_test,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.min_kver = "4.14",
>  };
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 73e70d26e..1ef667fa0 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -547,3 +547,18 @@ void tst_find_backing_dev(const char *path, char *dev)
>  	if (S_ISBLK(buf.st_mode) != 1)
>  		tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
>  }
> +
> +int tst_dev_block_size(const char *path)
> +{
> +	int fd;
> +	int size;
> +	char dev_name[1024];
> +
> +	tst_find_backing_dev(path, dev_name);
> +
> +	fd = SAFE_OPEN(NULL, dev_name, O_RDONLY);
> +	SAFE_IOCTL(NULL, fd, BLKSSZGET, &size);
> +	SAFE_CLOSE(NULL, fd);
> +
> +	return size;
> +}

I guess that it would make sense to add tst_dev_block_size_fd(int fd)
that would do the same but for a file descriptor later on, but apart
from the minor things the patch looks good.
diff mbox series

Patch

diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 123d3f1fc..0cb391a0a 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -1034,6 +1034,17 @@  uint64_t tst_get_device_size(const char *dev_path);
 This function gets size of the given block device, it checks the 'dev_path' is
 valid first, if yes, return the size in MB, otherwise return -1.
 
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+int tst_dev_block_size(const char *path);
+-------------------------------------------------------------------------------
+
+This function returns the size of a single IO block for the specific `path`.
+It finds the device where `path` is located and then uses `ioctl` to get a
+single device block size.
+
 1.16 Formatting a device with a filesystem
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/include/tst_device.h b/include/tst_device.h
index 72c560c02..bf0fb5320 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -112,4 +112,11 @@  void tst_purge_dir(const char *path);
  */
 void tst_find_backing_dev(const char *path, char *dev);
 
+/*
+ * Returns the size of a single IO block for the specific path
+ * @path   Path to find the block size
+ * @return Size of the block size
+ */
+int tst_dev_block_size(const char *path);
+
 #endif	/* TST_DEVICE_H__ */
diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
index 0bee0a939..3f9a43576 100644
--- a/lib/newlib_tests/tst_device.c
+++ b/lib/newlib_tests/tst_device.c
@@ -1,47 +1,103 @@ 
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2016 Linux Test Project
+ * Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
+#define _GNU_SOURCE
 
 #include <stdlib.h>
 #include <sys/mount.h>
 #include <stdint.h>
+#include <stdio.h>
+#include <lapi/loop.h>
+#include <time.h>
 
 #include "tst_test.h"
 
-static void do_test(void)
+#define PAGESIZE 2048
+#define DEV_MIN_SIZE 300
+
+static char *mntpoint;
+static uint64_t ltp_dev_size;
+
+static void setup(void)
 {
 	int fd;
-	const char *dev;
-	char block_dev[100];
-	uint64_t ltp_dev_size;
+	int ret;
 
-	dev = tst_device->dev;
-	if (!dev)
-		tst_brk(TCONF, "Failed to acquire test device");
+	ret = asprintf(&mntpoint, "%s/mnt", tst_get_tmpdir());
+	if (ret < 0)
+		tst_brk(TBROK, "asprintf failure");
 
-	SAFE_MKFS(dev, "ext2", NULL, NULL);
+	while ((fd = SAFE_OPEN(tst_device->dev, O_RDONLY, 0666)) < 0)
+		usleep(100);
 
-	fd = SAFE_OPEN(dev, O_RDONLY);
 	SAFE_IOCTL(fd, BLKGETSIZE64, &ltp_dev_size);
+	SAFE_IOCTL(fd, LOOP_SET_BLOCK_SIZE, PAGESIZE);
 	SAFE_CLOSE(fd);
 
-	if (ltp_dev_size/1024/1024 == 300)
-		tst_res(TPASS, "Got expected device size");
+	SAFE_MKFS(tst_device->dev, tst_device->fs_type, NULL, NULL);
+
+	SAFE_MKDIR(mntpoint, 0777);
+	SAFE_MOUNT(tst_device->dev, mntpoint, tst_device->fs_type, 0, 0);
+}
+
+static void cleanup(void)
+{
+	if (tst_is_mounted(mntpoint))
+		SAFE_UMOUNT(mntpoint);
+}
+
+static void test_dev_min_size(void)
+{
+	uint64_t size;
+
+	size = ltp_dev_size / 1024 / 1024;
+
+	if (size == DEV_MIN_SIZE)
+		tst_res(TPASS, "Got expected device size %lu", size);
+	else
+		tst_res(TFAIL, "Expected device size is %d but got %lu", DEV_MIN_SIZE, size);
+}
+
+static void test_tst_find_backing_dev(void)
+{
+	char block_dev[100];
+
+	tst_find_backing_dev(mntpoint, block_dev);
+
+	if (!strcmp(tst_device->dev, block_dev))
+		tst_res(TPASS, "%s belongs to %s block dev", mntpoint, block_dev);
 	else
-		tst_res(TFAIL, "Got unexpected device size");
-
-	tst_find_backing_dev("/boot", block_dev);
-	tst_res(TPASS, "/boot belongs to %s block dev", block_dev);
-	tst_find_backing_dev("/", block_dev);
-	tst_res(TPASS, "/ belongs to %s block dev", block_dev);
-	tst_find_backing_dev("/tmp", block_dev);
-	tst_find_backing_dev("/boot/xuyang", block_dev);
+		tst_res(TFAIL, "%s should belong to %s, but %s is returned", mntpoint, tst_device->dev, block_dev);
+}
+
+static void test_tst_dev_block_size(void)
+{
+	int block_size;
+
+	block_size = tst_dev_block_size(mntpoint);
+
+	if (block_size == PAGESIZE)
+		tst_res(TPASS, "%s has %d block size", mntpoint, block_size);
+	else
+		tst_res(TFAIL, "%s has %d block size, but expected is %i", mntpoint, block_size, PAGESIZE);
+}
+
+static void do_test(void)
+{
+	test_dev_min_size();
+	test_tst_find_backing_dev();
+	test_tst_dev_block_size();
 }
 
 static struct tst_test test = {
 	.needs_root = 1,
 	.needs_device = 1,
-	.dev_min_size = 300,
+	.needs_tmpdir = 1,
+	.dev_min_size = DEV_MIN_SIZE,
 	.test_all = do_test,
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_kver = "4.14",
 };
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 73e70d26e..1ef667fa0 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -547,3 +547,18 @@  void tst_find_backing_dev(const char *path, char *dev)
 	if (S_ISBLK(buf.st_mode) != 1)
 		tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
 }
+
+int tst_dev_block_size(const char *path)
+{
+	int fd;
+	int size;
+	char dev_name[1024];
+
+	tst_find_backing_dev(path, dev_name);
+
+	fd = SAFE_OPEN(NULL, dev_name, O_RDONLY);
+	SAFE_IOCTL(NULL, fd, BLKSSZGET, &size);
+	SAFE_CLOSE(NULL, fd);
+
+	return size;
+}