From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46504) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBD7t-000767-U9 for qemu-devel@nongnu.org; Mon, 21 Dec 2015 21:56:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBD7t-0000g5-12 for qemu-devel@nongnu.org; Mon, 21 Dec 2015 21:56:37 -0500 Received: from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f]:36836) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBD7s-0000fu-Qj for qemu-devel@nongnu.org; Mon, 21 Dec 2015 21:56:36 -0500 Received: by mail-wm0-x22f.google.com with SMTP id p187so91803885wmp.1 for ; Mon, 21 Dec 2015 18:56:36 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1450696004-47043-1-git-send-email-yanmiaobest@gmail.com> <1450696004-47043-2-git-send-email-yanmiaobest@gmail.com> Date: Tue, 22 Dec 2015 10:56:36 +0800 Message-ID: From: Miao Yan Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 1/3] net/vmxnet3: return 1 on device activation failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: P J P Cc: Dmitry Fleytman , Jason Wang , QEMU 2015-12-22 2:15 GMT+08:00 P J P : > +-- On Mon, 21 Dec 2015, Miao Yan wrote --+ > | So return 1 on device activation failure instead of -1; > | > | Signed-off-by: Miao Yan > | --- > | hw/net/vmxnet3.c | 2 +- > | 1 file changed, 1 insertion(+), 1 deletion(-) > | > | diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > | index e168285..9185408 100644 > | --- a/hw/net/vmxnet3.c > | +++ b/hw/net/vmxnet3.c > | @@ -1652,7 +1652,7 @@ static uint64_t vmxnet3_get_command_status(VMXNET3State *s) > | > | switch (s->last_command) { > | case VMXNET3_CMD_ACTIVATE_DEV: > | - ret = (s->device_active) ? 0 : -1; > | + ret = (s->device_active) ? 0 : 1; > | VMW_CFPRN("Device active: %" PRIx64, ret); > | break; > > It seems okay. Considering that the function returns 'uint64_t', -1 would > become an extremely large value. I wonder if that is intended. > > If '1' indicates the error, the 'default:' case in the same switch needs to be > updated too. '1' indicates an error on device activation. Not sure about the 'unknown command' case. > > default: > VMW_WRPRN("Received request for unknown command: %x", s->last_command); > ret = -1; > break; > > > Why does the function return 'uint64_t' type? All return values from other > cases seem to be within uint32_t type. Linux driver uses VXMNET3_READ_BAR1_REG() which calls readl(). That should be an indication that the driver expects 32bit values. But the prototype in MemoryRegionOps requires uint64_t. > > -- > Prasad J Pandit / Red Hat Product Security Team > 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F