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 <c...@fr.ibm.com>
>>>> ---
>>>> 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 <stdio.h>
>>>> #include <string.h>
>>>> #include <stdint.h>
>>>> +#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.
