Re: [PATCH v5 3/3] Load option with short device path for boot vars

2023-05-26 Thread Raymond Mao
Hi Heinrich,

Will keep the device path as-is if no short-term returns in v6 patch

On Fri, 26 May 2023 at 03:14, Heinrich Schuchardt 
wrote:

> On 5/26/23 00:04, Raymond Mao wrote:
> > The boot variables automatically generated for removable medias
> > should be with short form of device path without device nodes.
> > This is a requirement for the case that a removable media is
> > plugged into a different port but is still able to work with the
> > existing boot variables.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - Ignore EFI_NOT_FOUND returned from
> >efi_bootmgr_update_media_device_boot_option which means no boot
> >options scanned.
> > Changes in v3
> > - Split the patch into moving and renaming functions and
> >individual patches for each changed functionality
> > Changes in v4
> > - Revert the change of introducing a bool parameter when updating
> >the boot option. Use short-form of device path by default
> > Changes in v5
> > - Move function call of efi_bootmgr_update_media_device_boot_option()
> >from efi_init_variables() to efi_init_obj_list()
> > - Add warning log when a short-form device path doesn't exist
> >
> >   lib/efi_loader/efi_bootmgr.c | 7 +++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index c329428973..67e833141f 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -387,6 +387,13 @@ static efi_status_t
> efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
> >   p = dev_name;
> >   utf8_utf16_strncpy(, buf, strlen(buf));
> >
> > + /* use short form device path */
> > + device_path = efi_dp_shorten(device_path);
> > + if (!device_path) {
> > + log_warning("No short-form device path for device
> %s, skip it\n", buf);
> > + continue;
>
> efi_dp_shorten() returns NULL if the device-path contains neither of
> - a USB WWI node
> - a hard drive node, HD()
> - a file path node
>
> Why should we skip a device with a simple file protocol here if it does
> not contain one of the above nodes?
>
> The warning makes no sense at all.
>
> Best regards
>
> Heinrich
>
> > + }
> > +
> >   lo.label = dev_name;
> >   lo.attributes = LOAD_OPTION_ACTIVE;
> >   lo.file_path = device_path;
>
>


Re: [PATCH v5 3/3] Load option with short device path for boot vars

2023-05-26 Thread Heinrich Schuchardt

On 5/26/23 00:04, Raymond Mao wrote:

The boot variables automatically generated for removable medias
should be with short form of device path without device nodes.
This is a requirement for the case that a removable media is
plugged into a different port but is still able to work with the
existing boot variables.

Signed-off-by: Raymond Mao 
---
Changes in v2
- Ignore EFI_NOT_FOUND returned from
   efi_bootmgr_update_media_device_boot_option which means no boot
   options scanned.
Changes in v3
- Split the patch into moving and renaming functions and
   individual patches for each changed functionality
Changes in v4
- Revert the change of introducing a bool parameter when updating
   the boot option. Use short-form of device path by default
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
   from efi_init_variables() to efi_init_obj_list()
- Add warning log when a short-form device path doesn't exist

  lib/efi_loader/efi_bootmgr.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index c329428973..67e833141f 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -387,6 +387,13 @@ static efi_status_t 
efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
p = dev_name;
utf8_utf16_strncpy(, buf, strlen(buf));

+   /* use short form device path */
+   device_path = efi_dp_shorten(device_path);
+   if (!device_path) {
+   log_warning("No short-form device path for device %s, skip 
it\n", buf);
+   continue;


efi_dp_shorten() returns NULL if the device-path contains neither of
- a USB WWI node
- a hard drive node, HD()
- a file path node

Why should we skip a device with a simple file protocol here if it does
not contain one of the above nodes?

The warning makes no sense at all.

Best regards

Heinrich


+   }
+
lo.label = dev_name;
lo.attributes = LOAD_OPTION_ACTIVE;
lo.file_path = device_path;




[PATCH v5 3/3] Load option with short device path for boot vars

2023-05-25 Thread Raymond Mao
The boot variables automatically generated for removable medias
should be with short form of device path without device nodes.
This is a requirement for the case that a removable media is
plugged into a different port but is still able to work with the
existing boot variables.

Signed-off-by: Raymond Mao 
---
Changes in v2
- Ignore EFI_NOT_FOUND returned from
  efi_bootmgr_update_media_device_boot_option which means no boot
  options scanned.
Changes in v3
- Split the patch into moving and renaming functions and
  individual patches for each changed functionality
Changes in v4
- Revert the change of introducing a bool parameter when updating
  the boot option. Use short-form of device path by default
Changes in v5
- Move function call of efi_bootmgr_update_media_device_boot_option()
  from efi_init_variables() to efi_init_obj_list()
- Add warning log when a short-form device path doesn't exist

 lib/efi_loader/efi_bootmgr.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index c329428973..67e833141f 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -387,6 +387,13 @@ static efi_status_t 
efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo
p = dev_name;
utf8_utf16_strncpy(, buf, strlen(buf));
 
+   /* use short form device path */
+   device_path = efi_dp_shorten(device_path);
+   if (!device_path) {
+   log_warning("No short-form device path for device %s, 
skip it\n", buf);
+   continue;
+   }
+
lo.label = dev_name;
lo.attributes = LOAD_OPTION_ACTIVE;
lo.file_path = device_path;
-- 
2.25.1