diff --git a/virtcontainers/default_network.go b/virtcontainers/default_network.go index 7e7b8b8af..4b14c1254 100644 --- a/virtcontainers/default_network.go +++ b/virtcontainers/default_network.go @@ -40,7 +40,7 @@ func (n *defNetwork) run(networkNSPath string, cb func() error) error { } // add adds all needed interfaces inside the network namespace. -func (n *defNetwork) add(s *Sandbox) error { +func (n *defNetwork) add(s *Sandbox, hotplug bool) error { span, _ := n.trace(s.ctx, "add") defer span.Finish() @@ -53,9 +53,15 @@ func (n *defNetwork) add(s *Sandbox) error { err = doNetNS(s.config.NetworkConfig.NetNSPath, func(_ ns.NetNS) error { for _, endpoint := range s.networkNS.Endpoints { - n.logger().WithField("endpoint-type", endpoint.Type()).Info("Attaching endpoint") - if err := endpoint.Attach(s.hypervisor); err != nil { - return err + n.logger().WithField("endpoint-type", endpoint.Type()).WithField("hotplug", hotplug).Info("Attaching endpoint") + if hotplug { + if err := endpoint.HotAttach(s.hypervisor); err != nil { + return err + } + } else { + if err := endpoint.Attach(s.hypervisor); err != nil { + return err + } } } @@ -72,16 +78,22 @@ func (n *defNetwork) add(s *Sandbox) error { // remove network endpoints in the network namespace. It also deletes the network // namespace in case the namespace has been created by us. -func (n *defNetwork) remove(s *Sandbox) error { +func (n *defNetwork) remove(s *Sandbox, hotunplug bool) error { span, _ := n.trace(s.ctx, "remove") defer span.Finish() for _, endpoint := range s.networkNS.Endpoints { // Detach for an endpoint should enter the network namespace // if required. - n.logger().WithField("endpoint-type", endpoint.Type()).Info("Detaching endpoint") - if err := endpoint.Detach(s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil { - return err + n.logger().WithField("endpoint-type", endpoint.Type()).WithField("hotunplug", hotunplug).Info("Detaching endpoint") + if hotunplug { + if err := endpoint.HotDetach(s.hypervisor, s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil { + return err + } + } else { + if err := endpoint.Detach(s.networkNS.NetNsCreated, s.networkNS.NetNsPath); err != nil { + return err + } } } diff --git a/virtcontainers/network.go b/virtcontainers/network.go index 3475d609a..ae276e007 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -1430,9 +1430,9 @@ type network interface { run(networkNSPath string, cb func() error) error // add adds all needed interfaces inside the network namespace. - add(sandbox *Sandbox) error + add(sandbox *Sandbox, hotplug bool) error // remove unbridges and deletes TAP interfaces. It also removes virtual network // interfaces and deletes the network namespace. - remove(sandbox *Sandbox) error + remove(sandbox *Sandbox, hotunplug bool) error } diff --git a/virtcontainers/noop_network.go b/virtcontainers/noop_network.go index c4035d0e6..6553a96b1 100644 --- a/virtcontainers/noop_network.go +++ b/virtcontainers/noop_network.go @@ -19,13 +19,13 @@ func (n *noopNetwork) run(networkNSPath string, cb func() error) error { // add adds all needed interfaces inside the network namespace the Noop network. // It does nothing. -func (n *noopNetwork) add(sandbox *Sandbox) error { +func (n *noopNetwork) add(sandbox *Sandbox, hotattach bool) error { return nil } // remove unbridges and deletes TAP interfaces. It also removes virtual network // interfaces and deletes the network namespace for the Noop network. // It does nothing. -func (n *noopNetwork) remove(sandbox *Sandbox) error { +func (n *noopNetwork) remove(sandbox *Sandbox, hotdetach bool) error { return nil } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 7cffa51f6..a35f03315 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1049,22 +1049,18 @@ func (s *Sandbox) createNetwork() error { NetNsCreated: s.config.NetworkConfig.NetNsCreated, } - // In case there is a factory, the network should be handled - // through some calls at the API level, in order to add or - // remove interfaces and routes. - // This prevents from any assumptions that could be made from - // virtcontainers, in particular that the VM has not been started - // before it starts to scan the current network. + // In case there is a factory, network interfaces are hotplugged + // after vm is started. if s.factory == nil { // Add the network - if err := s.network.add(s); err != nil { + if err := s.network.add(s, false); err != nil { return err } - } - if s.config.NetworkConfig.NetmonConfig.Enable { - if err := s.startNetworkMonitor(); err != nil { - return err + if s.config.NetworkConfig.NetmonConfig.Enable { + if err := s.startNetworkMonitor(); err != nil { + return err + } } } @@ -1082,14 +1078,7 @@ func (s *Sandbox) removeNetwork() error { } } - // In case there is a factory, the network has been handled through - // some API calls to hotplug some interfaces and routes. This means - // the removal of the network should follow the same logic. - if s.factory != nil { - return nil - } - - return s.network.remove(s) + return s.network.remove(s, s.factory != nil) } func (s *Sandbox) generateNetInfo(inf *types.Interface) (NetworkInfo, error) { @@ -1200,8 +1189,6 @@ func (s *Sandbox) startVM() error { s.Logger().Info("Starting VM") - // FIXME: This would break cached VMs. We need network hotplug and move - // oci hooks and netns handling to cli. See #273. if err := s.network.run(s.networkNS.NetNsPath, func() error { if s.factory != nil { vm, err := s.factory.GetVM(ctx, VMConfig{ @@ -1231,6 +1218,24 @@ func (s *Sandbox) startVM() error { return err } + // In case of vm factory, network interfaces are hotplugged + // after vm is started. + if s.factory != nil { + if err := s.network.add(s, true); err != nil { + return err + } + + if s.config.NetworkConfig.NetmonConfig.Enable { + if err := s.startNetworkMonitor(); err != nil { + return err + } + } + if err := s.storage.storeSandboxNetwork(s.id, s.networkNS); err != nil { + return err + } + } + + // Store the network s.Logger().Info("VM started") return nil