Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cptofs: remount as read-only prior to exiting #534

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

Mic92
Copy link

@Mic92 Mic92 commented Oct 11, 2023

depends on #532

@Mic92 Mic92 changed the title Fix umount cptofs: remount as read-only prior to exiting Oct 11, 2023
@Mic92 Mic92 mentioned this pull request Oct 11, 2023
1 task
@@ -675,6 +675,26 @@ int main(int argc, char **argv)
break;
}

/* FIXME: Remouting as readonly seems to be required to make sure the filessytem is cleanly umounted. Otherwise the journal will be still dirty... */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as a workaround, but I would like to investigate the root cause. Do you have a way to reproduce this?

Copy link
Author

@Mic92 Mic92 Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ truncate -s 10G /tmp/foo.img
$ mkfs.ext4 /tmp/foo.img         
$ ./tools/lkl/cptofs -p -o 1000 -g 1000 -t ext4 -i /tmp/foo.img ./tools/lkl/tests  / 

Now if you try to mount the image read-only, you can see hat it fails because of the unclean journal

$ mount -o ro /tmp/foo.img /mnt

The error will show up in dmesg.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tavip did you managed to reproduce the issue? This would block us updating lkl in NixOS, where we use it for building disk images without the need for KVM.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found the root cause but it is turning out more difficult to fix than I expected. Lets merge this for now. Thanks for the ping.

also umount returns and /proc/self/mounts no longer lists the filesystem, ext4 seems to only flush the filesystem properly after a certain delay.
To work around this, we now remount the filesystem as read-only to ensure that it is actually finished by the time we exited

Signed-off-by: Jörg Thalheim <[email protected]>
@Mic92
Copy link
Author

Mic92 commented Oct 12, 2023

I can fix the coding style issues in case you cannot find a better solution to fix the issue

@tavip tavip merged commit 970883c into lkl:master Nov 7, 2023
11 of 13 checks passed
@Mic92 Mic92 deleted the fix-umount branch November 8, 2023 07:49
@Mic92
Copy link
Author

Mic92 commented Nov 8, 2023

Ok. It's probably good to drop some insights on what the actual root cause is.

@Mic92 Mic92 mentioned this pull request Nov 8, 2023
12 tasks
@physics-enthusiast
Copy link

@tavip If you don't mind, care to give an update on the root cause of this? I suspect #466 might be linked to this workaround and issue in general based on the trace provided in NixOS/nixpkgs#59867, but that was as far as my limited C skills got me. From what I can tell, the lkl umount call follows an identical call path (sans wrapper) as the mainline kernel so I really am rather curious what is causing it to fail a journal flush.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants