UBI_READWRITE constraint when opening volumes to rename

[Andrew Murray]

On 20 October 2014 12:10, Richard Weinberger <richard at nod.at> wrote:
> Am 19.10.2014 um 22:58 schrieb Andrew Murray:
>> On 10 October 2014 22:09, Richard Weinberger <richard at nod.at> wrote:
>>>
>>> As discussed with Ezequiel on IRC the right thing is to fix the UBI rename logic.
>>>
>>>> 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).
>>>
>>> Correct. Renaming a volume does not alter nor read volume data.
>>> Only the UBI volume table is altered.
>>> This is why I've cooked up the following patch.
>>> Please give it some testing!
>>
>> This appears to work for me - I can rename a mounted volume without
>> any obvious side effects. I've slightly tweaked (added a break
>> statement) and submitted your patch.
>>
>> Though I've had a further look and have an additional questions.
>>
>> This patch means that UBI_EXCLUSIVE is no longer exclusive (unless
>> these constraints are with respect to the volume data only and not the
>> volume table as well) - thus should we update the ubi_open_volume
>> function to prevent UBI_EXCLUSIVE when metaonly is already held (and
>> to prevent UBI_METAONLY when exclusive is already held)? I can't see
>> the harm in doing this and it probably won't impact the use case that
>> led to this patch.
>>
>
> Correct. See my first mail. :)
>
>> I can't see any locking around the table fields and thus (with present
>> patch) I assume you could perform a rename and volume remove at the
>> same time - I wonder if this would sometimes break. I assume that
>> anything that currently modifies the volume table will be doing so
>> with exclusive held.
>
> This is why I need to review all code paths first.
> My initial patch was not supposed to be a final solution, more a base for discussion.
> I.e. to follow the "less talk, more code" rule.

Sure I understand.

I'm happy to further investigate this and propose a v2.

Andrew Murray

>
> Thanks,
> //richard