[libvirt] [go PATCH 28/37] interface: fix error reporting thread safety

Create wrapper functions for each interface C API that accepts a virErrorPtr parameter. This avoids accessing a thread local from a goroutine which may race with other goroutines doing native API calls in the same OS thread. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- interface.go | 45 +++++++++------- interface_wrapper.go | 124 +++++++++++++++++++++++++++++++++++++++++++ interface_wrapper.h | 44 +++++++++++++++ 3 files changed, 195 insertions(+), 18 deletions(-) diff --git a/interface.go b/interface.go index bd39ed0..9b6ebb2 100644 --- a/interface.go +++ b/interface.go @@ -49,27 +49,30 @@ type Interface struct { // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceCreate func (n *Interface) Create(flags uint32) error { - result := C.virInterfaceCreate(n.ptr, C.uint(flags)) + var err C.virError + result := C.virInterfaceCreateWrapper(n.ptr, C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceDestroy func (n *Interface) Destroy(flags uint32) error { - result := C.virInterfaceDestroy(n.ptr, C.uint(flags)) + var err C.virError + result := C.virInterfaceDestroyWrapper(n.ptr, C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceIsActive func (n *Interface) IsActive() (bool, error) { - result := C.virInterfaceIsActive(n.ptr) + var err C.virError + result := C.virInterfaceIsActiveWrapper(n.ptr, &err) if result == -1 { - return false, GetLastError() + return false, makeError(&err) } if result == 1 { return true, nil @@ -79,9 +82,10 @@ func (n *Interface) IsActive() (bool, error) { // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetMA... func (n *Interface) GetMACString() (string, error) { - result := C.virInterfaceGetMACString(n.ptr) + var err C.virError + result := C.virInterfaceGetMACStringWrapper(n.ptr, &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } mac := C.GoString(result) return mac, nil @@ -89,9 +93,10 @@ func (n *Interface) GetMACString() (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetName func (n *Interface) GetName() (string, error) { - result := C.virInterfaceGetName(n.ptr) + var err C.virError + result := C.virInterfaceGetNameWrapper(n.ptr, &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } name := C.GoString(result) return name, nil @@ -99,9 +104,10 @@ func (n *Interface) GetName() (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceGetXM... func (n *Interface) GetXMLDesc(flags InterfaceXMLFlags) (string, error) { - result := C.virInterfaceGetXMLDesc(n.ptr, C.uint(flags)) + var err C.virError + result := C.virInterfaceGetXMLDescWrapper(n.ptr, C.uint(flags), &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } xml := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -110,27 +116,30 @@ func (n *Interface) GetXMLDesc(flags InterfaceXMLFlags) (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceUndefine func (n *Interface) Undefine() error { - result := C.virInterfaceUndefine(n.ptr) + var err C.virError + result := C.virInterfaceUndefineWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceFree func (n *Interface) Free() error { - ret := C.virInterfaceFree(n.ptr) + var err C.virError + ret := C.virInterfaceFreeWrapper(n.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-interface.html#virInterfaceRef func (c *Interface) Ref() error { - ret := C.virInterfaceRef(c.ptr) + var err C.virError + ret := C.virInterfaceRefWrapper(c.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } diff --git a/interface_wrapper.go b/interface_wrapper.go index c9618fb..a33aea9 100644 --- a/interface_wrapper.go +++ b/interface_wrapper.go @@ -30,5 +30,129 @@ package libvirt #include <assert.h> #include "interface_wrapper.h" + +int +virInterfaceCreateWrapper(virInterfacePtr iface, + unsigned int flags, + virErrorPtr err) +{ + int ret = virInterfaceCreate(iface, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virInterfaceDestroyWrapper(virInterfacePtr iface, + unsigned int flags, + virErrorPtr err) +{ + int ret = virInterfaceDestroy(iface, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virInterfaceFreeWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + int ret = virInterfaceFree(iface); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +virConnectPtr +virInterfaceGetConnectWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + virConnectPtr ret = virInterfaceGetConnect(iface); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +const char * +virInterfaceGetMACStringWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + const char * ret = virInterfaceGetMACString(iface); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +const char * +virInterfaceGetNameWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + const char * ret = virInterfaceGetName(iface); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +char * +virInterfaceGetXMLDescWrapper(virInterfacePtr iface, + unsigned int flags, + virErrorPtr err) +{ + char * ret = virInterfaceGetXMLDesc(iface, flags); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virInterfaceIsActiveWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + int ret = virInterfaceIsActive(iface); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virInterfaceRefWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + int ret = virInterfaceRef(iface); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virInterfaceUndefineWrapper(virInterfacePtr iface, + virErrorPtr err) +{ + int ret = virInterfaceUndefine(iface); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + */ import "C" diff --git a/interface_wrapper.h b/interface_wrapper.h index 8b1f8f1..b7cef76 100644 --- a/interface_wrapper.h +++ b/interface_wrapper.h @@ -29,4 +29,48 @@ #include <libvirt/libvirt.h> #include <libvirt/virterror.h> +int +virInterfaceCreateWrapper(virInterfacePtr iface, + unsigned int flags, + virErrorPtr err); + +int +virInterfaceDestroyWrapper(virInterfacePtr iface, + unsigned int flags, + virErrorPtr err); + +int +virInterfaceFreeWrapper(virInterfacePtr iface, + virErrorPtr err); + +virConnectPtr +virInterfaceGetConnectWrapper(virInterfacePtr iface, + virErrorPtr err); + +const char * +virInterfaceGetMACStringWrapper(virInterfacePtr iface, + virErrorPtr err); + +const char * +virInterfaceGetNameWrapper(virInterfacePtr iface, + virErrorPtr err); + +char * +virInterfaceGetXMLDescWrapper(virInterfacePtr iface, + unsigned int flags, + virErrorPtr err); + +int +virInterfaceIsActiveWrapper(virInterfacePtr iface, + virErrorPtr err); + +int +virInterfaceRefWrapper(virInterfacePtr iface, + virErrorPtr err); + +int +virInterfaceUndefineWrapper(virInterfacePtr iface, + virErrorPtr err); + + #endif /* LIBVIRT_GO_INTERFACE_WRAPPER_H__ */ -- 2.17.1