fsverity: introduce EnableVerifyError::FilesystemNotSupported error #68
Conversation
| /// Enabling fsverity failed. | ||
| #[derive(Error, Debug, PartialEq)] | ||
| pub enum EnableVerifyError { | ||
| #[error("Filesystem not supported by fs-verity")] |
There was a problem hiding this comment.
I'd describe the situation with slightly different language: the filesystem doesn't support fs-verity. The enablement code is in the filesystem, not in fs-verity.
| @@ -35,12 +44,16 @@ pub struct FsVerityEnableArg { | |||
| // #define FS_IOC_ENABLE_VERITY _IOW('f', 133, struct fsverity_enable_arg) | |||
| type FsIocEnableVerity = ioctl::WriteOpcode<b'f', 133, FsVerityEnableArg>; | |||
|
|
|||
| // ENOTTY 25 Inappropriate ioctl for device | |||
| // XXX: should we use an errno crate instead? | |||
| static ENOTTY: i32 = 25; | |||
There was a problem hiding this comment.
Are we missing that in rustix::io::Errno?
https://docs.rs/rustix/latest/rustix/io/struct.Errno.html#associatedconstant.NOTTY
There was a problem hiding this comment.
Yeah, makes sense, I removed this, I did missed this :/
| @@ -53,7 +66,13 @@ pub fn fs_ioc_enable_verity<F: AsFd, H: FsVerityHashValue>(fd: F) -> std::io::Re | |||
| sig_ptr: 0, | |||
| __reserved2: [0; 11], | |||
| }), | |||
| )?; | |||
| ) { | |||
| Err(e) if e.raw_os_error() == ENOTTY => { | |||
There was a problem hiding this comment.
It should be possible to match directly on Err(rustix::io::Errno::ENOTTY) here, no?
There was a problem hiding this comment.
Indeed, silly me! Thanks and I simplified this now as suggested.
| @@ -17,6 +17,15 @@ pub enum MeasureVerityError { | |||
| InvalidDigestSize { expected: u16 }, | |||
| } | |||
|
|
|||
| /// Enabling fsverity failed. | |||
| #[derive(Error, Debug, PartialEq)] | |||
| pub enum EnableVerifyError { | |||
There was a problem hiding this comment.
I wonder if this might be a bit short-sighted. Currently this is no problem because it gets smoothed out by anyhow, but in the future if we want to start reporting errors from high-level operations on the repository, maybe we want some unified error type for that?
On the other hand, we already have MeasureVerityError and we can chain those things via #from in the unified error type, I suppose.
Also: we never said we won't break API, anyway!
mvo5
force-pushed
the
fs-verity-no-fs
branch
2 times, most recently
from
ede9fb9 to
5d04f74
Compare
This commit improves the UX of the error reporting when enabling fs-verity fails because the underlying filesystem does not support this. Here we currently just error with: ``` Inappropriate ioctl for device" ``` with the new code we error with: ``` Filesystem not supported by fs-verity ``` Note that the test relies on the fact that /dev/shm is not supported by fs-verity. Closes: containers#59 Signed-off-by: Michael Vogt <[email protected]>
mvo5
force-pushed
the
fs-verity-no-fs
branch
from
5d04f74 to
ccfc88e
Compare
This commit improves the UX of the error reporting when enabling fs-verity fails because the underlying filesystem does not support this. Here we currently just error with:
with the new code we error with:
Note that the test relies on the fact that /dev/shm is not supported by fs-verity.
Closes: #59