diff mbox

UBI_READWRITE constraint when opening volumes to rename

Message ID 20141009102546.GA2755@arch.hh.imgtec.org
State Rejected
Headers show

Commit Message

Ezequiel Garcia Oct. 9, 2014, 10:25 a.m. UTC
On 07 Oct 03:31 PM, Andrew Murray wrote:
> Hello,
> 
> I'd like to be able to safely rename a UBI volume that contains a
> mounted UBIFS volume.
> 
> This allows for firmware upgrade via the following steps:
> 
> - ubimkvol rootfs_new
> - ubiupdatevol rootfs_new
> - ubirename rootfs rootfs_old rootfs_new rootfs
> - reboot
> - ubirmvol rootfs_old
> 
> Such an approach makes upgrade of a root filesystem simple as there is
> no need to unmount the root filesystem. I believe this question has
> been asked before on this mailing list
> (http://lists.infradead.org/pipermail/linux-mtd/2012-February/039743.html).
> 
> This process isn't possible at the moment as 'rename_volumes' opens
> the UBI volume with UBI_READWRITE. Unfortunately UBIFS always opens
> UBI with UBI_READWRITE regardless to if the user mounts as read-only.

How about making UBIFS honour the read-only mount flag?

A quick look to fs/ubifs/io.c shows that UBIFS will show an error message
if a LEB erase/write operation is attempted on a read-only mounted or
read-only media. So, hopefully, this is reasonable (the patch is completely
untested!):

Comments

Andrew Murray Oct. 9, 2014, 11:03 a.m. UTC | #1
Hi Ezequiel,

On 9 October 2014 11:25, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On 07 Oct 03:31 PM, Andrew Murray wrote:
>> Hello,
>>
>> I'd like to be able to safely rename a UBI volume that contains a
>> mounted UBIFS volume.
>>
>> This allows for firmware upgrade via the following steps:
>>
>> - ubimkvol rootfs_new
>> - ubiupdatevol rootfs_new
>> - ubirename rootfs rootfs_old rootfs_new rootfs
>> - reboot
>> - ubirmvol rootfs_old
>>
>> Such an approach makes upgrade of a root filesystem simple as there is
>> no need to unmount the root filesystem. I believe this question has
>> been asked before on this mailing list
>> (http://lists.infradead.org/pipermail/linux-mtd/2012-February/039743.html).
>>
>> This process isn't possible at the moment as 'rename_volumes' opens
>> the UBI volume with UBI_READWRITE. Unfortunately UBIFS always opens
>> UBI with UBI_READWRITE regardless to if the user mounts as read-only.
>
> How about making UBIFS honour the read-only mount flag?

Thanks for the suggestion, I didn't consider this as I assumed there
was a good reason for it being UBI_READWRITE - though it appears as
though it's always been UBI_READWRITE since the initial
implementation.

>
> A quick look to fs/ubifs/io.c shows that UBIFS will show an error message
> if a LEB erase/write operation is attempted on a read-only mounted or
> read-only media. So, hopefully, this is reasonable (the patch is completely
> untested!):

Is there any reason why UBIFS would need to write to UBI when the user
mounts UBIFS as R/O? I know that scrubbing can occur in the UBI layer
- will this still occur if the volume is opened as UBI_READONLY?

>
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 106bf20..ce445ce 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1998,11 +1998,16 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
>  {
>         struct ubifs_info *c = sb->s_fs_info;
>         struct inode *root;
> -       int err;
> +       int err, mode;
> +
> +       /*
> +        * Re-open the UBI device in read-write mode, or keep it read-only if
> +        * explicitly requested.
> +        */
> +       mode = (sb->s_flags & MS_RDONLY) ? UBI_READONLY : UBI_READWRITE;
>
>         c->vfs_sb = sb;
> -       /* Re-open the UBI device in read-write mode */
> -       c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READWRITE);
> +       c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, mode);
>         if (IS_ERR(c->ubi)) {
>                 err = PTR_ERR(c->ubi);
>                 goto out;
>

I will try this later and get back to you - this seems like the right fix.

To satisfy my curiosity - does the UBI rename function really need to
open the UBI volume as UBI_READWRITE to rename? Does it matter that a
UBIFS fileystem may be writing UBI whilst a UBI rename occurs - I've
assumed that UBIFS doesn't ever read/write the volume label - so it
doesn't matter if it changes beneath it? (I'm not too familiar with
UBI/UBIFS internals).

Thanks,

Andrew Murray

> --
> Ezequiel GarcĂ­a, Free Electrons
> Embedded Linux, Kernel and Android Engineering
> http://free-electrons.com
diff mbox

Patch

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 106bf20..ce445ce 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1998,11 +1998,16 @@  static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 {
        struct ubifs_info *c = sb->s_fs_info;
        struct inode *root;
-       int err;
+       int err, mode;
+
+       /*
+        * Re-open the UBI device in read-write mode, or keep it read-only if
+        * explicitly requested.
+        */
+       mode = (sb->s_flags & MS_RDONLY) ? UBI_READONLY : UBI_READWRITE;
 
        c->vfs_sb = sb;
-       /* Re-open the UBI device in read-write mode */
-       c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READWRITE);
+       c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, mode);
        if (IS_ERR(c->ubi)) {
                err = PTR_ERR(c->ubi);
                goto out;