virtiofs: add option for readonly mount

[damanm24]

Adds a readonly mode to the VirtioFs implementation, allowing a virtio-fs mount to be configured as read-only via LxVolumeOptions. When enabled, all mutating operations are rejected with EROFS (Read-only file system).

Operations that are guarded:

• open - Reject O_WRONLY and O_RDWR access modes using the O_NOACCESS mask
• set_attr - Reject attribute modifications for all attributes (except FATTR_FH, and FATTR_LOCKOWNER which are used to track states and do not cause any filesystem modifications)
• create, mkdir, mknod, symlink, link, write, unlink, rmdir, rename, setx_attr, remote_xattr

[github-actions]

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

[Copilot]

Pull request overview

Adds a configurable read-only mode for the virtio-fs backend, wired through LxVolumeOptions, so the guest can mount a shared filesystem as read-only and have mutating FUSE operations rejected with EROFS.

Changes:

• Add a readonly flag to VirtioFs and gate mutating FUSE ops via check_writable().
• Add readonly state + ro mount-option parsing to LxVolumeOptions, plus accessors (readonly(), is_readonly()).
• Enforce read-only behavior in virtio-fs paths such as create/mkdir/write/unlink/rename/set_xattr/remove_xattr/set_attr.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
vm/devices/virtio/virtiofs/src/lib.rs	Stores a read-only flag and rejects mutating FUSE operations with EROFS.
vm/devices/support/fs/lxutil/src/lib.rs	Extends LxVolumeOptions with a read-only flag, parsing support for ro, and getter/setter APIs.

[justus-camp-microsoft]

I don't have all the context here, but it seems like we should try to hoist check_writable up to the Fuse session layer so that we don't have to check it in every method if possible. Otherwise LGTM, let me know if that's not possible.

[benhillis]

I don't have all the context here, but it seems like we should try to hoist check_writable up to the Fuse session layer so that we don't have to check it in every method if possible. Otherwise LGTM, let me know if that's not possible.

Brian also mentioned potentially mapping things as readonly versus readwrite as an option perhaps?

[damanm24]

I don't have all the context here, but it seems like we should try to hoist check_writable up to the Fuse session layer so that we don't have to check it in every method if possible. Otherwise LGTM, let me know if that's not possible.

Brian also mentioned potentially mapping things as readonly versus readwrite as an option perhaps?

So AFAICT the root LxVolume that's associated with a virtiofs share is already mapped with readonly permissions. The root volume is used to resolve paths so that subsequent filesystem operations on the share operate relative to the root. The reason the checks are interspersed within every possible destructive method is because the access rights on handles to files within the share are not inherited from the root handle.

What I can do it instead move the check into the lxutil layer, and then in the windows implementation I can centralize check_writeable in the create_file method.

openvmm/vm/devices/support/fs/lxutil/src/windows/mod.rs

Line 878 in 3275935

fn create_file(

However, you wouldn't be able to do the same centralization in the unix lxutil implementation because not all destructive operations require an open call with write permissions.

[Brian-Perkins]

Is this a common case? I just wonder if we should succeed a set attribute call even if it isn't actually changing anything.

[damanm24]

So FATTR_FH and FATTR_LOCKOWNER are FUSE specific attributes that don't actually change anything about the file, so if the operation only contains these flags the call will succeed. All of the other possible set_attribute flags do modify the file in some manner that violate the readonly principle so the operation will fail.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

In the open() method, lx::O_NOACCESS is used as a bit mask to extract the access mode from flags, but it is semantically an access mode value (meaning "no access"), not a mask. The correct constant to use as a mask here is lx::O_ACCESS_MASK, which has the same numeric value (0x3) but communicates the intent more clearly. The codebase already uses O_ACCESS_MASK for this purpose elsewhere (e.g., fuse/tests/fuse_hello.rs:93 uses flags & lx::O_ACCESS_MASK).

Suggested change

	let access_mode = (flags & lx::O_NOACCESS as u32) as i32;
	let access_mode = (flags & lx::O_ACCESS_MASK as u32) as i32;