Patchwork Offtopic: ubifs and best practice for supporting browser based firmware upgrades

login
register
mail settings
Submitter Artem Bityutskiy
Date Sept. 26, 2012, 11:13 a.m.
Message ID <1348658003.24309.70.camel@sauron.fi.intel.com>
Download mbox | patch
Permalink /patch/187044/
State New
Headers show

Comments

Artem Bityutskiy - Sept. 26, 2012, 11:13 a.m.
From df2a498002498e52025acf51e0b43064d766aa0a Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Wed, 26 Sep 2012 14:12:28 +0300
Subject: [PATCH] UBI: allow for delayed rename

Just an experimental patch.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
 drivers/mtd/ubi/vmt.c |   19 -------------------
 1 file changed, 19 deletions(-)
Artem Bityutskiy - Sept. 26, 2012, 11:17 a.m.
On Wed, 2012-09-26 at 14:13 +0300, Artem Bityutskiy wrote:
> From df2a498002498e52025acf51e0b43064d766aa0a Mon Sep 17 00:00:00 2001
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Date: Wed, 26 Sep 2012 14:12:28 +0300
> Subject: [PATCH] UBI: allow for delayed rename
> 
> Just an experimental patch.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

This have a lot of chances to work. If we do this, it would need to be
an extention to the existing ioctl, to preserve the original behaviour.
Something like a "delayed" flag, and then we can implement this in
ubirename as a --delayed option. Surely there may be better names for
this.
Jaya Kumar - Sept. 26, 2012, 12:38 p.m.
On Wed, Sep 26, 2012 at 7:13 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From df2a498002498e52025acf51e0b43064d766aa0a Mon Sep 17 00:00:00 2001
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Date: Wed, 26 Sep 2012 14:12:28 +0300
> Subject: [PATCH] UBI: allow for delayed rename
>
> Just an experimental patch.

Thanks Artem. I gave it a quick try.

I applied the patch, flashed new kernel and then attempted to rename
the rootfs-reserve volume to rootfs while running from the rootfs
volume.

root@localhost:/# ubirename /dev/ubi0 rootfs-reserve rootfs
UBI error: ubi_open_volume: cannot open device 0, volume 1, error -16
UBI error: rename_volumes: cannot open volume "rootfs", error -16
ubirename: error!: cannot rename volumes
           error 16 (Device or resource busy)

UBI DBG (pid 1786): ubi_cdev_ioctl: re-name volumes
UBI DBG (pid 1786): rename_volumes: will rename volume 0 from
"rootfs-reserve" to "rootfs"
UBI error: ubi_open_volume: cannot open device 0, volume 1, error -16
UBI error: rename_volumes: cannot open volume "rootfs", error -16

I also tried the reverse, renaming the running rootfs volume to something else.

root@localhost:/# ubirename /dev/ubi0 rootfs rootfsold
UBI error: ubi_open_volume: cannot open device 0, volume 1, error -16
UBI error: rename_volumes: cannot open volume 1, error -16
ubirename: error!: cannot rename volumes
           error 16 (Device or resource busy)
UBI DBG (pid 2600): ubi_cdev_ioctl: re-name volumes
UBI error: ubi_open_volume: cannot open device 0, volume 1, error -16
UBI error: rename_volumes: cannot open volume 1, error -16

I'm guessing I also need to hack the following in rename_volumes:
desc = ubi_open_volume_nm(ubi->ubi_num, re->new_name, UBI_EXCLUSIVE);

I'll try looking at it further tomorrow.

Thanks,
jayakumar
Artem Bityutskiy - Oct. 10, 2012, 2:18 p.m.
On Wed, 2012-09-26 at 20:38 +0800, Jaya Kumar wrote:
> I'm guessing I also need to hack the following in rename_volumes:
> desc = ubi_open_volume_nm(ubi->ubi_num, re->new_name, UBI_EXCLUSIVE);

Probably, you can use UBI_READONLY instead of UBI_EXCLUSIVE, I guess,
just to try if the approach works.

Patch

diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 9169e58..6d7bcd9 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -597,25 +597,6 @@  int ubi_rename_volumes(struct ubi_device *ubi, struct list_head *rename_list)
 	struct ubi_rename_entry *re;
 
 	err = ubi_vtbl_rename_volumes(ubi, rename_list);
-	if (err)
-		return err;
-
-	list_for_each_entry(re, rename_list, list) {
-		if (re->remove) {
-			err = ubi_remove_volume(re->desc, 1);
-			if (err)
-				break;
-		} else {
-			struct ubi_volume *vol = re->desc->vol;
-
-			spin_lock(&ubi->volumes_lock);
-			vol->name_len = re->new_name_len;
-			memcpy(vol->name, re->new_name, re->new_name_len + 1);
-			spin_unlock(&ubi->volumes_lock);
-			ubi_volume_notify(ubi, vol, UBI_VOLUME_RENAMED);
-		}
-	}
-
 	if (!err)
 		self_check_volumes(ubi);
 	return err;