On Wed, Dec 16, 2009 at 5:31 PM, Erez Zilber <erezzi.l...@gmail.com> wrote:
> On Wed, Dec 2, 2009 at 10:55 AM, Erez Zilber <erezzi.l...@gmail.com> 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
>> 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).
>
> I suggest that each kernel module will have its own events. Each event
> will be represented by a module parameter (with some default value).
>
>> 2. For userspace code, we could do the same (i.e. have events and a
>> log level per event).
>
> Regarding the 'events' in userspace - we will have events A, B & C for
> iscsid and events D, E & F for iscsiadm. For each event, we will
> probably have a default logging level. The user may want to run with
> another logging level for each event. For iscsid, I suggest that we
> add this to iscsid.conf. For iscsiadm, the user will be able to do
> something like:
>
> iscsiadm -d some_level - this will set all events to 'some_level'
> iscsiadm -dE level_for_E -dF level_for_F - this will set the event 'E'
> to 'level_for_E' and the event 'F' to 'level_for_F'. The event 'D'
> will use the default logging level.
>
> Comments?
>
> Thanks,
> Erez
>

I've started working on the new logging. I've started with iscsi_tcp.
Here's a glance of the general idea. If you have comments on the
general implementation, let's discuss them now. Later, it will be much
more difficult for me.

Here it is:

Added the following code in libiscsi.h:

#define iscsi_log(log_level, dev, dbg_fmt, arg...)                      \
        do {                                                            \
                char *log_level_str;                                    \
                switch (log_level) {                                    \
                case ISCSI_LOG_ERROR:                                   \
                        log_level_str = "ERROR";                        \
                        break;                                          \
                case ISCSI_LOG_WARN:                                    \
                        log_level_str = "WARN";                         \
                        break;                                          \
                case ISCSI_LOG_INFO:                                    \
                        log_level_str = "INFO";                         \
                        break;                                          \
                case ISCSI_LOG_DEBUG:                                   \
                        log_level_str = "DEBUG";                        \
                        break;                                          \
                case ISCSI_LOG_TRACE:                                   \
                        log_level_str = "TRACE";                        \
                        break;                                          \
                }                                                       \
                if (log_level > ISCSI_LOG_INFO) {                       \
                        if (dev) {                                      \
                                dev_printk(KERN_DEBUG,                  \
                                           (struct device *)dev,        \
                                           "%s:%d (%s) " dbg_fmt "\n",  \
                                           __func__, __LINE__,          \
                                           log_level_str, ##arg);       \
                        } else {                                        \
                                printk(KERN_DEBUG                       \
                                       "%s:%d (%s) " dbg_fmt "\n",      \
                                       __func__, __LINE__,              \
                                       log_level_str, ##arg);           \
                        }                                               \
                } else {                                                \
                        if (dev) {                                      \
                                dev_printk(KERN_INFO,                   \
                                           (struct device *)dev,        \
                                           "%s:%d (%s) " dbg_fmt "\n",  \
                                           __func__, __LINE__,          \
                                           log_level_str, ##arg);       \
                        } else {                                        \
                                printk(KERN_INFO                        \
                                       "%s:%d (%s) " dbg_fmt "\n",      \
                                       __func__, __LINE__,              \
                                       log_level_str, ##arg);           \
                        }                                               \
                }                                                       \
        } while (0);

added a new iscsi_log.h file:

enum iscsi_log_level {
        ISCSI_LOG_ERROR = 1,
        ISCSI_LOG_WARN,
        ISCSI_LOG_INFO,
        ISCSI_LOG_DEBUG,
        ISCSI_LOG_TRACE
};

and in iscsi_tcp.c:

/* Logging contexts */
struct {
        int general;
        int conn;
} iscsi_sw_tcp_log_ctxt = {ISCSI_LOG_INFO, ISCSI_LOG_INFO};

enum iscsi_sw_tcp_ctxt {
        ISCSI_SW_TCP_CTXT_GENERAL = 1,
        ISCSI_SW_TCP_CTXT_CONN,
};

module_param_named(general_log_ctxt, iscsi_sw_tcp_log_ctxt.general, int,
                   S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(general_log_ctxt, "blah blah - general");

module_param_named(conn_log_ctxt, iscsi_sw_tcp_log_ctxt.conn, int,
                   S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(conn_log_ctxt, "blah blah - conn");

#define _iscsi_sw_tcp_log(log_level, ctxt, dev, dbg_fmt, arg...)                
\
        do {                                                            \
                switch (ctxt) {                                         \
                case ISCSI_SW_TCP_CTXT_GENERAL:                         \
                        if (log_level <= iscsi_sw_tcp_log_ctxt.general) \
                                iscsi_log(log_level, dev, dbg_fmt,      \
                                          ##arg);                       \
                        break;                                          \
                case ISCSI_SW_TCP_CTXT_CONN:                            \
                        if (log_level <= iscsi_sw_tcp_log_ctxt.conn)    \
                                iscsi_log(log_level, dev, dbg_fmt,      \
                                          ##arg);                       \
                        break;                                          \
                }                                                       \
         } while (0);

#define iscsi_sw_tcp_log(log_level, ctxt, dbg_fmt, arg...)              \
        do {                                                            \
                _iscsi_sw_tcp_log(log_level, ctxt, NULL, dbg_fmt,       \
                                  ##arg);                               \
         } while (0);

#define iscsi_sw_tcp_conn_log(log_level, ctxt, conn, dbg_fmt, arg...)   \
        do {                                                            \
                struct device *dev;                                     \
                dev = &((struct iscsi_conn *)conn)->cls_conn->dev;      \
                _iscsi_sw_tcp_log(log_level, ctxt, dev, dbg_fmt, arg);  \
         } while (0);

#define iscsi_sw_tcp_sess_log(log_level, ctxt, sess, dbg_fmt, arg...)   \
        do {                                                            \
                struct device *dev;                                     \
                dev = &((struct iscsi_session *)sess)->cls_session->dev;\
                _iscsi_sw_tcp_log(log_level, ctxt, dev, dbg_fmt, arg);  \
         } while (0);

so now, instead of doing the following in iscsi_sw_tcp_send_hdr_done():

ISCSI_SW_TCP_DBG(tcp_conn->iscsi_conn,
                         "Header done. Next segment size %u total_size %u\n",
                         tcp_sw_conn->out.segment.size,
                         tcp_sw_conn->out.segment.total_size);

we will do the following:

iscsi_sw_tcp_conn_log(ISCSI_LOG_INFO, ISCSI_SW_TCP_CTXT_CONN,
                              tcp_conn->iscsi_conn,
                              "Header done. Next segment size %u total_size 
%u\n",
                              tcp_sw_conn->out.segment.size,
                              tcp_sw_conn->out.segment.total_size);
-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-is...@googlegroups.com.
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.


Reply via email to