Code review comment for lp:~thumper/juju-core/fast-lxc-no-apt-upgrade

Please take a look.

https://codereview.appspot.com/74370044/diff/1/container/lxc/clonetemplate.go
File container/lxc/clonetemplate.go (right):

https://codereview.appspot.com/74370044/diff/1/container/lxc/clonetemplate.go#newcode142
container/lxc/clonetemplate.go:142: if backingFilesystem == Btrfs {
On 2014/03/12 08:31:38, sidnei.da.silva wrote:
> I find it odd that backingFilesystem is special-cased for btrfs, since
the
> version of lxc in trusty has support for picking the right backing
filesystem if
> -Bbest is specified, which would work for either btrfs or lvm thin
provisioning.

man lxc-create doesn't show this.

Post chat: yes -B best does exist, but not documented. However, it
looks for the following in order: "btrfs", "zfs", "lvm", "dir". Since
we have no easy way to determine when to use "aufs" if we are allowing
"best", it is easier to be explicit in our support with btrfs now, and
have all others use aufs.
I'll talk with hallyn

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc.go
File container/lxc/lxc.go (right):

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc.go#newcode92
container/lxc/lxc.go:92: if conf["use-clone"] == "true" {
On 2014/03/12 05:42:25, wallyworld wrote:
> Is there a helper function for str -> bool?

Yes there is. Using it now.

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

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc_test.go#newcode165
container/lxc/lxc_test.go:165: func (s *LxcSuite) AssertEvent(c *gc.C,
event mock.Event, expected mock.Action, id string) {
On 2014/03/12 05:42:25, wallyworld wrote:
> does this need to be exported?

No it doesn't, but it fits with the other Assert statements.

Since "exporting" a test function has no meaning, I went with
consistency.

https://codereview.appspot.com/74370044/diff/1/container/lxc/lxc_test.go#newcode203
container/lxc/lxc_test.go:203: c.Assert(template.Name(), gc.Equals,
name)
On 2014/03/12 05:42:25, wallyworld wrote:
> Should we be checking (some key aspects of) the contents of the
template?

We could make sure that it isn't autostarting nor mounting the logdir by
looking at the config file. Worth it?

https://codereview.appspot.com/74370044/