From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=43804 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PLdBn-0001P2-2v for qemu-devel@nongnu.org; Thu, 25 Nov 2010 09:52:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PLdBl-0004jC-Qk for qemu-devel@nongnu.org; Thu, 25 Nov 2010 09:52:46 -0500 Received: from mail-wy0-f173.google.com ([74.125.82.173]:41510) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PLdBl-0004j3-LU for qemu-devel@nongnu.org; Thu, 25 Nov 2010 09:52:45 -0500 Received: by wyg36 with SMTP id 36so586273wyg.4 for ; Thu, 25 Nov 2010 06:52:44 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <4CEE77CB.8050007@suse.de> References: <1290597370-21365-1-git-send-email-hare@suse.de> <1290597370-21365-15-git-send-email-hare@suse.de> <4CEE77CB.8050007@suse.de> Date: Thu, 25 Nov 2010 14:52:44 +0000 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH 14/15] megasas: LSI Megaraid SAS emulation List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke Cc: nab@linux-iscsi.org, qemu-devel@nongnu.org, kraxel@redhat.com On Thu, Nov 25, 2010 at 2:50 PM, Hannes Reinecke wrote: > On 11/25/2010 03:36 PM, Stefan Hajnoczi wrote: >> On Wed, Nov 24, 2010 at 11:16 AM, Hannes Reinecke wrote: >>> +static int megasas_pd_get_info_submit(SCSIDevice * sdev, int lun, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0struct megasas_cmd_t *cmd) >>> +{ >>> + =A0 =A0struct mfi_pd_info * info =3D cmd->iov_buf; >>> + =A0 =A0uint8_t cmdbuf[6]; >>> + =A0 =A0SCSIRequest *req; >>> + >>> + =A0 =A0if (info->inquiry_data[4] =3D=3D 0) { >>> + =A0 =A0 =A0 =A0/* Additional length is zero, resubmit */ >>> + =A0 =A0 =A0 =A0megasas_setup_inquiry(cmdbuf, 0, info->inquiry_data, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sizeof(inf= o->inquiry_data)); >>> + =A0 =A0 =A0 =A0req =3D sdev->info->alloc_req(sdev, (uint32_t) -1, lun= ); >>> + =A0 =A0 =A0 =A0if (!req) { >>> + =A0 =A0 =A0 =A0 =A0 =A0return MFI_STAT_FLASH_ALLOC_FAIL; >>> + =A0 =A0 =A0 =A0} >>> + =A0 =A0 =A0 =A0DPRINTF_DCMD("PD get info submit std inquiry to dev %d= \n", lun); >>> + =A0 =A0 =A0 =A0req->hba_private =3D cmd; >>> + =A0 =A0 =A0 =A0if (cmd->sdev->info->send_command(req, cmdbuf) > 0) >>> + =A0 =A0 =A0 =A0 =A0 =A0cmd->sdev->info->read_data(req); >>> + =A0 =A0 =A0 =A0return MFI_STAT_INVALID_STATUS; >> >> Here... >> >>> + =A0 =A0} else if (info->vpd_page83[3] =3D=3D 0) { >>> + =A0 =A0 =A0 =A0/* Additional length is zero, resubmit */ >>> + =A0 =A0 =A0 =A0megasas_setup_inquiry(cmdbuf, 0x83,(uint8_t *)info->vp= d_page83, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sizeof(inf= o->vpd_page83)); >>> + =A0 =A0 =A0 =A0req =3D sdev->info->alloc_req(sdev, (uint32_t) -1, lun= ); >>> + =A0 =A0 =A0 =A0if (!req) { >>> + =A0 =A0 =A0 =A0 =A0 =A0return MFI_STAT_FLASH_ALLOC_FAIL; >>> + =A0 =A0 =A0 =A0} >>> + =A0 =A0 =A0 =A0DPRINTF_DCMD("PD get info submit vpd inquiry to dev %d= \n", lun); >>> + =A0 =A0 =A0 =A0req->hba_private =3D cmd; >>> + =A0 =A0 =A0 =A0if (cmd->sdev->info->send_command(req, cmdbuf) > 0) >>> + =A0 =A0 =A0 =A0 =A0 =A0cmd->sdev->info->read_data(req); >>> + =A0 =A0 =A0 =A0return 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. =A0On 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