[PATCH 0/2] added support for renaming UBI volumes.

[PATCH 0/2] added support for renaming UBI volumes.

[Giorgio Dal Molin]

These two patches implement the ioctl UBI_IOCRNVOL for UBI devices and a new
command, 'ubirename' that calls the ioctl from the shell to rename UBI volumes.

The ioctl implementation was ported from the linux kernel v4.7.4, with minimal
changes; a further change was needed for the function 'ubi_rename_volumes()' in
drivers/mtd/ubi/vmt.c to also change the name(s) of the file(s) corresponding to
the renamed UBI volumes under /dev/...

Expecially this last change needs to be reviewed and validated by a barebox
developer more experienced than me.

	
Giorgio Dal Molin (2):
  mtd: UBI: add support (ioctl) for renaming ubi volumes.
  commands: ubi: added the new command 'ubirename' to rename ubi
    volumes.

 commands/ubi.c            |  87 ++++++++++++++++++++++
 drivers/mtd/ubi/barebox.c | 182 ++++++++++++++++++++++++++++++++++++++++++++--
 drivers/mtd/ubi/vmt.c     |   3 +
 3 files changed, 267 insertions(+), 5 deletions(-)

-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] mtd: UBI: add support (ioctl) for renaming ubi volumes.

[Giorgio Dal Molin]

From: Giorgio Dal Molin <giorgio.dal.molin@mobotix.com>

The actual implementation was taken from the current linux kernel v4.7.4.

Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
---
 drivers/mtd/ubi/barebox.c | 182 ++++++++++++++++++++++++++++++++++++++++++++--
 drivers/mtd/ubi/vmt.c     |   3 +
 2 files changed, 180 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/ubi/barebox.c b/drivers/mtd/ubi/barebox.c
index fc60aae..6d3bdaf 100644
--- a/drivers/mtd/ubi/barebox.c
+++ b/drivers/mtd/ubi/barebox.c
@@ -203,6 +203,168 @@ static int ubi_volume_cdev_ioctl(struct cdev *cdev, int cmd, void *buf)
 	return err;
 }
 
+/**
+ * rename_volumes - rename UBI volumes.
+ * @ubi: UBI device description object
+ * @req: volumes re-name request
+ *
+ * This is a helper function for the volume re-name IOCTL which validates the
+ * the request, opens the volume and calls corresponding volumes management
+ * function. Returns zero in case of success and a negative error code in case
+ * of failure.
+ */
+static int rename_volumes(struct ubi_device *ubi,
+			  struct ubi_rnvol_req *req)
+{
+	int i, n, err;
+	struct list_head rename_list;
+	struct ubi_rename_entry *re, *re1;
+
+	if (req->count < 0 || req->count > UBI_MAX_RNVOL)
+		return -EINVAL;
+
+	if (req->count == 0)
+		return 0;
+
+	/* Validate volume IDs and names in the request */
+	for (i = 0; i < req->count; i++) {
+		if (req->ents[i].vol_id < 0 ||
+		    req->ents[i].vol_id >= ubi->vtbl_slots)
+			return -EINVAL;
+		if (req->ents[i].name_len < 0)
+			return -EINVAL;
+		if (req->ents[i].name_len > UBI_VOL_NAME_MAX)
+			return -ENAMETOOLONG;
+		req->ents[i].name[req->ents[i].name_len] = '\0';
+		n = strlen(req->ents[i].name);
+		if (n != req->ents[i].name_len)
+			return -EINVAL;
+	}
+
+	/* Make sure volume IDs and names are unique */
+	for (i = 0; i < req->count - 1; i++) {
+		for (n = i + 1; n < req->count; n++) {
+			if (req->ents[i].vol_id == req->ents[n].vol_id) {
+				ubi_err(ubi, "duplicated volume id %d",
+					req->ents[i].vol_id);
+				return -EINVAL;
+			}
+			if (!strcmp(req->ents[i].name, req->ents[n].name)) {
+				ubi_err(ubi, "duplicated volume name \"%s\"",
+					req->ents[i].name);
+				return -EINVAL;
+			}
+		}
+	}
+
+	/* Create the re-name list */
+	INIT_LIST_HEAD(&rename_list);
+	for (i = 0; i < req->count; i++) {
+		int vol_id = req->ents[i].vol_id;
+		int name_len = req->ents[i].name_len;
+		const char *name = req->ents[i].name;
+
+		re = kzalloc(sizeof(struct ubi_rename_entry), GFP_KERNEL);
+		if (!re) {
+			err = -ENOMEM;
+			goto out_free;
+		}
+
+		re->desc = ubi_open_volume(ubi->ubi_num, vol_id, UBI_READONLY);
+		if (IS_ERR(re->desc)) {
+			err = PTR_ERR(re->desc);
+			ubi_err(ubi, "cannot open volume %d, error %d",
+				vol_id, err);
+			kfree(re);
+			goto out_free;
+		}
+
+		/* Skip this re-naming if the name does not really change */
+		if (re->desc->vol->name_len == name_len &&
+		    !memcmp(re->desc->vol->name, name, name_len)) {
+			ubi_close_volume(re->desc);
+			kfree(re);
+			continue;
+		}
+
+		re->new_name_len = name_len;
+		memcpy(re->new_name, name, name_len);
+		list_add_tail(&re->list, &rename_list);
+		dbg_gen("will rename volume %d from \"%s\" to \"%s\"",
+			vol_id, re->desc->vol->name, name);
+	}
+
+	if (list_empty(&rename_list))
+		return 0;
+
+	/* Find out the volumes which have to be removed */
+	list_for_each_entry(re, &rename_list, list) {
+		struct ubi_volume_desc *desc;
+		int no_remove_needed = 0;
+
+		/*
+		 * Volume @re->vol_id is going to be re-named to
+		 * @re->new_name, while its current name is @name. If a volume
+		 * with name @re->new_name currently exists, it has to be
+		 * removed, unless it is also re-named in the request (@req).
+		 */
+		list_for_each_entry(re1, &rename_list, list) {
+			if (re->new_name_len == re1->desc->vol->name_len &&
+			    !memcmp(re->new_name, re1->desc->vol->name,
+				    re1->desc->vol->name_len)) {
+				no_remove_needed = 1;
+				break;
+			}
+		}
+
+		if (no_remove_needed)
+			continue;
+
+		/*
+		 * It seems we need to remove volume with name @re->new_name,
+		 * if it exists.
+		 */
+		desc = ubi_open_volume_nm(ubi->ubi_num, re->new_name,
+					  UBI_EXCLUSIVE);
+		if (IS_ERR(desc)) {
+			err = PTR_ERR(desc);
+			if (err == -ENODEV)
+				/* Re-naming into a non-existing volume name */
+				continue;
+
+			/* The volume exists but busy, or an error occurred */
+			ubi_err(ubi, "cannot open volume \"%s\", error %d",
+				re->new_name, err);
+			goto out_free;
+		}
+
+		re1 = kzalloc(sizeof(struct ubi_rename_entry), GFP_KERNEL);
+		if (!re1) {
+			err = -ENOMEM;
+			ubi_close_volume(desc);
+			goto out_free;
+		}
+
+		re1->remove = 1;
+		re1->desc = desc;
+		list_add(&re1->list, &rename_list);
+		dbg_gen("will remove volume %d, name \"%s\"",
+			re1->desc->vol->vol_id, re1->desc->vol->name);
+	}
+
+	mutex_lock(&ubi->device_mutex);
+	err = ubi_rename_volumes(ubi, &rename_list);
+	mutex_unlock(&ubi->device_mutex);
+
+out_free:
+	list_for_each_entry_safe(re, re1, &rename_list, list) {
+		ubi_close_volume(re->desc);
+		list_del(&re->list);
+		kfree(re);
+	}
+	return err;
+}
+
 static struct file_operations ubi_volume_fops = {
 	.open	= ubi_volume_cdev_open,
 	.close	= ubi_volume_cdev_close,
@@ -261,10 +423,11 @@ static int ubi_cdev_ioctl(struct cdev *cdev, int cmd, void *buf)
 {
 	struct ubi_volume_desc *desc;
 	struct ubi_device *ubi = cdev->priv;
-	struct ubi_mkvol_req *req = buf;
 
 	switch (cmd) {
-	case UBI_IOCRMVOL:
+	case UBI_IOCRMVOL: {
+		struct ubi_mkvol_req *req = buf;
+
 		desc = ubi_open_volume_nm(ubi->ubi_num, req->name,
                                            UBI_EXCLUSIVE);
 		if (IS_ERR(desc))
@@ -272,13 +435,22 @@ static int ubi_cdev_ioctl(struct cdev *cdev, int cmd, void *buf)
 		ubi_remove_volume(desc, 0);
 		ubi_close_volume(desc);
 		break;
-	case UBI_IOCMKVOL:
+	}
+	case UBI_IOCMKVOL: {
+		struct ubi_mkvol_req *req = buf;
+
 		if (!req->bytes)
 			req->bytes = (__s64)ubi->avail_pebs * ubi->leb_size;
 		return ubi_create_volume(ubi, req);
-	};
+	}
+	case UBI_IOCRNVOL: {
+		struct ubi_rnvol_req *req = buf;
 
-	return 0;
+		return rename_volumes(ubi, req);
+	}
+	default:
+		return -ENOTTY;
+	};
 }
 
 static struct file_operations ubi_fops = {
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 41b814c..ed04364 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -411,6 +411,9 @@ int ubi_rename_volumes(struct ubi_device *ubi, struct list_head *rename_list)
 
 			vol->name_len = re->new_name_len;
 			memcpy(vol->name, re->new_name, re->new_name_len + 1);
+			free(vol->cdev.name);
+			vol->cdev.name = basprintf("%s.%s", ubi->cdev.name, vol->name);
+			vol->cdev.size = vol->used_bytes;
 			ubi_volume_notify(ubi, vol, UBI_VOLUME_RENAMED);
 		}
 	}
-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] commands: ubi: added the new command 'ubirename' to rename ubi volumes.

[Giorgio Dal Molin]

From: Giorgio Dal Molin <giorgio.dal.molin@mobotix.com>

The syntax was taken from the corresponding command of the 'mts-utils'
userland package:

# ubirename UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]

Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
---
 commands/ubi.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/commands/ubi.c b/commands/ubi.c
index 26b521f..3479340 100644
--- a/commands/ubi.c
+++ b/commands/ubi.c
@@ -6,6 +6,8 @@
 #include <errno.h>
 #include <getopt.h>
 #include <linux/mtd/mtd.h>
+#include <linux/mtd/ubi.h>
+#include <libgen.h>
 #include <linux/kernel.h>
 #include <linux/stat.h>
 #include <linux/mtd/mtd-abi.h>
@@ -306,3 +308,88 @@ BAREBOX_CMD_START(ubirmvol)
 	BAREBOX_CMD_GROUP(CMD_GRP_PART)
 	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
 BAREBOX_CMD_END
+
+
+static int get_vol_id(const char *vol_name)
+{
+	struct ubi_volume_desc *desc;
+	struct cdev *vol_cdev;
+	struct ubi_volume_info vi;
+
+	vol_cdev = cdev_by_name(vol_name);
+	if(!vol_cdev) {
+		perror("cdev_by_name");
+		return -1;
+	}
+	desc = ubi_open_volume_cdev(vol_cdev, UBI_READONLY);
+	if(IS_ERR(desc)) {
+		perror("ubi_open_volume_cdev");
+		return -1;
+	}
+	ubi_get_volume_info(desc, &vi);
+	ubi_close_volume(desc);
+
+	return vi.vol_id;
+};
+
+static int do_ubirename(int argc, char *argv[])
+{
+	struct ubi_rnvol_req req;
+	struct cdev *ubi_cd;
+	int i, j, fd, ret;
+
+	if ((argc < 4) || (argc % 2))
+		return COMMAND_ERROR_USAGE;
+
+	req.count = (argc / 2) - 1;
+	if (req.count > UBI_MAX_RNVOL) {
+		printf("too many volume renames. (max: %u)\n", UBI_MAX_RNVOL);
+		return COMMAND_ERROR_USAGE;
+	}
+
+	ubi_cd = cdev_by_name(basename(argv[1]));
+	if(!ubi_cd || !ubi_cd->name || (strlen(ubi_cd->name)>127)) {
+		printf("arg 1 (%s) is not an ubi device.\n", argv[1]);
+		return COMMAND_ERROR_USAGE;
+	}
+
+	for(i=2, j=0; i<argc; ++j, i+=2) {
+		char vol_name[128 + UBI_MAX_VOLUME_NAME];
+
+		snprintf(vol_name, sizeof(vol_name), "%s.%s", ubi_cd->name, argv[i]);
+		req.ents[j].vol_id = get_vol_id(vol_name);
+		if(req.ents[j].vol_id < 0) {
+			printf("'%s' is not a volume name.\n", vol_name);
+			return COMMAND_ERROR_USAGE;
+		}
+		strncpy(req.ents[j].name, argv[i+1], UBI_MAX_VOLUME_NAME);
+		req.ents[j].name_len = strlen(req.ents[j].name);
+	}
+
+	fd = open(argv[1], O_WRONLY);
+	if (fd < 0) {
+		perror("open");
+		return 1;
+	}
+
+	ret = ioctl(fd, UBI_IOCRNVOL, &req);
+	if (ret)
+		printf("failed to rename: %s\n", strerror(-ret));
+
+	close(fd);
+
+	return ret ? 1 : 0;
+
+}
+
+BAREBOX_CMD_HELP_START(ubirename)
+BAREBOX_CMD_HELP_TEXT("Rename UBI volume(s) from UBIDEV")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(ubirename)
+	.cmd		= do_ubirename,
+	BAREBOX_CMD_DESC("rename UBI volume(s)")
+	BAREBOX_CMD_OPTS("UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]")
+	BAREBOX_CMD_GROUP(CMD_GRP_PART)
+	BAREBOX_CMD_HELP(cmd_ubirename_help)
+BAREBOX_CMD_END
-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] commands: ubi: added the new command 'ubirename' to rename ubi volumes.

[Sascha Hauer]

On Wed, Sep 21, 2016 at 10:04:43AM +0200, Giorgio Dal Molin wrote:
> From: Giorgio Dal Molin <giorgio.dal.molin@mobotix.com>
> 
> The syntax was taken from the corresponding command of the 'mts-utils'
> userland package:
> 
> # ubirename UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]
> 
> Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
> ---
>  commands/ubi.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
> 
> diff --git a/commands/ubi.c b/commands/ubi.c
> index 26b521f..3479340 100644
> --- a/commands/ubi.c
> +++ b/commands/ubi.c
> @@ -6,6 +6,8 @@
>  #include <errno.h>
>  #include <getopt.h>
>  #include <linux/mtd/mtd.h>
> +#include <linux/mtd/ubi.h>
> +#include <libgen.h>
>  #include <linux/kernel.h>
>  #include <linux/stat.h>
>  #include <linux/mtd/mtd-abi.h>
> @@ -306,3 +308,88 @@ BAREBOX_CMD_START(ubirmvol)
>  	BAREBOX_CMD_GROUP(CMD_GRP_PART)
>  	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
>  BAREBOX_CMD_END
> +
> +
> +static int get_vol_id(const char *vol_name)
> +{
> +	struct ubi_volume_desc *desc;
> +	struct cdev *vol_cdev;
> +	struct ubi_volume_info vi;
> +
> +	vol_cdev = cdev_by_name(vol_name);
> +	if(!vol_cdev) {
> +		perror("cdev_by_name");
> +		return -1;
> +	}
> +	desc = ubi_open_volume_cdev(vol_cdev, UBI_READONLY);
> +	if(IS_ERR(desc)) {
> +		perror("ubi_open_volume_cdev");
> +		return -1;
> +	}
> +	ubi_get_volume_info(desc, &vi);
> +	ubi_close_volume(desc);
> +
> +	return vi.vol_id;
> +};

This get_vol_id() is not particularly nice. Also I do not like having
the rename operation inside the ioctl parser as this means we cannot
make the ubirename code optional for users that do not need renaming.

I just sent out a series which should help you in this regard. Can you
base your code ontop of this?

You should be able to get the vol_id with ubi_open_volume_nm() followed
by ubi_get_volume_info().

The rename_volumes() from patch #1 should become a
ubi_api_rename_volumes() directly called from the command code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 0/2] mtd: ubi: implement the new command 'ubirename'

[Giorgio Dal Molin]

Here a new set of patches implementing the command 'ubirename'.

They are based on a patch serie from Sascha that restructures a bit
the barebox UBI APIs. In particular it is now easier to find the ubi
volume ids from the command code. Moreover the command code uses now
api functions to do the rename.

Giorgio Dal Molin (2):
  mtd: ubi: add API call to rename volumes.
  mtd: ubi: commands: added the new command 'ubirename'.

 commands/ubi.c            |  71 +++++++++++++++++++++
 drivers/mtd/ubi/barebox.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/vmt.c     |   3 +
 include/linux/mtd/ubi.h   |   1 +
 4 files changed, 231 insertions(+)

-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] mtd: ubi: add API call to rename volumes.

[Giorgio Dal Molin]

Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
---
 drivers/mtd/ubi/barebox.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/vmt.c     |   3 +
 include/linux/mtd/ubi.h   |   1 +
 3 files changed, 160 insertions(+)

diff --git a/drivers/mtd/ubi/barebox.c b/drivers/mtd/ubi/barebox.c
index 23c4bfd..329cf45 100644
--- a/drivers/mtd/ubi/barebox.c
+++ b/drivers/mtd/ubi/barebox.c
@@ -280,6 +280,162 @@ int ubi_api_remove_volume(struct ubi_volume_desc *desc, int no_vtbl)
 	return ubi_remove_volume(desc, no_vtbl);
 }
 
+int ubi_api_rename_volumes(int ubi_num, struct ubi_rnvol_req *req)
+{
+	int i, n, err;
+	struct list_head rename_list;
+	struct ubi_rename_entry *re, *re1;
+	struct ubi_device *ubi;
+
+	if (req->count < 0 || req->count > UBI_MAX_RNVOL)
+		return -EINVAL;
+
+	if (req->count == 0)
+		return 0;
+
+	ubi = ubi_get_device(ubi_num);
+	if (!ubi)
+		return -ENODEV;
+
+	/* Validate volume IDs and names in the request */
+	for (i = 0; i < req->count; i++) {
+		if (req->ents[i].vol_id < 0 ||
+		    req->ents[i].vol_id >= ubi->vtbl_slots)
+			return -EINVAL;
+		if (req->ents[i].name_len < 0)
+			return -EINVAL;
+		if (req->ents[i].name_len > UBI_VOL_NAME_MAX)
+			return -ENAMETOOLONG;
+		req->ents[i].name[req->ents[i].name_len] = '\0';
+		n = strlen(req->ents[i].name);
+		if (n != req->ents[i].name_len)
+			return -EINVAL;
+	}
+
+	/* Make sure volume IDs and names are unique */
+	for (i = 0; i < req->count - 1; i++) {
+		for (n = i + 1; n < req->count; n++) {
+			if (req->ents[i].vol_id == req->ents[n].vol_id) {
+				ubi_err(ubi, "duplicated volume id %d",
+					req->ents[i].vol_id);
+				return -EINVAL;
+			}
+			if (!strcmp(req->ents[i].name, req->ents[n].name)) {
+				ubi_err(ubi, "duplicated volume name \"%s\"",
+					req->ents[i].name);
+				return -EINVAL;
+			}
+		}
+	}
+
+	/* Create the re-name list */
+	INIT_LIST_HEAD(&rename_list);
+	for (i = 0; i < req->count; i++) {
+		int vol_id = req->ents[i].vol_id;
+		int name_len = req->ents[i].name_len;
+		const char *name = req->ents[i].name;
+
+		re = kzalloc(sizeof(struct ubi_rename_entry), GFP_KERNEL);
+		if (!re) {
+			err = -ENOMEM;
+			goto out_free;
+		}
+
+		re->desc = ubi_open_volume(ubi->ubi_num, vol_id, UBI_READONLY);
+		if (IS_ERR(re->desc)) {
+			err = PTR_ERR(re->desc);
+			ubi_err(ubi, "cannot open volume %d, error %d",
+				vol_id, err);
+			kfree(re);
+			goto out_free;
+		}
+
+		/* Skip this re-naming if the name does not really change */
+		if (re->desc->vol->name_len == name_len &&
+		    !memcmp(re->desc->vol->name, name, name_len)) {
+			ubi_close_volume(re->desc);
+			kfree(re);
+			continue;
+		}
+
+		re->new_name_len = name_len;
+		memcpy(re->new_name, name, name_len);
+		list_add_tail(&re->list, &rename_list);
+		dbg_gen("will rename volume %d from \"%s\" to \"%s\"",
+			vol_id, re->desc->vol->name, name);
+	}
+
+	if (list_empty(&rename_list))
+		return 0;
+
+	/* Find out the volumes which have to be removed */
+	list_for_each_entry(re, &rename_list, list) {
+		struct ubi_volume_desc *desc;
+		int no_remove_needed = 0;
+
+		/*
+		 * Volume @re->vol_id is going to be re-named to
+		 * @re->new_name, while its current name is @name. If a volume
+		 * with name @re->new_name currently exists, it has to be
+		 * removed, unless it is also re-named in the request (@req).
+		 */
+		list_for_each_entry(re1, &rename_list, list) {
+			if (re->new_name_len == re1->desc->vol->name_len &&
+			    !memcmp(re->new_name, re1->desc->vol->name,
+				    re1->desc->vol->name_len)) {
+				no_remove_needed = 1;
+				break;
+			}
+		}
+
+		if (no_remove_needed)
+			continue;
+
+		/*
+		 * It seems we need to remove volume with name @re->new_name,
+		 * if it exists.
+		 */
+		desc = ubi_open_volume_nm(ubi->ubi_num, re->new_name,
+					  UBI_EXCLUSIVE);
+		if (IS_ERR(desc)) {
+			err = PTR_ERR(desc);
+			if (err == -ENODEV)
+				/* Re-naming into a non-existing volume name */
+				continue;
+
+			/* The volume exists but busy, or an error occurred */
+			ubi_err(ubi, "cannot open volume \"%s\", error %d",
+				re->new_name, err);
+			goto out_free;
+		}
+
+		re1 = kzalloc(sizeof(struct ubi_rename_entry), GFP_KERNEL);
+		if (!re1) {
+			err = -ENOMEM;
+			ubi_close_volume(desc);
+			goto out_free;
+		}
+
+		re1->remove = 1;
+		re1->desc = desc;
+		list_add(&re1->list, &rename_list);
+		dbg_gen("will remove volume %d, name \"%s\"",
+			re1->desc->vol->vol_id, re1->desc->vol->name);
+	}
+
+	mutex_lock(&ubi->device_mutex);
+	err = ubi_rename_volumes(ubi, &rename_list);
+	mutex_unlock(&ubi->device_mutex);
+
+out_free:
+	list_for_each_entry_safe(re, re1, &rename_list, list) {
+		ubi_close_volume(re->desc);
+		list_del(&re->list);
+		kfree(re);
+	}
+	return err;
+}
+
 static int ubi_cdev_ioctl(struct cdev *cdev, int cmd, void *buf)
 {
 	struct ubi_device *ubi = cdev->priv;
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 41b814c..ed04364 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -411,6 +411,9 @@ int ubi_rename_volumes(struct ubi_device *ubi, struct list_head *rename_list)
 
 			vol->name_len = re->new_name_len;
 			memcpy(vol->name, re->new_name, re->new_name_len + 1);
+			free(vol->cdev.name);
+			vol->cdev.name = basprintf("%s.%s", ubi->cdev.name, vol->name);
+			vol->cdev.size = vol->used_bytes;
 			ubi_volume_notify(ubi, vol, UBI_VOLUME_RENAMED);
 		}
 	}
diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
index c72f95b..c215ff2 100644
--- a/include/linux/mtd/ubi.h
+++ b/include/linux/mtd/ubi.h
@@ -220,6 +220,7 @@ int ubi_flush(int ubi_num, int vol_id, int lnum);
 
 int ubi_api_create_volume(int ubi_num, struct ubi_mkvol_req *req);
 int ubi_api_remove_volume(struct ubi_volume_desc *desc, int no_vtbl);
+int ubi_api_rename_volumes(int ubi_num, struct ubi_rnvol_req *req);
 
 /*
  * This function is the same as the 'ubi_leb_read()' function, but it does not
-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] mtd: ubi: commands: added the new command 'ubirename'.

[Giorgio Dal Molin]

Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
---
 commands/ubi.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/commands/ubi.c b/commands/ubi.c
index 7c55195..3c5729e 100644
--- a/commands/ubi.c
+++ b/commands/ubi.c
@@ -328,3 +328,74 @@ BAREBOX_CMD_START(ubirmvol)
 	BAREBOX_CMD_GROUP(CMD_GRP_PART)
 	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
 BAREBOX_CMD_END
+
+static int get_vol_id(u32 ubi_num, const char *name)
+{
+	struct ubi_volume_desc *desc;
+	struct ubi_volume_info vi;
+
+	desc = ubi_open_volume_nm(ubi_num, name, UBI_READONLY);
+	if(IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	ubi_get_volume_info(desc, &vi);
+
+	return vi.vol_id;
+};
+
+static int do_ubirename(int argc, char *argv[])
+{
+	struct ubi_rnvol_req req;
+	u32 ubi_num;
+	int i, j, fd, ret;
+
+	if ((argc < 4) || (argc % 2))
+		return COMMAND_ERROR_USAGE;
+
+	req.count = (argc / 2) - 1;
+	if (req.count > UBI_MAX_RNVOL) {
+		printf("too many volume renames. (max: %u)\n", UBI_MAX_RNVOL);
+		return COMMAND_ERROR_USAGE;
+	}
+
+	fd = open(argv[1], O_WRONLY);
+	if (fd < 0) {
+		perror("open ubi dev");
+		return 1;
+	}
+
+	ret = ioctl(fd, UBI_IOCGETUBINUM, &ubi_num);
+	if (ret) {
+		perror("failed to get the ubi num");
+		return COMMAND_ERROR_USAGE;
+	}
+	close(fd);
+
+	for(i=2, j=0; i<argc; ++j, i+=2) {
+		req.ents[j].vol_id = get_vol_id(ubi_num, argv[i]);
+		if(req.ents[j].vol_id < 0) {
+			printf("'%s' is not a volume name.\n", argv[i]);
+			return COMMAND_ERROR_USAGE;
+		}
+		strncpy(req.ents[j].name, argv[i+1], UBI_MAX_VOLUME_NAME);
+		req.ents[j].name_len = strlen(req.ents[j].name);
+	}
+
+	ret = ubi_api_rename_volumes(ubi_num, &req);
+	if (ret)
+		perror("failed to rename.");
+
+	return ret ? 1 : 0;
+};
+
+BAREBOX_CMD_HELP_START(ubirename)
+BAREBOX_CMD_HELP_TEXT("Rename UBI volume(s) from UBIDEV")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(ubirename)
+	.cmd		= do_ubirename,
+	BAREBOX_CMD_DESC("rename UBI volume(s)")
+	BAREBOX_CMD_OPTS("UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]")
+	BAREBOX_CMD_GROUP(CMD_GRP_PART)
+	BAREBOX_CMD_HELP(cmd_ubirename_help)
+BAREBOX_CMD_END
-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: ubi: commands: added the new command 'ubirename'.

[Sascha Hauer]

Hi Giorgio,


Looks good mostly. Patch 1/2 is fine, some small remarks for this one.

On Fri, Sep 23, 2016 at 11:07:39AM +0200, Giorgio Dal Molin wrote:
> Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
> ---
>  commands/ubi.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/commands/ubi.c b/commands/ubi.c
> index 7c55195..3c5729e 100644
> --- a/commands/ubi.c
> +++ b/commands/ubi.c
> @@ -328,3 +328,74 @@ BAREBOX_CMD_START(ubirmvol)
>  	BAREBOX_CMD_GROUP(CMD_GRP_PART)
>  	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
>  BAREBOX_CMD_END
> +
> +	ret = ioctl(fd, UBI_IOCGETUBINUM, &ubi_num);
> +	if (ret) {
> +		perror("failed to get the ubi num");
> +		return COMMAND_ERROR_USAGE;
> +	}
> +	close(fd);
> +
> +	for(i=2, j=0; i<argc; ++j, i+=2) {

Please add a space pn both sides of binary operators.

> +		req.ents[j].vol_id = get_vol_id(ubi_num, argv[i]);
> +		if(req.ents[j].vol_id < 0) {
> +			printf("'%s' is not a volume name.\n", argv[i]);

It is a volume name, but not a valid one. Better something like

			printf("Volume '%s' does not exist on %s\n", argv[i], argv[1]);

?

> +			return COMMAND_ERROR_USAGE;
> +		}
> +		strncpy(req.ents[j].name, argv[i+1], UBI_MAX_VOLUME_NAME);
> +		req.ents[j].name_len = strlen(req.ents[j].name);
> +	}
> +
> +	ret = ubi_api_rename_volumes(ubi_num, &req);
> +	if (ret)
> +		perror("failed to rename.");

perror("failed to rename: %s", strerror(-ret));

It's always nice to print the error reason if we can.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 0/2 (try 2)] mtd: ubi: implement the new command 'ubirename'

[Giorgio Dal Molin]

Here a new set of patches implementing the command 'ubirename'.

They are based on a patch serie from Sascha that restructures a bit
the barebox UBI APIs. In particular it is now easier to find the ubi
volume ids from the command code. Moreover the command code uses now
api functions to do the rename.

This new set also fixes some minor cosmetic details noted by Sascha and
a real problem found in the function 'get_vol_id()': it was missing
a call to 'ubi_close_volume(desc)' hence leaving ubi volumes busy.
A simple test that showed the problem was:

bb> ubimkvol /dev/nand0.ubi_volumes.ubi vol_A 10MiB
bb> ubirename /dev/nand0.ubi_volumes.ubi vol_A vol_B
bb> ubirmvol /dev/nand0.ubi_volumes.ubi vol_B

The last command, ubirmvol, failed because 'vol_B' appeared to be still
busy after the rename (vol->readers==1).
	
Giorgio Dal Molin (2):
  mtd: ubi: add API call to rename volumes.
  mtd: ubi: commands: added the new command 'ubirename'.

 commands/ubi.c            |  72 +++++++++++++++++++++
 drivers/mtd/ubi/barebox.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/vmt.c     |   3 +
 include/linux/mtd/ubi.h   |   1 +
 4 files changed, 232 insertions(+)

-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2 (try 2)] mtd: ubi: implement the new command 'ubirename'

[Sascha Hauer]

On Mon, Sep 26, 2016 at 12:52:31PM +0200, Giorgio Dal Molin wrote:
> Here a new set of patches implementing the command 'ubirename'.
> 
> They are based on a patch serie from Sascha that restructures a bit
> the barebox UBI APIs. In particular it is now easier to find the ubi
> volume ids from the command code. Moreover the command code uses now
> api functions to do the rename.
> 
> This new set also fixes some minor cosmetic details noted by Sascha and
> a real problem found in the function 'get_vol_id()': it was missing
> a call to 'ubi_close_volume(desc)' hence leaving ubi volumes busy.
> A simple test that showed the problem was:
> 
> bb> ubimkvol /dev/nand0.ubi_volumes.ubi vol_A 10MiB
> bb> ubirename /dev/nand0.ubi_volumes.ubi vol_A vol_B
> bb> ubirmvol /dev/nand0.ubi_volumes.ubi vol_B
> 
> The last command, ubirmvol, failed because 'vol_B' appeared to be still
> busy after the rename (vol->readers==1).
> 	
> Giorgio Dal Molin (2):
>   mtd: ubi: add API call to rename volumes.
>   mtd: ubi: commands: added the new command 'ubirename'.

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] mtd: ubi: add API call to rename volumes.

[Giorgio Dal Molin]

Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
---
 drivers/mtd/ubi/barebox.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ubi/vmt.c     |   3 +
 include/linux/mtd/ubi.h   |   1 +
 3 files changed, 160 insertions(+)

diff --git a/drivers/mtd/ubi/barebox.c b/drivers/mtd/ubi/barebox.c
index 23c4bfd..329cf45 100644
--- a/drivers/mtd/ubi/barebox.c
+++ b/drivers/mtd/ubi/barebox.c
@@ -280,6 +280,162 @@ int ubi_api_remove_volume(struct ubi_volume_desc *desc, int no_vtbl)
 	return ubi_remove_volume(desc, no_vtbl);
 }
 
+int ubi_api_rename_volumes(int ubi_num, struct ubi_rnvol_req *req)
+{
+	int i, n, err;
+	struct list_head rename_list;
+	struct ubi_rename_entry *re, *re1;
+	struct ubi_device *ubi;
+
+	if (req->count < 0 || req->count > UBI_MAX_RNVOL)
+		return -EINVAL;
+
+	if (req->count == 0)
+		return 0;
+
+	ubi = ubi_get_device(ubi_num);
+	if (!ubi)
+		return -ENODEV;
+
+	/* Validate volume IDs and names in the request */
+	for (i = 0; i < req->count; i++) {
+		if (req->ents[i].vol_id < 0 ||
+		    req->ents[i].vol_id >= ubi->vtbl_slots)
+			return -EINVAL;
+		if (req->ents[i].name_len < 0)
+			return -EINVAL;
+		if (req->ents[i].name_len > UBI_VOL_NAME_MAX)
+			return -ENAMETOOLONG;
+		req->ents[i].name[req->ents[i].name_len] = '\0';
+		n = strlen(req->ents[i].name);
+		if (n != req->ents[i].name_len)
+			return -EINVAL;
+	}
+
+	/* Make sure volume IDs and names are unique */
+	for (i = 0; i < req->count - 1; i++) {
+		for (n = i + 1; n < req->count; n++) {
+			if (req->ents[i].vol_id == req->ents[n].vol_id) {
+				ubi_err(ubi, "duplicated volume id %d",
+					req->ents[i].vol_id);
+				return -EINVAL;
+			}
+			if (!strcmp(req->ents[i].name, req->ents[n].name)) {
+				ubi_err(ubi, "duplicated volume name \"%s\"",
+					req->ents[i].name);
+				return -EINVAL;
+			}
+		}
+	}
+
+	/* Create the re-name list */
+	INIT_LIST_HEAD(&rename_list);
+	for (i = 0; i < req->count; i++) {
+		int vol_id = req->ents[i].vol_id;
+		int name_len = req->ents[i].name_len;
+		const char *name = req->ents[i].name;
+
+		re = kzalloc(sizeof(struct ubi_rename_entry), GFP_KERNEL);
+		if (!re) {
+			err = -ENOMEM;
+			goto out_free;
+		}
+
+		re->desc = ubi_open_volume(ubi->ubi_num, vol_id, UBI_READONLY);
+		if (IS_ERR(re->desc)) {
+			err = PTR_ERR(re->desc);
+			ubi_err(ubi, "cannot open volume %d, error %d",
+				vol_id, err);
+			kfree(re);
+			goto out_free;
+		}
+
+		/* Skip this re-naming if the name does not really change */
+		if (re->desc->vol->name_len == name_len &&
+		    !memcmp(re->desc->vol->name, name, name_len)) {
+			ubi_close_volume(re->desc);
+			kfree(re);
+			continue;
+		}
+
+		re->new_name_len = name_len;
+		memcpy(re->new_name, name, name_len);
+		list_add_tail(&re->list, &rename_list);
+		dbg_gen("will rename volume %d from \"%s\" to \"%s\"",
+			vol_id, re->desc->vol->name, name);
+	}
+
+	if (list_empty(&rename_list))
+		return 0;
+
+	/* Find out the volumes which have to be removed */
+	list_for_each_entry(re, &rename_list, list) {
+		struct ubi_volume_desc *desc;
+		int no_remove_needed = 0;
+
+		/*
+		 * Volume @re->vol_id is going to be re-named to
+		 * @re->new_name, while its current name is @name. If a volume
+		 * with name @re->new_name currently exists, it has to be
+		 * removed, unless it is also re-named in the request (@req).
+		 */
+		list_for_each_entry(re1, &rename_list, list) {
+			if (re->new_name_len == re1->desc->vol->name_len &&
+			    !memcmp(re->new_name, re1->desc->vol->name,
+				    re1->desc->vol->name_len)) {
+				no_remove_needed = 1;
+				break;
+			}
+		}
+
+		if (no_remove_needed)
+			continue;
+
+		/*
+		 * It seems we need to remove volume with name @re->new_name,
+		 * if it exists.
+		 */
+		desc = ubi_open_volume_nm(ubi->ubi_num, re->new_name,
+					  UBI_EXCLUSIVE);
+		if (IS_ERR(desc)) {
+			err = PTR_ERR(desc);
+			if (err == -ENODEV)
+				/* Re-naming into a non-existing volume name */
+				continue;
+
+			/* The volume exists but busy, or an error occurred */
+			ubi_err(ubi, "cannot open volume \"%s\", error %d",
+				re->new_name, err);
+			goto out_free;
+		}
+
+		re1 = kzalloc(sizeof(struct ubi_rename_entry), GFP_KERNEL);
+		if (!re1) {
+			err = -ENOMEM;
+			ubi_close_volume(desc);
+			goto out_free;
+		}
+
+		re1->remove = 1;
+		re1->desc = desc;
+		list_add(&re1->list, &rename_list);
+		dbg_gen("will remove volume %d, name \"%s\"",
+			re1->desc->vol->vol_id, re1->desc->vol->name);
+	}
+
+	mutex_lock(&ubi->device_mutex);
+	err = ubi_rename_volumes(ubi, &rename_list);
+	mutex_unlock(&ubi->device_mutex);
+
+out_free:
+	list_for_each_entry_safe(re, re1, &rename_list, list) {
+		ubi_close_volume(re->desc);
+		list_del(&re->list);
+		kfree(re);
+	}
+	return err;
+}
+
 static int ubi_cdev_ioctl(struct cdev *cdev, int cmd, void *buf)
 {
 	struct ubi_device *ubi = cdev->priv;
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 41b814c..ed04364 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -411,6 +411,9 @@ int ubi_rename_volumes(struct ubi_device *ubi, struct list_head *rename_list)
 
 			vol->name_len = re->new_name_len;
 			memcpy(vol->name, re->new_name, re->new_name_len + 1);
+			free(vol->cdev.name);
+			vol->cdev.name = basprintf("%s.%s", ubi->cdev.name, vol->name);
+			vol->cdev.size = vol->used_bytes;
 			ubi_volume_notify(ubi, vol, UBI_VOLUME_RENAMED);
 		}
 	}
diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
index c72f95b..c215ff2 100644
--- a/include/linux/mtd/ubi.h
+++ b/include/linux/mtd/ubi.h
@@ -220,6 +220,7 @@ int ubi_flush(int ubi_num, int vol_id, int lnum);
 
 int ubi_api_create_volume(int ubi_num, struct ubi_mkvol_req *req);
 int ubi_api_remove_volume(struct ubi_volume_desc *desc, int no_vtbl);
+int ubi_api_rename_volumes(int ubi_num, struct ubi_rnvol_req *req);
 
 /*
  * This function is the same as the 'ubi_leb_read()' function, but it does not
-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] mtd: ubi: commands: added the new command 'ubirename'.

[Giorgio Dal Molin]

Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
---
 commands/ubi.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/commands/ubi.c b/commands/ubi.c
index 7c55195..34247d6 100644
--- a/commands/ubi.c
+++ b/commands/ubi.c
@@ -328,3 +328,75 @@ BAREBOX_CMD_START(ubirmvol)
 	BAREBOX_CMD_GROUP(CMD_GRP_PART)
 	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
 BAREBOX_CMD_END
+
+static int get_vol_id(u32 ubi_num, const char *name)
+{
+	struct ubi_volume_desc *desc;
+	struct ubi_volume_info vi;
+
+	desc = ubi_open_volume_nm(ubi_num, name, UBI_READONLY);
+	if(IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	ubi_get_volume_info(desc, &vi);
+	ubi_close_volume(desc);
+
+	return vi.vol_id;
+};
+
+static int do_ubirename(int argc, char *argv[])
+{
+	struct ubi_rnvol_req req;
+	u32 ubi_num;
+	int i, j, fd, ret;
+
+	if ((argc < 4) || (argc % 2))
+		return COMMAND_ERROR_USAGE;
+
+	req.count = (argc / 2) - 1;
+	if (req.count > UBI_MAX_RNVOL) {
+		printf("too many volume renames. (max: %u)\n", UBI_MAX_RNVOL);
+		return COMMAND_ERROR_USAGE;
+	}
+
+	fd = open(argv[1], O_WRONLY);
+	if (fd < 0) {
+		perror("unable to open the UBI device");
+		return 1;
+	}
+
+	ret = ioctl(fd, UBI_IOCGETUBINUM, &ubi_num);
+	if (ret) {
+		perror("failed to get the ubi num");
+		return COMMAND_ERROR_USAGE;
+	}
+	close(fd);
+
+	for(i=2, j=0; i < argc; ++j, i += 2) {
+		req.ents[j].vol_id = get_vol_id(ubi_num, argv[i]);
+		if(req.ents[j].vol_id < 0) {
+			printf("Volume '%s' does not exist on %s\n", argv[i], argv[1]);
+			return COMMAND_ERROR_USAGE;
+		}
+		strncpy(req.ents[j].name, argv[i+1], UBI_MAX_VOLUME_NAME);
+		req.ents[j].name_len = strlen(req.ents[j].name);
+	}
+
+	ret = ubi_api_rename_volumes(ubi_num, &req);
+	if (ret)
+		perror("failed to rename.");
+
+	return ret ? 1 : 0;
+};
+
+BAREBOX_CMD_HELP_START(ubirename)
+BAREBOX_CMD_HELP_TEXT("Rename UBI volume(s) from UBIDEV")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(ubirename)
+	.cmd		= do_ubirename,
+	BAREBOX_CMD_DESC("rename UBI volume(s)")
+	BAREBOX_CMD_OPTS("UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]")
+	BAREBOX_CMD_GROUP(CMD_GRP_PART)
+	BAREBOX_CMD_HELP(cmd_ubirename_help)
+BAREBOX_CMD_END
-- 
2.10.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Aw: Re: [PATCH 2/2] commands: ubi: added the new command 'ubirename' to rename ubi volumes.

[iw3gtf]

Hi,


----- Original Nachricht ----
Von:     Sascha Hauer <s.hauer@pengutronix.de>
An:      Giorgio Dal Molin <iw3gtf@arcor.de>
Datum:   22.09.2016 10:04
Betreff: Re: [PATCH 2/2] commands: ubi: added the new command 'ubirename' to
 rename ubi volumes.

> On Wed, Sep 21, 2016 at 10:04:43AM +0200, Giorgio Dal Molin wrote:
> > From: Giorgio Dal Molin <giorgio.dal.molin@mobotix.com>
> > 
> > The syntax was taken from the corresponding command of the 'mts-utils'
> > userland package:
> > 
> > # ubirename UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]
> > 
> > Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
> > ---
> >  commands/ubi.c | 87
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 87 insertions(+)
> > 
> > diff --git a/commands/ubi.c b/commands/ubi.c
> > index 26b521f..3479340 100644
> > --- a/commands/ubi.c
> > +++ b/commands/ubi.c
> > @@ -6,6 +6,8 @@
> >  #include <errno.h>
> >  #include <getopt.h>
> >  #include <linux/mtd/mtd.h>
> > +#include <linux/mtd/ubi.h>
> > +#include <libgen.h>
> >  #include <linux/kernel.h>
> >  #include <linux/stat.h>
> >  #include <linux/mtd/mtd-abi.h>
> > @@ -306,3 +308,88 @@ BAREBOX_CMD_START(ubirmvol)
> >  	BAREBOX_CMD_GROUP(CMD_GRP_PART)
> >  	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
> >  BAREBOX_CMD_END
> > +
> > +
> > +static int get_vol_id(const char *vol_name)
> > +{
> > +	struct ubi_volume_desc *desc;
> > +	struct cdev *vol_cdev;
> > +	struct ubi_volume_info vi;
> > +
> > +	vol_cdev = cdev_by_name(vol_name);
> > +	if(!vol_cdev) {
> > +		perror("cdev_by_name");
> > +		return -1;
> > +	}
> > +	desc = ubi_open_volume_cdev(vol_cdev, UBI_READONLY);
> > +	if(IS_ERR(desc)) {
> > +		perror("ubi_open_volume_cdev");
> > +		return -1;
> > +	}
> > +	ubi_get_volume_info(desc, &vi);
> > +	ubi_close_volume(desc);
> > +
> > +	return vi.vol_id;
> > +};
> 
> This get_vol_id() is not particularly nice. Also I do not like having
> the rename operation inside the ioctl parser as this means we cannot
> make the ubirename code optional for users that do not need renaming.
> 
> I just sent out a series which should help you in this regard. Can you
> base your code ontop of this?

I've noticed a problem in the 3rd patch ([PATCH 3/4] mtd: ubi: commands: use function API to access ubi volumes)
of your last UBI serie: in the resulting code in 'commands/ubi.c', function 'do_ubirmvol()':

...
	desc = ubi_open_volume_nm(ubinum, argv[2], UBI_EXCLUSIVE);
	if (IS_ERR(desc)) {
		ret = PTR_ERR(desc);
		printf("failed to open volume %s: %s\n", argv[2], strerror(-ret));
		goto err1;
	}

	ret = ubi_api_remove_volume(desc, 0);
	if (ret)
		printf("failed to remove volume %s: %s\n", argv[2], strerror(-ret));

err1:
	ubi_close_volume(desc);
err:
	close(fd);

	return ret ? 1 : 0;
}

You have a wrong 'goto err1': in that case the 'desc' pointer is not a memory address, it
represent a negative error code and you cannot use it in ubi_close_volume(desc) (it segfaults).

> 
> You should be able to get the vol_id with ubi_open_volume_nm() followed
> by ubi_get_volume_info().
> 
> The rename_volumes() from patch #1 should become a
> ubi_api_rename_volumes() directly called from the command code.
> 
> Sascha
> 

giorgio


Giorgio, iw3gtf@arcor.de

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: Re: [PATCH 2/2] commands: ubi: added the new command 'ubirename' to rename ubi volumes.

[Sascha Hauer]

On Fri, Sep 23, 2016 at 12:11:56PM +0200, iw3gtf@arcor.de wrote:
> Hi,
> 
> 
> ----- Original Nachricht ----
> Von:     Sascha Hauer <s.hauer@pengutronix.de>
> An:      Giorgio Dal Molin <iw3gtf@arcor.de>
> Datum:   22.09.2016 10:04
> Betreff: Re: [PATCH 2/2] commands: ubi: added the new command 'ubirename' to
>  rename ubi volumes.
> 
> > On Wed, Sep 21, 2016 at 10:04:43AM +0200, Giorgio Dal Molin wrote:
> > > From: Giorgio Dal Molin <giorgio.dal.molin@mobotix.com>
> > > 
> > > The syntax was taken from the corresponding command of the 'mts-utils'
> > > userland package:
> > > 
> > > # ubirename UBIDEV OLD_NAME NEW_NAME [OLD_NAME NEW_NAME ...]
> > > 
> > > Signed-off-by: Giorgio Dal Molin <iw3gtf@arcor.de>
> > > ---
> > >  commands/ubi.c | 87
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 87 insertions(+)
> > > 
> > > diff --git a/commands/ubi.c b/commands/ubi.c
> > > index 26b521f..3479340 100644
> > > --- a/commands/ubi.c
> > > +++ b/commands/ubi.c
> > > @@ -6,6 +6,8 @@
> > >  #include <errno.h>
> > >  #include <getopt.h>
> > >  #include <linux/mtd/mtd.h>
> > > +#include <linux/mtd/ubi.h>
> > > +#include <libgen.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/stat.h>
> > >  #include <linux/mtd/mtd-abi.h>
> > > @@ -306,3 +308,88 @@ BAREBOX_CMD_START(ubirmvol)
> > >  	BAREBOX_CMD_GROUP(CMD_GRP_PART)
> > >  	BAREBOX_CMD_HELP(cmd_ubirmvol_help)
> > >  BAREBOX_CMD_END
> > > +
> > > +
> > > +static int get_vol_id(const char *vol_name)
> > > +{
> > > +	struct ubi_volume_desc *desc;
> > > +	struct cdev *vol_cdev;
> > > +	struct ubi_volume_info vi;
> > > +
> > > +	vol_cdev = cdev_by_name(vol_name);
> > > +	if(!vol_cdev) {
> > > +		perror("cdev_by_name");
> > > +		return -1;
> > > +	}
> > > +	desc = ubi_open_volume_cdev(vol_cdev, UBI_READONLY);
> > > +	if(IS_ERR(desc)) {
> > > +		perror("ubi_open_volume_cdev");
> > > +		return -1;
> > > +	}
> > > +	ubi_get_volume_info(desc, &vi);
> > > +	ubi_close_volume(desc);
> > > +
> > > +	return vi.vol_id;
> > > +};
> > 
> > This get_vol_id() is not particularly nice. Also I do not like having
> > the rename operation inside the ioctl parser as this means we cannot
> > make the ubirename code optional for users that do not need renaming.
> > 
> > I just sent out a series which should help you in this regard. Can you
> > base your code ontop of this?
> 
> I've noticed a problem in the 3rd patch ([PATCH 3/4] mtd: ubi: commands: use function API to access ubi volumes)
> of your last UBI serie: in the resulting code in 'commands/ubi.c', function 'do_ubirmvol()':
> 
> ...
> 	desc = ubi_open_volume_nm(ubinum, argv[2], UBI_EXCLUSIVE);
> 	if (IS_ERR(desc)) {
> 		ret = PTR_ERR(desc);
> 		printf("failed to open volume %s: %s\n", argv[2], strerror(-ret));
> 		goto err1;
> 	}
> 
> 	ret = ubi_api_remove_volume(desc, 0);
> 	if (ret)
> 		printf("failed to remove volume %s: %s\n", argv[2], strerror(-ret));
> 
> err1:
> 	ubi_close_volume(desc);
> err:
> 	close(fd);
> 
> 	return ret ? 1 : 0;
> }
> 
> You have a wrong 'goto err1': in that case the 'desc' pointer is not a memory address, it
> represent a negative error code and you cannot use it in ubi_close_volume(desc) (it segfaults).

Fixed that. Thanks for noting.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox