From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=35029 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PLd7B-0005rx-MR for qemu-devel@nongnu.org; Thu, 25 Nov 2010 09:48:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PLd7A-0003pk-Ht for qemu-devel@nongnu.org; Thu, 25 Nov 2010 09:48:01 -0500 Received: from cantor.suse.de ([195.135.220.2]:54223 helo=mx1.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PLd79-0003pG-6L for qemu-devel@nongnu.org; Thu, 25 Nov 2010 09:48:00 -0500 Message-ID: <4CEE77CB.8050007@suse.de> Date: Thu, 25 Nov 2010 15:50:51 +0100 From: Hannes Reinecke MIME-Version: 1.0 References: <1290597370-21365-1-git-send-email-hare@suse.de> <1290597370-21365-15-git-send-email-hare@suse.de> In-Reply-To: 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: Stefan Hajnoczi Cc: nab@linux-iscsi.org, qemu-devel@nongnu.org, kraxel@redhat.com 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, >> + struct megasas_cmd_t *cmd) >> +{ >> + struct mfi_pd_info * info =3D cmd->iov_buf; >> + uint8_t cmdbuf[6]; >> + SCSIRequest *req; >> + >> + if (info->inquiry_data[4] =3D=3D 0) { >> + /* Additional length is zero, resubmit */ >> + megasas_setup_inquiry(cmdbuf, 0, info->inquiry_data, >> + sizeof(info->inquiry_data)); >> + req =3D 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", lu= n); >> + req->hba_private =3D cmd; >> + if (cmd->sdev->info->send_command(req, cmdbuf) > 0) >> + cmd->sdev->info->read_data(req); >> + return MFI_STAT_INVALID_STATUS; >=20 > Here... >=20 >> + } else if (info->vpd_page83[3] =3D=3D 0) { >> + /* Additional length is zero, resubmit */ >> + megasas_setup_inquiry(cmdbuf, 0x83,(uint8_t *)info->vpd_page8= 3, >> + sizeof(info->vpd_page83)); >> + req =3D 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", lu= n); >> + req->hba_private =3D cmd; >> + if (cmd->sdev->info->send_command(req, cmdbuf) > 0) >> + cmd->sdev->info->read_data(req); >> + return MFI_STAT_INVALID_STATUS; >=20 > ...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. >=20 > 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. >=20 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. Cheers, Hannes --=20 Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: Markus Rex, HRB 16746 (AG N=FCrnberg)