On 08/10/2024 03.15, jrossi@linux.ibm.com wrote:
+ if (!vs_io_assert(virtio_run(vdev, VR_REQUEST, cmd) == 0, title)) {
+ puts(title);
Should there be a "return" with a non-0 value here? ...
+ }
+
+ return 0;
}
/* SCSI protocol implementation routines */
-static bool scsi_inquiry(VDev *vdev, uint8_t evpd, uint8_t page,
+static int scsi_inquiry(VDev *vdev, uint8_t evpd, uint8_t page,
void *data, uint32_t data_size)
{
ScsiCdbInquiry cdb = {
@@ -110,12 +122,13 @@ static bool scsi_inquiry(VDev *vdev, uint8_t
evpd, uint8_t page,
{ data, data_size, VRING_DESC_F_WRITE },
};
- vs_run("inquiry", inquiry, vdev, &cdb, sizeof(cdb), data,
data_size);
+ int cc = vs_run("inquiry", inquiry,
+ vdev, &cdb, sizeof(cdb), data, data_size);
... since the caller site now obviously checks for a non-zero value!
- return virtio_scsi_response_ok(&resp);
+ return cc ? cc : virtio_scsi_response_ok(&resp);
}
...
@@ -207,7 +221,7 @@ static int virtio_scsi_locate_device(VDev *vdev)
static uint8_t data[16 + 8 * 63];
ScsiLunReport *r = (void *) data;
ScsiDevice *sdev = vdev->scsi_device;
- int i, luns;
+ int i, cc, luns;
/* QEMU has hardcoded channel #0 in many places.
* If this hardcoded value is ever changed, we'll need to add
code for
@@ -233,13 +247,21 @@ static int virtio_scsi_locate_device(VDev *vdev)
sdev->channel = channel;
sdev->target = target;
sdev->lun = 0; /* LUN has to be 0 for REPORT LUNS */
- if (!scsi_report_luns(vdev, data, sizeof(data))) {
+ cc = scsi_report_luns(vdev, data, sizeof(data));
+ if (cc < 0) {
+ return cc;
+ }
By the way, calling a variable "cc" reminds me of the CC of the PSW,
so I'd expect values from 0 to 3 for this variable. Obviously this was
meant to convey negative error codes instead, so I'd like to suggest
to rename that variable to "ret" or something similar instead.
+ else if (cc == 0) {
if (resp.response == VIRTIO_SCSI_S_BAD_TARGET) {
continue;
}
printf("target 0x%X", target);
- virtio_scsi_verify_response(&resp, "SCSI cannot report
LUNs");
+ if (!virtio_scsi_verify_response(&resp, "SCSI cannot
report LUNs")) {
+ return -EIO;
+ }
}
Thomas