feat(vmm): Make SerialOut into Write+Send+Debug

[DavidVentura]

Changes

Remove the SerialOut specialized enum and replace it with a generic Trait.

Reason

Now other Write types can be used, such as std::fs::File, sockets, etc. I'm using vmm as a crate to launch multiple VMs on the same process, and they all have un-synchronized access to stdout.

Other

I've decided to not make changes to stdin without first having these reviewed first

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check CONTRIBUTING.md.

PR Checklist

• [X] If a specific issue led to this PR, this PR closes the issue.
• [X] The description of changes is clear and encompassing.
• [X] Any required documentation changes (code and docs) are included in this PR.
• [X] API changes follow the Runbook for Firecracker API changes.
• [X] User-facing changes are mentioned in CHANGELOG.md.
• [X] All added/changed functionality is tested.
• [X] New TODOs link to an issue.
• [X] Commits meet contribution quality standards.

---

• [X] This functionality cannot be added in rust-vmm.

[DavidVentura]

My pre-commit hook was broken :cry:

[DavidVentura]

I wasn't aware :sweat_smile: I'll try to go back to a static dispatch implementation

[DavidVentura]

@JonathanWoollett-Light I don't know how to change to be statically dispatched while allowing anything that is Write to be used.

I only could come up with one option; an Other variant of the enum which itself is dynamically dispatched:

pub enum SerialOut {
    Sink(std::io::Sink),
    Stdout(std::io::Stdout),
    Other(Box<dyn Write>),
}

Alternatively, we could embed my two use-cases into the enum: a File variant and a log variant (which needs to keep its own buffer, as write is called per character)

[bchalios]

Hi @DavidVentura,

I wanted to comment on this:

I'm using vmm as a crate to launch multiple VMs on the same process, and they all have un-synchronized access to stdout.

Firstly, even though vmm is a library crate, it's not designed to be a generic library. It is tightly coupled with the firecracker binary crate (and other utility crates we maintain). That is to say, you cannot rely on the public interface of the crate to be stable in any way, i.e. any commit we do to it might break your tool.

Second (and most important), it seems you imply that you are launching multiple microVMs from the same process without forking. If that is the case, we would strongly advise against that. KVM is the strongest security barrier we rely on, but the process barrier is important as well. In fact we suggest that users launch Firecracker microVMs using the jailer (or similar technology), to ensure workload isolation.

If that is not the case, and indeed you launch each microVM in a separate process, I think you could achieve what you want (redirecting stdout to file descriptors other than stdout) by other means.

Looking forward to hear your thoughts.

[DavidVentura]

you cannot rely on the public interface of the crate to be stable in any way

That's clear to me

The workload isolation is not so important for my current experiments, so indeed I'm launching multiple VMs from the same process -- the workloads are trusted and only get started inside virtual machines to get dedicated kernels for integration testing.

I understand if this is far outside what you consider reasonable for firecracker to support

[bchalios]

Hey @DavidVentura, really sorry for the late response. Thanks for confirming my understanding. Given that your use-case is not something we want to support (quite the contrary), I don't think it would make sense to merge this PR, especially since in order to support this using static dispatch we need to add code that we would not be testing. We typically don't integrate code that we don't plan to test in any way.

I hope that makes sense to you. Glad to hear your thoughts.

[bchalios]

@DavidVentura I will close this PR now. Feel free to re-open this if you think there is more to discuss in the future