[LTP] [PATCH V2 2/2] tbio: fixes

[LTP] [PATCH V2 2/2] tbio: fixes

[Stanislav Kholmanskikh]

* Wait until udev creates /dev/tbio. If udev can not create
  the node file in a defined timeout, create it manually.
* Clean /dev/tbio after test execution, if /dev/tbio was created
  manually.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---

Changes since V1:
 * implemented a more active waiting
 * fixed the wrong logical condition next to unlink() in cleanup()


 .../kernel/device-drivers/tbio/tbio_user/tbio.c    |   62 +++++++++-----------
 1 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c b/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c
index 81085a3..fdb55b5 100644
--- a/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c
+++ b/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c
@@ -75,12 +75,19 @@ void cleanup(void)
 	if (module_loaded)
 		tst_module_unload(NULL, module_name);
 
+	if (unlink(DEVICE_NAME) && (errno != ENOENT))
+		tst_brkm(TBROK | TERRNO, NULL, "unlink failed");
+
 	TEST_CLEANUP;
 }
 
 
 void setup(void)
 {
+	dev_t devt;
+	struct stat st;
+	unsigned int i, valid_node_created;
+
 	tst_require_root(NULL);
 
 	if (tst_kvercmp(2, 6, 0) < 0) {
@@ -91,50 +98,37 @@ void setup(void)
 	tst_module_load(cleanup, module_name, NULL);
 	module_loaded = 1;
 
-	dev_t devt;
-	struct stat st;
-
 	SAFE_FILE_SCANF(cleanup, "/sys/class/block/tbio/dev",
 		"%d:0", &TBIO_MAJOR);
 
 	devt = makedev(TBIO_MAJOR, 0);
+
 	/*
-	 * Check for the /dev/tbase node, and create if it does not
-	 * exist.
+	 * Wait until udev creates the device node.
+	 * If the node is not created or invalid, create it manually.
 	 */
-	errno = 0;
-	if (stat(DEVICE_NAME, &st)) {
-		if (errno == ENOENT) {
-			/* dev node does not exist */
-			if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR | S_IWUSR |
-				S_IRGRP | S_IWGRP, devt)) {
+	valid_node_created = 0;
+	for (i = 0; i < 5; i++) {
+		if (stat(DEVICE_NAME, &st)) {
+			if (errno != ENOENT)
 				tst_brkm(TBROK | TERRNO, cleanup,
-					"mknod failed at %s:%d",
-					__FILE__, __LINE__);
-			}
+					 "stat() failed");
 		} else {
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"problem with tbase device node directory");
-		}
-	} else {
-		/*
-		 * /dev/tbio CHR device exists.  Check to make sure it is for a
-		 * block device and that it has the right major and minor.
-		 */
-		if ((!(st.st_mode & S_IFCHR)) || (st.st_rdev != devt)) {
-			/* Recreate the dev node */
-			if (!unlink(DEVICE_NAME)) {
-				if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR |
-					S_IWUSR | S_IRGRP | S_IWGRP, devt)) {
-					tst_brkm(TBROK | TERRNO, cleanup,
-						"mknod failed at %s:%d",
-						__FILE__, __LINE__);
-				}
-			} else {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					"unlink failed");
+			if ((st.st_mode & S_IFBLK) && (st.st_rdev == devt)) {
+				valid_node_created = 1;
+				break;
 			}
 		}
+
+		sleep(1);
+	}
+
+	if (!valid_node_created) {
+		if (unlink(DEVICE_NAME) && (errno != ENOENT))
+			tst_brkm(TBROK | TERRNO, cleanup, "unlink() failed");
+		if (mknod(DEVICE_NAME, S_IFBLK | S_IRUSR | S_IWUSR |
+			  S_IRGRP | S_IWGRP, devt))
+			tst_brkm(TBROK | TERRNO, cleanup, "mknod() failed");
 	}
 
 	tbio_fd = open(DEVICE_NAME, O_RDWR);
-- 
1.7.1


------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V2 2/2] tbio: fixes

[Cyril Hrubis]

Hi!
> +			if ((st.st_mode & S_IFBLK) && (st.st_rdev == devt)) {
> +				valid_node_created = 1;
> +				break;
>  			}
>  		}
> +
> +		sleep(1);

I would be inclined to check at least ten times in second. I.e.
usleep(100000); here and do the for loop 50 times instead.

Each second we spend doing nothing is second not spend doing actual
testing.

> +	}
> +
> +	if (!valid_node_created) {

Maybe we should issue TINFO message here that device wasn't created by
udev and we have fallen back to manual creation.

> +		if (unlink(DEVICE_NAME) && (errno != ENOENT))
> +			tst_brkm(TBROK | TERRNO, cleanup, "unlink() failed");
> +		if (mknod(DEVICE_NAME, S_IFBLK | S_IRUSR | S_IWUSR |
> +			  S_IRGRP | S_IWGRP, devt))
> +			tst_brkm(TBROK | TERRNO, cleanup, "mknod() failed");
>  	}

Otherwise this looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH V3 2/2] tbio: fixes

[Stanislav Kholmanskikh]

* Wait until udev creates /dev/tbio. If udev can not create
  the node file in a defined timeout, create it manually.
* Clean /dev/tbio after test execution, if /dev/tbio was created
  manually.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
Changes since V2:

* Check for the file ten times a second.


 .../kernel/device-drivers/tbio/tbio_user/tbio.c    |   66 ++++++++++----------
 1 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c b/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c
index 81085a3..b27f709 100644
--- a/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c
+++ b/testcases/kernel/device-drivers/tbio/tbio_user/tbio.c
@@ -75,12 +75,19 @@ void cleanup(void)
 	if (module_loaded)
 		tst_module_unload(NULL, module_name);
 
+	if (unlink(DEVICE_NAME) && (errno != ENOENT))
+		tst_brkm(TBROK | TERRNO, NULL, "unlink failed");
+
 	TEST_CLEANUP;
 }
 
 
 void setup(void)
 {
+	dev_t devt;
+	struct stat st;
+	unsigned int i, valid_node_created;
+
 	tst_require_root(NULL);
 
 	if (tst_kvercmp(2, 6, 0) < 0) {
@@ -91,50 +98,41 @@ void setup(void)
 	tst_module_load(cleanup, module_name, NULL);
 	module_loaded = 1;
 
-	dev_t devt;
-	struct stat st;
-
 	SAFE_FILE_SCANF(cleanup, "/sys/class/block/tbio/dev",
 		"%d:0", &TBIO_MAJOR);
 
 	devt = makedev(TBIO_MAJOR, 0);
+
 	/*
-	 * Check for the /dev/tbase node, and create if it does not
-	 * exist.
+	 * Wait until udev creates the device node.
+	 * If the node is not created or invalid, create it manually.
 	 */
-	errno = 0;
-	if (stat(DEVICE_NAME, &st)) {
-		if (errno == ENOENT) {
-			/* dev node does not exist */
-			if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR | S_IWUSR |
-				S_IRGRP | S_IWGRP, devt)) {
+	valid_node_created = 0;
+	for (i = 0; i < 50; i++) {
+		if (stat(DEVICE_NAME, &st)) {
+			if (errno != ENOENT)
 				tst_brkm(TBROK | TERRNO, cleanup,
-					"mknod failed at %s:%d",
-					__FILE__, __LINE__);
-			}
+					 "stat() failed");
 		} else {
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"problem with tbase device node directory");
-		}
-	} else {
-		/*
-		 * /dev/tbio CHR device exists.  Check to make sure it is for a
-		 * block device and that it has the right major and minor.
-		 */
-		if ((!(st.st_mode & S_IFCHR)) || (st.st_rdev != devt)) {
-			/* Recreate the dev node */
-			if (!unlink(DEVICE_NAME)) {
-				if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR |
-					S_IWUSR | S_IRGRP | S_IWGRP, devt)) {
-					tst_brkm(TBROK | TERRNO, cleanup,
-						"mknod failed at %s:%d",
-						__FILE__, __LINE__);
-				}
-			} else {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					"unlink failed");
+			if ((st.st_mode & S_IFBLK) && (st.st_rdev == devt)) {
+				valid_node_created = 1;
+				break;
 			}
 		}
+
+		usleep(100000);
+	}
+
+	if (!valid_node_created) {
+		tst_resm(TINFO,
+			 "The device file was not created by udev, "
+			 "proceeding with manual creation");
+
+		if (unlink(DEVICE_NAME) && (errno != ENOENT))
+			tst_brkm(TBROK | TERRNO, cleanup, "unlink() failed");
+		if (mknod(DEVICE_NAME, S_IFBLK | S_IRUSR | S_IWUSR |
+			  S_IRGRP | S_IWGRP, devt))
+			tst_brkm(TBROK | TERRNO, cleanup, "mknod() failed");
 	}
 
 	tbio_fd = open(DEVICE_NAME, O_RDWR);
-- 
1.7.1


------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH V3 2/2] tbio: fixes

[Cyril Hrubis]

Hi!
Acked.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list