Re: [PATCH v3 1/2] virtio-scsi: first version

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/19/2011 01:22 PM, Hannes Reinecke wrote:
+	case VIRTIO_SCSI_S_UNDERRUN:
+		set_host_byte(sc, DID_ERROR);
+		break;
Hmm. Not sure this is correct.
UNDERRUN could be a valid state, eg it is quite common to send
an INQUIRY with a length of 255 bytes. And only evaluate the
bytes which are actually send back.

This happens when you send an INQUIRY with a sglist of 255 bytes and an ALLOCATION LENGTH of 300 bytes. The spec says "VIRTIO_SCSI_S_UNDERRUN [is returned] if the content of the CDB requires transferring more data than is available in the data buffers".

And, it's not good style to match the same DID_XXX error code to
several internal errors. I would prefer having a 1:1 match for
VIRTIO_SCSI_S_XXX and DID_XXX error codes.

There is no DID_XXX value for underrun, but it's quite useful to have it diagnosed separately in traces.

+static struct scsi_host_template virtscsi_host_template = {
+	.module = THIS_MODULE,
+	.name = "Virtio SCSI HBA",
+	.proc_name = "virtio_scsi",
+	.queuecommand = virtscsi_queuecommand,
+	.this_id = -1,
+
+	.can_queue = 1024,
+	.dma_boundary = UINT_MAX,
+	.use_clustering = ENABLE_CLUSTERING,
+};
+
Hmm. Apparently you support more than one LUN, yet no
->slave_alloc() callback is present. So how do you scan the bus here?

What does slave_alloc have to do with scanning? I don't need hostdata, so I don't have slave_alloc.

Scanning is here:

        err = scsi_add_host(shost, &vdev->dev);
        if (err)
                goto scsi_add_host_failed;

        scsi_scan_host(shost);

Do you actually have a working backend implementation?

Hmm... yes, of course...

Paolo
--
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


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux