Code review comment for lp:~thumper/juju-core/kvm-local-provider

LGTM with primarily the consolidation of the code to check kvm container
dependencies with the code to handle kvm initialisation. I also would
like the command code in apt moved. I'm not sure about the need for a
ContainerConfig interface but anytime we need to rely on UnknownAttrs()
seems a bit hacky.

https://codereview.appspot.com/31870043/diff/20001/container/kvm/libvirt.go
File container/kvm/libvirt.go (right):

https://codereview.appspot.com/31870043/diff/20001/container/kvm/libvirt.go#newcode36
container/kvm/libvirt.go:36: output, err = utils.RunCommand(command,
args...)
\o/

https://codereview.appspot.com/31870043/diff/20001/provider/local/config.go
File provider/local/config.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/config.go#newcode23
provider/local/config.go:23: // to explicitly get from the config
unknown params.
My brain found it hard to parse the 2nd line above

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprovider.go
File provider/local/environprovider.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprovider.go#newcode49
provider/local/environprovider.go:49: containerType, ok :=
cfg.UnknownAttrs()[containerConfigKey].(string)
Part of me really doesn't like this UnknownAttrs() business and would
love to see a proper cfg.ContainerType() API available. Not sure if we
should introduce a ContainerConfig interface and cast to that?

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprovider.go#newcode148
provider/local/environprovider.go:148: if localConfig.container() !=
"lxc" && localConfig.container() != "kvm" {
Can we use the ContainerType consts here instead of "lxc", "kvm"

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go
File provider/local/prereqs.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go#newcode50
provider/local/prereqs.go:50: const kvmNeedsUbuntu = `Sorry, but KVM
support with the local provider is only supported
s/but//

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go#newcode154
provider/local/prereqs.go:154: packagesNeeded := []string{"libvirt-bin",
"uvtool-libvirt", "kvm"}
There's also code in the container.kvm package to deal with installing
the required packages - see initialisation.go

I'd prefer this verifyKvm() function to be moved there so it's all done
in one place.

https://codereview.appspot.com/31870043/diff/20001/utils/apt.go
File utils/apt.go (right):

https://codereview.appspot.com/31870043/diff/20001/utils/apt.go#newcode87
utils/apt.go:87: func RunCommand(command string, args ...string) (output
string, err error) {
I'd like to see this in a file called command.go since it's not really
anything to do with apt per se. And tests in command_test.go

https://codereview.appspot.com/31870043/