diff mbox series

handlers: ubivol: Allow specifying MTD device

Message ID 20240408125620.2510328-1-ernestas.k@wilibox.com
State Accepted
Delegated to: Stefano Babic
Headers show
Series handlers: ubivol: Allow specifying MTD device | expand

Commit Message

Ernestas Kulik April 8, 2024, 12:56 p.m. UTC
Updates will typically fail in devices that use multiple MTD partitions
for failsafe upgrades and identical UBI volume names, since only the
first partition will match. Allowing to specify the device as well as
volume name in images allows such cases to work.

Signed-off-by: Ernestas Kulik <ernestas.k@wilibox.com>
---
 handlers/ubivol_handler.c | 45 +++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

Comments

Ernestas Kulik May 13, 2024, 11:35 a.m. UTC | #1
Ping

On Monday, April 8, 2024 at 3:57:46 PM UTC+3 Ernestas Kulik wrote:

> Updates will typically fail in devices that use multiple MTD partitions
> for failsafe upgrades and identical UBI volume names, since only the
> first partition will match. Allowing to specify the device as well as
> volume name in images allows such cases to work.
>
> Signed-off-by: Ernestas Kulik <ernes...@wilibox.com>
> ---
> handlers/ubivol_handler.c | 45 +++++++++++++++++++++++++++++++++------
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
> index 251f917..9db6472 100644
> --- a/handlers/ubivol_handler.c
> +++ b/handlers/ubivol_handler.c
> @@ -51,6 +51,26 @@ static struct ubi_part *search_volume_global(const char 
> *str)
> return NULL;
> }
>
> +/* search for a UBI volume by name on a specified MTD partition */
> +static struct ubi_part *search_volume_local(char *device, const char 
> *volname)
> +{
> + struct flash_description *flash = get_flash_info();
> + int mtdnum;
> + struct mtd_ubi_info *mtd_ubi_info;
> +
> + mtdnum = get_mtd_from_device(device);
> + if (mtdnum < 0) {
> + mtdnum = get_mtd_from_name(device);
> + }
> + if (mtdnum < 0 || !mtd_dev_present(flash->libmtd, mtdnum)) {
> + ERROR("%s does not exist", device);
> + return NULL;
> + }
> + mtd_ubi_info = &flash->mtd_info[mtdnum];
> +
> + return search_volume(volname, &mtd_ubi_info->ubi_partitions);
> +}
> +
> /**
> * check_replace - check for and validate replace property
> * @img: image information
> @@ -77,7 +97,10 @@ static int check_replace(struct img_type *img,
> if (tmpvol_name == NULL)
> return 0;
>
> - tmpvol = search_volume_global(tmpvol_name);
> + if (strlen(img->device))
> + tmpvol = search_volume_local(img->device, tmpvol_name);
> + else
> + tmpvol = search_volume_global(tmpvol_name);
>
> if (!tmpvol) {
> INFO("replace: unable to find a volume %s, will rename", tmpvol_name);
> @@ -400,7 +423,11 @@ static int wait_volume(struct img_type *img)
> struct stat buf;
> char node[64];
>
> - ubivol = search_volume_global(img->volname);
> + if (strlen(img->device))
> + ubivol = search_volume_local(img->device, img->volname);
> + else
> + ubivol = search_volume_global(img->volname);
> +
> if (!ubivol) {
> ERROR("can't found volume %s", img->volname);
> return -1;
> @@ -451,7 +478,10 @@ static int install_ubivol_image(struct img_type *img,
> }
>
> /* find the volume to be updated */
> - ubivol = search_volume_global(img->volname);
> + if (strlen(img->device))
> + ubivol = search_volume_local(img->device, img->volname);
> + else
> + ubivol = search_volume_global(img->volname);
>
> if (!ubivol) {
> ERROR("Image %s should be stored in volume "
> @@ -472,11 +502,14 @@ static int adjust_volume(struct img_type *cfg,
> return resize_volume(cfg, cfg->partsize);
> }
>
> -static int ubi_volume_get_info(char *name, int *dev_num, int *vol_id)
> +static int ubi_volume_get_info(char *device, char *name, int *dev_num, 
> int *vol_id)
> {
> struct ubi_part *ubi_part;
>
> - ubi_part = search_volume_global(name);
> + if (device)
> + ubi_part = search_volume_local(device, name);
> + else
> + ubi_part = search_volume_global(name);
> if (!ubi_part) {
> ERROR("could not found UBI volume %s", name);
> return -1;
> @@ -533,7 +566,7 @@ static int swap_volume(struct img_type *img, void 
> *data)
> }
>
> name[num] = volume->value;
> - if (ubi_volume_get_info(volume->value,
> + if (ubi_volume_get_info(img->device, volume->value,
> &dev_num[num],
> &vol_id[num]) < 0)
> goto out;
> -- 
> 2.39.2
>
>
Stefano Babic May 13, 2024, 6:46 p.m. UTC | #2
Hi Ernestas,

On 08.04.24 14:56, 'Ernestas Kulik' via swupdate wrote:
> Updates will typically fail in devices that use multiple MTD partitions
> for failsafe upgrades and identical UBI volume names, since only the
> first partition will match. Allowing to specify the device as well as
> volume name in images allows such cases to work.
>
> Signed-off-by: Ernestas Kulik <ernestas.k@wilibox.com>
> ---
>   handlers/ubivol_handler.c | 45 +++++++++++++++++++++++++++++++++------
>   1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
> index 251f917..9db6472 100644
> --- a/handlers/ubivol_handler.c
> +++ b/handlers/ubivol_handler.c
> @@ -51,6 +51,26 @@ static struct ubi_part *search_volume_global(const char *str)
>   	return NULL;
>   }
>
> +/* search for a UBI volume by name on a specified MTD partition */
> +static struct ubi_part *search_volume_local(char *device, const char *volname)
> +{
> +	struct flash_description *flash = get_flash_info();
> +	int mtdnum;
> +	struct mtd_ubi_info *mtd_ubi_info;
> +
> +	mtdnum = get_mtd_from_device(device);
> +	if (mtdnum < 0) {
> +		mtdnum = get_mtd_from_name(device);
> +	}
> +	if (mtdnum < 0 || !mtd_dev_present(flash->libmtd, mtdnum)) {
> +		ERROR("%s does not exist", device);
> +		return NULL;
> +	}
> +	mtd_ubi_info = &flash->mtd_info[mtdnum];
> +
> +	return search_volume(volname, &mtd_ubi_info->ubi_partitions);
> +}
> +
>   /**
>    * check_replace - check for and validate replace property
>    * @img: image information
> @@ -77,7 +97,10 @@ static int check_replace(struct img_type *img,
>   	if (tmpvol_name == NULL)
>   		return 0;
>
> -	tmpvol = search_volume_global(tmpvol_name);
> +	if (strlen(img->device))
> +		tmpvol = search_volume_local(img->device, tmpvol_name);
> +	else
> +		tmpvol = search_volume_global(tmpvol_name);
>
>   	if (!tmpvol) {
>   		INFO("replace: unable to find a volume %s, will rename", tmpvol_name);
> @@ -400,7 +423,11 @@ static int wait_volume(struct img_type *img)
>   	struct stat buf;
>   	char node[64];
>
> -	ubivol = search_volume_global(img->volname);
> +	if (strlen(img->device))
> +		ubivol = search_volume_local(img->device, img->volname);
> +	else
> +		ubivol = search_volume_global(img->volname);
> +
>   	if (!ubivol) {
>   		ERROR("can't found volume %s", img->volname);
>   		return -1;
> @@ -451,7 +478,10 @@ static int install_ubivol_image(struct img_type *img,
>   	}
>
>   	/* find the volume to be updated */
> -	ubivol = search_volume_global(img->volname);
> +	if (strlen(img->device))
> +		ubivol = search_volume_local(img->device, img->volname);
> +	else
> +		ubivol = search_volume_global(img->volname);
>
>   	if (!ubivol) {
>   		ERROR("Image %s should be stored in volume "
> @@ -472,11 +502,14 @@ static int adjust_volume(struct img_type *cfg,
>   	return resize_volume(cfg, cfg->partsize);
>   }
>
> -static int ubi_volume_get_info(char *name, int *dev_num, int *vol_id)
> +static int ubi_volume_get_info(char *device, char *name, int *dev_num, int *vol_id)
>   {
>   	struct ubi_part *ubi_part;
>
> -	ubi_part = search_volume_global(name);
> +	if (device)
> +		ubi_part = search_volume_local(device, name);
> +	else
> +		ubi_part = search_volume_global(name);
>   	if (!ubi_part) {
>   		ERROR("could not found UBI volume %s", name);
>   		return -1;
> @@ -533,7 +566,7 @@ static int swap_volume(struct img_type *img, void *data)
>   			}
>
>   			name[num] = volume->value;
> -			if (ubi_volume_get_info(volume->value,
> +			if (ubi_volume_get_info(img->device, volume->value,
>   						&dev_num[num],
>   						&vol_id[num]) < 0)
>   				goto out;

It looks good to me.

Reviewed-by: Stefano Babic <stefano.babic@swupdate.org>
diff mbox series

Patch

diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
index 251f917..9db6472 100644
--- a/handlers/ubivol_handler.c
+++ b/handlers/ubivol_handler.c
@@ -51,6 +51,26 @@  static struct ubi_part *search_volume_global(const char *str)
 	return NULL;
 }
 
+/* search for a UBI volume by name on a specified MTD partition */
+static struct ubi_part *search_volume_local(char *device, const char *volname)
+{
+	struct flash_description *flash = get_flash_info();
+	int mtdnum;
+	struct mtd_ubi_info *mtd_ubi_info;
+
+	mtdnum = get_mtd_from_device(device);
+	if (mtdnum < 0) {
+		mtdnum = get_mtd_from_name(device);
+	}
+	if (mtdnum < 0 || !mtd_dev_present(flash->libmtd, mtdnum)) {
+		ERROR("%s does not exist", device);
+		return NULL;
+	}
+	mtd_ubi_info = &flash->mtd_info[mtdnum];
+
+	return search_volume(volname, &mtd_ubi_info->ubi_partitions);
+}
+
 /**
  * check_replace - check for and validate replace property
  * @img: image information
@@ -77,7 +97,10 @@  static int check_replace(struct img_type *img,
 	if (tmpvol_name == NULL)
 		return 0;
 
-	tmpvol = search_volume_global(tmpvol_name);
+	if (strlen(img->device))
+		tmpvol = search_volume_local(img->device, tmpvol_name);
+	else
+		tmpvol = search_volume_global(tmpvol_name);
 
 	if (!tmpvol) {
 		INFO("replace: unable to find a volume %s, will rename", tmpvol_name);
@@ -400,7 +423,11 @@  static int wait_volume(struct img_type *img)
 	struct stat buf;
 	char node[64];
 
-	ubivol = search_volume_global(img->volname);
+	if (strlen(img->device))
+		ubivol = search_volume_local(img->device, img->volname);
+	else
+		ubivol = search_volume_global(img->volname);
+
 	if (!ubivol) {
 		ERROR("can't found volume %s", img->volname);
 		return -1;
@@ -451,7 +478,10 @@  static int install_ubivol_image(struct img_type *img,
 	}
 
 	/* find the volume to be updated */
-	ubivol = search_volume_global(img->volname);
+	if (strlen(img->device))
+		ubivol = search_volume_local(img->device, img->volname);
+	else
+		ubivol = search_volume_global(img->volname);
 
 	if (!ubivol) {
 		ERROR("Image %s should be stored in volume "
@@ -472,11 +502,14 @@  static int adjust_volume(struct img_type *cfg,
 	return resize_volume(cfg, cfg->partsize);
 }
 
-static int ubi_volume_get_info(char *name, int *dev_num, int *vol_id)
+static int ubi_volume_get_info(char *device, char *name, int *dev_num, int *vol_id)
 {
 	struct ubi_part *ubi_part;
 
-	ubi_part = search_volume_global(name);
+	if (device)
+		ubi_part = search_volume_local(device, name);
+	else
+		ubi_part = search_volume_global(name);
 	if (!ubi_part) {
 		ERROR("could not found UBI volume %s", name);
 		return -1;
@@ -533,7 +566,7 @@  static int swap_volume(struct img_type *img, void *data)
 			}
 
 			name[num] = volume->value;
-			if (ubi_volume_get_info(volume->value,
+			if (ubi_volume_get_info(img->device, volume->value,
 						&dev_num[num],
 						&vol_id[num]) < 0)
 				goto out;