Code review comment for ~crass/casper:fix-unsafe-mount-iso-loopback

Revision history for this message
Glenn Washburn (crass) wrote :

> Thanks for this proposal. As this is a substantive change, I would like to
> see a bug opened against https://bugs.launchpad.net/ubuntu/+source/casper
> before accepting it, so that we ensure we have a way to track
> discussion/feedback from users in the future.
>
> A few thoughts:
> - Ubuntu does not support hibernation anymore, for various reasons; TL;DR: the
> usability of it is sufficiently poor, and the power savings advantage
> sufficiently limited versus suspend on modern systems, that it was not worth
> carrying on. I don't know what other Linux distributions do with respect to
> hibernation support these days, but if the situation is similar, then I think
> this is an uncommon use case.

Yes, I imagine it is. And I shouldn't need to point out that users with uncommon setups are no less deserving of data loss protection.

> - This patch changes the default behavior of casper when loading images from
> nearly all supported writable filesystems. I'm not sure we should change the
> default. It is *safer*, but it reduces usability for most of our users
> (again, a hibernated Ubuntu filesystem is uncommon; booting a livefs on a
> system that's hibernated is even more uncommon). This is partly why I think
> we should have a bug report attached to this discussion.

This is reasonable. So you want a bug report that it explicit about the data loss scenario, correct?

> - The mount manpage mentions using '-o ro,noload' with ext3 and ext4
> filesystems to let it be mounted without the journal being replayed. Wouldn't
> this be a robust solution that doesn't require refusing to load from ext3/ext4
> filesystems by default?

For these filesystems, it might be a robust solution. Personally, I would feel uneasy about it. I'd rather a better guarantee that nothing gets written to the filesystem. For instance, that may still write to the superblock and I don't know if that will corrupt the resume process. I also don't like this because its not general. What about XFS and BTRFS?

> - This is only an issue when the user boots a casper-based system
> *specifically pointing it* at an image which is located on a journalling
> filesystem that has a dirty journal because it was mounted by a Linux system
> that has been hibernated and will be confused by a journal replay happening
> behind its back. So uh seriously, who does that and why?

I think you addressed this in a later comment.

« Back to merge proposal