[PATCH v2] cmd: ubi: add a command to rename volume

[PATCH v2] cmd: ubi: add a command to rename volume

[Philippe Reynes]

This commit add the command ubi rename to rename an ubi volume.
The format of the command is: ubi rename <oldname> <newname>.
To enable this command, the option CMD_UBI_RENAME must be selected.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 cmd/Kconfig |  8 ++++++++
 cmd/ubi.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

Changelog:
v2:
- added an option CMD_UBI_RENAME (feedback from Wolfgang)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 6403bc4..1564694 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2172,6 +2172,14 @@ config CMD_UBI
 	  It is also strongly encouraged to also enable CONFIG_MTD to get full
 	  partition support.
 
+config CMD_UBI_RENAME
+       bool "Enable rename"
+       depends on CMD_UBI
+       default n
+       help
+         Enable a "ubi" command to rename ubi volume:
+	   ubi rename <oldname> <newname>
+
 config CMD_UBIFS
 	tristate "Enable UBIFS - Unsorted block images filesystem commands"
 	depends on CMD_UBI
diff --git a/cmd/ubi.c b/cmd/ubi.c
index cecf251..4885661 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -251,6 +251,41 @@ out_err:
 	return err;
 }
 
+#ifdef CONFIG_CMD_UBI_RENAME
+static int ubi_rename_vol(char *oldname, char *newname)
+{
+	struct ubi_volume *vol;
+	struct ubi_rename_entry rename;
+	struct ubi_volume_desc desc;
+	struct list_head list;
+
+	vol = ubi_find_volume(oldname);
+	if (!vol) {
+		printf("%s: volume %s doesn't exist\n", __func__, oldname);
+		return ENODEV;
+	}
+
+	printf("Rename UBI volume %s to %s\n", oldname, newname);
+
+	if (ubi->ro_mode) {
+		printf("%s: ubi device is in read-only mode\n", __func__);
+		return EROFS;
+	}
+
+	rename.new_name_len = strlen(newname);
+	strcpy(rename.new_name, newname);
+	rename.remove = 0;
+	desc.vol = vol;
+	desc.mode = 0;
+	rename.desc = &desc;
+	INIT_LIST_HEAD(&rename.list);
+	INIT_LIST_HEAD(&list);
+	list_add(&rename.list, &list);
+
+	return ubi_rename_volumes(ubi, &list);
+}
+#endif /* CONFIG_CMD_UBI_RENAME */
+
 static int ubi_volume_continue_write(char *volume, void *buf, size_t size)
 {
 	int err = 1;
@@ -604,6 +639,11 @@ static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			return ubi_remove_vol(argv[2]);
 	}
 
+#ifdef CONFIG_CMD_UBI_RENAME
+	if (strncmp(argv[1], "rename", 6) == 0)
+		return ubi_rename_vol(argv[2], argv[3]);
+#endif
+
 	if (strncmp(argv[1], "skipcheck", 9) == 0) {
 		/* E.g., change skip_check flag */
 		if (argc == 4) {
@@ -692,6 +732,9 @@ U_BOOT_CMD(
 		" - Read volume to address with size\n"
 	"ubi remove[vol] volume"
 		" - Remove volume\n"
+#ifdef CONFIG_CMD_UBI_RENAME
+	"ubi rename oldname newname\n"
+#endif
 	"ubi skipcheck volume on/off - Set or clear skip_check flag in volume header\n"
 	"[Legends]\n"
 	" volume: character name\n"
-- 
2.7.4

[PATCH v2] cmd: ubi: add a command to rename volume

[Simon Glass]

Hi Philippe,

On Fri, 20 Mar 2020 at 03:55, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> This commit add the command ubi rename to rename an ubi volume.

adds

> The format of the command is: ubi rename <oldname> <newname>.
> To enable this command, the option CMD_UBI_RENAME must be selected.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  cmd/Kconfig |  8 ++++++++
>  cmd/ubi.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>
> Changelog:
> v2:
> - added an option CMD_UBI_RENAME (feedback from Wolfgang)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 6403bc4..1564694 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2172,6 +2172,14 @@ config CMD_UBI
>           It is also strongly encouraged to also enable CONFIG_MTD to get full
>           partition support.
>
> +config CMD_UBI_RENAME
> +       bool "Enable rename"
> +       depends on CMD_UBI
> +       default n
> +       help
> +         Enable a "ubi" command to rename ubi volume:
> +          ubi rename <oldname> <newname>
> +
>  config CMD_UBIFS
>         tristate "Enable UBIFS - Unsorted block images filesystem commands"
>         depends on CMD_UBI
> diff --git a/cmd/ubi.c b/cmd/ubi.c
> index cecf251..4885661 100644
> --- a/cmd/ubi.c
> +++ b/cmd/ubi.c
> @@ -251,6 +251,41 @@ out_err:
>         return err;
>  }
>
> +#ifdef CONFIG_CMD_UBI_RENAME
> +static int ubi_rename_vol(char *oldname, char *newname)
> +{
> +       struct ubi_volume *vol;
> +       struct ubi_rename_entry rename;
> +       struct ubi_volume_desc desc;
> +       struct list_head list;
> +
> +       vol = ubi_find_volume(oldname);
> +       if (!vol) {
> +               printf("%s: volume %s doesn't exist\n", __func__, oldname);
> +               return ENODEV;
> +       }
> +
> +       printf("Rename UBI volume %s to %s\n", oldname, newname);
> +
> +       if (ubi->ro_mode) {
> +               printf("%s: ubi device is in read-only mode\n", __func__);
> +               return EROFS;
> +       }
> +
> +       rename.new_name_len = strlen(newname);
> +       strcpy(rename.new_name, newname);
> +       rename.remove = 0;
> +       desc.vol = vol;
> +       desc.mode = 0;
> +       rename.desc = &desc;
> +       INIT_LIST_HEAD(&rename.list);
> +       INIT_LIST_HEAD(&list);
> +       list_add(&rename.list, &list);
> +
> +       return ubi_rename_volumes(ubi, &list);
> +}
> +#endif /* CONFIG_CMD_UBI_RENAME */
> +
>  static int ubi_volume_continue_write(char *volume, void *buf, size_t size)
>  {
>         int err = 1;
> @@ -604,6 +639,11 @@ static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>                         return ubi_remove_vol(argv[2]);
>         }
>
> +#ifdef CONFIG_CMD_UBI_RENAME

Can you use IS_ENABLED(CONFIG_...) and drop the #ifdef way above?

> +       if (strncmp(argv[1], "rename", 6) == 0)
> +               return ubi_rename_vol(argv[2], argv[3]);
> +#endif
> +
[..]

Regards,
Simon

[PATCH v2] cmd: ubi: add a command to rename volume

[Philippe REYNES]

Hi Simon,

> Hi Philippe,
> 
> On Fri, 20 Mar 2020 at 03:55, Philippe Reynes
> <philippe.reynes@softathome.com> wrote:
>> 
>> This commit add the command ubi rename to rename an ubi volume.
> 
> adds
> 
>> The format of the command is: ubi rename <oldname> <newname>.
>> To enable this command, the option CMD_UBI_RENAME must be selected.
>> 
>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>> ---
>> cmd/Kconfig | 8 ++++++++
>> cmd/ubi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 51 insertions(+)
>> 
>> Changelog:
>> v2:
>> - added an option CMD_UBI_RENAME (feedback from Wolfgang)
>> 
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 6403bc4..1564694 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -2172,6 +2172,14 @@ config CMD_UBI
>> It is also strongly encouraged to also enable CONFIG_MTD to get full
>> partition support.
>> 
>> +config CMD_UBI_RENAME
>> + bool "Enable rename"
>> + depends on CMD_UBI
>> + default n
>> + help
>> + Enable a "ubi" command to rename ubi volume:
>> + ubi rename <oldname> <newname>
>> +
>> config CMD_UBIFS
>> tristate "Enable UBIFS - Unsorted block images filesystem commands"
>> depends on CMD_UBI
>> diff --git a/cmd/ubi.c b/cmd/ubi.c
>> index cecf251..4885661 100644
>> --- a/cmd/ubi.c
>> +++ b/cmd/ubi.c
>> @@ -251,6 +251,41 @@ out_err:
>> return err;
>> }
>> 
>> +#ifdef CONFIG_CMD_UBI_RENAME
>> +static int ubi_rename_vol(char *oldname, char *newname)
>> +{
>> + struct ubi_volume *vol;
>> + struct ubi_rename_entry rename;
>> + struct ubi_volume_desc desc;
>> + struct list_head list;
>> +
>> + vol = ubi_find_volume(oldname);
>> + if (!vol) {
>> + printf("%s: volume %s doesn't exist\n", __func__, oldname);
>> + return ENODEV;
>> + }
>> +
>> + printf("Rename UBI volume %s to %s\n", oldname, newname);
>> +
>> + if (ubi->ro_mode) {
>> + printf("%s: ubi device is in read-only mode\n", __func__);
>> + return EROFS;
>> + }
>> +
>> + rename.new_name_len = strlen(newname);
>> + strcpy(rename.new_name, newname);
>> + rename.remove = 0;
>> + desc.vol = vol;
>> + desc.mode = 0;
>> + rename.desc = &desc;
>> + INIT_LIST_HEAD(&rename.list);
>> + INIT_LIST_HEAD(&list);
>> + list_add(&rename.list, &list);
>> +
>> + return ubi_rename_volumes(ubi, &list);
>> +}
>> +#endif /* CONFIG_CMD_UBI_RENAME */
>> +
>> static int ubi_volume_continue_write(char *volume, void *buf, size_t size)
>> {
>> int err = 1;
>> @@ -604,6 +639,11 @@ static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc,
>> char * const argv[])
>> return ubi_remove_vol(argv[2]);
>> }
>> 
>> +#ifdef CONFIG_CMD_UBI_RENAME
> 
> Can you use IS_ENABLED(CONFIG_...) and drop the #ifdef way above?

I've sent a v3 where I use IS_ENABLED instead of #ifdef.
But I've kept a #if IS_ENABLED(...) around the function ubi_rename_vol.
Otherwise if this option is not enabled, there is a warning when building :

  CC      cmd/ubi.o
cmd/ubi.c:254:12: warning: 'ubi_rename_vol' defined but not used [-Wunused-function]
 static int ubi_rename_vol(char *oldname, char *newname)
            ^~~~~~~~~~~~~~

 
>> + if (strncmp(argv[1], "rename", 6) == 0)
>> + return ubi_rename_vol(argv[2], argv[3]);
>> +#endif
>> +
> [..]
> 
> Regards,
> Simon

Regards,
Philippe

[PATCH v2] cmd: ubi: add a command to rename volume

[Rasmus Villemoes]

On 23/03/2020 18.16, Philippe REYNES wrote:

>>> +#ifdef CONFIG_CMD_UBI_RENAME
>>
>> Can you use IS_ENABLED(CONFIG_...) and drop the #ifdef way above?
> 
> I've sent a v3 where I use IS_ENABLED instead of #ifdef.
> But I've kept a #if IS_ENABLED(...) around the function ubi_rename_vol.
> Otherwise if this option is not enabled, there is a warning when building :
> 
>   CC      cmd/ubi.o
> cmd/ubi.c:254:12: warning: 'ubi_rename_vol' defined but not used [-Wunused-function]
>  static int ubi_rename_vol(char *oldname, char *newname)
>             ^~~~~~~~~~~~~~

Use C

  if (IS_ENABLED(CONFIG_...) && !strcmp(cmd, "rename"))

not cpp

#if IS_ENABLED(CONFIG_...)
   if (!strcmp(cmd, "rename"))

That way the compiler sees ubi_rename_vol is used, but also knows that
it is dead code that can be eliminated. Less ifdeffery (none, actually),
and the code in ubi_rename_vol gets compile coverage regardless of
configuration.

Rasmus

[PATCH v2] cmd: ubi: add a command to rename volume

[Philippe REYNES]

Hi Rasmus,

> On 23/03/2020 18.16, Philippe REYNES wrote:
> 
>>>> +#ifdef CONFIG_CMD_UBI_RENAME
>>> 
>>> Can you use IS_ENABLED(CONFIG_...) and drop the #ifdef way above?
>> 
>> I've sent a v3 where I use IS_ENABLED instead of #ifdef.
>> But I've kept a #if IS_ENABLED(...) around the function ubi_rename_vol.
>> Otherwise if this option is not enabled, there is a warning when building :
>> 
>> CC cmd/ubi.o
>> cmd/ubi.c:254:12: warning: 'ubi_rename_vol' defined but not used
>> [-Wunused-function]
>> static int ubi_rename_vol(char *oldname, char *newname)
>> ^~~~~~~~~~~~~~
> 
> Use C
> 
> if (IS_ENABLED(CONFIG_...) && !strcmp(cmd, "rename"))
> 
> not cpp
> 
> #if IS_ENABLED(CONFIG_...)
> if (!strcmp(cmd, "rename"))
> 
> That way the compiler sees ubi_rename_vol is used, but also knows that
> it is dead code that can be eliminated. Less ifdeffery (none, actually),
> and the code in ubi_rename_vol gets compile coverage regardless of
> configuration.

Thanks a lot for this explanation.

I have sent a v4. And it works as expected, without the #if ... before
the function ubi_rename_vol, there isn't a warning if ubi rename is not
enabled in the config.

 
> Rasmus

Regards,
Philippe