[libvirt] [go PATCH 26/37] node device: fix error reporting thread safety

Create wrapper functions for each node device 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; --- node_device.go | 62 ++++++++++------- node_device_wrapper.go | 150 +++++++++++++++++++++++++++++++++++++++++ node_device_wrapper.h | 55 +++++++++++++++ 3 files changed, 242 insertions(+), 25 deletions(-) diff --git a/node_device.go b/node_device.go index 0caf98b..474f288 100644 --- a/node_device.go +++ b/node_device.go @@ -57,45 +57,50 @@ type NodeDevice struct { // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceFree func (n *NodeDevice) Free() error { - ret := C.virNodeDeviceFree(n.ptr) + var err C.virError + ret := C.virNodeDeviceFreeWrapper(n.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceRef func (c *NodeDevice) Ref() error { - ret := C.virNodeDeviceRef(c.ptr) + var err C.virError + ret := C.virNodeDeviceRefWrapper(c.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy func (n *NodeDevice) Destroy() error { - result := C.virNodeDeviceDestroy(n.ptr) + var err C.virError + result := C.virNodeDeviceDestroyWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReset func (n *NodeDevice) Reset() error { - result := C.virNodeDeviceReset(n.ptr) + var err C.virError + result := C.virNodeDeviceResetWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDettach func (n *NodeDevice) Detach() error { - result := C.virNodeDeviceDettach(n.ptr) + var err C.virError + result := C.virNodeDeviceDettachWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } @@ -104,36 +109,40 @@ func (n *NodeDevice) Detach() error { func (n *NodeDevice) DetachFlags(driverName string, flags uint32) error { cDriverName := C.CString(driverName) defer C.free(unsafe.Pointer(cDriverName)) - result := C.virNodeDeviceDetachFlags(n.ptr, cDriverName, C.uint(flags)) + var err C.virError + result := C.virNodeDeviceDetachFlagsWrapper(n.ptr, cDriverName, C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReAttach func (n *NodeDevice) ReAttach() error { - result := C.virNodeDeviceReAttach(n.ptr) + var err C.virError + result := C.virNodeDeviceReAttachWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetName func (n *NodeDevice) GetName() (string, error) { - name := C.virNodeDeviceGetName(n.ptr) + var err C.virError + name := C.virNodeDeviceGetNameWrapper(n.ptr, &err) if name == nil { - return "", GetLastError() + return "", makeError(&err) } return C.GoString(name), nil } // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetXML... func (n *NodeDevice) GetXMLDesc(flags uint32) (string, error) { - result := C.virNodeDeviceGetXMLDesc(n.ptr, C.uint(flags)) + var err C.virError + result := C.virNodeDeviceGetXMLDescWrapper(n.ptr, C.uint(flags), &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } xml := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -142,9 +151,10 @@ func (n *NodeDevice) GetXMLDesc(flags uint32) (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetParent func (n *NodeDevice) GetParent() (string, error) { - result := C.virNodeDeviceGetParent(n.ptr) + var err C.virError + result := C.virNodeDeviceGetParentWrapper(n.ptr, &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } defer C.free(unsafe.Pointer(result)) return C.GoString(result), nil @@ -152,9 +162,10 @@ func (n *NodeDevice) GetParent() (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceNumOfCaps func (p *NodeDevice) NumOfCaps() (int, error) { - result := int(C.virNodeDeviceNumOfCaps(p.ptr)) + var err C.virError + result := int(C.virNodeDeviceNumOfCapsWrapper(p.ptr, &err)) if result == -1 { - return 0, GetLastError() + return 0, makeError(&err) } return result, nil } @@ -164,12 +175,13 @@ func (p *NodeDevice) ListCaps() ([]string, error) { const maxCaps = 1024 var names [maxCaps](*C.char) namesPtr := unsafe.Pointer(&names) - numCaps := C.virNodeDeviceListCaps( + var err C.virError + numCaps := C.virNodeDeviceListCapsWrapper( p.ptr, (**C.char)(namesPtr), - maxCaps) + maxCaps, &err) if numCaps == -1 { - return nil, GetLastError() + return nil, makeError(&err) } goNames := make([]string, numCaps) for k := 0; k < int(numCaps); k++ { diff --git a/node_device_wrapper.go b/node_device_wrapper.go index bf8f987..c4e173a 100644 --- a/node_device_wrapper.go +++ b/node_device_wrapper.go @@ -30,5 +30,155 @@ package libvirt #include <assert.h> #include "node_device_wrapper.h" + +int +virNodeDeviceDestroyWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceDestroy(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceDetachFlagsWrapper(virNodeDevicePtr dev, + const char *driverName, + unsigned int flags, + virErrorPtr err) +{ + int ret = virNodeDeviceDetachFlags(dev, driverName, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceDettachWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceDettach(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceFreeWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceFree(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +const char * +virNodeDeviceGetNameWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + const char * ret = virNodeDeviceGetName(dev); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +const char * +virNodeDeviceGetParentWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + const char * ret = virNodeDeviceGetParent(dev); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +char * +virNodeDeviceGetXMLDescWrapper(virNodeDevicePtr dev, + unsigned int flags, + virErrorPtr err) +{ + char * ret = virNodeDeviceGetXMLDesc(dev, flags); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceListCapsWrapper(virNodeDevicePtr dev, + char ** const names, + int maxnames, + virErrorPtr err) +{ + int ret = virNodeDeviceListCaps(dev, names, maxnames); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceNumOfCapsWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceNumOfCaps(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceReAttachWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceReAttach(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceRefWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceRef(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNodeDeviceResetWrapper(virNodeDevicePtr dev, + virErrorPtr err) +{ + int ret = virNodeDeviceReset(dev); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + */ import "C" diff --git a/node_device_wrapper.h b/node_device_wrapper.h index 22fc791..7670415 100644 --- a/node_device_wrapper.h +++ b/node_device_wrapper.h @@ -30,4 +30,59 @@ #include <libvirt/virterror.h> #include "node_device_compat.h" + +int +virNodeDeviceDestroyWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +int +virNodeDeviceDetachFlagsWrapper(virNodeDevicePtr dev, + const char *driverName, + unsigned int flags, + virErrorPtr err); + +int +virNodeDeviceDettachWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +int +virNodeDeviceFreeWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +const char * +virNodeDeviceGetNameWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +const char * +virNodeDeviceGetParentWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +char * +virNodeDeviceGetXMLDescWrapper(virNodeDevicePtr dev, + unsigned int flags, + virErrorPtr err); + +int +virNodeDeviceListCapsWrapper(virNodeDevicePtr dev, + char **const names, + int maxnames, + virErrorPtr err); + +int +virNodeDeviceNumOfCapsWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +int +virNodeDeviceReAttachWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +int +virNodeDeviceRefWrapper(virNodeDevicePtr dev, + virErrorPtr err); + +int +virNodeDeviceResetWrapper(virNodeDevicePtr dev, + virErrorPtr err); + + #endif /* LIBVIRT_GO_NODE_DEVICE_WRAPPER_H__ */ -- 2.17.1