Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[12.0-stable] IoBundle.KeepInHost should not forbid assignment #4110

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

milan-zededa
Copy link
Contributor

KeepInHost is used to keep unassigned devices visible to the host, i.e., not reserved for future PCI passthrough. However, once a device with KeepInHost=true is selected for assignment to an application, the assignment takes precedence over KeepInHost. Only IsPort=true devices are forbidden from assignment.

After a recent change, everything is kept in the host until assignment, and therefore we may consider removing the KeepInHost flag. See commit: aad213f

But here we only fix a bug introduced in the commit referenced above. Specifically, KeepInHost was misunderstood and used to forbid (or more precisely skip) device assignment. This makes it impossible, for example, to assign WiFi adapters or cellular modems.

(cherry picked from commit 5b9cbd9)

KeepInHost is used to keep unassigned devices visible to the host,
i.e., not reserved for future PCI passthrough. However, once a device
with KeepInHost=true is selected for assignment to an application,
the assignment takes precedence over KeepInHost. Only IsPort=true
devices are forbidden from assignment.

After a recent change, everything is kept in the host until assignment,
and therefore we may consider removing the KeepInHost flag.
See commit: aad213f

But here we only fix a bug introduced in the commit referenced above.
Specifically, KeepInHost was misunderstood and used to forbid (or more
precisely skip) device assignment. This makes it impossible, for example,
to assign WiFi adapters or cellular modems.

Signed-off-by: Milan Lenco <milan@zededa.com>
(cherry picked from commit 5b9cbd9)
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@milan-zededa
Copy link
Contributor Author

I have added a commit increasing the eden version to the latest.
Many test failures have been fixed recently and I think it makes sense to bring this to 12.0-stable to get greener results on backported PRs.

@milan-zededa
Copy link
Contributor Author

milan-zededa commented Jul 30, 2024

So we do have 4 locally-reproducible test failures on 12.0 with the latest eden:

 # line 48:
 ./eden test tests/eclient -e app_nonat -v debug
 # line 66:
 ./eden test tests/volume -e volumes_test -v debug
 # line 51:
 ./eden test tests/eclient -e nw_switch -v debug
 # line 76:
 ./eden test tests/eclient -e app_local_info -v debug

They are not related to this PR but to changes done in the sequence of application states when purging.
The problem is that the latest eden was updated to reflect these changes, which were merged to master after 12.0 was released.
So we cannot really pull the latest eden in 12.0. But at the same time I think it would make sense to have green test results also on this LTS branch. I'm removing eden-upgrade commit and will think about how to address this...

@eriknordmark eriknordmark merged commit 61288ae into lf-edge:12.0-stable Jul 30, 2024
37 of 43 checks passed
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