Code review comment for lp:~waigani/juju-core/lxc-autorestart-setting

I think we should hold off on the local provider changes until suspend
and resume are done.

https://codereview.appspot.com/73390043/diff/40001/container/lxc/lxc.go
File container/lxc/lxc.go (right):

https://codereview.appspot.com/73390043/diff/40001/container/lxc/lxc.go#newcode93
container/lxc/lxc.go:93: autoRestart := conf["auto-restart"] == "" ||
conf["auto-restart"] == "true"
autoRestart, err := strconv.ParseBool(conf.PopValue("auto-restart"))
if err != nil {
   // An empty string is an error.
   autoRestart = true
}

https://codereview.appspot.com/73390043/diff/40001/container/lxc/lxc.go#newcode97
container/lxc/lxc.go:97: logger.Warningf(`Found unused config option
with key: "%v" and value: "%v"`, k, v)
Please delete this. It is now handled by the

   conf.WarnAboutUnused()

below.

https://codereview.appspot.com/73390043/diff/40001/container/lxc/lxc_test.go
File container/lxc/lxc_test.go (right):

https://codereview.appspot.com/73390043/diff/40001/container/lxc/lxc_test.go#newcode346
container/lxc/lxc_test.go:346: s.autoRestartFalse = true
Don't set here and reset at the end, as an assert stops the end from
happening. Instead use PatchValue...

   s.PatchValue(&s.autoRestartFalse, true)

https://codereview.appspot.com/73390043/diff/40001/container/lxc/lxc_test.go#newcode355
container/lxc/lxc_test.go:355: s.autoRestartFalse = true
ditto.

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

https://codereview.appspot.com/73390043/diff/40001/provider/local/config.go#newcode121
provider/local/config.go:121: value, _ :=
c.attrs["lxc-auto-restart"].(bool)
I think we want a general auto-restart, not just an lxc- one. We can do
the same for kvm. Which leads me to point out that you need to fix kvm.

Although I don't think we should add the ability to set it in the local
provider until suspend and resume are done.

https://codereview.appspot.com/73390043/