[systemd-devel] [PATCH] log: add log_errno() helper
Syscalls may fail for a lot of reasons, but most times these errors are unexpected (we cannot recover). Especially when dealing with device nodes that can be revoked asynchronously, a series of syscalls may start failing at any point. Normally, we can silently ignore errors and just bail out, but for debugging purposes log messages are quite helpful. The log_errno() helper can be used in such situations where we don't expect a syscall error, but also don't want to add a custom log-message to reduce memory-consumption. The helper just prints the file+line+func information and the errno-content. Usage: r = ioctl(fd, ..); if (r 0) return log_errno(); It is basically the same as log_oom() but generic for all kernel errno-messages. If we added a custom log-message for each syscall failure, our .text section would increase heavily. --- src/shared/log.c | 5 + src/shared/log.h | 6 ++ 2 files changed, 11 insertions(+) diff --git a/src/shared/log.c b/src/shared/log.c index 8f4995a..5edb05f 100644 --- a/src/shared/log.c +++ b/src/shared/log.c @@ -713,6 +713,11 @@ int log_oom_internal(const char *file, int line, const char *func) { return -ENOMEM; } +int log_errno_internal(const char *file, int line, const char *func) { +log_meta(LOG_ERR, file, line, func, Syscall failed unexpectedly: %m); +return -errno; +} + int log_struct_internal( int level, const char *file, diff --git a/src/shared/log.h b/src/shared/log.h index 0dc5c26..6f05f7c 100644 --- a/src/shared/log.h +++ b/src/shared/log.h @@ -116,6 +116,11 @@ int log_oom_internal( int line, const char *func); +int log_errno_internal( +const char *file, +int line, +const char *func); + /* This modifies the buffer passed! */ int log_dump_internal( int level, @@ -151,6 +156,7 @@ do { \ #define log_struct(level, ...) log_struct_internal(level, __FILE__, __LINE__, __func__, __VA_ARGS__) #define log_oom() log_oom_internal(__FILE__, __LINE__, __func__) +#define log_errno() log_errno_internal(__FILE__, __LINE__, __func__) /* This modifies the buffer passed! */ #define log_dump(level, buffer) log_dump_internal(level, __FILE__, __LINE__, __func__, buffer) -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] log: add log_errno() helper
On Fri, Nov 15, 2013 at 11:22:59AM +0100, David Herrmann wrote: Syscalls may fail for a lot of reasons, but most times these errors are unexpected (we cannot recover). Especially when dealing with device nodes that can be revoked asynchronously, a series of syscalls may start failing at any point. Normally, we can silently ignore errors and just bail out, but for debugging purposes log messages are quite helpful. I'm not sold of the idea of completely generic message. If I'm sitting in front of a failed boot, seeing only Syscall failed unexpectedly: no such file or directory, I'm going to be pretty frustrated. Maybe there should be a custom message argument like ioctl AUTOFS_DEV_IOCTL_VERSION. This isn't going to increase the footprint too much, but will help undertand bug reprots. Or if you thyink it's too annoying to write, than maybe just print the file name and line automatically. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] log: add log_errno() helper
Hi On Fri, Nov 15, 2013 at 2:33 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Fri, Nov 15, 2013 at 11:22:59AM +0100, David Herrmann wrote: Syscalls may fail for a lot of reasons, but most times these errors are unexpected (we cannot recover). Especially when dealing with device nodes that can be revoked asynchronously, a series of syscalls may start failing at any point. Normally, we can silently ignore errors and just bail out, but for debugging purposes log messages are quite helpful. I'm not sold of the idea of completely generic message. If I'm sitting in front of a failed boot, seeing only Syscall failed unexpectedly: no such file or directory, I'm going to be pretty frustrated. Maybe there should be a custom message argument like ioctl AUTOFS_DEV_IOCTL_VERSION. This isn't going to increase the footprint too much, but will help undertand bug reprots. Or if you thyink it's too annoying to write, than maybe just print the file name and line automatically. file-name and line is actually what I wanted (does log_meta() suppress these?). Custom error-messages don't really make much sense. For example: Assume you have a function to open an input device. The function itself has to run a bunch of syscalls: - open() (open device node) - ioctl() (get device bits, like 10 EVIOC* ioctls) - write() (to initialize LED values) These are all mostly straightforward and shouldn't fail if the device is valid. However, they *can* fail for various reasons (daemon not running with enough privileges, device being unplugged during operation, ...) but it's almost impossible to detect these properly. So we want debug/error messages. If we now add a custom log_error() message for each of these calls, they will be something like EVIOCGABS failed: %m. To a user this is as helpful as syscall failed in $file:$line:$func. So I thought, we can avoid cluttering .text with useless error-messages and just print $file:$line:$func and %m. Does that make sense? Note that this obviously is only for developer error-messages. For functions like open() it makes sense to handle it separately. A message like cannot open /dev/$sth: Access denied even helps users, not only developers. The log_errno() is really intended for stuff like custom ioctls/etc. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] log: add log_errno() helper
On Fri, 15.11.13 15:26, David Herrmann (dh.herrm...@gmail.com) wrote: file-name and line is actually what I wanted (does log_meta() suppress these?). Custom error-messages don't really make much sense. For example: Assume you have a function to open an input device. The function itself has to run a bunch of syscalls: - open() (open device node) - ioctl() (get device bits, like 10 EVIOC* ioctls) - write() (to initialize LED values) These are all mostly straightforward and shouldn't fail if the device is valid. However, they *can* fail for various reasons (daemon not running with enough privileges, device being unplugged during operation, ...) but it's almost impossible to detect these properly. So we want debug/error messages. If we now add a custom log_error() message for each of these calls, they will be something like EVIOCGABS failed: %m. To a user this is as helpful as syscall failed in $file:$line:$func. So I thought, we can avoid cluttering .text with useless error-messages and just print $file:$line:$func and %m. They might not be helpful to the user, but they are for the developer who the user complains to. Much unlike the file/line which is highly dependent on the version you have built. I don't think we really need to optimize the log messages on error away. Doing log_error(EVIOCGABS: %m) doesn't look like too much. Does that make sense? Note that this obviously is only for developer error-messages. For functions like open() it makes sense to handle it separately. A message like cannot open /dev/$sth: Access denied even helps users, not only developers. The log_errno() is really intended for stuff like custom ioctls/etc. I'd side with Zbsyzek here... It's difficult to draw a line between user and developer messages, and people frequently are somewhat in the middle of it too (i think those folks are called devops these days ;-). I am pretty sure we should keep proper log messages for these cases too. You can keep them terse of course, they'd don't need to have the highest quality of proper sentences, but they should give an educated man an idea what failed without him having to figure out the version of the binary and then look into the sources... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel