[edk2-devel] [edk2-redfish-client][PATCH 0/4] change method for provisioning + minor fixes

[edk2-devel] [edk2-redfish-client][PATCH 0/4] change method for provisioning + minor fixes

[Mike Maslenkin]

This patch set contains minor fixes and the one major change related
to BIOS resources provisioning.

PR: https://github.com/tianocore/edk2-redfish-client/pull/64

Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112573): https://edk2.groups.io/g/devel/message/112573
Mute This Topic: https://groups.io/mt/103181637/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-redfish-client][PATCH 1/4] RedfishClientPkg: add check for NULL pointer to avoid ASSERT

[Mike Maslenkin]

Initially RedfishPlatformConfigGetConfigureLang could return success
even if ConfigureLangList is empty. After fixing this condition,
RedfishPlatformConfigGetConfigureLang returns an error, but this doesn't
help to avoid ASSERT because the error path is the same as for non-empty
list.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c       | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 4cb7621c25c4..0f0b050d7eba 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -3118,7 +3118,9 @@ LeaveFunction:
     FreePool (ConfigureLangBuffer);
   }
 
-  FreePool (ConfigureLangList);
+  if (ConfigureLangList != NULL) {
+    FreePool (ConfigureLangList);
+  }
 
   *NumberOfValues = (UINT32)ListCount;
   return FirstEmptyPropKeyValueList;
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112574): https://edk2.groups.io/g/devel/message/112574
Mute This Topic: https://groups.io/mt/103181638/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-redfish-client][PATCH 1/4] RedfishClientPkg: add check for NULL pointer to avoid ASSERT

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Friday, December 15, 2023 8:04 AM
> To: devel@edk2.groups.io
> Cc: abner.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [edk2-redfish-client][PATCH 1/4] RedfishClientPkg: add check for NULL
> pointer to avoid ASSERT
> 
> External email: Use caution opening links or attachments
> 
> 
> Initially RedfishPlatformConfigGetConfigureLang could return success even if
> ConfigureLangList is empty. After fixing this condition,
> RedfishPlatformConfigGetConfigureLang returns an error, but this doesn't help to
> avoid ASSERT because the error path is the same as for non-empty list.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c       | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> index 4cb7621c25c4..0f0b050d7eba 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> +++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUt
> +++ ilityLib.c
> @@ -3118,7 +3118,9 @@ LeaveFunction:
>      FreePool (ConfigureLangBuffer);
> 
>    }
> 
> 
> 
> -  FreePool (ConfigureLangList);
> 
> +  if (ConfigureLangList != NULL) {
> 
> +    FreePool (ConfigureLangList);
> 
> +  }
> 
> 
> 
>    *NumberOfValues = (UINT32)ListCount;
> 
>    return FirstEmptyPropKeyValueList;
> 
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112580): https://edk2.groups.io/g/devel/message/112580
Mute This Topic: https://groups.io/mt/103181638/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-redfish-client][PATCH 2/4] RedfishClientPkg: fix typo in EfiRestJsonStructureProtocolIsReady

[Mike Maslenkin]

Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
index 8b9bdc313832..85dc546120e2 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
@@ -562,7 +562,7 @@ EDKII_REDFISH_CONFIG_HANDLER_PROTOCOL  mRedfishConfigHandler = {
 **/
 VOID
 EFIAPI
-EfiRestJasonStructureProtocolIsReady (
+EfiRestJsonStructureProtocolIsReady (
   IN  EFI_EVENT  Event,
   IN  VOID       *Context
   )
@@ -829,7 +829,7 @@ RedfishResourceEntryPoint (
   EfiCreateProtocolNotifyEvent (
     &gEfiRestJsonStructureProtocolGuid,
     TPL_CALLBACK,
-    EfiRestJasonStructureProtocolIsReady,
+    EfiRestJsonStructureProtocolIsReady,
     NULL,
     &Registration
     );
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112575): https://edk2.groups.io/g/devel/message/112575
Mute This Topic: https://groups.io/mt/103181639/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-redfish-client][PATCH 2/4] RedfishClientPkg: fix typo in EfiRestJsonStructureProtocolIsReady

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Friday, December 15, 2023 8:04 AM
> To: devel@edk2.groups.io
> Cc: abner.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [edk2-redfish-client][PATCH 2/4] RedfishClientPkg: fix typo in
> EfiRestJsonStructureProtocolIsReady
> 
> External email: Use caution opening links or attachments
> 
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index 8b9bdc313832..85dc546120e2 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -562,7 +562,7 @@ EDKII_REDFISH_CONFIG_HANDLER_PROTOCOL
> mRedfishConfigHandler = {  **/
> 
>  VOID
> 
>  EFIAPI
> 
> -EfiRestJasonStructureProtocolIsReady (
> 
> +EfiRestJsonStructureProtocolIsReady (
> 
>    IN  EFI_EVENT  Event,
> 
>    IN  VOID       *Context
> 
>    )
> 
> @@ -829,7 +829,7 @@ RedfishResourceEntryPoint (
>    EfiCreateProtocolNotifyEvent (
> 
>      &gEfiRestJsonStructureProtocolGuid,
> 
>      TPL_CALLBACK,
> 
> -    EfiRestJasonStructureProtocolIsReady,
> 
> +    EfiRestJsonStructureProtocolIsReady,
> 
>      NULL,
> 
>      &Registration
> 
>      );
> 
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112581): https://edk2.groups.io/g/devel/message/112581
Mute This Topic: https://groups.io/mt/103181639/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-redfish-client][PATCH 3/4] RedfishClientPkg: fix access to unitialized variable.

[Mike Maslenkin]

It is possible that at the time of accessing to AsciiLocation pointer
the memory is not allocated.

Also gcc version 4.8.5 20150623 (Red Hat 4.8.5-44) emits a warning for
this case:

RedfishFeatureUtilityLib.c:1889:37: error: 'AsciiLocation' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       *Location = StrAsciiToUnicode (AsciiLocation);

Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c        | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 0f0b050d7eba..01c054ae3b70 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -1856,7 +1856,8 @@ GetEtagAndLocation (
   }
 
   if (Location != NULL) {
-    *Location = NULL;
+    *Location     = NULL;
+    AsciiLocation = NULL;
 
     if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
       Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_LOCATION);
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112576): https://edk2.groups.io/g/devel/message/112576
Mute This Topic: https://groups.io/mt/103181640/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-redfish-client][PATCH 3/4] RedfishClientPkg: fix access to unitialized variable.

[Nickle Wang via groups.io]

Thanks for catching this issue.


Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Friday, December 15, 2023 8:04 AM
> To: devel@edk2.groups.io
> Cc: abner.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [edk2-redfish-client][PATCH 3/4] RedfishClientPkg: fix access to
> unitialized variable.
> 
> External email: Use caution opening links or attachments
> 
> 
> It is possible that at the time of accessing to AsciiLocation pointer the memory is
> not allocated.
> 
> Also gcc version 4.8.5 20150623 (Red Hat 4.8.5-44) emits a warning for this case:
> 
> RedfishFeatureUtilityLib.c:1889:37: error: 'AsciiLocation' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>        *Location = StrAsciiToUnicode (AsciiLocation);
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c        | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> index 0f0b050d7eba..01c054ae3b70 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> +++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUt
> +++ ilityLib.c
> @@ -1856,7 +1856,8 @@ GetEtagAndLocation (
>    }
> 
> 
> 
>    if (Location != NULL) {
> 
> -    *Location = NULL;
> 
> +    *Location     = NULL;
> 
> +    AsciiLocation = NULL;
> 
> 
> 
>      if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> 
>        Header = HttpFindHeader (Response->HeaderCount, Response->Headers,
> HTTP_HEADER_LOCATION);
> 
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112582): https://edk2.groups.io/g/devel/message/112582
Mute This Topic: https://groups.io/mt/103181640/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property.

[Mike Maslenkin]

If EdkIIRedfishResourceConfigCheck fails according to the logic and
comment: new resources should be provisioned, so the POST method must be
used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is
BOOLEAN HttpPostMode, so we pass TRUE here.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index a26a1083cd74..4fd4845f3420 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -818,9 +818,9 @@ HandleResource (
     // The target property does not exist, do the provision to create property.
     //
     DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri));
-    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, Private->InformationExchange, FALSE);
+    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, Private->InformationExchange, TRUE);
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\n", __func__, Status));
+      DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode: %r\n", __func__, Status));
     }
 
     return Status;
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112577): https://edk2.groups.io/g/devel/message/112577
Mute This Topic: https://groups.io/mt/103181641/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property.

[Nickle Wang via groups.io]

Hi Mike,

Per Redfish specification 7.9 POST(create)

"The POST request is submitted to the resource collection to which the new resource will belong."

If this is not a collection resource, we cannot use POST method. And /redfish/v1/Systems/SYS_ID/Bios is not a collection resource. The allowed method returned from BMC for BIOS resource is usually "GET" and "PUT".

So, I think that the fourth parameter is still FALSE here. But I admit that the function header below is confusing and did not express above rule clearly.

/**
  Provisioning redfish resource by given URI.

  @param[in]   Schema              Redfish schema information.
  @param[in]   Uri                 Target URI to create resource.
  @param[in]   InformationExchange Pointer to RESOURCE_INFORMATION_EXCHANGE.
  @param[in]   HttpPostMode        TRUE if resource does not exist, HTTP POST method is used.
                                   FALSE if the resource exist but some of properties are missing,
                                   HTTP PUT method is used.

  @retval EFI_SUCCESS              Value is returned successfully.
  @retval Others                   Some error happened.

**/

Below is my suggestion.

  @param[in]   HttpPostMode        TRUE if target resource is a member of collection resource, HTTP POST method is used.
                                                             FALSE if target resource is non-collection resource, HTTP PUT method is used.

Do you think this helps to explain the use-case of fourth parameter more clearly?

Thanks,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Friday, December 15, 2023 8:04 AM
> To: devel@edk2.groups.io
> Cc: abner.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method
> while provisioning new property.
> 
> External email: Use caution opening links or attachments
> 
> 
> If EdkIIRedfishResourceConfigCheck fails according to the logic and
> comment: new resources should be provisioned, so the POST method must be
> used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is BOOLEAN
> HttpPostMode, so we pass TRUE here.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> index a26a1083cd74..4fd4845f3420 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -818,9 +818,9 @@ HandleResource (
>      // The target property does not exist, do the provision to create property.
> 
>      //
> 
>      DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri));
> 
> -    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, Private-
> >InformationExchange, FALSE);
> 
> +    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri,
> + Private->InformationExchange, TRUE);
> 
>      if (EFI_ERROR (Status)) {
> 
> -      DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\n",
> __func__, Status));
> 
> +      DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode:
> + %r\n", __func__, Status));
> 
>      }
> 
> 
> 
>      return Status;
> 
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112583): https://edk2.groups.io/g/devel/message/112583
Mute This Topic: https://groups.io/mt/103181641/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property.

[Mike Maslenkin]

[-- Attachment #1: Type: text/plain, Size: 5686 bytes --]


Hi Nickle,

> On 15. 12. 2023., at 04:53, Nickle Wang <nicklew@nvidia.com> wrote:
> 
> Hi Mike,
> 
> Per Redfish specification 7.9 POST(create)
> 
> "The POST request is submitted to the resource collection to which the new resource will belong."
> 
> If this is not a collection resource, we cannot use POST method. And /redfish/v1/Systems/SYS_ID/Bios is not a collection resource. The allowed method returned from BMC for BIOS resource is usually "GET" and "PUT".
> 
> So, I think that the fourth parameter is still FALSE here. But I admit that the function header below is confusing and did not express above rule clearly.
> 
> /**
>  Provisioning redfish resource by given URI.
> 
>  @param[in]   Schema              Redfish schema information.
>  @param[in]   Uri                 Target URI to create resource.
>  @param[in]   InformationExchange Pointer to RESOURCE_INFORMATION_EXCHANGE.
>  @param[in]   HttpPostMode        TRUE if resource does not exist, HTTP POST method is used.
>                                   FALSE if the resource exist but some of properties are missing,
>                                   HTTP PUT method is used.
> 
>  @retval EFI_SUCCESS              Value is returned successfully.
>  @retval Others                   Some error happened.
> 
> **/
> 
> Below is my suggestion.
> 
>  @param[in]   HttpPostMode        TRUE if target resource is a member of collection resource, HTTP POST method is used.
>                                                             FALSE if target resource is non-collection resource, HTTP PUT method is used.
> 
> Do you think this helps to explain the use-case of fourth parameter more clearly?
> 
> Thanks,
> Nickle


Seems like more comments need to be changed....
The idea behind this patch is the basis of the current implementation of RedfishClientPkg/Features/Bios/v1_0_9.
The EdkIIRedfishResourceConfigProvisioning function calls the EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL::Provisioning function,
this is RedfishResourceProvisioningResource() for this driver[1], then it calls RedfishProvisioningResourceCommon() where the logic
of this flag is completely changed [2].
So, for the current implementation of this flag, it is not a choice between POST and PUT, but between POST and PATCH (see [1]).
May be flag should not be inverted here [3]? 
The value of FALSE here (you expect PUT to be used) means the following ProvisioningBiosExistResource() function calls
ProvisioningBiosProperties() with ProvisionMode == TRUE. This ProvisionMode == TRUE forces PropertyChanged set into TRUE,
so finally ProvisioningBiosProperties() returns success even for elements that do not exist.

Here I mean elements of /redfish/v1/Systems/{SystemID}/Bios/Attributes. I'm in situation when Attributes exists,
but it is empty.

Currently the PUT method is not used anywhere in RedfishClientPkg/Features/Bios and "PUT back to instance" actually performs the PATCH.

I will drop this patch from the current PR until it becomes clear how this can be improved.

Regards,
Mike.
> 
>> -----Original Message-----
>> From: Mike Maslenkin <mike.maslenkin@gmail.com>
>> Sent: Friday, December 15, 2023 8:04 AM
>> To: devel@edk2.groups.io
>> Cc: abner.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
>> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
>> Subject: [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method
>> while provisioning new property.
>> 
>> External email: Use caution opening links or attachments
>> 
>> 
>> If EdkIIRedfishResourceConfigCheck fails according to the logic and
>> comment: new resources should be provisioned, so the POST method must be
>> used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is BOOLEAN
>> HttpPostMode, so we pass TRUE here.
>> 
>> Cc: Abner Chang <abner.chang@amd.com>
>> Cc: Igor Kulchytskyy <igork@ami.com>
>> Cc: Nickle Wang <nicklew@nvidia.com>
>> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
>> ---
>> RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>> index a26a1083cd74..4fd4845f3420 100644
>> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>> @@ -818,9 +818,9 @@ HandleResource (
>>     // The target property does not exist, do the provision to create property.
>> 
>>     //
>> 
>>     DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri));
>> 
>> -    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, Private-
>>> InformationExchange, FALSE);
>> 
>> +    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri,
>> + Private->InformationExchange, TRUE);
>> 
>>     if (EFI_ERROR (Status)) {
>> 
>> -      DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\n",
>> __func__, Status));
>> 
>> +      DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode:
>> + %r\n", __func__, Status));
>> 
>>     }
>> 
>> 
>> 
>>     return Status;
>> 
>> --
>> 2.32.0 (Apple Git-132)

> 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112610): https://edk2.groups.io/g/devel/message/112610
Mute This Topic: https://groups.io/mt/103181641/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 12041 bytes --]

Re: [edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property.

[Mike Maslenkin]

[-- Attachment #1: Type: text/plain, Size: 6802 bytes --]



> On 18. 12. 2023., at 03:33, Mike Maslenkin via groups.io <mike.maslenkin=gmail.com@groups.io> wrote:
> 
> 
> Hi Nickle,
> 
>> On 15. 12. 2023., at 04:53, Nickle Wang <nicklew@nvidia.com <mailto:nicklew@nvidia.com>> wrote:
>> 
>> Hi Mike,
>> 
>> Per Redfish specification 7.9 POST(create)
>> 
>> "The POST request is submitted to the resource collection to which the new resource will belong."
>> 
>> If this is not a collection resource, we cannot use POST method. And /redfish/v1/Systems/SYS_ID/Bios is not a collection resource. The allowed method returned from BMC for BIOS resource is usually "GET" and "PUT".
>> 
>> So, I think that the fourth parameter is still FALSE here. But I admit that the function header below is confusing and did not express above rule clearly.
>> 
>> /**
>>  Provisioning redfish resource by given URI.
>> 
>>  @param[in]   Schema              Redfish schema information.
>>  @param[in]   Uri                 Target URI to create resource.
>>  @param[in]   InformationExchange Pointer to RESOURCE_INFORMATION_EXCHANGE.
>>  @param[in]   HttpPostMode        TRUE if resource does not exist, HTTP POST method is used.
>>                                   FALSE if the resource exist but some of properties are missing,
>>                                   HTTP PUT method is used.
>> 
>>  @retval EFI_SUCCESS              Value is returned successfully.
>>  @retval Others                   Some error happened.
>> 
>> **/
>> 
>> Below is my suggestion.
>> 
>>  @param[in]   HttpPostMode        TRUE if target resource is a member of collection resource, HTTP POST method is used.
>>                                                             FALSE if target resource is non-collection resource, HTTP PUT method is used.
>> 
>> Do you think this helps to explain the use-case of fourth parameter more clearly?
>> 
>> Thanks,
>> Nickle
> 
> 
> Seems like more comments need to be changed....
> The idea behind this patch is the basis of the current implementation of RedfishClientPkg/Features/Bios/v1_0_9.
> The EdkIIRedfishResourceConfigProvisioning function calls the EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL::Provisioning function,
> this is RedfishResourceProvisioningResource() for this driver[1], then it calls RedfishProvisioningResourceCommon() where the logic
> of this flag is completely changed [2].
> So, for the current implementation of this flag, it is not a choice between POST and PUT, but between POST and PATCH (see [1]).
> May be flag should not be inverted here [3]? 
> The value of FALSE here (you expect PUT to be used) means the following ProvisioningBiosExistResource() function calls
> ProvisioningBiosProperties() with ProvisionMode == TRUE. This ProvisionMode == TRUE forces PropertyChanged set into TRUE,
> so finally ProvisioningBiosProperties() returns success even for elements that do not exist.
> 
> Here I mean elements of /redfish/v1/Systems/{SystemID}/Bios/Attributes. I'm in situation when Attributes exists,
> but it is empty.
> 
> Currently the PUT method is not used anywhere in RedfishClientPkg/Features/Bios and "PUT back to instance" actually performs the PATCH.
> 
> I will drop this patch from the current PR until it becomes clear how this can be improved.

Forgot to add the links:

[1] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c#L50
[2] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c#L518 <https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c#L518>
[3] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c#L68


> 
> Regards,
> Mike.
>> 
>>> -----Original Message-----
>>> From: Mike Maslenkin <mike.maslenkin@gmail.com <mailto:mike.maslenkin@gmail.com>>
>>> Sent: Friday, December 15, 2023 8:04 AM
>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>> Cc: abner.chang@amd.com <mailto:abner.chang@amd.com>; Nickle Wang <nicklew@nvidia.com <mailto:nicklew@nvidia.com>>;
>>> igork@ami.com <mailto:igork@ami.com>; Mike Maslenkin <mike.maslenkin@gmail.com <mailto:mike.maslenkin@gmail.com>>
>>> Subject: [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method
>>> while provisioning new property.
>>> 
>>> External email: Use caution opening links or attachments
>>> 
>>> 
>>> If EdkIIRedfishResourceConfigCheck fails according to the logic and
>>> comment: new resources should be provisioned, so the POST method must be
>>> used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is BOOLEAN
>>> HttpPostMode, so we pass TRUE here.
>>> 
>>> Cc: Abner Chang <abner.chang@amd.com <mailto:abner.chang@amd.com>>
>>> Cc: Igor Kulchytskyy <igork@ami.com <mailto:igork@ami.com>>
>>> Cc: Nickle Wang <nicklew@nvidia.com <mailto:nicklew@nvidia.com>>
>>> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com <mailto:mike.maslenkin@gmail.com>>
>>> ---
>>> RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>>> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>>> index a26a1083cd74..4fd4845f3420 100644
>>> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>>> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
>>> @@ -818,9 +818,9 @@ HandleResource (
>>>     // The target property does not exist, do the provision to create property.
>>> 
>>>     //
>>> 
>>>     DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri));
>>> 
>>> -    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, Private-
>>>> InformationExchange, FALSE);
>>> 
>>> +    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri,
>>> + Private->InformationExchange, TRUE);
>>> 
>>>     if (EFI_ERROR (Status)) {
>>> 
>>> -      DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\n",
>>> __func__, Status));
>>> 
>>> +      DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode:
>>> + %r\n", __func__, Status));
>>> 
>>>     }
>>> 
>>> 
>>> 
>>>     return Status;
>>> 
>>> --
>>> 2.32.0 (Apple Git-132)
> 
>> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112611): https://edk2.groups.io/g/devel/message/112611
Mute This Topic: https://groups.io/mt/103181641/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 14317 bytes --]

Re: [edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property.

[Chang, Abner via groups.io]

[-- Attachment #1: Type: text/plain, Size: 7039 bytes --]

[AMD Official Use Only - General]

Yes, this is something we have to make it clear to support PUT, POST and PATCH for different scenarios. Agree we can just drop 4/4 for now, I will add this to the TODO list.

Thanks
Abner

From: M M <mike.maslenkin@gmail.com>
Sent: Monday, December 18, 2023 8:58 AM
To: devel@edk2.groups.io; mike.maslenkin@gmail.com
Cc: Nickle Wang <nicklew@nvidia.com>; Chang, Abner <Abner.Chang@amd.com>; igork@ami.com
Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property.

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.




On 18. 12. 2023., at 03:33, Mike Maslenkin via groups.io<http://groups.io> <mike.maslenkin=gmail.com@groups.io<mailto:mike.maslenkin=gmail.com@groups.io>> wrote:


Hi Nickle,


On 15. 12. 2023., at 04:53, Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>> wrote:

Hi Mike,

Per Redfish specification 7.9 POST(create)

"The POST request is submitted to the resource collection to which the new resource will belong."

If this is not a collection resource, we cannot use POST method. And /redfish/v1/Systems/SYS_ID/Bios is not a collection resource. The allowed method returned from BMC for BIOS resource is usually "GET" and "PUT".

So, I think that the fourth parameter is still FALSE here. But I admit that the function header below is confusing and did not express above rule clearly.

/**
 Provisioning redfish resource by given URI.

 @param[in]   Schema              Redfish schema information.
 @param[in]   Uri                 Target URI to create resource.
 @param[in]   InformationExchange Pointer to RESOURCE_INFORMATION_EXCHANGE.
 @param[in]   HttpPostMode        TRUE if resource does not exist, HTTP POST method is used.
                                  FALSE if the resource exist but some of properties are missing,
                                  HTTP PUT method is used.

 @retval EFI_SUCCESS              Value is returned successfully.
 @retval Others                   Some error happened.

**/

Below is my suggestion.

 @param[in]   HttpPostMode        TRUE if target resource is a member of collection resource, HTTP POST method is used.
                                                            FALSE if target resource is non-collection resource, HTTP PUT method is used.

Do you think this helps to explain the use-case of fourth parameter more clearly?

Thanks,
Nickle


Seems like more comments need to be changed....
The idea behind this patch is the basis of the current implementation of RedfishClientPkg/Features/Bios/v1_0_9.
The EdkIIRedfishResourceConfigProvisioning function calls the EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL::Provisioning function,
this is RedfishResourceProvisioningResource() for this driver[1], then it calls RedfishProvisioningResourceCommon() where the logic
of this flag is completely changed [2].
So, for the current implementation of this flag, it is not a choice between POST and PUT, but between POST and PATCH (see [1]).
May be flag should not be inverted here [3]?
The value of FALSE here (you expect PUT to be used) means the following ProvisioningBiosExistResource() function calls
ProvisioningBiosProperties() with ProvisionMode == TRUE. This ProvisionMode == TRUE forces PropertyChanged set into TRUE,
so finally ProvisioningBiosProperties() returns success even for elements that do not exist.

Here I mean elements of /redfish/v1/Systems/{SystemID}/Bios/Attributes. I'm in situation when Attributes exists,
but it is empty.

Currently the PUT method is not used anywhere in RedfishClientPkg/Features/Bios and "PUT back to instance" actually performs the PATCH.

I will drop this patch from the current PR until it becomes clear how this can be improved.

Forgot to add the links:

[1] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c#L50
[2] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c#L518
[3] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c#L68




Regards,
Mike.



-----Original Message-----
From: Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>
Sent: Friday, December 15, 2023 8:04 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: abner.chang@amd.com<mailto:abner.chang@amd.com>; Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>;
igork@ami.com<mailto:igork@ami.com>; Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>
Subject: [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method
while provisioning new property.

External email: Use caution opening links or attachments


If EdkIIRedfishResourceConfigCheck fails according to the logic and
comment: new resources should be provisioned, so the POST method must be
used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is BOOLEAN
HttpPostMode, so we pass TRUE here.

Cc: Abner Chang <abner.chang@amd.com<mailto:abner.chang@amd.com>>
Cc: Igor Kulchytskyy <igork@ami.com<mailto:igork@ami.com>>
Cc: Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>
---
RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index a26a1083cd74..4fd4845f3420 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -818,9 +818,9 @@ HandleResource (
    // The target property does not exist, do the provision to create property.

    //

    DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri));

-    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, Private-

InformationExchange, FALSE);

+    Status = EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri,
+ Private->InformationExchange, TRUE);

    if (EFI_ERROR (Status)) {

-      DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\n",
__func__, Status));

+      DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode:
+ %r\n", __func__, Status));

    }



    return Status;

--
2.32.0 (Apple Git-132)






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112634): https://edk2.groups.io/g/devel/message/112634
Mute This Topic: https://groups.io/mt/103181641/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 17805 bytes --]