[libvirt] [go PATCH 4/8] Switch typedParamsUnpackLen to use accessor methods

Daniel P. Berrangé berrange at redhat.com
Thu Jan 24 13:17:03 UTC 2019


Stop playing games accessing struct fields directly from go and call the
supported libvirt APIs instead. This makes the code clearer and safer
from Go.

The string list code still needs direct pointer games, since libvirt
does not expose any virTypedParamsGetStringList() method, as none of its
APIs actually require this feature yet. The Go binding still wants this
impl at least for the test suite in order to validate the packing of
string lists.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
 connect.go             |  18 +++---
 domain.go              |   8 +--
 domain_events.go       |  10 ++--
 typedparams.go         | 128 ++++++++++++++++++++++-------------------
 typedparams_test.go    |   2 +-
 typedparams_wrapper.go | 101 ++++++++++++++++++++++++++++++++
 typedparams_wrapper.h  |  44 ++++++++++++++
 7 files changed, 233 insertions(+), 78 deletions(-)

diff --git a/connect.go b/connect.go
index 5044def..e2ff88a 100644
--- a/connect.go
+++ b/connect.go
@@ -2811,7 +2811,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 		state := &DomainStatsState{}
 		stateInfo := getDomainStatsStateFieldInfo(state)
 
-		count, gerr := typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), stateInfo)
+		count, gerr := typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, stateInfo)
 		if gerr != nil {
 			return []DomainStats{}, gerr
 		}
@@ -2822,7 +2822,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 		cpu := &DomainStatsCPU{}
 		cpuInfo := getDomainStatsCPUFieldInfo(cpu)
 
-		count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), cpuInfo)
+		count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, cpuInfo)
 		if gerr != nil {
 			return []DomainStats{}, gerr
 		}
@@ -2833,7 +2833,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 		balloon := &DomainStatsBalloon{}
 		balloonInfo := getDomainStatsBalloonFieldInfo(balloon)
 
-		count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), balloonInfo)
+		count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, balloonInfo)
 		if gerr != nil {
 			return []DomainStats{}, gerr
 		}
@@ -2844,7 +2844,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 		perf := &DomainStatsPerf{}
 		perfInfo := getDomainStatsPerfFieldInfo(perf)
 
-		count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), perfInfo)
+		count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, perfInfo)
 		if gerr != nil {
 			return []DomainStats{}, gerr
 		}
@@ -2855,7 +2855,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 		lengths := domainStatsLengths{}
 		lengthsInfo := getDomainStatsLengthsFieldInfo(&lengths)
 
-		count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), lengthsInfo)
+		count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, lengthsInfo)
 		if gerr != nil {
 			return []DomainStats{}, gerr
 		}
@@ -2871,7 +2871,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 				vcpu := DomainStatsVcpu{}
 				vcpuInfo := getDomainStatsVcpuFieldInfo(j, &vcpu)
 
-				count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), vcpuInfo)
+				count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, vcpuInfo)
 				if gerr != nil {
 					return []DomainStats{}, gerr
 				}
@@ -2889,7 +2889,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 				block := DomainStatsBlock{}
 				blockInfo := getDomainStatsBlockFieldInfo(j, &block)
 
-				count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), blockInfo)
+				count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, blockInfo)
 				if gerr != nil {
 					return []DomainStats{}, gerr
 				}
@@ -2905,7 +2905,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
 				net := DomainStatsNet{}
 				netInfo := getDomainStatsNetFieldInfo(j, &net)
 
-				count, gerr = typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), netInfo)
+				count, gerr = typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, netInfo)
 				if gerr != nil {
 					return []DomainStats{}, gerr
 				}
@@ -2977,7 +2977,7 @@ func (c *Connect) GetSEVInfo(flags uint32) (*NodeSEVParameters, error) {
 
 	defer C.virTypedParamsFree(cparams, nparams)
 
-	_, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+	_, gerr := typedParamsUnpackLen(cparams, nparams, info)
 	if gerr != nil {
 		return nil, gerr
 	}
diff --git a/domain.go b/domain.go
index 8f7f030..09d6a71 100644
--- a/domain.go
+++ b/domain.go
@@ -3207,7 +3207,7 @@ func (d *Domain) GetJobStats(flags DomainGetJobStatsFlags) (*DomainJobInfo, erro
 	params := DomainJobInfo{}
 	info := getDomainJobInfoFieldInfo(&params)
 
-	_, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+	_, gerr := typedParamsUnpackLen(cparams, nparams, info)
 	if gerr != nil {
 		return nil, gerr
 	}
@@ -3585,7 +3585,7 @@ func (d *Domain) GetPerfEvents(flags DomainModificationImpact) (*DomainPerfEvent
 
 	defer C.virTypedParamsFree(cparams, nparams)
 
-	_, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+	_, gerr := typedParamsUnpackLen(cparams, nparams, info)
 	if gerr != nil {
 		return nil, gerr
 	}
@@ -4644,7 +4644,7 @@ func (d *Domain) GetGuestVcpus(flags uint32) (*DomainGuestVcpus, error) {
 
 	defer C.virTypedParamsFree(cparams, C.int(nparams))
 
-	_, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+	_, gerr := typedParamsUnpackLen(cparams, C.int(nparams), info)
 	if gerr != nil {
 		return nil, gerr
 	}
@@ -4861,7 +4861,7 @@ func (d *Domain) GetLaunchSecurityInfo(flags uint32) (*DomainLaunchSecurityParam
 
 	defer C.virTypedParamsFree(cparams, nparams)
 
-	_, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+	_, gerr := typedParamsUnpackLen(cparams, nparams, info)
 	if gerr != nil {
 		return nil, gerr
 	}
diff --git a/domain_events.go b/domain_events.go
index fe46c5e..b4bff7b 100644
--- a/domain_events.go
+++ b/domain_events.go
@@ -763,7 +763,7 @@ func domainEventTunableGetPin(params C.virTypedParameterPtr, nparams C.int) *Dom
 	numvcpus, numiothreads := countPinInfo(params, nparams)
 	pinInfo := getDomainPinTempFieldInfo(numvcpus, numiothreads, &pin)
 
-	num, err := typedParamsUnpackLen(params, int(nparams), pinInfo)
+	num, err := typedParamsUnpackLen(params, nparams, pinInfo)
 	if num == 0 || err != nil {
 		return nil
 	}
@@ -820,7 +820,7 @@ func domainEventTunableCallback(c C.virConnectPtr, d C.virDomainPtr, params C.vi
 	var sched DomainSchedulerParameters
 	schedInfo := getDomainTuneSchedulerParametersFieldInfo(&sched)
 
-	num, _ := typedParamsUnpackLen(params, int(nparams), schedInfo)
+	num, _ := typedParamsUnpackLen(params, nparams, schedInfo)
 	if num > 0 {
 		eventDetails.CpuSched = &sched
 	}
@@ -831,12 +831,12 @@ func domainEventTunableCallback(c C.virConnectPtr, d C.virDomainPtr, params C.vi
 			s:   &eventDetails.BlkdevDisk,
 		},
 	}
-	typedParamsUnpackLen(params, int(nparams), blknameInfo)
+	typedParamsUnpackLen(params, nparams, blknameInfo)
 
 	var blktune DomainBlockIoTuneParameters
 	blktuneInfo := getTuneBlockIoTuneParametersFieldInfo(&blktune)
 
-	num, _ = typedParamsUnpackLen(params, int(nparams), blktuneInfo)
+	num, _ = typedParamsUnpackLen(params, nparams, blktuneInfo)
 	if num > 0 {
 		eventDetails.BlkdevTune = &blktune
 	}
@@ -910,7 +910,7 @@ func domainEventJobCompletedCallback(c C.virConnectPtr, d C.virDomainPtr, params
 	eventDetails := &DomainEventJobCompleted{}
 	info := getDomainJobInfoFieldInfo(&eventDetails.Info)
 
-	typedParamsUnpackLen(params, int(nparams), info)
+	typedParamsUnpackLen(params, nparams, info)
 
 	callbackFunc := getCallbackId(goCallbackId)
 	callback, ok := callbackFunc.(DomainEventJobCompletedCallback)
diff --git a/typedparams.go b/typedparams.go
index a1bdffd..bda785f 100644
--- a/typedparams.go
+++ b/typedparams.go
@@ -53,76 +53,86 @@ type typedParamsFieldInfo struct {
 	sl  *[]string
 }
 
-func typedParamsUnpackLen(cparams *C.virTypedParameter, nparams int, infomap map[string]typedParamsFieldInfo) (uint, error) {
+func typedParamsUnpackLen(cparams *C.virTypedParameter, cnparams C.int, infomap map[string]typedParamsFieldInfo) (uint, error) {
 	count := uint(0)
-	for i := 0; i < nparams; i++ {
-		var cparam *C.virTypedParameter
-		cparam = (*C.virTypedParameter)(unsafe.Pointer(uintptr(unsafe.Pointer(cparams)) + unsafe.Sizeof(*cparam)*uintptr(i)))
-		name := C.GoString((*C.char)(unsafe.Pointer(&cparam.field)))
-		info, ok := infomap[name]
-		if !ok {
-			// Ignore unknown keys so that we don't break if
-			// run against a newer libvirt that returns more
-			// parameters than we currently have code to
-			// consume
-			continue
-		}
-		switch cparam._type {
-		case C.VIR_TYPED_PARAM_INT:
-			if info.i == nil {
-				return 0, fmt.Errorf("field %s expects an int", name)
-			}
-			*info.i = int(*(*C.int)(unsafe.Pointer(&cparam.value)))
-			*info.set = true
-		case C.VIR_TYPED_PARAM_UINT:
-			if info.ui == nil {
-				return 0, fmt.Errorf("field %s expects a uint", name)
-			}
-			*info.ui = uint(*(*C.uint)(unsafe.Pointer(&cparam.value)))
-			*info.set = true
-		case C.VIR_TYPED_PARAM_LLONG:
-			if info.l == nil {
-				return 0, fmt.Errorf("field %s expects an int64", name)
-			}
-			*info.l = int64(*(*C.longlong)(unsafe.Pointer(&cparam.value)))
-			*info.set = true
-		case C.VIR_TYPED_PARAM_ULLONG:
-			if info.ul == nil {
-				return 0, fmt.Errorf("field %s expects a uint64", name)
-			}
-			*info.ul = uint64(*(*C.ulonglong)(unsafe.Pointer(&cparam.value)))
-			*info.set = true
-		case C.VIR_TYPED_PARAM_DOUBLE:
-			if info.d == nil {
-				return 0, fmt.Errorf("field %s expects a float64", name)
+	for name, value := range infomap {
+		var err C.virError
+		var ret C.int
+		cname := C.CString(name)
+		defer C.free(unsafe.Pointer(cname))
+		if value.sl != nil {
+			for i := 0; i < int(cnparams); i++ {
+				var cparam *C.virTypedParameter
+				cparam = (*C.virTypedParameter)(unsafe.Pointer(uintptr(unsafe.Pointer(cparams)) +
+					(unsafe.Sizeof(*cparam) * uintptr(i))))
+				var cs *C.char
+				ret = C.virTypedParamsGetStringWrapper(cparam, 1, cname, &cs, &err)
+				if ret == 1 {
+					*value.sl = append(*value.sl, C.GoString(cs))
+					*value.set = true
+					count++
+				}
 			}
-			*info.d = float64(*(*C.double)(unsafe.Pointer(&cparam.value)))
-			*info.set = true
-		case C.VIR_TYPED_PARAM_BOOLEAN:
-			if info.b == nil {
-				return 0, fmt.Errorf("field %s expects a bool", name)
+		} else {
+			if value.i != nil {
+				var ci C.int
+				ret = C.virTypedParamsGetIntWrapper(cparams, cnparams, cname, &ci, &err)
+				if ret == 1 {
+					*value.i = int(ci)
+				}
+			} else if value.ui != nil {
+				var cui C.uint
+				ret = C.virTypedParamsGetUIntWrapper(cparams, cnparams, cname, &cui, &err)
+				if ret == 1 {
+					*value.ui = uint(cui)
+				}
+			} else if value.l != nil {
+				var cl C.longlong
+				ret = C.virTypedParamsGetLLongWrapper(cparams, cnparams, cname, &cl, &err)
+				if ret == 1 {
+					*value.l = int64(cl)
+				}
+			} else if value.ul != nil {
+				var cul C.ulonglong
+				ret = C.virTypedParamsGetULLongWrapper(cparams, cnparams, cname, &cul, &err)
+				if ret == 1 {
+					*value.ul = uint64(cul)
+				}
+			} else if value.d != nil {
+				var cd C.double
+				ret = C.virTypedParamsGetDoubleWrapper(cparams, cnparams, cname, &cd, &err)
+				if ret == 1 {
+					*value.d = float64(cd)
+				}
+			} else if value.b != nil {
+				var cb C.int
+				ret = C.virTypedParamsGetBooleanWrapper(cparams, cnparams, cname, &cb, &err)
+				if ret == 1 {
+					if cb == 1 {
+						*value.b = true
+					} else {
+						*value.b = false
+					}
+				}
+			} else if value.s != nil {
+				var cs *C.char
+				ret = C.virTypedParamsGetStringWrapper(cparams, cnparams, cname, &cs, &err)
+				if ret == 1 {
+					*value.s = C.GoString(cs)
+				}
 			}
-			*info.b = *(*C.char)(unsafe.Pointer(&cparam.value)) == 1
-			*info.set = true
-		case C.VIR_TYPED_PARAM_STRING:
-			if info.s != nil {
-				*info.s = C.GoString(*(**C.char)(unsafe.Pointer(&cparam.value)))
-				*info.set = true
-			} else if info.sl != nil {
-				*info.sl = append(*info.sl, C.GoString(*(**C.char)(unsafe.Pointer(&cparam.value))))
-				*info.set = true
-			} else {
-				return 0, fmt.Errorf("field %s expects a string/string list", name)
+			if ret == 1 {
+				*value.set = true
+				count++
 			}
 		}
-		count++
 	}
 
 	return count, nil
 }
 
 func typedParamsUnpack(cparams []C.virTypedParameter, infomap map[string]typedParamsFieldInfo) (uint, error) {
-	return typedParamsUnpackLen(&cparams[0], len(cparams), infomap)
+	return typedParamsUnpackLen(&cparams[0], C.int(len(cparams)), infomap)
 }
 
 func typedParamsPackLen(cparams *C.virTypedParameter, nparams int, infomap map[string]typedParamsFieldInfo) error {
diff --git a/typedparams_test.go b/typedparams_test.go
index ff3783b..dca65b2 100644
--- a/typedparams_test.go
+++ b/typedparams_test.go
@@ -86,7 +86,7 @@ func TestPackUnpack(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	nout, err := typedParamsUnpackLen(params, int(nparams), infoout)
+	nout, err := typedParamsUnpackLen(params, nparams, infoout)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/typedparams_wrapper.go b/typedparams_wrapper.go
index c0248ce..2209d60 100644
--- a/typedparams_wrapper.go
+++ b/typedparams_wrapper.go
@@ -135,5 +135,106 @@ virTypedParamsAddStringWrapper(virTypedParameterPtr *params,
     return ret;
 }
 
+
+int
+virTypedParamsGetIntWrapper(virTypedParameterPtr params,
+			    int nparams,
+			    const char *name,
+			    int *value,
+			    virErrorPtr err)
+{
+    int ret = virTypedParamsGetInt(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetUIntWrapper(virTypedParameterPtr params,
+			     int nparams,
+			     const char *name,
+			     unsigned int *value,
+			     virErrorPtr err)
+{
+    int ret = virTypedParamsGetUInt(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetLLongWrapper(virTypedParameterPtr params,
+			      int nparams,
+			      const char *name,
+			      long long *value,
+			      virErrorPtr err)
+{
+    int ret = virTypedParamsGetLLong(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetULLongWrapper(virTypedParameterPtr params,
+			       int nparams,
+			       const char *name,
+			       unsigned long long *value,
+			       virErrorPtr err)
+{
+    int ret = virTypedParamsGetULLong(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetDoubleWrapper(virTypedParameterPtr params,
+			       int nparams,
+			       const char *name,
+			       double *value,
+			       virErrorPtr err)
+{
+    int ret = virTypedParamsGetDouble(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetBooleanWrapper(virTypedParameterPtr params,
+				int nparams,
+				const char *name,
+				int *value,
+				virErrorPtr err)
+{
+    int ret = virTypedParamsGetBoolean(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetStringWrapper(virTypedParameterPtr params,
+			       int nparams,
+			       const char *name,
+			       const char **value,
+			       virErrorPtr err)
+{
+    int ret = virTypedParamsGetString(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+
+
 */
 import "C"
diff --git a/typedparams_wrapper.h b/typedparams_wrapper.h
index d2ef7d6..ee626b1 100644
--- a/typedparams_wrapper.h
+++ b/typedparams_wrapper.h
@@ -79,4 +79,48 @@ virTypedParamsAddStringWrapper(virTypedParameterPtr *params,
 			       const char *value,
 			       virErrorPtr err);
 
+int
+virTypedParamsGetIntWrapper(virTypedParameterPtr params,
+			    int nparams,
+			    const char *name,
+			    int *value,
+			    virErrorPtr err);
+int
+virTypedParamsGetUIntWrapper(virTypedParameterPtr params,
+			     int nparams,
+			     const char *name,
+			     unsigned int *value,
+			     virErrorPtr err);
+int
+virTypedParamsGetLLongWrapper(virTypedParameterPtr params,
+			      int nparams,
+			      const char *name,
+			      long long *value,
+			      virErrorPtr err);
+int
+virTypedParamsGetULLongWrapper(virTypedParameterPtr params,
+			       int nparams,
+			       const char *name,
+			       unsigned long long *value,
+			       virErrorPtr err);
+int
+virTypedParamsGetDoubleWrapper(virTypedParameterPtr params,
+			       int nparams,
+			       const char *name,
+			       double *value,
+			       virErrorPtr err);
+int
+virTypedParamsGetBooleanWrapper(virTypedParameterPtr params,
+				int nparams,
+				const char *name,
+				int *value,
+				virErrorPtr err);
+int
+virTypedParamsGetStringWrapper(virTypedParameterPtr params,
+			       int nparams,
+			       const char *name,
+			       const char **value,
+			       virErrorPtr err);
+
+
 #endif /* LIBVIRT_GO_TYPEDPARAMS_WRAPPER_H__ */
-- 
2.20.1




More information about the libvir-list mailing list