-
Notifications
You must be signed in to change notification settings - Fork 376
WASIp2 component support for wasmtime #1877
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
base: main
Are you sure you want to change the base?
Changes from all commits
543cb94
d878baf
c05cf2e
ce7d656
dc2e2e1
b1c4da5
9a073f6
e8cbc72
741045a
cf36a28
877ca9a
d6b9778
49f0f42
2170c02
3777fba
4ddd82a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
#define _GNU_SOURCE | ||
|
||
#include <config.h> | ||
#include <string.h> | ||
#include "../container.h" | ||
#include "../utils.h" | ||
#include "handler-utils.h" | ||
|
@@ -69,3 +70,28 @@ wasm_can_handle_container (libcrun_container_t *container, libcrun_error_t *err | |
|
||
return 0; | ||
} | ||
|
||
wasm_encoding_t | ||
wasm_interpret_header (const char *header, const size_t len) | ||
{ | ||
if (len < 8) | ||
return WASM_ENC_INVALID; | ||
|
||
// Check for the WebAssembly magic bytes | ||
// See: https://webassembly.github.io/spec/core/binary/modules.html#binary-module | ||
if (memcmp (header, "\0asm", 4)) | ||
return WASM_ENC_INVALID; | ||
|
||
/* The next four bytes are the WebAssembly version. | ||
We don't care for the specific WebAssembly version | ||
so we only read the value of the `layer` field which | ||
was defined by the component spec. | ||
See: https://github.com/WebAssembly/component-model/blob/main/design/mvp/Binary.md#component-definitions | ||
*/ | ||
if (header[6] == '\0' && header[7] == '\0') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to make sure the header length is at least 8 before accessing these bytes. Since you already need the length for the first comment, I'd suggest to store the header length first before any other check ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree but |
||
return WASM_ENC_MODULE; | ||
|
||
// `layer` does not equal `0x00 0x00` so we are working | ||
// with a component. | ||
return WASM_ENC_COMPONENT; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you write it in commit message or here, about why is this change needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry forgot to mention it ...
After adding the pass through of argv to the component I wanted to try it out. Upon running
podman run mywasm-image:latest arg1 arg2
podman would replaceCMD
(the wasm binary) witharg1
which of course does not work. Changing toENTRYPOINT
resolves this.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a breaking change. I think adding this message to commit logs can help us revisit this and fix this if needed.
cc @giuseppe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this to the commit logs should be possible but I don't really see where this is a breaking change? Without the changes of this PR a wasm module runs into the same problem. IIRC this is also the same behavior a non-wasm container would show.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the message in ce7d656 OK?