Hey Allison,

How interested would you be in patches for Unix family operating systems other than Linux?

I think it makes sense to support other Unix families. We just have to make sure we keep a clean separation and general tidiness in the code while adding support for other platforms/OSes/stuff. This can be discussed during code review.

As a concrete example of how non-disruptive this can be, the patch I'm debating whether to submit is a 4 line substitution in vmm-sys-util, which changes [#cfg(unix)] to [#cfg(linux)]. Right now, those lines are checking for target_family = "unix", but the code in question is actually unique to Linux, and fails to compile on other Unix family operating systems (mostly due to type errors, and using features that only exist on Linux), so it really should be checking target_os = "linux" instead.

This sounds like a bug in vmm-sys-util to me. If the code only works on Linux, it should be labeled as such. I don't see any problem in changing it from Unix to Linux.

src/errno.rs testing a very specific string message as the text for libc::EBADF, which has slightly different wording on other Unix family operating systems.

I don't think Display should be tested at all because this particular implementation of Display relies on `io::Error` implementation of Display. We're just basically forwarding the implementation. We can either delete the Display implementation (which would make me very happy :)) ) or use some other errors where the wording is the same (are there any errors like that?). I look forward to your PRs. Andreea On Thu, Apr 16, 2020 at 5:58 AM Iggy Jackson (Brian) <notiggy@gmail.com> wrote:

...

On Apr 15, 2020, at 1:39 PM, Allison Randal <allison@lohutok.net> wrote:

Hi all,

How interested would you be in patches for Unix family operating systems other than Linux? I'm working on a prototype lightweight VMM on the Illumos Kernel and Bhyve ioctl interface, and I'm planning to reuse rust-vmm libraries where I can. I prefer to work upstream on shared libraries, but I totally understand if the focus here is Linux, and patches for other Unix family operating systems are undesirable.

Rust-VMM is such a moving target at the moment, you basically _have_ to work on upstream.

...

As a concrete example of how non-disruptive this can be, the patch I'm debating whether to submit is a 4 line substitution in vmm-sys-util, which changes [#cfg(unix)] to [#cfg(linux)]. Right now, those lines are checking for target_family = "unix", but the code in question is actually unique to Linux, and fails to compile on other Unix family operating systems (mostly due to type errors, and using features that only exist on Linux), so it really should be checking target_os = "linux" instead. The specific changes are lines 8, 17, and 19, in src/lib.rs, which determine whether to use the code in the architecture-specific 'unix' directory, and whether to use the bitflags library (which is only used by the 'unix' directory). And, line 183 in src/errno.rs testing a very specific string message as the text for libc::EBADF, which has slightly different wording on other Unix family operating systems.

That simple substitution makes it possible to use both vmm-sys-util and vm-memory on Illumos (since vm-memory was only using architecture-independent parts of vmm-sys-util for temp files in tests, and nothing in the 'unix' directory). I haven't started systematically testing other rust-vmm libraries for non-Linux compatibility yet, but I expect I'll find similar patterns.

I can't speak for everyone (or anyone for that matter, I'm just a bystander here), but I think as long as things don't turn into a spaghetti ball of conditionals, targeting other unixes is a good thing.

I think your best bet is to send a couple of small PRs and see what the response is. I suspect the devs will be open to most of it. Anything that is untenable can either live outside in a separate library or just a straight fork of certain crates. That already kind of exists currently between firecracker, cloud-hypervisor, and rust-vmm (although they are working to merge some of that where it makes sense).

...

(Full disclosure: I personally prefer Linux Kernel and KVM, but this is paid work, and sparks enough intellectual curiosity for me to be willing to do it.)

Good to hear. The more companies that start building around rust-vmm the better.

...

Thanks for any thoughts, Allison

_______________________________________________ Rust-vmm mailing list Rust-vmm@lists.opendev.org http://lists.opendev.org/cgi-bin/mailman/listinfo/rust-vmm

_______________________________________________ Rust-vmm mailing list Rust-vmm@lists.opendev.org http://lists.opendev.org/cgi-bin/mailman/listinfo/rust-vmm

UNSUBSCRIBE On Thu, Apr 16, 2020 at 4:46 AM Andreea Florescu < andreea.florescu15@gmail.com> wrote:

Hey Allison,

...

How interested would you be in patches for Unix family operating systems other than Linux?

I think it makes sense to support other Unix families. We just have to make sure we keep a clean separation and general tidiness in the code while adding support for other platforms/OSes/stuff. This can be discussed during code review.

...

As a concrete example of how non-disruptive this can be, the patch I'm debating whether to submit is a 4 line substitution in vmm-sys-util, which changes [#cfg(unix)] to [#cfg(linux)]. Right now, those lines are checking for target_family = "unix", but the code in question is actually unique to Linux, and fails to compile on other Unix family operating systems (mostly due to type errors, and using features that only exist on Linux), so it really should be checking target_os = "linux" instead.

This sounds like a bug in vmm-sys-util to me. If the code only works on Linux, it should be labeled as such. I don't see any problem in changing it from Unix to Linux.

...

src/errno.rs testing a very specific string message as the text for libc::EBADF, which has slightly different wording on other Unix family operating systems.

I don't think Display should be tested at all because this particular implementation of Display relies on `io::Error` implementation of Display. We're just basically forwarding the implementation. We can either delete the Display implementation (which would make me very happy :)) ) or use some other errors where the wording is the same (are there any errors like that?).

I look forward to your PRs.

Andreea

On Thu, Apr 16, 2020 at 5:58 AM Iggy Jackson (Brian) <notiggy@gmail.com> wrote:

...

...

On Apr 15, 2020, at 1:39 PM, Allison Randal <allison@lohutok.net> wrote:

Hi all,

How interested would you be in patches for Unix family operating systems other than Linux? I'm working on a prototype lightweight VMM on the Illumos Kernel and Bhyve ioctl interface, and I'm planning to reuse rust-vmm libraries where I can. I prefer to work upstream on shared libraries, but I totally understand if the focus here is Linux, and patches for other Unix family operating systems are undesirable.

Rust-VMM is such a moving target at the moment, you basically _have_ to work on upstream.

...

As a concrete example of how non-disruptive this can be, the patch I'm debating whether to submit is a 4 line substitution in vmm-sys-util, which changes [#cfg(unix)] to [#cfg(linux)]. Right now, those lines are checking for target_family = "unix", but the code in question is actually unique to Linux, and fails to compile on other Unix family operating systems (mostly due to type errors, and using features that only exist on Linux), so it really should be checking target_os = "linux" instead. The specific changes are lines 8, 17, and 19, in src/lib.rs, which determine whether to use the code in the architecture-specific 'unix' directory, and whether to use the bitflags library (which is only used by the 'unix' directory). And, line 183 in src/errno.rs testing a very specific string message as the text for libc::EBADF, which has slightly different wording on other Unix family operating systems.

That simple substitution makes it possible to use both vmm-sys-util and vm-memory on Illumos (since vm-memory was only using architecture-independent parts of vmm-sys-util for temp files in tests, and nothing in the 'unix' directory). I haven't started systematically testing other rust-vmm libraries for non-Linux compatibility yet, but I expect I'll find similar patterns.

I can't speak for everyone (or anyone for that matter, I'm just a bystander here), but I think as long as things don't turn into a spaghetti ball of conditionals, targeting other unixes is a good thing.

I think your best bet is to send a couple of small PRs and see what the response is. I suspect the devs will be open to most of it. Anything that is untenable can either live outside in a separate library or just a straight fork of certain crates. That already kind of exists currently between firecracker, cloud-hypervisor, and rust-vmm (although they are working to merge some of that where it makes sense).

...

(Full disclosure: I personally prefer Linux Kernel and KVM, but this is paid work, and sparks enough intellectual curiosity for me to be willing to do it.)

Good to hear. The more companies that start building around rust-vmm the better.

...

Thanks for any thoughts, Allison

_______________________________________________ Rust-vmm mailing list Rust-vmm@lists.opendev.org http://lists.opendev.org/cgi-bin/mailman/listinfo/rust-vmm

_______________________________________________ Rust-vmm mailing list Rust-vmm@lists.opendev.org http://lists.opendev.org/cgi-bin/mailman/listinfo/rust-vmm

_______________________________________________ Rust-vmm mailing list Rust-vmm@lists.opendev.org http://lists.opendev.org/cgi-bin/mailman/listinfo/rust-vmm

-- -- [image: Summit Virtual Experience] <https://www.redhat.com/en/summit?sc_cid=7013a000002D2QxAAK> Scott McCarty Product Management - Containers, Red Hat Enterprise Linux & OpenShift Email: smccarty@redhat.com Phone: 312-660-3535 Cell: 330-807-1043 Web: http://crunchtools.com Using Azure Pipelines with Red Hat Universal Base Image and Quay.io: https://red.ht/2TvYo3Y

On 4/16/20 4:44 AM, Andreea Florescu wrote:

Hey Allison,

...

How interested would you be in patches for Unix family operating systems other than Linux?

I think it makes sense to support other Unix families. We just have to make sure we keep a clean separation and general tidiness in the code while adding support for other platforms/OSes/stuff. This can be discussed during code review.

Yes, I'm trying to be mindful of changes to shared libraries, to keep them minimal and cleanly separated. And, sometimes I've decided that creating a separate library is better than modifying a shared library, like introducing a new library bhyve-ioctls, which doesn't borrow anything from kvm-ioctls, but presents a similar interface as much as possible.

...

As a concrete example of how non-disruptive this can be, the patch I'm debating whether to submit is a 4 line substitution in vmm-sys-util, which changes [#cfg(unix)] to [#cfg(linux)]. Right now, those lines are checking for target_family = "unix", but the code in question is actually unique to Linux, and fails to compile on other Unix family operating systems (mostly due to type errors, and using features that only exist on Linux), so it really should be checking target_os = "linux" instead.

This sounds like a bug in vmm-sys-util to me. If the code only works on Linux, it should be labeled as such. I don't see any problem in changing it from Unix to Linux.

Nod, I considered it little more than a simple typo, someone typed X where they meant Y. At some point, you may want to consider changing the name of the 'unix' directory to 'linux' too. I won't put that in this PR, because it moves a lot of files, and is really more a matter of cosmetics/maintainability, than functionality.

...

src/errno.rs <http://errno.rs> testing a very specific string message as the text for libc::EBADF, which has slightly different wording on other Unix family operating systems.

I don't think Display should be tested at all because this particular implementation of Display relies on `io::Error` implementation of Display. We're just basically forwarding the implementation. We can either delete the Display [-implementation/+test] (which would make me very happy :)) ) or use some other errors where the wording is the same (are there any errors like that?).

For now, I'll only submit a PR for the change to [#cfg(linux)], which effectively means the text comparison test only runs on Linux and Windows, where the text is known. For a more long-term fix, one option would be to change the test so it isn't comparing to a literal string, but instead tests that the Display for vmm-sys-util::errno::Error produces the same text as std::io::Error. Ensuring that the error message text wasn't mangled by vmm-sys-util::errno seems to be the original intent of the test, even though it was written as a literal string comparison. That would be a different one-line change: #[cfg(unix)] assert_eq!( format!("{}", Error::new(libc::EBADF)), - "Bad file descriptor (os error 9)" + format!("{}", io::Error::from_raw_os_error(9)) );

On Thu, Apr 16, 2020 at 5:58 AM Iggy Jackson (Brian) <notiggy@gmail.com <mailto:notiggy@gmail.com>> wrote:

Rust-VMM is such a moving target at the moment, you basically _have_ to work on upstream.

Yeah, continually reapplying a local patch set on top of an upstream library has never been my idea of fun. :(

Good to hear. The more companies that start building around rust-vmm the better.

Very helpful perspective, thanks! Allison