Allow to change internetworking model #887
Conversation
jcvenegas
force-pushed
the
networkingmodel-op-20180103
branch
from
4bb0b98
to
0662abc
Compare
config.go
Outdated
| } | ||
|
|
||
| ccLog.WithFields( | ||
| logrus.Fields{ |
There was a problem hiding this comment.
Nit: as you are only logging a single field, you could make this:
ccLog.WithField("model", modelName).Info("Setting internetworking model")
config.go
Outdated
| ccLog.WithFields( | ||
| logrus.Fields{ | ||
| "model": modelName, | ||
| }).Infof("Setting internetworking model") |
There was a problem hiding this comment.
This can be Info() rather than Infof() here.
config.go
Outdated
| if err != nil { | ||
| return vc.ModelDefault, err | ||
| } | ||
| return model, nil |
There was a problem hiding this comment.
Missing blank line between closing brace and return.
config.go
Outdated
| @@ -265,6 +286,10 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { | |||
| if err != nil { | |||
| return vc.HypervisorConfig{}, err | |||
| } | |||
| internetworkingModel, err := h.internetworkingModel() | |||
There was a problem hiding this comment.
Missing blank line before this one.
config/configuration.toml.in
Outdated
| # the container network interface | ||
| # Options: | ||
| # | ||
| # - bridged |
There was a problem hiding this comment.
I'd be tempted to indent these two options (and indent their descriptions further) for clarity:
# Options:
#
# - bridged
#
# Uses a linux bridge to interconnect
# the container interface to the VM. ...
# :
#
# - macvtap
#
# Used when the Container network
# :
#
0662abc
to
62e3926
Compare
|
Hi @jodh-intel thanks for the review, changes applied. |
|
I noticed that the PR was raised using an alternative github id, but the commit authorship and sign-offs are as expected, so lgtm. Now, we just need to wait for containers/virtcontainers#528 to land and be re-vendored into the runtime.
|
config.go
Outdated
| @@ -219,6 +220,24 @@ func (h hypervisor) defaultBridges() uint32 { | |||
| return h.DefaultBridges | |||
| } | |||
|
|
|||
| func (h hypervisor) internetworkingModel() (vc.NetInterworkingModel, error) { | |||
There was a problem hiding this comment.
Could you please rename every internetworkingModel occurrences into interNetworkingModel.
jcvenegas
force-pushed
the
networkingmodel-op-20180103
branch
4 times, most recently
from
f677d77
to
bba0873
Compare
jcvenegas
force-pushed
the
networkingmodel-op-20180103
branch
from
bba0873
to
cc338ac
Compare
Internetworking model is the way the hypervisor will be connected to the container network. Add option to define how to connect the vm. Fixes: clearcontainers#886 Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>
For testing Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>
delete before merge Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>
jcvenegas
force-pushed
the
networkingmodel-op-20180103
branch
from
cc338ac
to
4a20b47
Compare
|
@jcvenegas containers/virtcontainers#528 has been merged. Could you please rebase this PR on latest origin/master and update the vendoring too ? |
This PR add configuation option to allow change how VM will be connected to container network.
Allowed options are :
bridgedandmacvtapDepends on containers/virtcontainers#528