add a C compatible interface #43
Conversation
rageagainsthepc
force-pushed
the
add-ffi
branch
5 times, most recently
from
ac11fe4 to
ef83fc4
Compare
There was a problem hiding this comment.
I added some comments over your work so far.
I've tested it on my machine and it compiled and worked for me, except for the mem-dump.
We should also provide a Makefile in c_examples, automatically compiling and linking with libmicrovmi in $root/target/debug.
A c_examples/README will be useful as well, explaining how to generate the libmicrovmi.h here, and how to set your LD_LIBRARY_PATH.
Congrats for all your work @rageagainsthepc ! 🎉 👍
rageagainsthepc
force-pushed
the
add-ffi
branch
2 times, most recently
from
ab6dd53 to
a7c2f8f
Compare
rageagainsthepc
force-pushed
the
add-ffi
branch
from
a7c2f8f to
967db34
Compare
c_examples/README.md
Outdated
| Command to execute an example as an unprivileged user: | ||
|
|
||
| ~~~ | ||
| sudo LD_LIBRARY_PATH="$LD_LIBRARY_PATH:../target/debug" <example> <vm_name> |
There was a problem hiding this comment.
sudo 🤔
If you are working with Xen, you need to be root, that's correct.
But in the general case, I would prefer to remove this indication, since it's not needed for KVM and VirtualBox.
| @@ -64,4 +67,6 @@ pub trait Introspectable { | |||
| fn resume(&mut self) -> Result<(), Box<dyn Error>> { | |||
| unimplemented!(); | |||
| } | |||
|
|
|||
| fn get_driver_type(&self) -> DriverType; | |||
There was a problem hiding this comment.
@rageagainsthepc can you explain why you had to introduce a new API here, it's hard to follow for me ?
I would like to avoid changing the core of the Rust library just to provide a C compatibility layer.
There was a problem hiding this comment.
Well, if we initialize the driver with None, we don't know the type of the driver in advance. But we need to know since we are casting back and forth between void* and the actual type of the driver. As far as I know it's not possible to cast directly from void* to a trait type. There are probably different ways to approach this, but this one seemed pretty straight forward to me. I'm open to alternatives though ;)
There was a problem hiding this comment.
I think the issue was that pointers to traits are fat pointers. Maybe there is way to pass fat pointers to C code and cast them back to traits afterwards. I'll look into that.
There was a problem hiding this comment.
There exists a std::raw::TraitObject but it's marked as unstable because its representation may change in the future rust-lang/rust#27751 . Also cbindgen reports TraitObject types as unsupported. In theory we could split the fat pointer into two separate pointers, store them in a struct and transmute the memory back to an Introspectable, but imho that sounds super ugly and unstable.
There was a problem hiding this comment.
On the other hand, we could just loose the support for passing None instead of the DriverType, at least for the C-API ;)
There was a problem hiding this comment.
I've found the crate downcast-rs. With this crate you are able to check a trait object for its concrete type.
There was a problem hiding this comment.
Okay, thanks for the update.
I'm okay to merge this as long as you add a comment that this API has been introduced for the solve purpose of the C interoperability, and that it's a workaround, to be deprecated in the future when new better solutions will emerge to deal with this, either from Rust unstable, or simply by switching to dynamic library loading.
rageagainsthepc
force-pushed
the
add-ffi
branch
from
967db34 to
6f8f8e9
Compare
| @@ -64,4 +67,6 @@ pub trait Introspectable { | |||
| fn resume(&mut self) -> Result<(), Box<dyn Error>> { | |||
| unimplemented!(); | |||
| } | |||
|
|
|||
| fn get_driver_type(&self) -> DriverType; | |||
There was a problem hiding this comment.
Okay, thanks for the update.
I'm okay to merge this as long as you add a comment that this API has been introduced for the solve purpose of the C interoperability, and that it's a workaround, to be deprecated in the future when new better solutions will emerge to deal with this, either from Rust unstable, or simply by switching to dynamic library loading.
No description provided.