Hi Samer
I agree with you to keep DEBUG message and add a another "Download
failed "message to give user a hint, as for adding a URL where the file is
being downloaded, But we already print it in the HttpBootClient.c file
Best Regards
Lubo
-----Original Message-----
From: Fu, Siyuan
Sent: Friday, May 20, 2016 10:04 AM
To: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; Zhang, Lubo
<lubo.zh...@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
Subject: RE: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.
Hi, Samer
I think in PXE this issue was solved by the
EFI_PXE_BASE_CODE_CALLBACK_PROTOCOL, which allows the platform owner to provide
an callback implementation which will take precedence over the default
implementation in PXE driver. The callback protocol allows the platform owner
to display the process messages or even abort the PXE boot. Do you think we
could reference this practice in HTTP boot?
Best Regards
Siyuan
> -----Original Message-----
> From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Thursday, May 19, 2016 9:46 PM
> To: Zhang, Lubo <lubo.zh...@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>;
> Wu, Jiaxin <jiaxin...@intel.com>
> Subject: RE: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.
>
> Thanks Zhang.
>
> I know this makes sense (and does help during lengthy download), and I
> also know that PXE does something similar. I am only concerned about
> this happening after BGRT logo is displayed on the screen (if PXE/HTTP
> boot options are tried after a failed HDD boot option for instance).
> The result is that the messages will be printed on top of the already
> displayed logo, which is also reported in ACPI BGRT table. It may not
> be a big issue, but it did result in some user complaints at times. What do
> you think?
>
> Other than that, I would improve the messaging a bit. See feedback below.
>
>
>
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zhang Lubo
> Sent: Wednesday, May 18, 2016 10:53 PM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting <ting...@intel.com>; Fu Siyuan <siyuan...@intel.com>; Wu
> Jiaxin <jiaxin...@intel.com>
> Subject: [edk2] [patch] NetworkPkg: Refine codes of Http boot driver.
>
> When downloading a big image as ram disk iso,we can print the progress
> message on screen to enhance the user experience.
>
> Cc: Ye Ting <ting...@intel.com>
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Wu Jiaxin <jiaxin...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo <lubo.zh...@intel.com>
> ---
> NetworkPkg/HttpBootDxe/HttpBootClient.c | 29
> +++++++++++++++++++++++++++++
> NetworkPkg/HttpBootDxe/HttpBootSupport.c | 4 ++--
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 46cf9ca..9b2a8bd 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -753,10 +753,12 @@ HttpBootGetBootFile (
> HTTP_BOOT_CACHE_CONTENT *Cache;
> UINT8 *Block;
> CHAR16 *Url;
> BOOLEAN IdentityMode;
> UINTN ReceivedSize;
> + UINTN Ratio;
> + UINTN NewRatio;
>
> ASSERT (Private != NULL);
> ASSERT (Private->HttpCreated);
>
> if (BufferSize == NULL || ImageType == NULL) { @@ -992,10 +994,13
> @@ HttpBootGetBootFile (
> //
> // 3.4.2, start the message-body download, the identity and
> chunked transfer-coding
> // is handled in different path here.
> //
> ZeroMem (&ResponseBody, sizeof (HTTP_IO_RESPONSE_DATA));
> + AsciiPrint ("\n");
> + AsciiPrint ("\n Downloading NBP file... ");
>
>
> [Samer] How about printing the URL where the file is being downloaded
> from so the user knows that this is happening from HTTP Boot and not
> PXE (since the messages are identical)
>
>
>
> + Ratio = 0;
> if (IdentityMode) {
> //
> // In identity transfer-coding there is no need to parse the message
> body,
> // just download the message body to the user provided buffer directly.
> //
> @@ -1010,10 +1015,25 @@ HttpBootGetBootFile (
> );
> if (EFI_ERROR (Status)) {
> goto ERROR_6;
> }
> ReceivedSize += ResponseBody.BodyLength;
> + //
> + // Display the download progress.
> + //
> + NewRatio = (ReceivedSize * 100) / ContentLength;
> + if (NewRatio != Ratio) {
> + if (Ratio !=0) {
> + if (Ratio >=10) {
> + AsciiPrint ("\b\b\b");
> + } else {
> + AsciiPrint ("\b\b");
> + }
> + }
> + Ratio = NewRatio;
> + AsciiPrint ("%d%%", NewRatio);
> + }
> }
> } else {
> //
> // In "chunked" transfer-coding mode, so we need to parse the received
> // data to get the real entity content.
> @@ -1058,10 +1078,19 @@ HttpBootGetBootFile (
> ResponseBody.Body
> );
> if (EFI_ERROR (Status)) {
> goto ERROR_6;
> }
> +
> + //
> + // Print '.' when using chunked mode to download bootfile.
> + //
> + Ratio ++;
> + if ((Ratio % 1024) == 0) {
> + AsciiPrint (".");
> + }
> +
> }
> }
> }
>
> //
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> index 66eca78..71e4826 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> @@ -1121,11 +1121,11 @@ HttpBootRegisterRamDisk (
> ASSERT (Buffer != NULL);
> ASSERT (BufferSize != 0);
>
> Status = gBS->LocateProtocol (&gEfiRamDiskProtocolGuid, NULL,
> (VOID**) &RamDisk);
> if (EFI_ERROR (Status)) {
> - DEBUG ((EFI_D_ERROR, "HTTP Boot: Couldn't find the RAM Disk protocol -
> %r\n", Status));
> + AsciiPrint ("\n HTTP Boot: Couldn't find the RAM Disk protocol -
> + %r\n", Status);
>
> [Samer] Such messages should not be printed to the user. The user of
> the machine does not know what a "RAM Disk Protocol" is. Instead, keep
> the DEBUG message, and print a generic "Download failed" message to
> the console.
>
> return Status;
> }
>
> if (ImageType == ImageTypeVirtualCd) {
> RamDiskType = &gEfiVirtualCdGuid; @@ -1141,11 +1141,11 @@
> HttpBootRegisterRamDisk (
> RamDiskType,
> Private->UsingIpv6 ? Private->Ip6Nic->DevicePath :
> Private->Ip4Nic-
> >DevicePath,
> &DevicePath
> );
> if (EFI_ERROR (Status)) {
> - DEBUG ((EFI_D_ERROR, "HTTP Boot: Failed to register RAM Disk - %r\n",
> Status));
> + AsciiPrint ("\n HTTP Boot: Failed to register RAM Disk - %r\n",
> + Status);
> }
>
> [Samer] Same as above. Keep the DEBUG message, and print a generic
> "Download failed" message to the console.
>
> return Status;
> }
>
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
