Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout
On Tue, Apr 21, 2015 at 12:41 AM, Tom Gundersen wrote: > On Mon, Apr 13, 2015 at 3:04 PM, Nir Soffer wrote: >> On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann >> wrote: >>>> A program running this tool can detect a timeout (expected) or an error >>>> (unexpected), and can change the program flow based on this result. >>>> >>>> Without this, the only way to detect a timeout is to implement the timeout >>>> in the program calling udevadm. >>> >>> I cannot really see a use-case here. I mean, yeah, the commit-message >>> says it warns about timeouts but fails loudly on real errors. But >>> again, what's the use-case? Why is a timeout not a real error? Why do >>> you need to handle it differently? >> >> Timeout means that the value I chose may be too small, or the machine >> is overloaded. The administrator may need to configure the system >> differently. >> >> Other errors are not expected, and typically unexpected errors in an >> underlying tool means getting the developer of the underlying tool >> involved. >> >>> Anyway, if it's only about diagnostics this patch seems fine to me. >> >> Yes, it is mainly about diagnostics, and making it easier to debug and >> support. > > Wouldn't a better solution be to improve the udevadm logging? If we > change the exit codes this is basically ABI. Do we really want to make > such promises only for diagnostics? Improving logging is orthogonal, as it does allow the program to change the flow based the exit code. Adding a timeout exit code may break code using undocumented behavior, since the return code is not documented. Nir ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v3 1/2] udev: settle should return immediately when timeout is 0
udevadm manual says:
A value of 0 will check if the queue is empty and always return
immediately.
However, currently we ignore the deadline if the value is 0, and wait
without any limit.
Zero timeout behaved according to the documentation until commit
ead7c62ab7 (udevadm: settle - kill alarm()). Looking at this patch, it
seems that the behavior change was unintended.
This patch restores the documented behavior.
---
src/udev/udevadm-settle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 2c84ada..437c794 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -142,7 +142,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
-if (timeout > 0 && now(CLOCK_MONOTONIC) >= deadline)
+if (now(CLOCK_MONOTONIC) >= deadline)
break;
/* wake up when queue is empty */
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v3 2/2] udev: Skip ping if timeout is 0
When running udevadm settle --timeout=0, udev_ctrl_send_ping always
times out, and settle returns 0 without checking the queue.
Now we skip ping in this case, and return the queue state.
---
src/udev/udevadm-settle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 437c794..614768f 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -102,7 +102,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
deadline = now(CLOCK_MONOTONIC) + timeout * USEC_PER_SEC;
/* guarantee that the udev daemon isn't pre-processing */
-if (getuid() == 0) {
+if (timeout > 0 && getuid() == 0) {
struct udev_ctrl *uctrl;
uctrl = udev_ctrl_new(udev);
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] udev: Fix ping timeout when settle timeout is 0
When running udevadm settle --timeout=0, the ping always times out, and
udevadm will return 0 without checking the queue state.
Since zero timeout is considered as unlimited timeout, we use now
unlimited ping timeout.
---
src/udev/udevadm-settle.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 2c84ada..424 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -107,7 +107,9 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
uctrl = udev_ctrl_new(udev);
if (uctrl != NULL) {
-if (udev_ctrl_send_ping(uctrl, timeout) < 0) {
+int ping_timeout = timeout > 0 ? (int)timeout : -1;
+
+if (udev_ctrl_send_ping(uctrl, ping_timeout) < 0) {
log_debug("no connection to daemon");
udev_ctrl_unref(uctrl);
return EXIT_SUCCESS;
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2] udev: Allow detection of udevadm settle timeout
When udevadm settle times out, it exits with exit code 1. This make it
impossible for users to detect a timeout and handle real errors. Now we
use exit code 3 on timeouts.
---
src/udev/udevadm-settle.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 2c84ada..114951c 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -29,6 +29,8 @@
#include "udev.h"
#include "util.h"
+#define EXIT_TIMEOUT 3
+
static void help(void) {
printf("%s settle OPTIONS\n\n"
"Wait for pending udev events.\n\n"
@@ -142,8 +144,10 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
-if (timeout > 0 && now(CLOCK_MONOTONIC) >= deadline)
+if (timeout > 0 && now(CLOCK_MONOTONIC) >= deadline) {
+rc = EXIT_TIMEOUT;
break;
+}
/* wake up when queue is empty */
if (poll(pfd, 1, MSEC_PER_SEC) > 0 && pfd[0].revents & POLLIN)
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] udev interface naming for SR-IOV VFs
On Tue, Apr 14, 2015 at 1:11 PM, Ido Barkan wrote: > We are implementing support for SR-IOV network cards. Afer the changing of > the number of VFs on the card and programmatically querying for all links > (we use libnl for this) we observe that *during the iteration* over the links > some of them were renamed by udev and still were not. Meanning, some of them > are called 'ethN' and some are called 'enp2sNf2' (where N is an arbitrary > number). Also, there are times that not all of the devices are returned from > libnl. > After a _while_ everything stabilizes (# of interfaces and names). > > My questions: > 1. Is this what you would expect from udev? Meaning, this is just >async behavior? Generally yes. You can investigate that by monitoring udev events. One way is to use udevadm monitor: 1. Start the monitor: udevadm monitor --udev --property 2. Configure the SR-IOV card 3. Watch the events - you can use the timestamp to tell how much time it took until the new interfaces are ready. Another way is to turn on debug level, and watch the system log. 1. Set log level to debug: udevadm control --log-priority=debug 2. Start to watch the log in another shell journalctl -f 3. Setup the SR-IOV card > 2. Is there a way to _know_ programmticaly that everything was probed >and renamed? Not a heuristic? Using udevadm settle, you can wait until the current events are handled, but there are some issues: - You don't know when events from the kernel are starting - while you wait for the SR-IOV modification (e.g, write to sysfs attribute), or after a while, and if all of them are triggered one after another, or after some delay. So udevadm settle may return before the devices are ready. - It may be possible that one event will synthesize another event, so udev settle may return just before another event is triggered. - You may wait for unrelated events that happen to trigger in the same time, waiting after the new interfaces are ready. I think you need something like: while True: try: udevadm.settle(1) except udevadm.Timeout: pass else: if all devices are ready: break time.sleep(1) Note that it is not possible to detect a timeout in udevadm settle, see http://lists.freedesktop.org/archives/systemd-devel/2015-April/030391.html Nir ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout
On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann wrote: >> A program running this tool can detect a timeout (expected) or an error >> (unexpected), and can change the program flow based on this result. >> >> Without this, the only way to detect a timeout is to implement the timeout >> in the program calling udevadm. > > I cannot really see a use-case here. I mean, yeah, the commit-message > says it warns about timeouts but fails loudly on real errors. But > again, what's the use-case? Why is a timeout not a real error? Why do > you need to handle it differently? Timeout means that the value I chose may be too small, or the machine is overloaded. The administrator may need to configure the system differently. Other errors are not expected, and typically unexpected errors in an underlying tool means getting the developer of the underlying tool involved. > Anyway, if it's only about diagnostics this patch seems fine to me. Yes, it is mainly about diagnostics, and making it easier to debug and support. Nir ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2] udev: settle should return immediately when timeout is 0
udevadm manual says:
A value of 0 will check if the queue is empty and always return
immediately.
However, currently we ignore the deadline if the value is 0, and wait
without any limit.
Zero timeout behaved according to the documentation until commit
ead7c62ab7 (udevadm: settle - kill alarm()). Looking at this patch, it
seems that the behavior change was unintended.
This patch restores the documented behavior.
---
src/udev/udevadm-settle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 2c84ada..437c794 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -142,7 +142,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
-if (timeout > 0 && now(CLOCK_MONOTONIC) >= deadline)
+if (now(CLOCK_MONOTONIC) >= deadline)
break;
/* wake up when queue is empty */
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2] udev: settle should return immediately when timeout is 0
udevadm manual says:
A value of 0 will check if the queue is empty and always return
immediately.
However, currently we ignore the deadline if the value is 0, and wait
without any limit.
Zero timeout behaved according to the documentation until commit
ead7c62ab7 (udevadm: settle - kill alarm()). Looking at this patch, it
seems that the behavior change was unintended.
This patch restores the documented behavior.
---
src/udev/udevadm-settle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 2c84ada..437c794 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -142,7 +142,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
-if (timeout > 0 && now(CLOCK_MONOTONIC) >= deadline)
+if (now(CLOCK_MONOTONIC) >= deadline)
break;
/* wake up when queue is empty */
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] udev: settle should return immediately when timeout is 0
udevadm manual says:
A value of 0 will check if the queue is empty and always return
immediately.
However, currently we ignore the deadline if the value is 0, and wait
without any limit.
Zero timeout behaved according to the documentation until commit
ead7c62ab7 (udevadm: settle - kill alarm()). Looking at this patch, it
seems that the behavior change was unintended.
This patch restores the documented behavior.
---
src/udev/udevadm-settle.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 2c84ada..0c9c1d9 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -49,7 +49,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
{ "quiet", no_argument, NULL, 'q' }, /* removed
*/
{}
};
-usec_t deadline;
+usec_t deadline = 0;
const char *exists = NULL;
unsigned int timeout = 120;
struct pollfd pfd[1] = { {.fd = -1}, };
@@ -99,7 +99,8 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
return EXIT_FAILURE;
}
-deadline = now(CLOCK_MONOTONIC) + timeout * USEC_PER_SEC;
+if (timeout != 0)
+deadline = now(CLOCK_MONOTONIC) + timeout * USEC_PER_SEC;
/* guarantee that the udev daemon isn't pre-processing */
if (getuid() == 0) {
@@ -142,7 +143,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
-if (timeout > 0 && now(CLOCK_MONOTONIC) >= deadline)
+if (timeout == 0 || now(CLOCK_MONOTONIC) >= deadline)
break;
/* wake up when queue is empty */
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout
On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann wrote:
> > @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, char
> > *argv[]) {
> > break;
> > }
> >
> > +if (now(CLOCK_MONOTONIC) >= deadline)
> > +break;
> > +
>
> Previous udevadm allowed timeout=0 to disable this. I added the condition.
Hi David,
I think the handling of timeout=0 is incorrect now. The manual says:
A value of 0 will check if the queue is empty and always return
immediately.
In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
is_timeout was set and settle was returning with rc=1.
This behavior changed in:
http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60
After this commit, zero timeout results in unlimited wait. Since this
patch did not
change the manual or the online help, and the commit message says:
"udevadm: settle - kill alarm()", I guess this was unintended change.
I don't see the use case for disabling the timeout, so it seems that
we should fix
this, restoring the behavior before this commit.
What do you think?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout
When udevadm settle times out, it exits with exit code 1. This make it
impossible for users to detect a timeout and handle real errors. Now we
use exit code 3 on timeouts.
---
src/udev/udevadm-settle.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 715d2e7..8f9ae81 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -29,6 +29,8 @@
#include "udev.h"
#include "util.h"
+#define EXIT_TIMEOUT 3
+
static void help(void) {
printf("%s settle OPTIONS\n\n"
"Wait for pending udev events.\n\n"
@@ -142,8 +144,10 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
-if (now(CLOCK_MONOTONIC) >= deadline)
+if (now(CLOCK_MONOTONIC) >= deadline) {
+rc = EXIT_TIMEOUT;
break;
+}
/* wake up when queue is empty */
if (poll(pfd, 1, MSEC_PER_SEC) > 0 && pfd[0].revents & POLLIN)
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] udev: Restore udevadm settle timeout
Commit 9ea28c55a2 (udev: remove seqnum API and all assumptions about
seqnums) introduced a regresion, ignoring the timeout option when
waiting until the event queue is empty.
Previously, if the udev event queue was not empty when the timeout was
expired, udevadm settle was returning with exit code 1. To check if the
queue is empty, you could invoke udevadm settle with timeout=0. This
patch restores the previous behavior.
---
src/udev/udevadm-settle.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 0d3025e..715d2e7 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -49,6 +49,7 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
{ "quiet", no_argument, NULL, 'q' }, /* removed
*/
{}
};
+usec_t deadline = 0;
const char *exists = NULL;
unsigned int timeout = 120;
struct pollfd pfd[1] = { {.fd = -1}, };
@@ -98,6 +99,8 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
return EXIT_FAILURE;
}
+deadline = now(CLOCK_MONOTONIC) + timeout * USEC_PER_SEC;
+
/* guarantee that the udev daemon isn't pre-processing */
if (getuid() == 0) {
struct udev_ctrl *uctrl;
@@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[]) {
break;
}
+if (now(CLOCK_MONOTONIC) >= deadline)
+break;
+
/* wake up when queue is empty */
if (poll(pfd, 1, MSEC_PER_SEC) > 0 && pfd[0].revents & POLLIN)
udev_queue_flush(queue);
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
