[libvirt] [go PATCH 6/8] Simplify setting of typed parameters

There is no need to get currently known parameters from libvirt before setting new typed parameters. Indeed this behaviour actually causes problems because it means when setting parameters we will explicitly set all known parameters, instead of only the parameters with changed (non-default) values. Fixes github bug #21 Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- connect.go | 23 ++----- domain.go | 182 +++++++++++-------------------------------------- typedparams.go | 73 -------------------- 3 files changed, 44 insertions(+), 234 deletions(-) diff --git a/connect.go b/connect.go index 11a6a8c..632b7a6 100644 --- a/connect.go +++ b/connect.go @@ -1995,28 +1995,15 @@ func (c *Connect) GetSecurityModel() (*NodeSecurityModel, error) { func (c *Connect) SetMemoryParameters(params *NodeMemoryParameters, flags uint32) error { info := getMemoryParameterFieldInfo(params) - var cnparams C.int - - var err C.virError - ret := C.virNodeGetMemoryParametersWrapper(c.ptr, nil, &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - cparams := make([]C.virTypedParameter, cnparams) - ret = C.virNodeGetMemoryParametersWrapper(c.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virNodeSetMemoryParametersWrapper(c.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virNodeSetMemoryParametersWrapper(c.ptr, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } diff --git a/domain.go b/domain.go index 6c40f10..c851bf6 100644 --- a/domain.go +++ b/domain.go @@ -1288,30 +1288,18 @@ func (d *Domain) GetInterfaceParameters(device string, flags DomainModificationI func (d *Domain) SetInterfaceParameters(device string, params *DomainInterfaceParameters, flags DomainModificationImpact) error { info := getInterfaceParameterFieldInfo(params) - var cnparams C.int - cdevice := C.CString(device) defer C.free(unsafe.Pointer(cdevice)) - var err C.virError - ret := C.virDomainGetInterfaceParametersWrapper(d.ptr, cdevice, nil, &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - cparams := make([]C.virTypedParameter, cnparams) - ret = C.virDomainGetInterfaceParametersWrapper(d.ptr, cdevice, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetInterfaceParametersWrapper(d.ptr, cdevice, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetInterfaceParametersWrapper(d.ptr, cdevice, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } @@ -2636,28 +2624,15 @@ func (d *Domain) GetBlkioParameters(flags DomainModificationImpact) (*DomainBlki func (d *Domain) SetBlkioParameters(params *DomainBlkioParameters, flags DomainModificationImpact) error { info := getBlkioParametersFieldInfo(params) - var cnparams C.int - - var err C.virError - ret := C.virDomainGetBlkioParametersWrapper(d.ptr, nil, &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - cparams := make([]C.virTypedParameter, cnparams) - ret = C.virDomainGetBlkioParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetBlkioParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetBlkioParametersWrapper(d.ptr, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } @@ -2831,28 +2806,15 @@ func (d *Domain) SetBlockIoTune(disk string, params *DomainBlockIoTuneParameters info := getBlockIoTuneParametersFieldInfo(params) - var cnparams C.int - - var err C.virError - ret := C.virDomainGetBlockIoTuneWrapper(d.ptr, cdisk, nil, &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - cparams := make([]C.virTypedParameter, cnparams) - ret = C.virDomainGetBlockIoTuneWrapper(d.ptr, cdisk, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetBlockIoTuneWrapper(d.ptr, cdisk, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetBlockIoTuneWrapper(d.ptr, cdisk, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } @@ -3314,28 +3276,15 @@ func (d *Domain) GetMemoryParameters(flags DomainModificationImpact) (*DomainMem func (d *Domain) SetMemoryParameters(params *DomainMemoryParameters, flags DomainModificationImpact) error { info := getDomainMemoryParametersFieldInfo(params) - var cnparams C.int - - var err C.virError - ret := C.virDomainGetMemoryParametersWrapper(d.ptr, nil, &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - cparams := make([]C.virTypedParameter, cnparams) - ret = C.virDomainGetMemoryParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetMemoryParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetMemoryParametersWrapper(d.ptr, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } @@ -3395,28 +3344,15 @@ func (d *Domain) GetNumaParameters(flags DomainModificationImpact) (*DomainNumaP func (d *Domain) SetNumaParameters(params *DomainNumaParameters, flags DomainModificationImpact) error { info := getDomainNumaParametersFieldInfo(params) - var cnparams C.int - - var err C.virError - ret := C.virDomainGetNumaParametersWrapper(d.ptr, nil, &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - cparams := make([]C.virTypedParameter, cnparams) - ret = C.virDomainGetNumaParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetNumaParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetNumaParametersWrapper(d.ptr, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } @@ -3601,22 +3537,14 @@ func (d *Domain) SetPerfEvents(params *DomainPerfEvents, flags DomainModificatio info := getDomainPerfEventsFieldInfo(params) - var cparams *C.virTypedParameter - var cnparams C.int - var err C.virError - ret := C.virDomainGetPerfEventsWrapper(d.ptr, (*C.virTypedParameterPtr)(unsafe.Pointer(&cparams)), &cnparams, C.uint(flags), &err) - if ret == -1 { - return makeError(&err) - } - - defer C.virTypedParamsFree(cparams, cnparams) - - gerr := typedParamsPackLen(cparams, int(cnparams), info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } + defer C.virTypedParamsFree(cparams, cnparams) - ret = C.virDomainSetPerfEventsWrapper(d.ptr, cparams, cnparams, C.uint(flags), &err) + var err C.virError + ret := C.virDomainSetPerfEventsWrapper(d.ptr, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } @@ -3789,31 +3717,15 @@ func (d *Domain) GetSchedulerParametersFlags(flags DomainModificationImpact) (*D func (d *Domain) SetSchedulerParameters(params *DomainSchedulerParameters) error { info := getDomainSchedulerParametersFieldInfo(params) - var cnparams C.int - var err C.virError - schedtype := C.virDomainGetSchedulerTypeWrapper(d.ptr, &cnparams, &err) - if schedtype == nil { - return makeError(&err) - } - - defer C.free(unsafe.Pointer(schedtype)) - if cnparams == 0 { - return nil - } - - cparams := make([]C.virTypedParameter, cnparams) - ret := C.virDomainGetSchedulerParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, &err) - if ret == -1 { - return makeError(&err) - } - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetSchedulerParametersWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetSchedulerParametersWrapper(d.ptr, cparams, cnparams, &err) if ret == -1 { return makeError(&err) } @@ -3825,31 +3737,15 @@ func (d *Domain) SetSchedulerParameters(params *DomainSchedulerParameters) error func (d *Domain) SetSchedulerParametersFlags(params *DomainSchedulerParameters, flags DomainModificationImpact) error { info := getDomainSchedulerParametersFieldInfo(params) - var cnparams C.int - var err C.virError - schedtype := C.virDomainGetSchedulerTypeWrapper(d.ptr, &cnparams, &err) - if schedtype == nil { - return makeError(&err) - } - - defer C.free(unsafe.Pointer(schedtype)) - if cnparams == 0 { - return nil - } - - cparams := make([]C.virTypedParameter, cnparams) - ret := C.virDomainGetSchedulerParametersFlagsWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), &cnparams, 0, &err) - if ret == -1 { - return makeError(&err) - } - defer C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams) - - gerr := typedParamsPack(cparams, info) + cparams, cnparams, gerr := typedParamsPackNew(info) if gerr != nil { return gerr } - ret = C.virDomainSetSchedulerParametersFlagsWrapper(d.ptr, (*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), cnparams, C.uint(flags), &err) + defer C.virTypedParamsFree(cparams, cnparams) + + var err C.virError + ret := C.virDomainSetSchedulerParametersFlagsWrapper(d.ptr, cparams, cnparams, C.uint(flags), &err) if ret == -1 { return makeError(&err) } diff --git a/typedparams.go b/typedparams.go index bda785f..ee531cc 100644 --- a/typedparams.go +++ b/typedparams.go @@ -135,79 +135,6 @@ func typedParamsUnpack(cparams []C.virTypedParameter, infomap map[string]typedPa return typedParamsUnpackLen(&cparams[0], C.int(len(cparams)), infomap) } -func typedParamsPackLen(cparams *C.virTypedParameter, nparams int, infomap map[string]typedParamsFieldInfo) error { - stringOffsets := make(map[string]uint) - - 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 - } - if !*info.set { - continue - } - switch cparam._type { - case C.VIR_TYPED_PARAM_INT: - if info.i == nil { - return fmt.Errorf("field %s expects an int", name) - } - *(*C.int)(unsafe.Pointer(&cparam.value)) = C.int(*info.i) - case C.VIR_TYPED_PARAM_UINT: - if info.ui == nil { - return fmt.Errorf("field %s expects a uint", name) - } - *(*C.uint)(unsafe.Pointer(&cparam.value)) = C.uint(*info.ui) - case C.VIR_TYPED_PARAM_LLONG: - if info.l == nil { - return fmt.Errorf("field %s expects an int64", name) - } - *(*C.longlong)(unsafe.Pointer(&cparam.value)) = C.longlong(*info.l) - case C.VIR_TYPED_PARAM_ULLONG: - if info.ul == nil { - return fmt.Errorf("field %s expects a uint64", name) - } - *(*C.ulonglong)(unsafe.Pointer(&cparam.value)) = C.ulonglong(*info.ul) - case C.VIR_TYPED_PARAM_DOUBLE: - if info.d == nil { - return fmt.Errorf("field %s expects a float64", name) - } - *(*C.double)(unsafe.Pointer(&cparam.value)) = C.double(*info.d) - case C.VIR_TYPED_PARAM_BOOLEAN: - if info.b == nil { - return fmt.Errorf("field %s expects a bool", name) - } - if *info.b { - *(*C.char)(unsafe.Pointer(&cparam.value)) = 1 - } else { - *(*C.char)(unsafe.Pointer(&cparam.value)) = 0 - } - case C.VIR_TYPED_PARAM_STRING: - if info.s != nil { - *(**C.char)(unsafe.Pointer(&cparam.value)) = C.CString(*info.s) - } else if info.sl != nil { - count := stringOffsets[name] - *(**C.char)(unsafe.Pointer(&cparam.value)) = C.CString((*info.sl)[count]) - stringOffsets[name] = count + 1 - } else { - return fmt.Errorf("field %s expects a string", name) - } - } - } - - return nil -} - -func typedParamsPack(cparams []C.virTypedParameter, infomap map[string]typedParamsFieldInfo) error { - return typedParamsPackLen(&cparams[0], len(cparams), infomap) -} - func typedParamsPackNew(infomap map[string]typedParamsFieldInfo) (*C.virTypedParameter, C.int, error) { var cparams C.virTypedParameterPtr var nparams C.int -- 2.20.1