sandia-minimega / minimega

minimega
GNU General Public License v3.0
148 stars 66 forks source link

minimega: don't error on kvm when configuring networks for containers #1376

Closed djfritz closed 4 years ago

djfritz commented 4 years ago

Describe your environment

  1. minimega version 2.6

  2. Linux distro/version ubuntu 19.10

  3. Go compiler version N/A

  4. VM types container

Describe the bug Issuing vm config net a on an install that doesn't have kvm/qemu installed throws an error:

unable to determine valid QEMU hosts -- exec: "kvm": executable not found in $PATH

Expected behavior If I'm trying to launch just containers, this shouldn't be an error...

@jcrussell

jcrussell commented 4 years ago

We could probably make it so that it warns instead. It uses "kvm" to check what the valid drivers are and doesn't know you will only launch containers.

djfritz commented 4 years ago

src/minimega/kvm.go:func validNIC

@jcrussell it looks like validNIC isn't used for anything? We're basically inlining this function in namespace.go instead?

djfritz commented 4 years ago

The logic in ParseNetConfig() is going to need some rethinking, as we look for the DefaultKVMDriver and otherwise take the first driver enumerated by kvm, regardless of what type of VM we eventually use this for. I wonder if driver detection needs to be deferred to launching, or if we should ignore checking for any default driver and just assert something like e1000 and let things go sideways later if it somehow doesn't exist...

jcrussell commented 4 years ago

Looks like validNIC is dead code.

There's ambiguity in the network spec format -- foo,bar could be bridge, VLAN alias or vlan alias, driver so we use isDriver to decide which is likely correct. We could defer the parsing until vm launch but then the error appears much later.

A better solution might be to remove magic from vm config network and pull driver out of the network spec. Do we ever need to have different network drivers per interface? If not, add a vm config network-driver API.

djfritz commented 4 years ago

The only person I can see ever needing a mixed driver setup on a single VM would be @mkunz7

djfritz commented 4 years ago

From the qemu docs:

The e1000 is the default network adapter in qemu. The rtl8139 is the default network adapter in qemu-kvm.

So why not just make DefaultKVMDriver rtl8139 and just assert that it must exist and deal with kvm failing if somehow it doesn't at launch time?

@jcrussell

djfritz commented 4 years ago

In that case we could leave the logic as is and just remove one case. We could still warn on not finding kvm in the path, but otherwise leave it alone. I like the magic...

mkunz7 commented 4 years ago

I’m a big fan of vm config network-driver. In my ideal world I wouldn’t be declaring vlan alias, drivers, Mac addresses, and bridge in one go without flags. It seems out of place with how the rest of the CLI works. Make sure to handle multiple adapters with different drivers.

I vote keep e1000 https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/virtualization/sect-virtualization-troubleshooting-kvm_networking_performance

jcrussell commented 4 years ago

I'm fine with warn instead of error. If we wanted to go full idiomatic minimega, we would have:

vm config net driver e1000
vm config net vlan foo
vm config net bridge bar
vm config net commit
vm config net vlan foo2
vm config net commit
djfritz commented 4 years ago

... you monster.