From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42524) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMIIm-0003hR-6f for qemu-devel@nongnu.org; Thu, 21 Jan 2016 11:41:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMIIj-0005Cf-EY for qemu-devel@nongnu.org; Thu, 21 Jan 2016 11:41:40 -0500 Received: from e06smtp06.uk.ibm.com ([195.75.94.102]:34802) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMIIj-0005B0-65 for qemu-devel@nongnu.org; Thu, 21 Jan 2016 11:41:37 -0500 Received: from localhost by e06smtp06.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 21 Jan 2016 16:41:34 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id A307F2190046 for ; Thu, 21 Jan 2016 16:41:19 +0000 (GMT) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u0LGfViT10748222 for ; Thu, 21 Jan 2016 16:41:31 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u0LGfVDe016952 for ; Thu, 21 Jan 2016 09:41:31 -0700 References: <1452015002-28493-6-git-send-email-clg@fr.ibm.com> <569B8350.2040305@gmail.com> <20160117161425-mutt-send-email-mst@redhat.com> <56A10948.8020306@gmail.com> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <56A10A33.9010705@fr.ibm.com> Date: Thu, 21 Jan 2016 17:41:23 +0100 MIME-Version: 1.0 In-Reply-To: <56A10948.8020306@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 5/8] ipmi: add ACPI power and GUID commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: minyard@acm.org, "Michael S. Tsirkin" , marcel@redhat.com Cc: Corey Minyard , qemu-devel@nongnu.org On 01/21/2016 05:37 PM, Corey Minyard wrote: > On 01/17/2016 08:16 AM, Michael S. Tsirkin wrote: >> On Sun, Jan 17, 2016 at 02:04:32PM +0200, Marcel Apfelbaum wrote: >>> On 01/05/2016 07:29 PM, Cédric Le Goater wrote: >>>> Signed-off-by: Cédric Le Goater >>>> --- >>>> hw/ipmi/ipmi_bmc_sim.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 55 insertions(+) >>>> >>>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c >>>> index 60586a67104e..c3a06d0ac7e4 100644 >>>> --- a/hw/ipmi/ipmi_bmc_sim.c >>>> +++ b/hw/ipmi/ipmi_bmc_sim.c >>>> @@ -25,6 +25,7 @@ >>>> #include >>>> #include >>>> #include >>>> +#include "sysemu/sysemu.h" >>>> #include "qemu/timer.h" >>>> #include "hw/ipmi/ipmi.h" >>>> #include "qemu/error-report.h" >>>> @@ -54,6 +55,9 @@ >>>> #define IPMI_CMD_GET_DEVICE_ID 0x01 >>>> #define IPMI_CMD_COLD_RESET 0x02 >>>> #define IPMI_CMD_WARM_RESET 0x03 >>>> +#define IPMI_CMD_SET_POWER_STATE 0x06 >>>> +#define IPMI_CMD_GET_POWER_STATE 0x07 >>>> +#define IPMI_CMD_GET_DEVICE_GUID 0x08 >>>> #define IPMI_CMD_RESET_WATCHDOG_TIMER 0x22 >>>> #define IPMI_CMD_SET_WATCHDOG_TIMER 0x24 >>>> #define IPMI_CMD_GET_WATCHDOG_TIMER 0x25 >>>> @@ -215,6 +219,9 @@ struct IPMIBmcSim { >>>> >>>> uint8_t restart_cause; >>>> >>>> + uint8_t power_state[2]; >>>> + uint8_t uuid[16]; >>>> + >>>> IPMISel sel; >>>> IPMISdr sdr; >>>> IPMIFru fru; >>>> @@ -842,6 +849,42 @@ static void warm_reset(IPMIBmcSim *ibs, >>>> k->reset(s, false); >>>> } >>>> } >>>> +static void set_power_state(IPMIBmcSim *ibs, >>>> + uint8_t *cmd, unsigned int cmd_len, >>>> + uint8_t *rsp, unsigned int *rsp_len, >>>> + unsigned int max_rsp_len) >>>> +{ >>>> + IPMI_CHECK_CMD_LEN(4); >>>> + ibs->power_state[0] = cmd[2]; >>>> + ibs->power_state[1] = cmd[3]; >>>> + out: >>>> + return; >>> >>> Hi, >>> >>> I am sorry for my late comment, but I find a little strange the use of >>> the "out" label here. >>> I understand this is because of its usage in IPMI_* macros, but >>> I looked into every usage(I hope I didn't miss anything) and the code >>> simply returns. >>> Also the correlation between those macros is a little odd. >>> >>> Thanks, >>> Marcel >> >> Yes - these macros with goto out are confusing. >> >> Please rewrite them to return bool, and put >> goto out in the caller. >> > > Marcel, do you want me to do this? I have the patch ready (and more). I will send this one to start with. C.