On Wed, Sep 2, 2015 at 6:21 PM, hotmail <jordan_hargr...@hotmail.com> wrote:
>
>
>> Date: Thu, 20 Aug 2015 18:49:08 +0200
>> Subject: Re: [Ipmitool-devel] [PATCH] New PEF policy framework
>> From: zdenek.styb...@gmail.com
>> To: jordan_hargr...@hotmail.com
>> CC: ipmitool-devel@lists.sourceforge.net; p...@sgi.com
>>
>> On Mon, Aug 17, 2015 at 10:46 PM, Jordan Hargrave
>> wrote:
>>> Some time ago I proposed a patch to enable enabling/disabling individual
>>> PEF policy entries.
>>> Support for this is needed for one of our utilities. A new rework of the
>>> whole PEF framework was
>>> requested. I'm attaching a patch that (partially) implements this new
>>> scheme, please review.
>>>
>>
>> Hello Jordan,
>>
>> I'm not aware that rework of the whole PEF framework was prerequisite
>> for the patch you've posted. Patch you've posted has been rejected
>> because 'setpolicy' didn't and doesn't make sense in broad view. As a
>> follow up, new CLI PEF interface with "99%" coverage has been proposed
>> and cooperated on. Yes, discussion has quieted down and I'm to blame.
>> My apologies to Pat Donlin at SGI.
>> Only request that has been made towards you, resp. Dell, was to change
>> 'setpolicy' to 'policy set' which makes much more sense. As far as I'm
>> aware, this was the only condition for the patch to be accepted and
>> merged in. If you've interpreted any of it as a
>> dependency/prerequisite, then I'm sorry.
>>
>
> Ah well that makes things much easier! Would something like ipmitool pef
> policy and ipmitool pef policy be OK?
>
You mean to tell me the whole code review was for nothing? That you've
deliberately wasted my time? And now you're asking me to do yet
another code review? Well, good luck with that!
Z.
> Signed-off-by: Jordan Hargrave
> ---
> doc/ipmitool.1 | 5 +++
> include/ipmitool/ipmi_pef.h | 14 +++++++++
> lib/ipmi_pef.c | 75
> +++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 92 insertions(+), 2 deletions(-)
>
> diff --git a/doc/ipmitool.1 b/doc/ipmitool.1
> index 0d8adcc..2cf7c99 100644
> --- a/doc/ipmitool.1
> +++ b/doc/ipmitool.1
> @@ -2425,6 +2425,11 @@ processed by the BMC, etc).
> This command lists the PEF policy table entries. Each policy
> entry describes an alert destination. A policy set is a
> collection of table entries. PEF alert actions reference policy sets.
> +.TP
> +\fIpolicy\fP
> +.br
> +
> +This command enables or disables individual PEF policy table entries.
> .TP
> \fIlist\fP
> .br
> diff --git a/include/ipmitool/ipmi_pef.h b/include/ipmitool/ipmi_pef.h
> index cdea4ec..2b392ff 100644
> --- a/include/ipmitool/ipmi_pef.h
> +++ b/include/ipmitool/ipmi_pef.h
> @@ -555,6 +555,19 @@ struct pef_cfgparm_policy_table_entry {
> #ifdef HAVE_PRAGMA_PACK
> #pragma pack(1)
> #endif
> +struct pef_cfgparm_set_policy_table_entry
> +{
> + uint8_t id;
> + uint8_t sel;
> + struct pef_policy_entry entry;
> +} ATTRIBUTE_PACKING;
> +#ifdef HAVE_PRAGMA_PACK
> +#pragma pack(0)
> +#endif
> +
> +#ifdef HAVE_PRAGMA_PACK
> +#pragma pack(1)
> +#endif
> struct pef_cfgparm_system_guid {
> #define PEF_SYSTEM_GUID_USED_IN_PET 0x01
> uint8_t data1;
> @@ -936,6 +949,7 @@ BIT_DESC_MAP_LIST,
> #endif
>
> #define IPMI_CMD_GET_PEF_CAPABILITIES 0x10
> +#define IPMI_CMD_SET_PEF_CONFIG_PARMS 0x12
> #define IPMI_CMD_GET_PEF_CONFIG_PARMS 0x13
> #define IPMI_CMD_GET_LAST_PROCESSED_EVT_ID 0x15
> #define IPMI_CMD_GET_SYSTEM_GUID 0x37
> diff --git a/lib/ipmi_pef.c b/lib/ipmi_pef.c
> index 1beebf0..305283d 100644
> --- a/lib/ipmi_pef.c
> +++ b/lib/ipmi_pef.c
> @@ -256,6 +256,56 @@ ipmi_pef_get_policy_table(struct ipmi_intf * intf,
> return(tbl_size);
> }
>
> +static int
> +ipmi_pef_set_policy_table_entry(struct ipmi_intf * intf, int set, struct
> pef_cfgparm_policy_table_entry *entry)
> +{
> + struct ipmi_rs *rsp;
> + struct ipmi_rq req;
> + struct pef_cfgparm_set_policy_table_entry psel;
> +
> + memset(&req, 0, sizeof(req));
> + req.msg.netfn = IPMI_NETFN_SE;
> + req.msg.cmd = IPMI_CMD_SET_PEF_CONFIG_PARMS;
> + req.msg.data = &psel;
> + req.msg.data_len = sizeof(psel);
> +
> + memset(&psel, 0, sizeof(psel));
> + psel.id = PEF_CFGPARM_ID_PEF_ALERT_POLICY_TABLE_ENTRY;
> + psel.sel = set & 0x3F;
> + memcpy(&psel.entry, &entry->entry, sizeof(entry->entry));
> +
> + rsp = ipmi_pef_msg_exchange(intf, &req, "Set Alert policy table entry");
> + if (!rsp) {
> + lprintf(LOG_ERR, " **Error setting Alert policy table entry");
> + return -1;
> + }
> + return 0;
> +}
> +
> +static void
> +ipmi_pef_policy_enable(struct ipmi_intf * intf, int set, int enable)
> +{
> + struct pef_cfgparm_policy_table_entry * ptbl = NULL;
> + int tbl_size;
> +
> + tbl_size = ipmi_pef_get_policy_table(intf, &ptbl);
> + if (!tbl_size) {
> + if (ptbl) {
> + free(ptbl);
> + }
> + return;
> + }
> + if (set> 0 && set <= tbl_size) {
> + if (enable)
> + ptbl[set-1].entry.policy |= PEF_POLICY_ENABLED;
> + else
> + ptbl[set-1].entry.policy &= ~PEF_POLICY_ENABLED;
> + ipmi_pef_set_policy_table_entry(intf, set, &ptbl[set-1]);
> + } else {
> + lprintf(LOG_ERR, "Invalid policy index, valid range = (1..%d)",
> tbl_size);
> + }
> +}
> +
> static void
> ipmi_pef_print_lan_dest(struct ipmi_intf * intf, uint8_t ch, uint8_t dest)
> { /*
> @@ -858,6 +908,12 @@ ipmi_pef_get_info(struct ipmi_intf * intf)
> ipmi_pef_print_flags(&pef_b2s_actions, P_SUPP, actions);
> }
>
> +struct valstr endis[] = {
> + { .str = "disable", .val = 0x00 },
> + { .str = "enable", .val = 0x01 },
> + { .val = 0xFFFF },
> +};
> +
> int ipmi_pef_main(struct ipmi_intf * intf, int argc, char ** argv)
> { /*
> // PEF subcommand handling
> @@ -871,8 +927,23 @@ int ipmi_pef_main(struct ipmi_intf * intf, int argc,
> char ** argv)
> help = 1;
> else if (!strncmp(argv[0], "status", 6))
> ipmi_pef_get_status(intf);
> - else if (!strncmp(argv[0], "policy", 6))
> - ipmi_pef_list_policies(intf);
> + else if (!strncmp(argv[0], "policy", 6)) {
> + if (argc == 3) {
> + uint16_t pol,en;
> +
> + if (str2int(argv[1], &pol) != 0 ||
> + (en = str2val(argv[2], endis)) == 0xFFFF) {
> + lprintf(LOG_ERR, "Usage: ipmitool pef policy \n");
> + return (-1);
> + }
> + ipmi_pef_policy_enable(intf, pol, en);
> + } else if (argc == 1) {
> + ipmi_pef_list_policies(intf);
> + } else {
> + lprintf(LOG_NOTICE, "PEF policy commands: policy | policy ");
> + return (-1);
> + }
> + }
> else if (!strncmp(argv[0], "list", 4))
> ipmi_pef_list_entries(intf);
> else {
> --
> 1.8.3.1
>
------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
