RFC : attach ubi in readonly mode

RFC : attach ubi in readonly mode

[Matthieu CASTET]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

Hi,

it could be interesting to attach ubi in read-only mode (for example getting ubi
volume info without altering flash).

A solution to do that could be to add an argument to UBI_IOCATT ioctl.

Another solution should be to have a way to switch MTD_WRITEABLE flags.

What do you think a that ?


PS : attached a proof of concept for solution 1

[-- Attachment #2: poc_ubi_attach_ro.diff --]
[-- Type: text/x-diff, Size: 2680 bytes --]

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 13b05cb..56c2249 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -841,6 +841,7 @@ static int autoresize(struct ubi_device *ubi, int vol_id)
  * @mtd: MTD device description object
  * @ubi_num: number to assign to the new UBI device
  * @vid_hdr_offset: VID header offset
+ * @ro : attach in readonly mode
  *
  * This function attaches MTD device @mtd_dev to UBI and assign @ubi_num number
  * to the newly created UBI device, unless @ubi_num is %UBI_DEV_NUM_AUTO, in
@@ -851,7 +852,7 @@ static int autoresize(struct ubi_device *ubi, int vol_id)
  * Note, the invocations of this function has to be serialized by the
  * @ubi_devices_mutex.
  */
-int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
+int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset, int ro)
 {
 	struct ubi_device *ubi;
 	int i, err, ref = 0;
@@ -922,6 +923,12 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
 
 	ubi_msg("attaching mtd%d to ubi%d", mtd->index, ubi_num);
 
+	if (ro) {
+		ubi_msg("attach in "
+			"read-only mode");
+		ubi->ro_mode = 1;
+	}
+
 	err = io_init(ubi);
 	if (err)
 		goto out_free;
@@ -1209,7 +1216,7 @@ static int __init ubi_init(void)
 
 		mutex_lock(&ubi_devices_mutex);
 		err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
-					 p->vid_hdr_offs);
+					 p->vid_hdr_offs, 0);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0) {
 			ubi_err("cannot attach mtd%d", mtd->index);
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 4dfa6b9..9be009f 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -1007,7 +1007,7 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd,
 		 * 'ubi_attach_mtd_dev()'.
 		 */
 		mutex_lock(&ubi_devices_mutex);
-		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset);
+		err = ubi_attach_mtd_dev(mtd, req.ubi_num, req.vid_hdr_offset, req.padding[0]);
 		mutex_unlock(&ubi_devices_mutex);
 		if (err < 0)
 			put_mtd_device(mtd);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index a637f02..aff929f 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -550,7 +550,7 @@ int ubi_io_write_vid_hdr(struct ubi_device *ubi, int pnum,
 			 struct ubi_vid_hdr *vid_hdr);
 
 /* build.c */
-int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset);
+int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset, int ro);
 int ubi_detach_mtd_dev(int ubi_num, int anyway);
 struct ubi_device *ubi_get_device(int ubi_num);
 void ubi_put_device(struct ubi_device *ubi);

Re: RFC : attach ubi in readonly mode

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On Fri, 2012-02-03 at 14:40 +0100, Matthieu CASTET wrote:
> Hi,
> 
> it could be interesting to attach ubi in read-only mode (for example getting ubi
> volume info without altering flash).
> 
> A solution to do that could be to add an argument to UBI_IOCATT ioctl.

No objections in general, especially if you also:

1. amend the mtd-utils and add support for this option
2. add a piece of doc for the mtd web site
3. add a test for this feature to mtd-utils.git/tests/ubi-test/
(probably simpler to improve one of the existing tests there)

> Another solution should be to have a way to switch MTD_WRITEABLE flags.

This can be done as well, up to you. But I guess UBI will need changes
anyway to handle gracefully situation like:

1. you have RW partition, do a lot of I/O, then reboot uncleanly, so
that there are many erase operations unfinished

2. You reboot, mark the partition R/O, try to attach it - UBI will
probably anyway try to erase some blocks and get -EFORS, not sure will
it panic or not. Should be tested, but probably not a big deal.

IOW, both approaches should be OK.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: RFC : attach ubi in readonly mode

[Marek Vasut]

Dear Matthieu CASTET,

> Hi,
> 
> it could be interesting to attach ubi in read-only mode (for example
> getting ubi volume info without altering flash).
> 
> A solution to do that could be to add an argument to UBI_IOCATT ioctl.
> 
> Another solution should be to have a way to switch MTD_WRITEABLE flags.
> 
> What do you think a that ?
> 
> 
> PS : attached a proof of concept for solution 1

Did you get any further with this patches? Is it now possible to safely mount 
UBIFS in read-only mode in some way please?

Thanks!

Best regards,
Marek Vasut