elliottt requested alexcrichton for a review on PR #7417.
elliottt requested wasmtime-core-reviewers for a review on PR #7417.
elliottt opened PR #7417 from elliottt:trevor/request-options-resource
to bytecodealliance:main
:
As we'd like to be able to modify
request-options
in the future without breaking compatibility with older guests, turnrequest-options
into a resource. This allows us to expose getters and setters instead of fields on a record, which gives the embedder flexibility in what additional options are allowed.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
elliottt edited PR #7417:
As we'd like to be able to modify
request-options
in the future without breaking compatibility with older guests, turnrequest-options
into a resource. This allows us to expose getters and setters instead of fields on a record, which gives the embedder flexibility in what additional options are allowed.Fixes #7409
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
elliottt requested pchickey for a review on PR #7417.
alexcrichton submitted PR review:
Looks good to me.
Random drive-by thought unrelated to this change specifically, could the
*-ms
bits be updated to take theduration
type in https://github.com/bytecodealliance/wasmtime/pull/7358 which would give more precision here as well?
pchickey submitted PR review.
pchickey submitted PR review.
pchickey created PR review comment:
Lets use either
std
ortokio::time::Duration
to represent these values in the host as early as we can.
pchickey created PR review comment:
Each of these setters needs to return result, where the error means that the setting is not supported.
elliottt submitted PR review.
elliottt created PR review comment:
The downside is that the getters become quite fallible as they're turning a
std::time::Duration
back into au32
.
elliottt updated PR #7417.
pchickey submitted PR review.
pchickey created PR review comment:
Isnt that resolved when we switch to using a monotonic duration, which is a u64?
elliottt updated PR #7417.
elliottt submitted PR review.
elliottt created PR review comment:
std::time::Duration::as_millis
returns a u128, so we won't avoid it completely.
elliottt submitted PR review.
elliottt created PR review comment:
I implemented this by treating conversion failure from
u128
as anOk(None)
return value. Since we default all these timeouts toNone
anyway, I think a conversion failure would mean that the guest somehow managed to set a larger than u32 value to begin with, then tried to get one back. The host setting an invalid default would be the other possibility, but we don't do this anywhere in wasmtime currently.
elliottt updated PR #7417.
elliottt updated PR #7417.
elliottt has enabled auto merge for PR #7417.
elliottt merged PR #7417.
Last updated: Dec 23 2024 at 12:05 UTC