On 09/20/2012 09:53 PM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <h...@suse.de> writes:
>
> Hannes> I recently got my hands on some weird drives, insisting on
> Hannes> having been formatted with protection type 7:
>
> Lovely :|
>
Hehe. Probably _really_ future drives :-)
>
> Hannes> I've attached a tentative patch, which allows the system to
> Hannes> boot. However, I'm not completely happy with that, as the
> Hannes> capacity is _still_ updated after revalidation:
>
> How about this?
>
>
> sd: Ensure we correctly disable devices with unknown protection type
>
> We set the capacity to zero when we discovered a device formatted with
> an unknown DIF protection type. However, the read_capacity code would
> override the capacity and cause the device to be enabled regardless.
>
> Make sd_read_protection_type() return an error if the protection type is
> unknown. Also prevent duplicate printk lines when the device is being
> revalidated.
>
> Reported-by: Hannes Reinecke <h...@suse.de>
> Signed-off-by: Martin K. Petersen <martin.peter...@oracle.com>
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 1cf2d5d..1c54564 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
> /*
> * Determine whether disk supports Data Integrity Field.
> */
> -static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char
> *buffer)
> +static int sd_read_protection_type(struct scsi_disk *sdkp, unsigned char
> *buffer)
> {
> struct scsi_device *sdp = sdkp->device;
> u8 type;
> + int ret = 0;
>
> if (scsi_device_protection(sdp) == 0 || (buffer[12] & 1) == 0)
> - return;
> + return ret;
>
> type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
>
> - if (type == sdkp->protection_type || !sdkp->first_scan)
> - return;
> + if (type > SD_DIF_TYPE3_PROTECTION)
> + ret = -ENODEV;
> + else if (scsi_host_dif_capable(sdp->host, type))
> + ret = 1;
> +
> + if (sdkp->first_scan || type != sdkp->protection_type)
> + switch (ret) {
> + case -ENODEV:
> + sd_printk(KERN_ERR, sdkp, "formatted with unsupported" \
> + " protection type %u. Disabling disk!\n",
> + type);
> + break;
> + case 1:
> + sd_printk(KERN_NOTICE, sdkp,
> + "Enabling DIF Type %u protection\n", type);
> + break;
> + case 0:
> + sd_printk(KERN_NOTICE, sdkp,
> + "Disabling DIF Type %u protection\n", type);
> + break;
> + }
>
> sdkp->protection_type = type;
>
> - if (type > SD_DIF_TYPE3_PROTECTION) {
> - sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
> - "protection type %u. Disabling disk!\n", type);
> - sdkp->capacity = 0;
> - return;
> - }
> -
> - if (scsi_host_dif_capable(sdp->host, type))
> - sd_printk(KERN_NOTICE, sdkp,
> - "Enabling DIF Type %u protection\n", type);
> - else
> - sd_printk(KERN_NOTICE, sdkp,
> - "Disabling DIF Type %u protection\n", type);
> + return ret;
> }
>
> static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device
> *sdp,
> @@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp,
> struct scsi_device *sdp,
> sector_size = get_unaligned_be32(&buffer[8]);
> lba = get_unaligned_be64(&buffer[0]);
>
> - sd_read_protection_type(sdkp, buffer);
> + if (sd_read_protection_type(sdkp, buffer) < 0) {
> + sdkp->capacity = 0;
> + return -ENODEV;
> + }
>
> if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) {
> sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
>
Yup. That's what I had in mind.
Acked-by: Hannes Reinecke <h...@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
