From: "Chang, Abner" <abner.chang@amd.com>
To: Nickle Wang <nicklew@nvidia.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Igor Kulchytskyy <igork@ami.com>
Subject: Re: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish Resource Addendum Library
Date: Mon, 15 May 2023 03:19:33 +0000 [thread overview]
Message-ID: <MN2PR12MB3966948709CBD9170EF11180EA789@MN2PR12MB3966.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20230512072301.17881-1-nicklew@nvidia.com>
[AMD Official Use Only - General]
Hi Nickle,
I read though this patch and was confused by function naming and the function description in the function header. I think we have to revise it and make the description of functionality clear for user to follow.
I apologize if we ever discussed this before however I don't have the synchronous mind now.
1. Protocol definitions
This protocol is provided by platform that would like to add an additional Redfish resource in "Oem" property or to add other platform specific property in the name other than "Oem".
Both of two protocol interfaces give platform an chance to add "OEM" or other Redfish property, however the naming of function prototypes are not consistent. The naming of "OemCallback" and "ProvisioningCallback" are also not consistent.
How do you think the naming below,
struct _EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL {
UINT64 Revision;
EDKII_REDFISH_RESOURCE_ADDENDUM_OEM AddAddendumOemRedfishResource; ///<
EDKII_REDFISH_RESOURCE_ADDENDUM_OTHER AddAddendumOtherRedfishResource; ///<
};
Would above look simple and clear? Also, please add some comments to these two functions if you are agree with this change.
2. Function naming in the library,
RedfishGetOemData -> RedfishGetAddendumOemData
RedfishGetAddendumData-> RedfishGetAddendumOtherData
3. The current description in function header looks to me not quite clear. Please elaborates some more functionality in the function header and parameters
4. Shall we have to update readme.md?
Thanks
Abner
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Friday, May 12, 2023 3:23 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>
> Subject: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish
> Resource Addendum Library
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Implement RedfishAddendumLib to support Redfish Resource Addendum
> Protocol.
> Feature driver calls this library to get addendum data before providing
> data to Redfish service
>
> Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> ---
> RedfishClientPkg/RedfishClientLibs.dsc.inc | 4 +-
> RedfishClientPkg/RedfishClientPkg.dsc | 2 +
> .../RedfishAddendumLib/RedfishAddendumLib.inf | 40 +++
> .../Include/Library/RedfishAddendumLib.h | 65 +++++
> .../RedfishAddendumLib/RedfishAddendumLib.c | 262
> ++++++++++++++++++
> 5 files changed, 372 insertions(+), 1 deletion(-)
> create mode 100644
> RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> create mode 100644
> RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> create mode 100644
> RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
>
> diff --git a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> index fe0c4b08..5ea38015 100644
> --- a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> +++ b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> @@ -2,10 +2,11 @@
> # Redfish DSC include file for [LibraryClasses*] section of all Architectures.
> #
> # This file can be included to the [LibraryClasses*] section(s) of a platform
> DSC file
> -# by using "!include RedfishPkg/RedfisLibs.dsc.inc" to specify the library
> instances
> +# by using "!include RedfishPkg/RedfishLibs.dsc.inc" to specify the library
> instances
> # of EDKII network library classes.
> #
> # (C) Copyright 2021-2022 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -37,3 +38,4 @@
>
> EdkIIRedfishResourceConfigLib|RedfishClientPkg/Library/EdkIIRedfishResou
> rceConfigLib/EdkIIRedfishResourceConfigLib.inf
>
> RedfishEventLib|RedfishClientPkg/Library/RedfishEventLib/RedfishEventLib.i
> nf
>
> RedfishVersionLib|RedfishClientPkg/Library/RedfishVersionLib/RedfishVersi
> onLib.inf
> +
> RedfishAddendumLib|RedfishClientPkg/Library/RedfishAddendumLib/Redfi
> shAddendumLib.inf
> diff --git a/RedfishClientPkg/RedfishClientPkg.dsc
> b/RedfishClientPkg/RedfishClientPkg.dsc
> index 2b2149cc..d3b645b6 100644
> --- a/RedfishClientPkg/RedfishClientPkg.dsc
> +++ b/RedfishClientPkg/RedfishClientPkg.dsc
> @@ -2,6 +2,7 @@
> # Redfish Client Package
> #
> # (C) Copyright 2021 Hewlett-Packard Enterprise Development LP.
> +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -49,5 +50,6 @@
>
>
> RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> RedfishClientPkg/PrivateLibrary/RedfishLib/RedfishLib.inf
> + RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
>
> !include RedfishClientPkg/RedfishClient.dsc.inc
> diff --git
> a/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> new file mode 100644
> index 00000000..1ecfaa69
> --- /dev/null
> +++
> b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> @@ -0,0 +1,40 @@
> +## @file
> +#
> +# Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010006
> + BASE_NAME = RedfishAddendumLib
> + FILE_GUID = 7B227D39-746D-4247-8291-27B0FA79A7AF
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = RedfishAddendumLib| DXE_DRIVER
> UEFI_DRIVER
> +
> +#
> +# VALID_ARCHITECTURES = IA32 X64 EBC
> +#
> +
> +[Sources]
> + RedfishAddendumLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + RedfishPkg/RedfishPkg.dec
> + RedfishClientPkg/RedfishClientPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + DebugLib
> + UefiBootServicesTableLib
> + MemoryAllocationLib
> + UefiLib
> + JsonLib
> +
> +[Protocols]
> + gEdkIIRedfishResourceAddendumProtocolGuid ## CONSUMED ##
> +
> diff --git a/RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> b/RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> new file mode 100644
> index 00000000..d77347fd
> --- /dev/null
> +++ b/RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> @@ -0,0 +1,65 @@
> +/** @file
> + This file defines the Redfish addendum library interface.
> +
> + Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef REDFISH_ADDENDUM_LIB_H_
> +#define REDFISH_ADDENDUM_LIB_H_
> +
> +#include <Uefi.h>
> +#include <Library/JsonLib.h>
> +#include <Protocol/EdkIIRedfishResourceAddendumProtocol.h>
> +
> +/**
> + Provide redfish resource with addendum data in given schema.
> +
> + @param[in] Uri Uri of input resource.
> + @param[in] Schema Redfish schema string.
> + @param[in] Version Schema version string.
> + @param[in] JsonText Input resource in JSON format string.
> + @param[out] JsonWithAddendum The input resource with addendum
> value attached.
> +
> + @retval EFI_SUCCESS Addendum data is attached.
> + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> + @retval EFI_UNSUPPORTED No addendum data is required in given
> schema.
> + @retval Others Some error happened.
> +
> +**/
> +EFI_STATUS
> +RedfishGetAddendumData (
> + IN EFI_STRING Uri,
> + IN CHAR8 *Schema,
> + IN CHAR8 *Version,
> + IN CHAR8 *JsonText,
> + OUT CHAR8 **JsonWithAddendum
> + );
> +
> +/**
> + Provide redfish OEM resource with given schema information.
> +
> + @param[in] Uri Uri of input resource.
> + @param[in] Schema Redfish schema string.
> + @param[in] Version Schema version string.
> + @param[in] JsonText Input resource in JSON format string.
> + @param[out] JsonWithOem The input resource with OEM value
> attached.
> +
> + @retval EFI_SUCCESS OEM data is attached.
> + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> + @retval EFI_UNSUPPORTED No OEM data is required in given schema.
> + @retval Others Some error happened.
> +
> +**/
> +EFI_STATUS
> +RedfishGetOemData (
> + IN EFI_STRING Uri,
> + IN CHAR8 *Schema,
> + IN CHAR8 *Version,
> + IN CHAR8 *JsonText,
> + OUT CHAR8 **JsonWithOem
> + );
> +
> +#endif
> diff --git
> a/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> new file mode 100644
> index 00000000..6aa6e3cc
> --- /dev/null
> +++
> b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> @@ -0,0 +1,262 @@
> +/** @file
> + Redfish addendum library helps Redfish application to get addendum data
> and OEM
> + data from platform driver.
> +
> + Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <RedfishBase.h>
> +
> +#include <Library/UefiLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/RedfishAddendumLib.h>
> +
> +/**
> +
> + Convert Unicode string to ASCII string. It's call responsibility to release
> returned buffer.
> +
> + @param[in] UnicodeStr Unicode string to convert.
> +
> + @retval CHAR8 * ASCII string returned.
> + @retval NULL Errors occur.
> +
> +**/
> +CHAR8 *
> +UnicodeToAscii (
> + IN EFI_STRING UnicodeStr
> + )
> +{
> + CHAR8 *AsciiStr;
> + UINTN AsciiStrSize;
> + EFI_STATUS Status;
> +
> + if (IS_EMPTY_STRING (UnicodeStr)) {
> + return NULL;
> + }
> +
> + AsciiStrSize = StrLen (UnicodeStr) + 1;
> + AsciiStr = AllocatePool (AsciiStrSize);
> + if (AsciiStr == NULL) {
> + return NULL;
> + }
> +
> + Status = UnicodeStrToAsciiStrS (UnicodeStr, AsciiStr, AsciiStrSize);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "UnicodeStrToAsciiStrS failed: %r\n", Status));
> + FreePool (AsciiStr);
> + return NULL;
> + }
> +
> + return AsciiStr;
> +}
> +
> +/**
> + Provide redfish resource with addendum data in given schema.
> +
> + @param[in] Uri Uri of input resource.
> + @param[in] Schema Redfish schema string.
> + @param[in] Version Schema version string.
> + @param[in] JsonText Input resource in JSON format string.
> + @param[out] JsonWithAddendum The input resource with addendum
> value attached.
> + It is the caller's responsibility to free this buffer.
> +
> + @retval EFI_SUCCESS Addendum data is attached.
> + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> + @retval EFI_UNSUPPORTED No addendum data is required in given
> schema.
> + @retval Others Some error happened.
> +
> +**/
> +EFI_STATUS
> +RedfishGetAddendumData (
> + IN EFI_STRING Uri,
> + IN CHAR8 *Schema,
> + IN CHAR8 *Version,
> + IN CHAR8 *JsonText,
> + OUT CHAR8 **JsonWithAddendum
> + )
> +{
> + REDFISH_RESOURCE_SCHEMA_INFO SchemaInfo;
> + EDKII_JSON_VALUE JsonValue;
> + EFI_STATUS Status;
> + EFI_HANDLE *HandleBuffer;
> + UINTN NumberOfHandles;
> + UINTN Index;
> + EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL *Protocol;
> +
> + if (IS_EMPTY_STRING (Uri) || IS_EMPTY_STRING (Schema) ||
> IS_EMPTY_STRING (Version) || IS_EMPTY_STRING (JsonText) ||
> (JsonWithAddendum == NULL)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *JsonWithAddendum = NULL;
> + SchemaInfo.Uri = UnicodeToAscii (Uri);
> + SchemaInfo.Schema = Schema;
> + SchemaInfo.Version = Version;
> + NumberOfHandles = 0;
> + HandleBuffer = NULL;
> +
> + JsonValue = JsonLoadString (JsonText, 0, NULL);
> + if ((JsonValue == NULL) || !JsonValueIsObject (JsonValue)) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + Status = gBS->LocateHandleBuffer (
> + ByProtocol,
> + &gEdkIIRedfishResourceAddendumProtocolGuid,
> + NULL,
> + &NumberOfHandles,
> + &HandleBuffer
> + );
> + if (EFI_ERROR (Status)) {
> + goto ON_RELEASE;
> + }
> +
> + for (Index = 0; Index < NumberOfHandles; Index++) {
> + Status = gBS->HandleProtocol (
> + HandleBuffer[Index],
> + &gEdkIIRedfishResourceAddendumProtocolGuid,
> + (VOID **)&Protocol
> + );
> + if (EFI_ERROR (Status)) {
> + continue;
> + }
> +
> + Status = Protocol->ProvisioningCallback (Protocol, &SchemaInfo,
> JsonValue);
> + if (!EFI_ERROR (Status)) {
> + *JsonWithAddendum = JsonDumpString (JsonValue,
> EDKII_JSON_COMPACT);
> + break;
> + }
> + }
> +
> +ON_RELEASE:
> +
> + if (HandleBuffer != NULL) {
> + FreePool (HandleBuffer);
> + }
> +
> + if (JsonValue != NULL) {
> + JsonValueFree (JsonValue);
> + }
> +
> + if (SchemaInfo.Uri != NULL) {
> + FreePool (SchemaInfo.Uri);
> + }
> +
> + return Status;
> +}
> +
> +/**
> + Provide redfish OEM resource with given schema information.
> +
> + @param[in] Uri Uri of input resource.
> + @param[in] Schema Redfish schema string.
> + @param[in] Version Schema version string.
> + @param[in] JsonText Input resource in JSON format string.
> + @param[out] JsonWithOem The input resource with OEM value
> attached.
> +
> + @retval EFI_SUCCESS OEM data is attached.
> + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> + @retval EFI_UNSUPPORTED No OEM data is required in given schema.
> + @retval Others Some error happened.
> +
> +**/
> +EFI_STATUS
> +RedfishGetOemData (
> + IN EFI_STRING Uri,
> + IN CHAR8 *Schema,
> + IN CHAR8 *Version,
> + IN CHAR8 *JsonText,
> + OUT CHAR8 **JsonWithOem
> + )
> +{
> + REDFISH_RESOURCE_SCHEMA_INFO SchemaInfo;
> + EDKII_JSON_VALUE JsonValue;
> + EDKII_JSON_VALUE JsonValueOem;
> + EFI_STATUS Status;
> + EFI_HANDLE *HandleBuffer;
> + UINTN NumberOfHandles;
> + UINTN Index;
> + EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL *Protocol;
> +
> + if (IS_EMPTY_STRING (Uri) || IS_EMPTY_STRING (Schema) ||
> IS_EMPTY_STRING (Version) || IS_EMPTY_STRING (JsonText) ||
> (JsonWithOem == NULL)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *JsonWithOem = NULL;
> + SchemaInfo.Uri = UnicodeToAscii (Uri);
> + SchemaInfo.Schema = Schema;
> + SchemaInfo.Version = Version;
> + JsonValue = NULL;
> + JsonValueOem = NULL;
> + NumberOfHandles = 0;
> + HandleBuffer = NULL;
> +
> + JsonValue = JsonLoadString (JsonText, 0, NULL);
> + if ((JsonValue == NULL) || !JsonValueIsObject (JsonValue)) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + JsonValueOem = JsonValueInitObject ();
> + if ((JsonValueOem == NULL) || !JsonValueIsObject (JsonValueOem)) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto ON_RELEASE;
> + }
> +
> + Status = gBS->LocateHandleBuffer (
> + ByProtocol,
> + &gEdkIIRedfishResourceAddendumProtocolGuid,
> + NULL,
> + &NumberOfHandles,
> + &HandleBuffer
> + );
> + if (EFI_ERROR (Status)) {
> + goto ON_RELEASE;
> + }
> +
> + for (Index = 0; Index < NumberOfHandles; Index++) {
> + Status = gBS->HandleProtocol (
> + HandleBuffer[Index],
> + &gEdkIIRedfishResourceAddendumProtocolGuid,
> + (VOID **)&Protocol
> + );
> + if (EFI_ERROR (Status)) {
> + continue;
> + }
> +
> + Status = Protocol->OemCallback (Protocol, &SchemaInfo, JsonValueOem);
> + if (!EFI_ERROR (Status)) {
> + Status = JsonObjectSetValue (JsonValue, "Oem", JsonValueOem);
> + if (!EFI_ERROR (Status)) {
> + *JsonWithOem = JsonDumpString (JsonValue, EDKII_JSON_COMPACT);
> + }
> +
> + break;
> + }
> + }
> +
> +ON_RELEASE:
> +
> + if (HandleBuffer != NULL) {
> + FreePool (HandleBuffer);
> + }
> +
> + if (JsonValue != NULL) {
> + JsonValueFree (JsonValue);
> + }
> +
> + if (JsonValueOem != NULL) {
> + JsonValueFree (JsonValueOem);
> + }
> +
> + if (SchemaInfo.Uri != NULL) {
> + FreePool (SchemaInfo.Uri);
> + }
> +
> + return Status;
> +}
> --
> 2.17.1
next prev parent reply other threads:[~2023-05-15 3:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-12 7:23 [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish Resource Addendum Library Nickle Wang
2023-05-15 3:19 ` Chang, Abner [this message]
2023-05-15 7:39 ` Nickle Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR12MB3966948709CBD9170EF11180EA789@MN2PR12MB3966.namprd12.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox