Thanks Joe. I'll send out another patch. > -----Original Message----- > From: Joe Perches [mailto:joe@xxxxxxxxxxx] > Sent: Thursday, December 3, 2015 6:28 PM > To: Long Li <longli@xxxxxxxxxxxxx>; KY Srinivasan <kys@xxxxxxxxxxxxx>; > Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; James E.J. Bottomley > <JBottomley@xxxxxxxx> > Cc: devel@xxxxxxxxxxxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] storvsc: add more logging for error and warning > messages > > On Thu, 2015-12-03 at 19:47 -0800, Long Li wrote: > > Introduce a logging level for storvsc to log certain error/warning > > messages. Those messages are helpful in some environments, e.g. > > Microsoft Azure, for customer support and troubleshooting purposes. > [] > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > [] > > +static inline bool do_logging(int level) { > > + return (logging_level >= level) ? true : false; > > The ternary is not necessary > > return logging_level >= level; > > is enough > > > +} > > + > > + > > struct vmscsi_win8_extension { > > /* > > * The following were added in Windows 8 @@ -1183,7 +1198,7 @@ > > static void storvsc_command_completion(struct storvsc_cmd_request > > *cmd_request) > > > > scmnd->result = vm_srb->scsi_status; > > > > - if (scmnd->result) { > > + if (scmnd->result && do_logging(STORVSC_LOGGING_ERROR)) { > > if (scsi_normalize_sense(scmnd->sense_buffer, > > SCSI_SENSE_BUFFERSIZE, &sense_hdr)) > > scsi_print_sense_hdr(scmnd->device, "storvsc", > > Is it appropriate to make this scsi_normalize_sense call conditional on > do_logging here? > > > @@ -1239,12 +1254,25 @@ static void storvsc_on_io_completion(struct > hv_device *device, > > stor_pkt->vm_srb.sense_info_length = > > vstor_packet->vm_srb.sense_info_length; > > > > + if (vstor_packet->vm_srb.scsi_status != 0 || > > + vstor_packet->vm_srb.srb_status != SRB_STATUS_SUCCESS) > > + if (do_logging(STORVSC_LOGGING_WARN)) > > + dev_warn(&device->device, > > + "cmd 0x%x scsi status 0x%x srb status > 0x%x\n", > > + stor_pkt->vm_srb.cdb[0], > > + vstor_packet->vm_srb.scsi_status, > > + vstor_packet->vm_srb.srb_status); > > It might make some sense to use another macro indirection like > > #define svc_log_warn(dev, level, fmt, ...) \ > do { \ > if (do_logging(STORSVC_LOGGING_##level) \ > dev_warn(&(dev)->device, fmt, ##__VA_ARGS__); \ > } while (0) > > So a use could be: > > if (vstore_packet...) > svc_log_warn(device, WARN, ...); > > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
