<font size=2 face="sans-serif">Hi Cyril,</font><br><font size=2 face="sans-serif">I had modified the code following your

recommended except for using the dbus_cold_reset function( replaced by

dbus_warm_reset ). Because I haven't found out a best way to do it. Please

give some detailed solution.</font><br><br><font size=3 color=#424282 face="Calibri">Regards,</font><br><br><font size=3 color=#424282 face="Calibri">William Li ( Li Nan, 川薨

)    </font><br><br><font size=3 color=#424282 face="Calibri">Firmware Engineering Professional</font><br><font size=3 color=#424282 face="Calibri">OpenPower AE Team | IBM System

& Technology Lab</font><br><font size=3 color=#424282 face="Calibri">Mobile: +86-186-1081 6605,

+86-138-1188 5954</font><br><br><font size=3 color=#424282 face="Calibri">Beijing, China</font><br><br><br><br><font size=1 color=#5f5f5f face="sans-serif">From:      

 </font><font size=1 face="sans-serif">Cyril Bur <cyrilbur@gmail.com></font><br><font size=1 color=#5f5f5f face="sans-serif">To:      

 </font><font size=1 face="sans-serif">Norman James <njames@us.ibm.com>,

Sam Mendoza-Jonas <sam@mendozajonas.com></font><br><font size=1 color=#5f5f5f face="sans-serif">Cc:      

 </font><font size=1 face="sans-serif">OpenBMC Patches <openbmc-patches@stwcx.xyz>,

Nan KX Li/China/IBM@IBMCN, openbmc@lists.ozlabs.org</font><br><font size=1 color=#5f5f5f face="sans-serif">Date:      

 </font><font size=1 face="sans-serif">02/02/2016 13:26</font><br><font size=1 color=#5f5f5f face="sans-serif">Subject:    

   </font><font size=1 face="sans-serif">Re: [PATCH phosphor-host-ipmid

v3] Add ipmi coldReset command, which call a dbus method, belongs to NETFUN_APP.

1. A method dbus_warm_reset() to talk with the dbus method 'warmRest';

2. Also get service name by ipmid_get_sd_bus_connection() instead of object_mapper_get_connection();

3. Register the ipmi command; 4. Add related .o to the Makefile; 5. Add

wildcard function.</font><br><hr noshade><br><br><br><font size=3><br></font><br><font size=3>On Tue, 2 Feb 2016 15:13 Norman James <</font><a href=mailto:njames@us.ibm.com><font size=3 color=blue><u>njames@us.ibm.com</u></font></a><font size=3>>

wrote:</font><br><font size=3>Too late. Cyril had already approved this code. If you

want to step up the priority on fixing the TODO, you could open an issue

so we can make sure it gets addressed in a timely fashion. </font><p><br><font size=3>I never 'approved' this patch. I reviewed it and pointed

out that it needed work in several places.</font><br><br><font size=3>By the looks of things not all my concerns were addressed

or explained to me in a way that makes me feel comfortable with the patch

as it is.</font><br><br><font size=3>Furthermore even if this version had addressed all my

issues with the previous version, that does not mean I automatically approve

this patch, I still need to give my actual approval to each patch no matter

what version we get to.</font><br><br><font size=3>You'll know when I 'approve' a patch as I will reply to

it with my "reviewed-by" tag.</font><br><br><font size=3>Regards,</font><br><font size=3>Cyril Bur</font><br><font size=3>Open Source Developer</font><br><br><font size=3><br><br>Regards,<br>Norman James<br>IBM - POWER Systems Architect<br>Phone: 1-512-286-6807 (T/L: 363-6807)<br>Internet: </font><a href=mailto:njames@us.ibm.com target=_blank><font size=3 color=blue><u>njames@us.ibm.com</u></font></a><font size=3><br><br><br></font><img src=cid:_1_104CCE2C104CC93400229FF148257F4D alt="Inactive hide details for Sam Mendoza-Jonas ---02/01/2016 09:44:21 PM---Hi William, a question below; On Mon, Feb 01, 2016 at 1" style="border:0px solid;"><font size=3 color=#424282>Sam

Mendoza-Jonas ---02/01/2016 09:44:21 PM---Hi William, a question below;

On Mon, Feb 01, 2016 at 12:40:25AM -0600, OpenBMC Patches wrote:<font size=3> <font size=2 color=#5f5f5f> From: <font size=2>Sam Mendoza-Jonas <<a href=mailto:sam@mendozajonas.com target=_blank><font size=2 color=blue>sam@mendozajonas.com</a><font size=2>><font size=2 color=#5f5f5f> To: <font size=2>OpenBMC Patches <<a href="mailto:openbmc-patches@stwcx.xyz" target=_blank><font size=2 color=blue>openbmc-patches@stwcx.xyz</a><font size=2>><font size=2 color=#5f5f5f> Cc: <a href=mailto:openbmc@lists.ozlabs.org target=_blank><font size=2 color=blue>openbmc@lists.ozlabs.org</a><font size=2>,

William <</font><a href=mailto:bjlinan@cn.ibm.com target=_blank><font size=2 color=blue><u>bjlinan@cn.ibm.com</u></font></a><font size=2>></font><font size=2 color=#5f5f5f><br>Date: </font><font size=2>02/01/2016 09:44 PM</font><font size=2 color=#5f5f5f><br>Subject: </font><font size=2>Re: [PATCH phosphor-host-ipmid v3] Add ipmi

coldReset command, which call a dbus method, belongs to NETFUN_APP. 1.

A method dbus_warm_reset() to talk with the dbus method 'warmRest'; 2.

Also get service name by ipmid_get_sd_bus_connection() instead of object_mapper_get_connection();

3. Register the ipmi command; 4. Add related .o to the Makefile; 5. Add

wildcard function.</font><font size=2 color=#5f5f5f><br>Sent by: </font><font size=2>"openbmc" <openbmc-bounces+njames=</font><a href=mailto:us.ibm.com@lists.ozlabs.org target=_blank><font size=2 color=blue><u>us.ibm.com@lists.ozlabs.org</u></font></a><font size=2>></font><br><hr noshade><p><font size=3><br><br></font><tt><font size=3><br>Hi William, a question below;<br><br>On Mon, Feb 01, 2016 at 12:40:25AM -0600, OpenBMC Patches wrote:<br>> From: William <</font></tt><a href=mailto:bjlinan@cn.ibm.com target=_blank><tt><font size=3 color=blue><u>bjlinan@cn.ibm.com</u></font></tt></a><tt><font size=3>><br>> <br>> ---<br>>  Makefile        |   1 +<br>>  globalhandler.C | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++<br>>  globalhandler.h |  12 ++++<br>>  3 files changed, 184 insertions(+)<br>>  create mode 100644 globalhandler.C<br>>  create mode 100644 globalhandler.h<br>> <br>> diff --git a/Makefile b/Makefile<br>> index 472bfaa..58e7310 100644<br>> --- a/Makefile<br>> +++ b/Makefile<br>> @@ -15,6 +15,7 @@ LIB_APP_OBJ = apphandler.o     \<br>>                ipmisensor.o

    \<br>>                storageaddsel.o

 \<br>>                transporthandler.o

 \<br>> +              globalhandler.o

 <br>>  <br>>  LIB_HOST_SRV_OBJ = host-services.o<br>>  <br>> diff --git a/globalhandler.C b/globalhandler.C<br>> new file mode 100644<br>> index 0000000..e68ea79<br>> --- /dev/null<br>> +++ b/globalhandler.C<br>> @@ -0,0 +1,171 @@<br>> +#include "globalhandler.h"<br>> +#include "ipmid-api.h"<br>> +#include <stdio.h><br>> +#include <string.h><br>> +#include <stdint.h><br>> +<br>> +const char  *control_object_name  =  "/org/openbmc/control/bmc0";<br>> +const char  *control_intf_name    =  "org.openbmc.control.Bmc";<br>> +<br>> +const char  *objectmapper_service_name =  "org.openbmc.objectmapper";<br>> +const char  *objectmapper_object_name  =  "/org/openbmc/objectmapper/objectmapper";<br>> +const char  *objectmapper_intf_name    =  "org.openbmc.objectmapper.ObjectMapper";<br>> +<br>> +void register_netfn_global_functions() __attribute__((constructor));<br>> +<br>> +int obj_mapper_get_connection(char** buf, const char* obj_path)<br>> +{<br>> +    sd_bus_error error = SD_BUS_ERROR_NULL;<br>> +    sd_bus_message *m = NULL;<br>> +    sd_bus *bus = NULL;<br>> +    char *temp_buf = NULL, *intf = NULL;<br>> +    size_t buf_size = 0;<br>> +    int r;<br>> +<br>> +    //Get the system bus where most system services are

provided.<br>> +    bus = ipmid_get_sd_bus_connection();<br>> +<br>> +    /*<br>> +     * Bus, service, object path, interface and method

are provided to call<br>> +     * the method.<br>> +     * Signatures and input arguments are provided by the

arguments at the<br>> +     * end.<br>> +     */<br>> +    r = sd_bus_call_method(bus,<br>> +            objectmapper_service_name,

                     /*

service to contact */<br>> +            objectmapper_object_name,

/* object path */<br>> +            objectmapper_intf_name,

  /* interface name */<br>> +            "GetObject",

             /* method name */<br>> +            &error,    

              /* object to return error

in */<br>> +            &m,      

                /* return message

on success */<br>> +            "s",    

                 /* input

signature */<br>> +            obj_path    

             /* first argument */<br>> +            );<br>> +<br>> +    if (r < 0) {<br>> +        fprintf(stderr, "Failed to issue

method call: %s\n", error.message);<br>> +        goto finish;<br>> +    }<br>> +<br>> +    // Get the key, aka, the connection name<br>> +    sd_bus_message_read(m, "a{sas}", 1, &temp_buf,

1, &intf);<br>> +    <br>> + /* <br>> +     * TODO: check the return code. Currently for no reason

the message<br>> +     * parsing of object mapper is always complaining about<br>> +     * "Device or resource busy", but the result

seems OK for now. Need<br>> +     *  further checks.<br>> +     */<br><br>I've seen this comment before and it worries me a bit. Why are we<br>getting the error? How do you know temp_buf is allocated properly? What<br>does it mean that the result "seems OK"?<br>You check return codes before and after this, but if you get unlucky<br>here you could easily segfault or read nonsense.<br><br>I think this issue should be figured out before merging the code, both<br>here and wherever else it appears.<br><br>Thanks,<br>Sam<br><br>> +<br>> +    buf_size = strlen(temp_buf) + 1;<br>> +    printf("IPMID connection name: %s\n", temp_buf);<br>> +    *buf = (char*)malloc(buf_size);<br>> +<br>> +    if (*buf == NULL) {<br>> +        fprintf(stderr, "Malloc failed for

warm reset");<br>> +        r = -1;<br>> +        goto finish;<br>> +    }<br>> +<br>> +    memcpy(*buf, temp_buf, buf_size);<br>> +<br>> +finish:<br>> +    sd_bus_error_free(&error);<br>> +    sd_bus_message_unref(m);<br>> +<br>> +    return r;<br>> +}<br>> +<br>> +int dbus_warm_reset()<br>> +{<br>> +    sd_bus_error error = SD_BUS_ERROR_NULL;<br>> +    sd_bus_message *m = NULL;<br>> +    sd_bus *bus = NULL;<br>> +    char* temp_buf = NULL;<br>> +    uint8_t* get_value = NULL;<br>> +    char* connection = NULL;<br>> +    int r, i;<br>> +<br>> +    r = obj_mapper_get_connection(&connection, control_object_name);<br>> +    if (r < 0) {<br>> +        fprintf(stderr, "Failed to get connection,

return value: %d.\n", r);<br>> +        goto finish;<br>> +    }<br>> +<br>> +    printf("connection: %s\n", connection);<br>> +<br>> +    // Open the system bus where most system services are

provided.<br>> +    bus = ipmid_get_sd_bus_connection();<br>> +    <br>> +    /*<br>> +     * Bus, service, object path, interface and method

are provided to call<br>> +     * the method.<br>> +     * Signatures and input arguments are provided by the

arguments at the<br>> +     * end.<br>> +  */<br>> +    r = sd_bus_call_method(bus,<br>> +            connection,    

     /* service to contact */<br>> +            control_object_name,  

                    /*

object path */<br>> +            control_intf_name,  

/* interface name */<br>> +            "warmReset",

      /* method name */<br>> +            &error,    

         /* object to return error in */<br>> +            &m,      

           /* return message on success */<br>> +            NULL,<br>> +            NULL<br>> +            );<br>> +<br>> +    if (r < 0) {<br>> +        fprintf(stderr, "Failed to issue

method call: %s\n", error.message);<br>> +        goto finish;<br>> +    }<br>> +<br>> +finish:<br>> +    sd_bus_error_free(&error);<br>> +    sd_bus_message_unref(m);<br>> +    free(connection);<br>> +<br>> +    return r;<br>> +}<br>> +<br>> +ipmi_ret_t ipmi_global_warm_reset(ipmi_netfn_t netfn, ipmi_cmd_t

cmd,<br>> +                    

         ipmi_request_t request, ipmi_response_t

response,<br>> +                    

         ipmi_data_len_t data_len, ipmi_context_t

context)<br>> +{<br>> +    printf("Handling GLOBAL warmReset Netfn:[0x%X],

Cmd:[0x%X]\n",netfn, cmd);<br>> +<br>> +    // TODO: call the correct dbus method for warmReset.<br>> +    dbus_warm_reset();<br>> +<br>> +    // Status code.<br>> +    ipmi_ret_t rc = IPMI_CC_OK;<br>> +    *data_len = 0;<br>> +    return rc;<br>> +}<br>> +<br>> +ipmi_ret_t ipmi_global_wildcard_handler(ipmi_netfn_t netfn, ipmi_cmd_t

cmd,<br>> +                    

         ipmi_request_t request, ipmi_response_t

response,<br>> +                    

         ipmi_data_len_t data_len, ipmi_context_t

context)<br>> +{<br>> +    printf("Handling WILDCARD Netfn:[0x%X], Cmd:[0x%X]\n",netfn,

cmd);<br>> +<br>> +    // Status code.<br>> +    ipmi_ret_t rc = IPMI_CC_OK;<br>> +<br>> +    *data_len = strlen("THIS IS WILDCARD");<br>> +<br>> +    // Now pack actual response<br>> +    memcpy(response, "THIS IS WILDCARD", *data_len);<br>> +<br>> +    return rc;<br>> +}<br>> +<br>> +void register_netfn_global_functions()<br>> +{<br>> +    printf("Registering NetFn:[0x%X], Cmd:[0x%X]\n",NETFUN_APP,

IPMI_CMD_WARM_RESET); > +    ipmi_register_callback(NETFUN_APP, IPMI_CMD_WARM_RESET,

NULL, ipmi_global_warm_reset); > + > +    printf("Registering NetFn:[0x%X], Cmd:[0x%X]\n",NETFUN_APP,

IPMI_CMD_WILDCARD); > +    ipmi_register_callback(NETFUN_APP, IPMI_CMD_WILDCARD,

NULL, ipmi_global_wildcard_handler);<br>> +<br>> + return;<br>> +}<br>> diff --git a/globalhandler.h b/globalhandler.h<br>> new file mode 100644<br>> index 0000000..608df3b<br>> --- /dev/null<br>> +++ b/globalhandler.h<br>> @@ -0,0 +1,12 @@<br>> +#ifndef __HOST_IPMI_GLOBAL_HANDLER_H__<br>> +#define __HOST_IPMI_GLOBAL_HANDLER_H__<br>> +<br>> +#include <stdint.h><br>> +<br>> +// Various GLOBAL operations under a single command.<br>> +enum ipmi_global_control_cmds : uint8_t<br>> +{<br>> +IPMI_CMD_WARM_RESET    = 0x02,<br>> +};<br>> +<br>> +#endif<br>> -- <br>> 2.6.4<br>> <br>> <br>> _______________________________________________<br>> openbmc mailing list<br>> </font></tt><a href=mailto:openbmc@lists.ozlabs.org target=_blank><tt><font size=3 color=blue><u>openbmc@lists.ozlabs.org</u></font></tt></a><tt><font size=3><br>> </font></tt><a href=https://lists.ozlabs.org/listinfo/openbmc target=_blank><tt><font size=3 color=blue><u>https://lists.ozlabs.org/listinfo/openbmc</u></font></tt></a><tt><font size=3><br><br>_______________________________________________<br>openbmc mailing list</font></tt><tt><font size=3 color=blue><u><br></u></font></tt><a href=mailto:openbmc@lists.ozlabs.org target=_blank><tt><font size=3 color=blue><u>openbmc@lists.ozlabs.org</u></font></tt></a><tt><font size=3 color=blue><u><br></u></font></tt><a href=https://lists.ozlabs.org/listinfo/openbmc target=_blank><tt><font size=3 color=blue><u>https://lists.ozlabs.org/listinfo/openbmc</u></font></tt></a><font size=3><br><br></font><br><font size=3>_______________________________________________<br>openbmc mailing list</font><font size=3 color=blue><u><br></u></font><a href=mailto:openbmc@lists.ozlabs.org target=_blank><font size=3 color=blue><u>openbmc@lists.ozlabs.org</u></font></a><font size=3 color=blue><u><br></u></font><a href=https://lists.ozlabs.org/listinfo/openbmc target=_blank><font size=3 color=blue><u>https://lists.ozlabs.org/listinfo/openbmc</u></font></a><br><BR>