Firecracker crashes if no argument is passed

[andynog]

Firecracker version 0.13.0

When running firecracker from a terminal if no arguments are passed then it crashes.

$ firecracker

2019-01-10T07:54:40.927708641 [anonymous-instance:ERROR:src/main.rs:50] Panic occurred: PanicInfo { payload: Any, message: Some(Cannot create VMM.: Kvm(Error(13))), location: Location { file: "libcore/result.rs", line: 1009, col: 5 } }
2019-01-10T07:54:40.932323586 [anonymous-instance:ERROR:src/main.rs:54] stack backtrace:
2019-01-10T07:54:40.932375699 [anonymous-instance:ERROR:src/main.rs:58] Failed to log metrics while panicking: Logger was not initialized.
Aborted (core dumped)

This might be a minor enhancement but one that can provide a better user experience. Probably a better user experience would be to show the default help (same as --help).

Since firecracker uses clap, I believe that might be as simple as using https://docs.rs/clap/2.32.0/clap/enum.AppSettings.html#variant.ArgRequiredElseHelp.

[andreeaflorescu]

KVM Error number 13 suggests that you do not have permissions on /dev/kvm.

[andynog]

Ok, that's interesting. I had kvm working with 0.12.0, but I guess since I got a new release I had to do again

sudo setfacl -m u:${USER}:rw /dev/kvm

Then it doesn't crash. So a couple of thoughts, why I had to run the command above again ? The only difference is that release 0.13.0 I'm running from a different path. Also I understand that there is no required argument to run it becaue it just creates the sock at /tmp/firecracker.socket so maybe a good user experience would be to show some output when running it with no arguments something like

$ firecracker
Running...
Socket: /tmp/firecracker.socket

Thanks.

[andreeaflorescu]

Hey, from my understanding ACLs on /dev/kvm are not persistent. It would be interesting to know if after you reboot the machine, you still have the proper permissions to open /dev/kvm. We can add a new issue to print the socket name in case the default is used. Do you want to pick that up or should I create an issue?

[andynog]

Or in case the issue is actually kvm not enabled then show a friendlier message with possible some info on how to fix that (e.g. running the command above), but I understand that this has been discussed before and doing checks when running firecracker might cause some performance issues (discussed in #693)

Not sure if it was caused by a reboot but I did reboot my machine.

[dhrgit]

ACLs on device "files" are never persistent across reboots. Moreover, for /dev/kvm, as soon as a process invokes KVM_CREATE_VM, a change event is generated for /dev/kvm, meaning the ACLs are reset. I.e. the setfacl method of granting KVM access is only good for exactly one invocation of Firecracker.

Yes, we should update our guide to describe this more in depth.

[andreeaflorescu]

We already have an issue related to better error messages: #808. It should be straightforward to replace the unwrap when opening /dev/kvm with something like "Cannot open /dev/kvm (errno = ${errno}". But we will still need to add the error code as is because I don't think we should create a mapping for errno. Maybe just advertise where you can get more details about the error (i.e. from errno linux header).

[andynog]

As long as there’s a proper exit code if it panics that should help because I’d believe firecracker will be run mostly from scripts or tools and they can handle the error properly with an exit code

Right now I get 134 exit code (echo $?) for both Kvm error 13 or IO Error Address in use. So not sure each error outputs a unique exit code

[andreeaflorescu]

Exit codes are not unique, but what should be unique is the error message. You get the same exit code because in both cases Firecracker exits the execution due to a panic. Before initializing the logger, the panic error messages are going to stdout and afterwards they're logged in the logger pipe.
In case a panic occurred ( exit code != 0), the logs have to be inspected. There is currently no plan on adding different exit codes for different errors.

[andreeaflorescu]

We merged some pull requests that make the error messages human readable. I will close this issue for now, but feel free to re-open it in case there is something we missed.