Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout
On Sat, May 2, 2015 at 12:21 PM, Nir Soffer nir...@gmail.com wrote: On Tue, Apr 21, 2015 at 12:41 AM, Tom Gundersen t...@jklm.no wrote: On Mon, Apr 13, 2015 at 3:04 PM, Nir Soffer nir...@gmail.com wrote: On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann dh.herrm...@gmail.com 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. I don't really have a strong argument against this, except that we should not add ABI without a strong usecase. So you mentioned that you want this for diagnostics, to which my suggestion was to improve the logging in udevadm itself. But are there other usecases? What is the case where the external tool actually needs to change its behavior? Cheers, Tom ___ 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 Tue, Apr 21, 2015 at 12:41 AM, Tom Gundersen t...@jklm.no wrote: On Mon, Apr 13, 2015 at 3:04 PM, Nir Soffer nir...@gmail.com wrote: On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann dh.herrm...@gmail.com 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
Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout
On Mon, Apr 13, 2015 at 3:04 PM, Nir Soffer nir...@gmail.com wrote: On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann dh.herrm...@gmail.com 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? Cheers, Tom ___ 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 dh.herrm...@gmail.com 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
Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout
(Please keep the ML in CC) On Sat, Apr 11, 2015 at 5:38 PM, Nir Soffer nir...@gmail.com wrote: On Sat, Apr 11, 2015 at 1:50 PM, David Herrmann dh.herrm...@gmail.com wrote: Hi On Wed, Apr 8, 2015 at 9:40 PM, Nir Soffer nir...@gmail.com wrote: 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. What's the use-case for this? 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. That's an explanation of your patch. For example, see https://gerrit.ovirt.org/39740 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? Anyway, if it's only about diagnostics this patch seems fine to me. Thanks David ___ 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
Hi
On Wed, Apr 8, 2015 at 9:40 PM, Nir Soffer nir...@gmail.com wrote:
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.
What's the use-case for this?
Thanks
David
---
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 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
