On 2 Dec 2009 at 16:17, Erez Zilber wrote: > On Wed, Dec 2, 2009 at 3:02 PM, Ulrich Windl > <[email protected]> wrote: > > 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 agree. > > > > > 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. > > Yes. It would be nice if we had the function + line number for each > log. I'm not sure that I understand the meaning of 'step'. Here's an > example (to make sure that I got you right): if you send a PDU, you > can print multiple messages while preparing & sending the PDU, each > with its own step number. For example: > > "(1) allocating PDU" > "(2) init PDU" > "(3) sending PDU" > "(4) PDU was sent" > > Is this what you mean?
Yes, more or less: If you have a loop that is performing the same code multiple times, it may be interesting in which iteration the message was triggered. For situations like that I "invented" the step. Also, if the step number is negative, the output is suppressed. So basically it's optional. 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.
