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, ...);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
