[Rust-VMM] interest in other Unixes?

[Allison Randal]

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 at gmail.com
> <mailto:notiggy at 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