[systemd-devel] [PATCH] log: add log_errno() helper

2013-11-15 Thread David Herrmann
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

2013-11-15 Thread Zbigniew Jędrzejewski-Szmek
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

2013-11-15 Thread David Herrmann
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

2013-11-15 Thread Lennart Poettering
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