On Thu, Nov 25, 2010 at 2:50 PM, Hannes Reinecke <h...@suse.de> wrote: > On 11/25/2010 03:36 PM, Stefan Hajnoczi wrote: >> On Wed, Nov 24, 2010 at 11:16 AM, Hannes Reinecke <h...@suse.de> wrote: >>> +static int megasas_pd_get_info_submit(SCSIDevice * sdev, int lun, >>> + struct megasas_cmd_t *cmd) >>> +{ >>> + struct mfi_pd_info * info = cmd->iov_buf; >>> + uint8_t cmdbuf[6]; >>> + SCSIRequest *req; >>> + >>> + if (info->inquiry_data[4] == 0) { >>> + /* Additional length is zero, resubmit */ >>> + megasas_setup_inquiry(cmdbuf, 0, info->inquiry_data, >>> + sizeof(info->inquiry_data)); >>> + req = sdev->info->alloc_req(sdev, (uint32_t) -1, lun); >>> + if (!req) { >>> + return MFI_STAT_FLASH_ALLOC_FAIL; >>> + } >>> + DPRINTF_DCMD("PD get info submit std inquiry to dev %d\n", lun); >>> + req->hba_private = cmd; >>> + if (cmd->sdev->info->send_command(req, cmdbuf) > 0) >>> + cmd->sdev->info->read_data(req); >>> + return MFI_STAT_INVALID_STATUS; >> >> Here... >> >>> + } else if (info->vpd_page83[3] == 0) { >>> + /* Additional length is zero, resubmit */ >>> + megasas_setup_inquiry(cmdbuf, 0x83,(uint8_t *)info->vpd_page83, >>> + sizeof(info->vpd_page83)); >>> + req = sdev->info->alloc_req(sdev, (uint32_t) -1, lun); >>> + if (!req) { >>> + return MFI_STAT_FLASH_ALLOC_FAIL; >>> + } >>> + DPRINTF_DCMD("PD get info submit vpd inquiry to dev %d\n", lun); >>> + req->hba_private = cmd; >>> + if (cmd->sdev->info->send_command(req, cmdbuf) > 0) >>> + cmd->sdev->info->read_data(req); >>> + return MFI_STAT_INVALID_STATUS; >> >> ...and here I can't tell for sure if an error status is returned >> intentionally or whether this is an if statement without {}-related >> bug. >> >> On one hand it looks like it could be intentional. On the other hand >> we went through the trouble of sending an inquiry command that may >> have succeeded but we're still returning an error status. >> > > MFI_STAT_INVALID_STATUS is _not_ an error status. It's used > internally to signal that a command is still being processed. > ->read_data() itself doesn't have a meaningful return code; > we need to wait for the ->complete callback to get results. > So we're returning MFI_STAT_INVALID_STATUS to signal this condition. > > So that's ok.
Okay, thanks for explaining. Stefan