Hi Abner,
Sorry for the delayed response.
Patch looks good to me.

Regards,
Mike.

On Mon, Apr 1, 2024 at 4:23 AM Chang, Abner <abner.ch...@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi Mike,
> Just letting you know that I merged this patch set to clean up the backlogs. 
> I believe your comment was addressed in V5. Just let me know if you still 
> find any improper code.
> Thanks
> Abner
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> > Abner via groups.io
> > Sent: Thursday, March 28, 2024 2:54 PM
> > To: Nickle Wang <nick...@nvidia.com>; devel@edk2.groups.io; Mike
> > Maslenkin <mike.maslen...@gmail.com>
> > Cc: Igor Kulchytskyy <ig...@ami.com>
> > Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH V5 1/2]
> > RedfishClientPkg: Set SettingsObject URI as the config language
> >
> > [AMD Official Use Only - General]
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > [AMD Official Use Only - General]
> >
> > Hi @Mike Maslenkin, would you like to review this patch again as I addressed
> > your comments in V5 patch set.
> > Thanks
> > Abner
> >
> > > -----Original Message-----
> > > From: Nickle Wang <nick...@nvidia.com>
> > > Sent: Tuesday, March 26, 2024 8:14 PM
> > > To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io
> > > Cc: Igor Kulchytskyy <ig...@ami.com>
> > > Subject: RE: [edk2-redfish-client][PATCH V5 1/2] RedfishClientPkg: Set
> > > SettingsObject URI as the config language
> > >
> > > Caution: This message originated from an External Source. Use proper
> > caution
> > > when opening attachments, clicking links, or responding.
> > >
> > >
> > > Thanks for working on this, Abner.
> > >
> > >
> > > Reviewed-by: Nickle Wang <nick...@nvidia.com>
> > >
> > > Regards,
> > > Nickle
> > >
> > > > -----Original Message-----
> > > > From: abner.ch...@amd.com <abner.ch...@amd.com>
> > > > Sent: Friday, March 22, 2024 10:41 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Igor Kulchytskyy <ig...@ami.com>; Nickle Wang
> > <nick...@nvidia.com>
> > > > Subject: [edk2-redfish-client][PATCH V5 1/2] RedfishClientPkg: Set
> > > SettingsObject
> > > > URI as the config language
> > > >
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > From: Abner Chang <abner.ch...@amd.com>
> > > >
> > > > Set SettingsObject URI in @Redfish.Settings resource as the config
> > language
> > > > which is the same as the config language of parent URI that mandates
> > > > @Redfish.Settings.
> > > > With this, we can find the config language of the properties in
> > SettingsObject
> > > URI.
> > > >
> > > > Signed-off-by: Abner Chang <abner.ch...@amd.com>
> > > > Cc: Igor Kulchytskyy <ig...@ami.com>
> > > > Co-authored-by: Nickle Wang <nick...@nvidia.com>
> > > > ---
> > > >  .../Library/RedfishFeatureUtilityLib.h        | 17 ++++
> > > >  .../RedfishFeatureUtilityLib.c                | 89 ++++++++++++++++---
> > > >  .../RedfishConfigLangMapDxe.c                 | 15 +++-
> > > >  3 files changed, 104 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > > b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > > index ba9ea01501..3c5f248eb7 100644
> > > > --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > > +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > > @@ -369,6 +369,23 @@ RedfishSetRedfishUri (
> > > >    IN    EFI_STRING  Uri
> > > >    );
> > > >
> > > > +/**
> > > > +
> > > > +  Save Redfish SettingsObject URI in database for further use.
> > > > +
> > > > +  @param[in]    ParentUri         Parent URI of @Redfish.Settings 
> > > > property.
> > > > +  @param[in]    SettingObjectUri  Redfish SettingsObject Uri to save.
> > > > +
> > > > +  @retval  EFI_INVALID_PARAMETER  SystemId is NULL or EMPTY
> > > > +  @retval  EFI_SUCCESS            Redfish uri is saved
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +SetRedfishSettingsObjectsUri (
> > > > +  IN EFI_STRING  ParentUri,
> > > > +  IN EFI_STRING  SettingObjectUri
> > > > +  );
> > > > +
> > > >  /**
> > > >
> > > >    Get the property name by given Configure Language.
> > > > diff --git
> > > >
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > > >
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > > > index cc2b37b796..dda80c4608 100644
> > > > ---
> > > >
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > > > +++
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUt
> > > > +++ ilityLib.c
> > > > @@ -2082,7 +2082,6 @@ GetConfigureLang (
> > > >    EFI_STRING  ResultStr;
> > > >    EFI_STRING  UnicodeUri;
> > > >    EFI_STATUS  Status;
> > > > -  EFI_STRING  StrFound;
> > > >
> > > >    if (IS_EMPTY_STRING (Uri)) {
> > > >      return NULL;
> > > > @@ -2101,18 +2100,6 @@ GetConfigureLang (
> > > >    }
> > > >
> > > >    ConfigLang = RedfishGetConfigLanguage (UnicodeUri);
> > > > -  if (ConfigLang == NULL) {
> > > > -    //
> > > > -    // @Redfish.Settings share the same schema as its parent.
> > > > -    // Remove "Settings" and try again.
> > > > -    //
> > > > -    StrFound = StrStr (UnicodeUri, L"/Settings");
> > > > -    if (StrFound != NULL) {
> > > > -      StrFound[0] = L'\0';
> > > > -      DEBUG ((REDFISH_DEBUG_TRACE, "%a: \"Settings\" found in URI, try:
> > > %s\n",
> > > > __func__, UnicodeUri));
> > > > -      ConfigLang = RedfishGetConfigLanguage (UnicodeUri);
> > > > -    }
> > > > -  }
> > > >
> > > >    FreePool (UnicodeUri);
> > > >
> > > > @@ -2172,6 +2159,64 @@ RedfishSetRedfishUri (
> > > >    return mConfigLangMapProtocol->Set (mConfigLangMapProtocol,
> > ConfigLang,
> > > > Uri);  }
> > > >
> > > > +/**
> > > > +
> > > > +  Save Redfish SettingsObject URI in database for further use.
> > > > +
> > > > +  @param[in]    ParentUri         Parent URI of @Redfish.Settings 
> > > > property.
> > > > +  @param[in]    SettingObjectUri  Redfish SettingsObject Uri to save.
> > > > +
> > > > +  @retval  EFI_INVALID_PARAMETER  ParentUri or SettingObjectUri is
> > NULL.
> > > > +  @retval  EFI_NOT_FOUND          Config language for ParentUri is not
> > found.
> > > > +  @retval  EFI_SUCCESS            Redfish URI is saved with 
> > > > corresponding
> > > > +                                  config language.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +SetRedfishSettingsObjectsUri (
> > > > +  IN EFI_STRING  ParentUri,
> > > > +  IN EFI_STRING  SettingObjectUri
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS  Status;
> > > > +  EFI_STRING  ConfigLang;
> > > > +
> > > > +  if ((ParentUri == NULL) || (SettingObjectUri == NULL)) {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +
> > > > +  //
> > > > +  // Check if the SettingsObject URI already in the database.
> > > > +  //
> > > > +  Status     = EFI_SUCCESS;
> > > > +  ConfigLang = RedfishGetConfigLanguage (SettingObjectUri);  if
> > > > + (ConfigLang == NULL) {
> > > > +    //
> > > > +    // No config language of SettingsObject URI is found.
> > > > +    // Get the config language of parent URI because the data model of
> > > > +    // SettingsObject URI resource is the same as the data model of 
> > > > parent
> > URI.
> > > > +    //
> > > > +    ConfigLang = RedfishGetConfigLanguage (ParentUri);
> > > > +    if (ConfigLang == NULL) {
> > > > +      DEBUG ((DEBUG_ERROR, "%a: Failed to get the config language of
> > parent
> > > > URI that mandates SettingsObject - %s.\n", __func__, ParentUri));
> > > > +      Status = EFI_NOT_FOUND;
> > > > +    } else {
> > > > +
> > > > +      // Set the config language of settings URI using parent's URI 
> > > > config
> > > language.
> > > > +      Status = RedfishSetRedfishUri (ConfigLang, SettingObjectUri);
> > > > +      if (EFI_ERROR (Status)) {
> > > > +        DEBUG ((DEBUG_ERROR, "%a: Fails to set the config language of
> > > > SettingsObject - %s.\n", __func__, SettingObjectUri));
> > > > +      } else {
> > > > +        DEBUG ((DEBUG_INFO, "%a: Set the config language of
> > SettingsObject -
> > > %s:
> > > > SUCCESS.\n", __func__, SettingObjectUri));
> > > > +      }
> > > > +      FreePool (ConfigLang); // Free the ConfigLang of parent URI.
> > > > +    }
> > > > +  } else {
> > > > +    FreePool (ConfigLang); // Free the ConfigLang of SettingObject URI.
> > > > +  }
> > > > +  return Status;
> > > > +}
> > > > +
> > > >  /**
> > > >
> > > >    Get @odata.id from give HTTP payload. It's call responsibility to 
> > > > release
> > > > returned buffer.
> > > > @@ -3532,6 +3577,7 @@ CompareRedfishBooleanArrayValues (
> > > >    payload and URI to pending settings. Caller has to release
> > "SettingPayload"
> > > and
> > > >    "SettingUri".
> > > >
> > > > +  @param[in]  RedfishService  Instance of REDFISH_SERVICE
> > > >    @param[in]  Payload         Payload that may contain 
> > > > "@Redfish.Settings"
> > > >    @param[out] SettingPayload  Payload keeps pending settings.
> > > >    @param[out] SettingUri      URI to pending settings.
> > > > @@ -3552,6 +3598,7 @@ GetPendingSettings (
> > > >    EDKII_JSON_VALUE  JsonValue;
> > > >    UINTN             Index;
> > > >    EFI_STATUS        Status;
> > > > +  EFI_STRING        StrFound;
> > > >
> > > >    if ((RedfishService == NULL) || (Payload == NULL) || 
> > > > (SettingResponse ==
> > > NULL)
> > > > || (SettingUri == NULL)) {
> > > >      return EFI_INVALID_PARAMETER;
> > > > @@ -3586,6 +3633,22 @@ GetPendingSettings (
> > > >        return Status;
> > > >      }
> > > >
> > > > +    //
> > > > +    // Setting URI exists, check if settings URI is valid or not.
> > > > +    //
> > > > +    StrFound = StrStr (*SettingUri, L"/Settings");
> > > > +    if (StrFound != NULL) {
> > > > +      DEBUG ((REDFISH_DEBUG_TRACE, "%a: \"Settings\" found in URI:
> > %s\n",
> > > > __func__, *SettingUri));
> > > > +    } else {
> > > > +      StrFound = StrStr (*SettingUri, L"/SD");
> > > > +      if (StrFound != NULL) {
> > > > +        DEBUG ((REDFISH_DEBUG_TRACE, "%a: \"SD\" found in URI: %s\n",
> > > > __func__, *SettingUri));
> > > > +      } else {
> > > > +        DEBUG ((DEBUG_ERROR, "%a: Not an valid @redfish.settings 
> > > > URI\n",
> > > > __func__, *SettingUri));
> > > > +        ASSERT (FALSE);
> > > > +      }
> > > > +    }
> > > > +
> > > >      return EFI_SUCCESS;
> > > >    }
> > > >
> > > > diff --git
> > > >
> > a/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> > > >
> > b/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> > > > index 8c93044580..4071e30c17 100644
> > > > ---
> > a/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> > > > +++
> > >
> > b/RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.c
> > > > @@ -2,6 +2,7 @@
> > > >
> > > >    (C) Copyright 2022 Hewlett Packard Enterprise Development LP<BR>
> > > >    Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> > > > reserved.
> > > > +  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> > > > + reserved.<BR>
> > > >
> > > >    SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > @@ -606,10 +607,16 @@ RedfishConfigLangMapSet (
> > > >    Status = EFI_NOT_FOUND;
> > > >    Target = FindConfigLangMapRecord (&Private-
> > >ConfigLangList.ListHeader,
> > > > ConfigLang, FALSE);
> > > >    if (Target != NULL) {
> > > > -    //
> > > > -    // Remove old one and create new one.
> > > > -    //
> > > > -    Status = DeleteConfigLangMapRecord (&Private->ConfigLangList,
> > Target);
> > > > +    if (Uri != NULL) {
> > > > +      if (StrCmp (Uri, Target->Uri) == 0) {
> > > > +        return EFI_SUCCESS;
> > > > +      }
> > > > +    } else {
> > > > +      //
> > > > +      // Remove old one and create new one.
> > > > +      //
> > > > +      Status = DeleteConfigLangMapRecord (&Private->ConfigLangList,
> > Target);
> > > > +    }
> > > >    }
> > > >
> > > >    //
> > > > --
> > > > 2.37.1.windows.1
> >
> >
> >
> > 
> >
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117286): https://edk2.groups.io/g/devel/message/117286
Mute This Topic: https://groups.io/mt/105086466/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to