Skip to content

Make the 'rawfd' feature a NOP on Windows#373

Open
germag wants to merge 2 commits intorust-vmm:mainfrom
germag:disable-rawfd-on-windows
Open

Make the 'rawfd' feature a NOP on Windows#373
germag wants to merge 2 commits intorust-vmm:mainfrom
germag:disable-rawfd-on-windows

Conversation

@germag
Copy link
Copy Markdown

@germag germag commented Mar 23, 2026

Summary of the PR

When `rawfd' feature was introduced on commit 0b7a437, it was enabled
by default to avoid breaking the default behavior on linux. So, on
Windows, vm-memory must be included with 'default-features = false'
otherwise it fails to compile. This is problematic for projects that
depend indirectly on vm-memory and fail if their dependencies don't
set 'default-features = false'.

Since there is no current way to set the default feature set for a
specific target OS, let's make the 'rawfd' feature a NOP for Windows
builds, the feature will still be enabled, but silently just not do
anything.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • [] All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@germag
Copy link
Copy Markdown
Author

germag commented Mar 23, 2026

I can make this a bit more clear using cfg_aliases, so instead of

#[cfg(all(feature = "rawfd", not(target_os = "windows")))]

we can use something like

#[cfg("rawfd")]

@XanClic
Copy link
Copy Markdown
Collaborator

XanClic commented Mar 24, 2026

I don’t like it very much that this way, on Windows, the feature will still be enabled, but silently just not do anything. I know that is a very lukewarm stance and a problem you have probably already considered, but it is not weighed on in the commit message, hence me commenting on it.

So: There is no other way, right? To change the default-features list only on Windows, or at least any way to detect the feature being enabled explicitly, and fail then.

(In any case, CHANGELOG.md should get an entry, I think.)

@germag
Copy link
Copy Markdown
Author

germag commented Mar 24, 2026

I don’t like it very much that this way, on Windows, the feature will still be enabled, but silently just not do anything. I know that is a very lukewarm stance and a problem you have probably already considered, but it is not weighed on in the commit message, hence me commenting on it.

Yes, I'm not happy too, but I don't see another way without breaking the default behavior on linux

So: There is no other way, right? To change the default-features list only on Windows, or at least any way to detect the feature being enabled explicitly, and fail then.

I'm not aware of another way, sadly there is nothing like [target.'cfg(windows)'.features].
I know other projects that have the same issue and doing this (some using cfg_aliases to make it more palatable)

(In any case, CHANGELOG.md should get an entry, I think.)

Ok, I'll explain the problem in the commit msg and add and changelog entry

@germag
Copy link
Copy Markdown
Author

germag commented Mar 24, 2026

any way to detect the feature being enabled explicitly, and fail then.

Maybe (not sure, I never used it) I can use cargo_metadata to do some heuristic inside a build.rs it feels it will be quite hacky, tho

@germag germag changed the title Disable 'rawfd' feature on windows Make the 'rawfd' feature a NOP on windows Mar 24, 2026
@germag germag force-pushed the disable-rawfd-on-windows branch from 4652699 to 1f96b9f Compare March 24, 2026 15:37
@germag
Copy link
Copy Markdown
Author

germag commented Mar 24, 2026

any way to detect the feature being enabled explicitly, and fail then.

Maybe (not sure, I never used it) I can use cargo_metadata to do some heuristic inside a build.rs it feels it will be quite hacky, tho

It seems it will not work for this purpose.

@germag
Copy link
Copy Markdown
Author

germag commented Mar 24, 2026

I did this to keep the SemVer compatibility, but since the next 0.18.0 version already includes a major change in the API, I can just remove the rawfd feature from the default set, @XanClic wdyt?

@bonzini
Copy link
Copy Markdown
Member

bonzini commented Mar 24, 2026

I can just remove the rawfd feature from the default set, @XanClic wdyt?

0.18.0 has already been released, unfortunately.

@germag
Copy link
Copy Markdown
Author

germag commented Mar 24, 2026

I can just remove the rawfd feature from the default set, @XanClic wdyt?

0.18.0 has already been released, unfortunately.

Ops my mistake I'll change the CHANGELOG then

Signed-off-by: German Maglione <gmaglione@redhat.com>
@germag germag changed the title Make the 'rawfd' feature a NOP on windows Make the 'rawfd' feature a NOP on Windows Mar 25, 2026
When `rawfd' feature was introduced on commit 0b7a437, it was enabled
by default to avoid breaking the default behavior on linux. So, on
Windows, vm-memory must be included with 'default-features = false'
otherwise it fails to compile. This is problematic for projects that
depend indirectly on vm-memory and fail if their dependencies don't
set 'default-features = false'.

Since there is no current way to set the default feature set for a
specific target OS, let's make the 'rawfd' feature a NOP for Windows
builds, the feature will still be enabled, but silently just not do
anything.

Signed-off-by: German Maglione <gmaglione@redhat.com>
@germag germag force-pushed the disable-rawfd-on-windows branch from 1f96b9f to f7e90d7 Compare March 25, 2026 09:02
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