diff mbox series

fastboot: properly handle unknown partition type

Message ID 20241113050607.1850472-1-caleb.connolly@linaro.org
State New
Headers show
Series fastboot: properly handle unknown partition type | expand

Commit Message

Caleb Connolly Nov. 13, 2024, 5:05 a.m. UTC
In getvar_partition_type() we attempt to find a filesystem driver for
the partition (of the list of driver enabled in U-Boot), on failure we
return the error to fastboot and completely bail out of the operation.

However, this should not be a failure, instead we should just default to
"raw". This allows commands like "fastboot format:ext4 userdata" to work
if userdata didn't already have an ext4 partition table (or if FS_EXT4
is disabled in U-Boot), as failing to determine the current partition
type is not an error in this case.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/fastboot/fb_getvar.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Mattijs Korpershoek Nov. 14, 2024, 1:25 p.m. UTC | #1
Hi Caleb,

Thank you for the patch.

On mer., nov. 13, 2024 at 06:05, Caleb Connolly <caleb.connolly@linaro.org> wrote:

> In getvar_partition_type() we attempt to find a filesystem driver for
> the partition (of the list of driver enabled in U-Boot), on failure we
> return the error to fastboot and completely bail out of the operation.
>
> However, this should not be a failure, instead we should just default to
> "raw". This allows commands like "fastboot format:ext4 userdata" to work
> if userdata didn't already have an ext4 partition table (or if FS_EXT4
> is disabled in U-Boot), as failing to determine the current partition
> type is not an error in this case.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  drivers/fastboot/fb_getvar.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 93cbd598e024..9c2ce65a4e5b 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -229,9 +229,10 @@ static void __maybe_unused getvar_partition_type(char *part_name, char *response
>  				       response);
>  	if (r >= 0) {
>  		r = fs_set_blk_dev_with_part(dev_desc, r);
>  		if (r < 0)
> -			fastboot_fail("failed to set partition", response);
> +			/* If we don't know then just default to raw */
> +			fastboot_okay("raw", response);
>  		else
>  			fastboot_okay(fs_get_type_name(), response);
>  	}
>  }
> -- 
> 2.47.0
Mattijs Korpershoek Nov. 19, 2024, 2:11 p.m. UTC | #2
Hi,

On Wed, 13 Nov 2024 06:05:59 +0100, Caleb Connolly wrote:
> In getvar_partition_type() we attempt to find a filesystem driver for
> the partition (of the list of driver enabled in U-Boot), on failure we
> return the error to fastboot and completely bail out of the operation.
> 
> However, this should not be a failure, instead we should just default to
> "raw". This allows commands like "fastboot format:ext4 userdata" to work
> if userdata didn't already have an ext4 partition table (or if FS_EXT4
> is disabled in U-Boot), as failing to determine the current partition
> type is not an error in this case.
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu)

[1/1] fastboot: properly handle unknown partition type
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/06b8aafd6810d86f37d5b1cd9c1966f1e42403ed

--
Mattijs
diff mbox series

Patch

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 93cbd598e024..9c2ce65a4e5b 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -229,9 +229,10 @@  static void __maybe_unused getvar_partition_type(char *part_name, char *response
 				       response);
 	if (r >= 0) {
 		r = fs_set_blk_dev_with_part(dev_desc, r);
 		if (r < 0)
-			fastboot_fail("failed to set partition", response);
+			/* If we don't know then just default to raw */
+			fastboot_okay("raw", response);
 		else
 			fastboot_okay(fs_get_type_name(), response);
 	}
 }