On 2 Dec 2009 at 10:55, Erez Zilber wrote:
> I'd like to make some changes in the logging in open-iscsi. The
> current status is as follows:
>
> kernel modules:
>
> * We use iscsi_cls_session_printk & iscsi_cls_conn_printk in
> scsi_transport_iscsi.c. They are sometimes wrapped by macros (e.g.
> ISCSI_DBG_TRANS_SESSION). These macros use KERN_INFO and are
> controlled by module parameters.
>
> * We use iscsi_session_printk & iscsi_conn_printk for the rest of the
> kernel code.These macros wrap iscsi_cls_session_printk &
> iscsi_cls_conn_printk accordingly. They are sometimes wrapped by
> macros (e.g. ISCSI_SW_TCP_DBG). These macros use KERN_INFO and are
> controlled by module parameters.
>
> * We sometimes use printk calls.
>
> userspace:
>
> We use log_warning, log_error & log_debug. They depend on the logging
> level that we use (0-8). if (log_level > level), the log is sent to
> syslog with the appropriate log level (LOG_WARNING/LOG_ERR/LOG_DEBUG).
>
> My motivation: with the current logging mechanism, if an error occurs,
> I'm unable to tell exactly what happened. The default logging level is
> too low. Increasing it affects performance. Another problem is that
> open-iscsi has too many logging mechanisms.
>
> I suggest that:
> 1. For kernel modules, we will have 'events' (or any better name that
I'd call them "contexts" or "event sources".
> you suggest) like 'session', 'conn', 'eh', 'cmd' etc. For each event,
> we will have a logging level. For example, the user may want to set
> the 'conn' event to 'DEBUG'. It means that we will print all conn
> related logs that are DEBUG and above (e.g. WARNING, ERROR).
> 2. For userspace code, we could do the same (i.e. have events and a
> log level per event).
> 3. Userspace logging uses the 'daemon' facility. This should
> definitely be the default, but we should allow the user to use another
> facility. The motivation for doing so is that if we want to send all
> iscsid logs to a separate file, we can set it to 'local2' for example
> (instead of 'daemon').
>
> Comments?
What I'd wish: For most messages, especially if informational messages are
enabled, it's not clear what type of severity a message has (i.e. debug, info,
warning, error, fatal error). I'd wish the severity would be obvious from the
message.
I once wrote some code to do something like that. Showing some calling examples
might make it obvious:
message(MSG_TYPE_DEBUG, __LINE__, my_step, instance,
"min_pause=%ld\n",
min_pause);
message(MSG_TYPE_ERROR, __LINE__, my_step, instance,
"Incompatible alert state: file %s, ID %06lx\n",
state_file, state.id);
message(MSG_TYPE_INFO, __LINE__, my_step, instance,
"OS error (%s: %s)\n",
state_file, strerror(last_errno));
message(MSG_TYPE_FATAL, __LINE__, my_step, instance,
"No memory for state file %s\n",
file);
message(MSG_TYPE_WARN, __LINE__, my_step, instance,
"file \"%s\" exceeded size threshold\n",
msg_file);
My "instance" is what you called "event"; "step" are steps in processing
something
(an integer), and __LINE__ is just handy if "Instance" includes the source file.
Regards,
Ulrich
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/open-iscsi?hl=en.
