Re: [PATCH] nilfs2: convert to use the new mount API

[Eric Sandeen <sandeen@redhat.com>]

On 4/4/24 3:11 PM, Ryusuke Konishi wrote:
> On Thu, Apr 4, 2024 at 7:12 AM Eric Sandeen wrote:
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> Note: This one was relatively more complex than others, so I would
>> appreciate review and testing. Basic checks of mounts, various mount
>> options, and snapshot mounts do seem to work. I may well have missed
>> something though, as I am not very familiar with nilfs.
>>
>> You may want to look closely at the handling of case Opt_err: which
>> no longer uses nilfs_write_opt() and open-codes the flag change, so
>> that I can use the enum. If you'd prefer to make 3 independent
>> Opt_err_XXXZ cases, that would be possible.
>>
>> If any of the other changes here are unclear, or problematic, please
>> let me know.
>>
>> Thanks!
>> -Eric
> 
> Hi Eric,
> 
> Thank you!  This is one of the modernizations that I thought I had to
> do with nilfs2.
> 
> I'm planning on doing a full review later, but when I ran a mount
> pattern test, the kernel restarted without any messages (probably
> caused a panic), so I'll give you some quick feedback.
> 
> The mount pattern that caused the kernel to restart was a simultaneous
> mount of the current tree and a snapshot, which occurred when the
> snapshot was mounted and then the current tree was mounted.  Something
> like below:
> 
> $ sudo losetup /dev/loop0 ./nilfs.iso
> $ sudo mount -t nilfs2 -o ro,cp=38866 /dev/loop0 /mnt/snapshot
> $ sudo mount -t nilfs2 /dev/loop0 /mnt/test
> --> panic
> 
> Here, 38866 is the snapshot number that can be created with the
> nilfs-utils "mkcp -s" command or "chcp" command, and the number can be
> checked with "lscp -s".
> 
> I have placed the mount test script I used in the following location:
> 
>  https://github.com/konis/nilfs-test-tools/blob/main/test-nilfs-mount.sh
> 
> The panic occurred in test #17 of that script.
> 
> I'll also try to track what's going on.

Thanks, I'll look - I was hoping/expecting that you had better tests for
mount options than I did! ;)

Feel free to debug if you like, but it must be a bug in my patch so
I'll take ownership of trying to track down the problem and get it to
pass your test script.

-Eric

> Thanks,
> Ryusuke Konishi