Skip to content

sys/storage: gate the sysfs device discovery to unix - #548

Open
obodnikov wants to merge 1 commit into
matrix-construct:mainfrom
obodnikov:fix/storage-sysfs-unix-gating
Open

sys/storage: gate the sysfs device discovery to unix#548
obodnikov wants to merge 1 commit into
matrix-construct:mainfrom
obodnikov:fix/storage-sysfs-unix-gating

Conversation

@obodnikov

Copy link
Copy Markdown
Contributor

What does this PR do?

sys/storage.rs walks /sys/dev/block/ and reaches the device through
MetadataExt::dev and libc::major/libc::minor, none of which exists off unix, so
tuwunel_core does not compile there:

error[E0599]: no method named `dev` found for struct `Metadata` in the current scope
error[E0425]: cannot find function `major` in crate `libc`
error[E0425]: cannot find function `minor` in crate `libc`

dev_from_path already carries #[cfg(target_family = "unix")] on its MetadataExt
import — which is what makes the method go missing rather than the import fail — but the
three call sites below it were never gated to match. This puts the whole discovery path
behind the same condition, so that attribute becomes redundant and goes with it.

md_discover and name_from_path are the two entry points, both called unconditionally
from database/pool/configure.rs, so each gains a #[cfg(not(unix))] arm rather than
pushing a condition onto the caller:

/// Get properties of a MultiDevice (md) storage system.
///
/// Reports no raid on a target without sysfs, which is what the unix arm also
/// returns for a path that is not on one.
#[cfg(not(unix))]
#[must_use]
pub fn md_discover(_path: &Path) -> MultiDevice { MultiDevice::default() }
/// Get the name of the block device on which Path is mounted.
///
/// Naming the device requires sysfs, so this reports the target cannot do it.
/// The unix arm already returns an error when the name is not there to be read.
#[cfg(not(unix))]
pub fn name_from_path(_path: &Path) -> Result<String> {
	use std::io::{Error, ErrorKind::Unsupported};

	Err(Error::new(Unsupported, "Block device discovery requires sysfs.").into())
}

Both mirror an outcome the unix arm already produces: md_discover returns the same
empty MultiDevice it returns for a path that is not on a raid, and name_from_path
already returns NotFound when DEVNAME is not there to be read, so
configure.rs handles an error from it today.

mq_discover, queue_discover, dev_from_path and block_path are private and
reachable only from the gated entry points, so they need no second arm. itertools and
libc::dev_t do resolve off unix but are used only by the gated items, so they follow
the same condition; Path and Result are used by both arms and stay ungated.

The module comment is rewritten to say which facility is missing rather than which
platform is running, since the condition is unix, not any one target.

No behaviour change on unix. Same class of fix as #526, #527, #528 and #536.

On verification. CI builds Linux only, where none of this appears, so CI cannot
demonstrate the change: the symptom is absent there both before and after. It was
observed building v1.8.3 for x86_64-pc-windows-msvc, and the three errors above are
reproducible on any host with a two-file crate that depends on libc and calls
stat.dev(), libc::major and libc::minor behind the same
#[cfg(target_family = "unix")] import this file has. cargo +nightly fmt --check,
cargo check and cargo clippy on the unix path are clean before and after.

Checklist

  • Code is formatted with nightly cargo fmt and satisfies clippy and
    rustc lints; any allowed lint is justified by an obvious reason or a
    comment.
  • Complement compliance changes (new passes or new failures), if any,
    are noted in the description above. — none; the unix path is unchanged.
  • Config option changes were made in src/core/config/mod.rs doc
    comments and the regenerated tuwunel-example.toml is committed. — n/a
  • User-facing changes are reflected in docs/. — n/a
  • I agree that my changes may be licensed under the Apache-2.0 licence
    and my conduct is in line with the Contributor's Covenant and
    Tuwunel's Code of Conduct.

`sys/storage.rs` walks `/sys/dev/block/` and reaches the device through
`MetadataExt::dev` and `libc::major`/`libc::minor`, none of which exist off
unix, so `tuwunel_core` does not compile there:

    error[E0599]: no method named `dev` found for struct `Metadata`
    error[E0425]: cannot find function `major` in crate `libc`
    error[E0425]: cannot find function `minor` in crate `libc`

`dev_from_path` already carries `#[cfg(target_family = "unix")]` on its
`MetadataExt` import, which is what makes the method go missing rather than the
import fail; the three call sites below it were never gated to match. This puts
the whole discovery path behind the same condition, so that attribute becomes
redundant and goes.

`md_discover` and `name_from_path` are the two entry points, both called
unconditionally from `database/pool/configure.rs`, so each gains a
`#[cfg(not(unix))]` arm rather than pushing a condition onto the caller:
`md_discover` reports no raid, which is what the unix arm returns for a path
that is not on one, and `name_from_path` reports the target cannot name a
device, alongside the `NotFound` the unix arm already returns when the name is
not there to be read.

`mq_discover`, `queue_discover`, `dev_from_path` and `block_path` are private
and reachable only from the gated entry points, so they need no second arm.
`itertools` and `libc::dev_t` do resolve off unix but are used only by the gated
items, so they follow the condition; `Path` and `Result` are used by both arms
and stay ungated.

No behaviour change on unix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant