On 6.3.2017 22:24, Don Brace wrote:
> - Add in a new case for volume offline. Resolves internal
> testing bug for multilun array management.
> - Return correct status for failed TURs.
>
> Reviewed-by: Scott Benesh <scott.benesh@xxxxxxxxxxxxx>
> Reviewed-by: Scott Teel <scott.teel@xxxxxxxxxxxxx>
> Signed-off-by: Don Brace <don.brace@xxxxxxxxxxxxx>
> ---
> drivers/scsi/hpsa.c | 26 +++++++++++++-------------
> drivers/scsi/hpsa_cmd.h | 2 ++
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 524a0c7..1adc4ec 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -3735,7 +3735,7 @@ static int hpsa_volume_offline(struct ctlr_info *h,
> DEFAULT_TIMEOUT);
> if (rc) {
> cmd_free(h, c);
Hi Don,
patch is ok, but this function returns a mix of either HPSA_LV_*
values or some magic numbers.
Could you replace the 0xff with HPSA_VPD_LV_STATUS_UNSUPPORTED
(like it is in hpsa_get_volume_status)
and in case of success return HPSA_LV_OK ?
Also it would make sense to change the return value
from 'int' to 'unsigned char' and to drop
this test
volume_offline = hpsa_volume_offline(h, scsi3addr);
if (volume_offline < 0 || volume_offline > 0xff)
volume_offline = HPSA_VPD_LV_STATUS_UNSUPPORTED;
this_device->volume_offline = volume_offline & 0xff;
from hpsa_update_device_info since the condition is never met.
Switching volume_offline to an unsigned char everywhere might be also possible.
tomash
> - return 0;
> + return 0xff;
> }
> sense = c->err_info->SenseInfo;
> if (c->err_info->SenseLen > sizeof(c->err_info->SenseInfo))
> @@ -3746,19 +3746,13 @@ static int hpsa_volume_offline(struct ctlr_info *h,
> cmd_status = c->err_info->CommandStatus;
> scsi_status = c->err_info->ScsiStatus;
> cmd_free(h, c);
> - /* Is the volume 'not ready'? */
> - if (cmd_status != CMD_TARGET_STATUS ||
> - scsi_status != SAM_STAT_CHECK_CONDITION ||
> - sense_key != NOT_READY ||
> - asc != ASC_LUN_NOT_READY) {
> - return 0;
> - }
>
> /* Determine the reason for not ready state */
> ldstat = hpsa_get_volume_status(h, scsi3addr);
>
> /* Keep volume offline in certain cases: */
> switch (ldstat) {
> + case HPSA_LV_FAILED:
> case HPSA_LV_UNDERGOING_ERASE:
> case HPSA_LV_NOT_AVAILABLE:
> case HPSA_LV_UNDERGOING_RPI:
> @@ -3853,10 +3847,10 @@ static int hpsa_update_device_info(struct ctlr_info *h,
> /* Do an inquiry to the device to see what it is. */
> if (hpsa_scsi_do_inquiry(h, scsi3addr, 0, inq_buff,
> (unsigned char) OBDR_TAPE_INQ_SIZE) != 0) {
> - /* Inquiry failed (msg printed already) */
> dev_err(&h->pdev->dev,
> - "hpsa_update_device_info: inquiry failed\n");
> - rc = -EIO;
> + "%s: inquiry failed, device will be skipped.\n",
> + __func__);
> + rc = HPSA_INQUIRY_FAILED;
> goto bail_out;
> }
>
> @@ -3894,6 +3888,13 @@ static int hpsa_update_device_info(struct ctlr_info *h,
> if (volume_offline < 0 || volume_offline > 0xff)
> volume_offline = HPSA_VPD_LV_STATUS_UNSUPPORTED;
> this_device->volume_offline = volume_offline & 0xff;
> + if (volume_offline == HPSA_LV_FAILED) {
> + rc = HPSA_LV_FAILED;
> + dev_err(&h->pdev->dev,
> + "%s: LV failed, device will be skipped.\n",
> + __func__);
> + goto bail_out;
> + }
> } else {
> this_device->raid_level = RAID_UNKNOWN;
> this_device->offload_config = 0;
> @@ -4379,8 +4380,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
> goto out;
> }
> if (rc) {
> - dev_warn(&h->pdev->dev,
> - "Inquiry failed, skipping device.\n");
> + h->drv_req_rescan = 1;
> continue;
> }
>
> diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
> index a584cdf..5961705 100644
> --- a/drivers/scsi/hpsa_cmd.h
> +++ b/drivers/scsi/hpsa_cmd.h
> @@ -156,6 +156,7 @@
> #define CFGTBL_BusType_Fibre2G 0x00000200l
>
> /* VPD Inquiry types */
> +#define HPSA_INQUIRY_FAILED 0x02
> #define HPSA_VPD_SUPPORTED_PAGES 0x00
> #define HPSA_VPD_LV_DEVICE_ID 0x83
> #define HPSA_VPD_LV_DEVICE_GEOMETRY 0xC1
> @@ -166,6 +167,7 @@
> /* Logical volume states */
> #define HPSA_VPD_LV_STATUS_UNSUPPORTED 0xff
> #define HPSA_LV_OK 0x0
> +#define HPSA_LV_FAILED 0x01
> #define HPSA_LV_NOT_AVAILABLE 0x0b
> #define HPSA_LV_UNDERGOING_ERASE 0x0F
> #define HPSA_LV_UNDERGOING_RPI 0x12
>
