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

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(a)redhat.com&gt; --- 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