persist: improve readability

Address some comments for code readability, also add some unit tests.

Signed-off-by: Wei Zhang <weizhang555.zw@gmail.com>

virtcontainers/bridgedmacvlan_endpoint.go

@@ -117,51 +117,22 @@ func (endpoint *BridgedMacvlanEndpoint) HotDetach(h hypervisor, netNsCreated boo
 	return fmt.Errorf("BridgedMacvlanEndpoint does not support Hot detach")
 }
 
-func (endpoint *BridgedMacvlanEndpoint) save() (s persistapi.NetworkEndpoint) {
-	s.Type = string(endpoint.Type())
-	s.BridgedMacvlan = &persistapi.BridgedMacvlanEndpoint{
-		NetPair: persistapi.NetworkInterfacePair{
-			TapInterface: persistapi.TapInterface{
-				ID:   endpoint.NetPair.TapInterface.ID,
-				Name: endpoint.NetPair.TapInterface.Name,
-				TAPIface: persistapi.NetworkInterface{
-					Name:     endpoint.NetPair.TapInterface.TAPIface.Name,
-					HardAddr: endpoint.NetPair.TapInterface.TAPIface.HardAddr,
-					Addrs:    endpoint.NetPair.TapInterface.TAPIface.Addrs,
-				},
-			},
-			VirtIface: persistapi.NetworkInterface{
-				Name:     endpoint.NetPair.VirtIface.Name,
-				HardAddr: endpoint.NetPair.VirtIface.HardAddr,
-				Addrs:    endpoint.NetPair.VirtIface.Addrs,
-			},
-			NetInterworkingModel: int(endpoint.NetPair.NetInterworkingModel),
+func (endpoint *BridgedMacvlanEndpoint) save() persistapi.NetworkEndpoint {
+	netpair := saveNetIfPair(&endpoint.NetPair)
+
+	return persistapi.NetworkEndpoint{
+		Type: string(endpoint.Type()),
+		BridgedMacvlan: &persistapi.BridgedMacvlanEndpoint{
+			NetPair: *netpair,
 		},
 	}
-	return
 }
 
 func (endpoint *BridgedMacvlanEndpoint) load(s persistapi.NetworkEndpoint) {
 	endpoint.EndpointType = BridgedMacvlanEndpointType
 
 	if s.BridgedMacvlan != nil {
-		iface := s.BridgedMacvlan
-		endpoint.NetPair = NetworkInterfacePair{
-			TapInterface: TapInterface{
-				ID:   iface.NetPair.TapInterface.ID,
-				Name: iface.NetPair.TapInterface.Name,
-				TAPIface: NetworkInterface{
-					Name:     iface.NetPair.TapInterface.TAPIface.Name,
-					HardAddr: iface.NetPair.TapInterface.TAPIface.HardAddr,
-					Addrs:    iface.NetPair.TapInterface.TAPIface.Addrs,
-				},
-			},
-			VirtIface: NetworkInterface{
-				Name:     iface.NetPair.VirtIface.Name,
-				HardAddr: iface.NetPair.VirtIface.HardAddr,
-				Addrs:    iface.NetPair.VirtIface.Addrs,
-			},
-			NetInterworkingModel: NetInterworkingModel(iface.NetPair.NetInterworkingModel),
-		}
+		netpair := loadNetIfPair(&s.BridgedMacvlan.NetPair)
+		endpoint.NetPair = *netpair
 	}
 }

virtcontainers/endpoint.go

@@ -107,3 +107,79 @@ func (endpointType *EndpointType) String() string {
 		return ""
 	}
 }
+
+func saveTapIf(tapif *TapInterface) *persistapi.TapInterface {
+	if tapif == nil {
+		return nil
+	}
+
+	return &persistapi.TapInterface{
+		ID:   tapif.ID,
+		Name: tapif.Name,
+		TAPIface: persistapi.NetworkInterface{
+			Name:     tapif.TAPIface.Name,
+			HardAddr: tapif.TAPIface.HardAddr,
+			Addrs:    tapif.TAPIface.Addrs,
+		},
+	}
+}
+
+func loadTapIf(tapif *persistapi.TapInterface) *TapInterface {
+	if tapif == nil {
+		return nil
+	}
+
+	return &TapInterface{
+		ID:   tapif.ID,
+		Name: tapif.Name,
+		TAPIface: NetworkInterface{
+			Name:     tapif.TAPIface.Name,
+			HardAddr: tapif.TAPIface.HardAddr,
+			Addrs:    tapif.TAPIface.Addrs,
+		},
+	}
+}
+
+func saveNetIfPair(pair *NetworkInterfacePair) *persistapi.NetworkInterfacePair {
+	if pair == nil {
+		return nil
+	}
+
+	epVirtIf := pair.VirtIface
+
+	tapif := saveTapIf(&pair.TapInterface)
+
+	virtif := persistapi.NetworkInterface{
+		Name:     epVirtIf.Name,
+		HardAddr: epVirtIf.HardAddr,
+		Addrs:    epVirtIf.Addrs,
+	}
+
+	return &persistapi.NetworkInterfacePair{
+		TapInterface:         *tapif,
+		VirtIface:            virtif,
+		NetInterworkingModel: int(pair.NetInterworkingModel),
+	}
+}
+
+func loadNetIfPair(pair *persistapi.NetworkInterfacePair) *NetworkInterfacePair {
+	if pair == nil {
+		return nil
+	}
+
+	savedVirtIf := pair.VirtIface
+
+	tapif := loadTapIf(&pair.TapInterface)
+
+	virtif := NetworkInterface{
+		Name:     savedVirtIf.Name,
+		HardAddr: savedVirtIf.HardAddr,
+		Addrs:    savedVirtIf.Addrs,
+	}
+
+	return &NetworkInterfacePair{
+		TapInterface:         *tapif,
+		VirtIface:            virtif,
+		NetInterworkingModel: NetInterworkingModel(pair.NetInterworkingModel),
+	}
+}

virtcontainers/endpoint_test.go

@@ -6,6 +6,10 @@
 package virtcontainers
 
 import (
+	"io/ioutil"
+	"net"
+	"os"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -79,3 +83,41 @@ func TestIncorrectEndpointTypeString(t *testing.T) {
 	var endpointType EndpointType
 	testEndpointTypeString(t, &endpointType, "")
 }
+
+func TestSaveLoadIfPair(t *testing.T) {
+	macAddr := net.HardwareAddr{0x02, 0x00, 0xCA, 0xFE, 0x00, 0x04}
+
+	tmpfile, err := ioutil.TempFile("", "vc-save-load-net-")
+	assert.Nil(t, err)
+	defer os.Remove(tmpfile.Name())
+
+	netPair := &NetworkInterfacePair{
+		TapInterface: TapInterface{
+			ID:   "uniqueTestID-4",
+			Name: "br4_kata",
+			TAPIface: NetworkInterface{
+				Name:     "tap4_kata",
+				HardAddr: macAddr.String(),
+			},
+			VMFds:    []*os.File{tmpfile}, // won't be saved to disk
+			VhostFds: []*os.File{tmpfile}, // won't be saved to disk
+		},
+		VirtIface: NetworkInterface{
+			Name:     "eth4",
+			HardAddr: macAddr.String(),
+		},
+		NetInterworkingModel: DefaultNetInterworkingModel,
+	}
+
+	// Save to disk then load it back.
+	savedIfPair := saveNetIfPair(netPair)
+	loadedIfPair := loadNetIfPair(savedIfPair)
+
+	// Since VMFds and VhostFds are't saved, netPair and loadedIfPair are not equal.
+	assert.False(t, reflect.DeepEqual(netPair, loadedIfPair))
+
+	netPair.TapInterface.VMFds = nil
+	netPair.TapInterface.VhostFds = nil
+	// They are equal now.
+	assert.True(t, reflect.DeepEqual(netPair, loadedIfPair))
+}

virtcontainers/ipvlan_endpoint.go

@@ -120,51 +120,22 @@ func (endpoint *IPVlanEndpoint) HotDetach(h hypervisor, netNsCreated bool, netNs
 	return fmt.Errorf("IPVlanEndpoint does not support Hot detach")
 }
 
-func (endpoint *IPVlanEndpoint) save() (s persistapi.NetworkEndpoint) {
-	s.Type = string(endpoint.Type())
-	s.IPVlan = &persistapi.IPVlanEndpoint{
-		NetPair: persistapi.NetworkInterfacePair{
-			TapInterface: persistapi.TapInterface{
-				ID:   endpoint.NetPair.TapInterface.ID,
-				Name: endpoint.NetPair.TapInterface.Name,
-				TAPIface: persistapi.NetworkInterface{
-					Name:     endpoint.NetPair.TapInterface.TAPIface.Name,
-					HardAddr: endpoint.NetPair.TapInterface.TAPIface.HardAddr,
-					Addrs:    endpoint.NetPair.TapInterface.TAPIface.Addrs,
-				},
-			},
-			VirtIface: persistapi.NetworkInterface{
-				Name:     endpoint.NetPair.VirtIface.Name,
-				HardAddr: endpoint.NetPair.VirtIface.HardAddr,
-				Addrs:    endpoint.NetPair.VirtIface.Addrs,
-			},
-			NetInterworkingModel: int(endpoint.NetPair.NetInterworkingModel),
+func (endpoint *IPVlanEndpoint) save() persistapi.NetworkEndpoint {
+	netpair := saveNetIfPair(&endpoint.NetPair)
+
+	return persistapi.NetworkEndpoint{
+		Type: string(endpoint.Type()),
+		IPVlan: &persistapi.IPVlanEndpoint{
+			NetPair: *netpair,
 		},
 	}
-	return
 }
 
 func (endpoint *IPVlanEndpoint) load(s persistapi.NetworkEndpoint) {
 	endpoint.EndpointType = IPVlanEndpointType
 
 	if s.IPVlan != nil {
-		iface := s.IPVlan
-		endpoint.NetPair = NetworkInterfacePair{
-			TapInterface: TapInterface{
-				ID:   iface.NetPair.TapInterface.ID,
-				Name: iface.NetPair.TapInterface.Name,
-				TAPIface: NetworkInterface{
-					Name:     iface.NetPair.TapInterface.TAPIface.Name,
-					HardAddr: iface.NetPair.TapInterface.TAPIface.HardAddr,
-					Addrs:    iface.NetPair.TapInterface.TAPIface.Addrs,
-				},
-			},
-			VirtIface: NetworkInterface{
-				Name:     iface.NetPair.VirtIface.Name,
-				HardAddr: iface.NetPair.VirtIface.HardAddr,
-				Addrs:    iface.NetPair.VirtIface.Addrs,
-			},
-			NetInterworkingModel: NetInterworkingModel(iface.NetPair.NetInterworkingModel),
-		}
+		netpair := loadNetIfPair(&s.IPVlan.NetPair)
+		endpoint.NetPair = *netpair
 	}
 }

virtcontainers/kata_agent.go

@@ -2076,10 +2076,11 @@ func (k *kataAgent) cleanup(id string) {
 	}
 }
 
-func (k *kataAgent) save() (s persistapi.AgentState) {
-	s.ProxyPid = k.state.ProxyPid
-	s.URL = k.state.URL
-	return
+func (k *kataAgent) save() persistapi.AgentState {
+	return persistapi.AgentState{
+		ProxyPid: k.state.ProxyPid,
+		URL:      k.state.URL,
+	}
 }
 
 func (k *kataAgent) load(s persistapi.AgentState) {

virtcontainers/macvtap_endpoint.go

@@ -105,15 +105,18 @@ func (endpoint *MacvtapEndpoint) NetworkPair() *NetworkInterfacePair {
 	return nil
 }
 
-func (endpoint *MacvtapEndpoint) save() (s persistapi.NetworkEndpoint) {
-	s.Type = string(endpoint.Type())
-	s.Macvtap = &persistapi.MacvtapEndpoint{
-		PCIAddr: endpoint.PCIAddr,
+func (endpoint *MacvtapEndpoint) save() persistapi.NetworkEndpoint {
+	return persistapi.NetworkEndpoint{
+		Type: string(endpoint.Type()),
+
+		Macvtap: &persistapi.MacvtapEndpoint{
+			PCIAddr: endpoint.PCIAddr,
+		},
 	}
-	return
 }
 func (endpoint *MacvtapEndpoint) load(s persistapi.NetworkEndpoint) {
 	endpoint.EndpointType = MacvtapEndpointType
+
 	if s.Macvtap != nil {
 		endpoint.PCIAddr = s.Macvtap.PCIAddr
 	}

virtcontainers/persist.go

@@ -285,6 +285,7 @@ func (s *Sandbox) loadNetwork(netInfo persistapi.NetworkInfo) {
 		case IPVlanEndpointType:
 			ep = &IPVlanEndpoint{}
 		default:
+			s.Logger().WithField("endpoint-type", e.Type).Error("unknown endpoint type")
 			continue
 		}
 		ep.load(e)

virtcontainers/persist/api/hypervisor.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2016 Intel Corporation
+// Copyright (c) 2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //

virtcontainers/persist_test.go

@@ -83,7 +83,7 @@ func TestSandboxRestore(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(sandbox.newStore)
 
-	// if we don't call ToDisk, we can get nothing from disk
+	// if we don't call Save(), we can get nothing from disk
 	err = sandbox.Restore()
 	assert.NotNil(t, err)
 	assert.True(os.IsNotExist(err))

virtcontainers/physical_endpoint.go

@@ -202,18 +202,21 @@ func bindNICToHost(endpoint *PhysicalEndpoint) error {
 	return drivers.BindDevicetoHost(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID)
 }
 
-func (endpoint *PhysicalEndpoint) save() (s persistapi.NetworkEndpoint) {
-	s.Type = string(endpoint.Type())
-	s.Physical = &persistapi.PhysicalEndpoint{
-		BDF:            endpoint.BDF,
-		Driver:         endpoint.Driver,
-		VendorDeviceID: endpoint.VendorDeviceID,
+func (endpoint *PhysicalEndpoint) save() persistapi.NetworkEndpoint {
+	return persistapi.NetworkEndpoint{
+		Type: string(endpoint.Type()),
+
+		Physical: &persistapi.PhysicalEndpoint{
+			BDF:            endpoint.BDF,
+			Driver:         endpoint.Driver,
+			VendorDeviceID: endpoint.VendorDeviceID,
+		},
 	}
-	return
 }
 
 func (endpoint *PhysicalEndpoint) load(s persistapi.NetworkEndpoint) {
 	endpoint.EndpointType = PhysicalEndpointType
+
 	if s.Physical != nil {
 		endpoint.BDF = s.Physical.BDF
 		endpoint.Driver = s.Physical.Driver

virtcontainers/tap_endpoint.go

@@ -189,34 +189,21 @@ func unTapNetwork(name string) error {
 	return nil
 }
 
-func (endpoint *TapEndpoint) save() (s persistapi.NetworkEndpoint) {
-	s.Type = string(endpoint.Type())
-	s.Tap = &persistapi.TapEndpoint{
-		TapInterface: persistapi.TapInterface{
-			ID:   endpoint.TapInterface.ID,
-			Name: endpoint.TapInterface.Name,
-			TAPIface: persistapi.NetworkInterface{
-				Name:     endpoint.TapInterface.TAPIface.Name,
-				HardAddr: endpoint.TapInterface.TAPIface.HardAddr,
-				Addrs:    endpoint.TapInterface.TAPIface.Addrs,
-			},
+func (endpoint *TapEndpoint) save() persistapi.NetworkEndpoint {
+	tapif := saveTapIf(&endpoint.TapInterface)
+
+	return persistapi.NetworkEndpoint{
+		Type: string(endpoint.Type()),
+		Tap: &persistapi.TapEndpoint{
+			TapInterface: *tapif,
 		},
 	}
-	return
 }
 func (endpoint *TapEndpoint) load(s persistapi.NetworkEndpoint) {
 	endpoint.EndpointType = TapEndpointType
 
 	if s.Tap != nil {
-		iface := s.Tap
-		endpoint.TapInterface = TapInterface{
-			ID:   iface.TapInterface.ID,
-			Name: iface.TapInterface.Name,
-			TAPIface: NetworkInterface{
-				Name:     iface.TapInterface.TAPIface.Name,
-				HardAddr: iface.TapInterface.TAPIface.HardAddr,
-				Addrs:    iface.TapInterface.TAPIface.Addrs,
-			},
-		}
+		tapif := loadTapIf(&s.Tap.TapInterface)
+		endpoint.TapInterface = *tapif
 	}
 }