[libvirt] [go PATCH 27/37] network: fix error reporting thread safety

Create wrapper functions for each network 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; --- network.go | 80 +++++++++------- network_wrapper.go | 228 +++++++++++++++++++++++++++++++++++++++++++-- network_wrapper.h | 78 +++++++++++++++- 3 files changed, 347 insertions(+), 39 deletions(-) diff --git a/network.go b/network.go index 015fe5e..d9ec9bf 100644 --- a/network.go +++ b/network.go @@ -121,45 +121,50 @@ type NetworkDHCPLease struct { // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkFree func (n *Network) Free() error { - ret := C.virNetworkFree(n.ptr) + var err C.virError + ret := C.virNetworkFreeWrapper(n.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkRef func (c *Network) Ref() error { - ret := C.virNetworkRef(c.ptr) + var err C.virError + ret := C.virNetworkRefWrapper(c.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkCreate func (n *Network) Create() error { - result := C.virNetworkCreate(n.ptr) + var err C.virError + result := C.virNetworkCreateWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkDestroy func (n *Network) Destroy() error { - result := C.virNetworkDestroy(n.ptr) + var err C.virError + result := C.virNetworkDestroyWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsActive func (n *Network) IsActive() (bool, error) { - result := C.virNetworkIsActive(n.ptr) + var err C.virError + result := C.virNetworkIsActiveWrapper(n.ptr, &err) if result == -1 { - return false, GetLastError() + return false, makeError(&err) } if result == 1 { return true, nil @@ -169,9 +174,10 @@ func (n *Network) IsActive() (bool, error) { // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsPersistent func (n *Network) IsPersistent() (bool, error) { - result := C.virNetworkIsPersistent(n.ptr) + var err C.virError + result := C.virNetworkIsPersistentWrapper(n.ptr, &err) if result == -1 { - return false, GetLastError() + return false, makeError(&err) } if result == 1 { return true, nil @@ -182,9 +188,10 @@ func (n *Network) IsPersistent() (bool, error) { // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetAutostart func (n *Network) GetAutostart() (bool, error) { var out C.int - result := C.virNetworkGetAutostart(n.ptr, (*C.int)(unsafe.Pointer(&out))) + var err C.virError + result := C.virNetworkGetAutostartWrapper(n.ptr, (*C.int)(unsafe.Pointer(&out)), &err) if result == -1 { - return false, GetLastError() + return false, makeError(&err) } switch out { case 1: @@ -203,18 +210,20 @@ func (n *Network) SetAutostart(autostart bool) error { default: cAutostart = 0 } - result := C.virNetworkSetAutostart(n.ptr, cAutostart) + var err C.virError + result := C.virNetworkSetAutostartWrapper(n.ptr, cAutostart, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetName func (n *Network) GetName() (string, error) { - name := C.virNetworkGetName(n.ptr) + var err C.virError + name := C.virNetworkGetNameWrapper(n.ptr, &err) if name == nil { - return "", GetLastError() + return "", makeError(&err) } return C.GoString(name), nil } @@ -223,9 +232,10 @@ func (n *Network) GetName() (string, error) { func (n *Network) GetUUID() ([]byte, error) { var cUuid [C.VIR_UUID_BUFLEN](byte) cuidPtr := unsafe.Pointer(&cUuid) - result := C.virNetworkGetUUID(n.ptr, (*C.uchar)(cuidPtr)) + var err C.virError + result := C.virNetworkGetUUIDWrapper(n.ptr, (*C.uchar)(cuidPtr), &err) if result != 0 { - return []byte{}, GetLastError() + return []byte{}, makeError(&err) } return C.GoBytes(cuidPtr, C.VIR_UUID_BUFLEN), nil } @@ -234,18 +244,20 @@ func (n *Network) GetUUID() ([]byte, error) { func (n *Network) GetUUIDString() (string, error) { var cUuid [C.VIR_UUID_STRING_BUFLEN](C.char) cuidPtr := unsafe.Pointer(&cUuid) - result := C.virNetworkGetUUIDString(n.ptr, (*C.char)(cuidPtr)) + var err C.virError + result := C.virNetworkGetUUIDStringWrapper(n.ptr, (*C.char)(cuidPtr), &err) if result != 0 { - return "", GetLastError() + return "", makeError(&err) } return C.GoString((*C.char)(cuidPtr)), nil } // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetBridge... func (n *Network) GetBridgeName() (string, error) { - result := C.virNetworkGetBridgeName(n.ptr) + var err C.virError + result := C.virNetworkGetBridgeNameWrapper(n.ptr, &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } bridge := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -254,9 +266,10 @@ func (n *Network) GetBridgeName() (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetXMLDesc func (n *Network) GetXMLDesc(flags NetworkXMLFlags) (string, error) { - result := C.virNetworkGetXMLDesc(n.ptr, C.uint(flags)) + var err C.virError + result := C.virNetworkGetXMLDescWrapper(n.ptr, C.uint(flags), &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } xml := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -265,9 +278,10 @@ func (n *Network) GetXMLDesc(flags NetworkXMLFlags) (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkUndefine func (n *Network) Undefine() error { - result := C.virNetworkUndefine(n.ptr) + var err C.virError + result := C.virNetworkUndefineWrapper(n.ptr, &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } @@ -276,9 +290,10 @@ func (n *Network) Undefine() error { func (n *Network) Update(cmd NetworkUpdateCommand, section NetworkUpdateSection, parentIndex int, xml string, flags NetworkUpdateFlags) error { cxml := C.CString(xml) defer C.free(unsafe.Pointer(cxml)) - result := C.virNetworkUpdate(n.ptr, C.uint(cmd), C.uint(section), C.int(parentIndex), cxml, C.uint(flags)) + var err C.virError + result := C.virNetworkUpdateWrapper(n.ptr, C.uint(cmd), C.uint(section), C.int(parentIndex), cxml, C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } @@ -289,9 +304,10 @@ func (n *Network) GetDHCPLeases() ([]NetworkDHCPLease, error) { return []NetworkDHCPLease{}, GetNotImplementedError("virNetworkGetDHCPLeases") } var cLeases *C.virNetworkDHCPLeasePtr - numLeases := C.virNetworkGetDHCPLeasesWrapper(n.ptr, nil, (**C.virNetworkDHCPLeasePtr)(&cLeases), C.uint(0)) + var err C.virError + numLeases := C.virNetworkGetDHCPLeasesWrapper(n.ptr, nil, (**C.virNetworkDHCPLeasePtr)(&cLeases), C.uint(0), &err) if numLeases == -1 { - return nil, GetLastError() + return nil, makeError(&err) } hdr := reflect.SliceHeader{ Data: uintptr(unsafe.Pointer(cLeases)), diff --git a/network_wrapper.go b/network_wrapper.go index 5c797e0..2fc443f 100644 --- a/network_wrapper.go +++ b/network_wrapper.go @@ -31,21 +31,237 @@ package libvirt #include <assert.h> #include "network_wrapper.h" -void virNetworkDHCPLeaseFreeWrapper(virNetworkDHCPLeasePtr lease) +int +virNetworkCreateWrapper(virNetworkPtr network, + virErrorPtr err) { + int ret = virNetworkCreate(network); + if (ret < 0) { + virCopyLastError(err); + } + return ret; } -int virNetworkGetDHCPLeasesWrapper(virNetworkPtr network, - const char *mac, - virNetworkDHCPLeasePtr **leases, - unsigned int flags) + +void +virNetworkDHCPLeaseFreeWrapper(virNetworkDHCPLeasePtr lease) { #if LIBVIR_VERSION_NUMBER < 1002006 assert(0); // Caller should have checked version #else - return virNetworkGetDHCPLeases(network, mac, leases, flags); + virNetworkDHCPLeaseFree(lease); #endif } + +int +virNetworkDestroyWrapper(virNetworkPtr network, + virErrorPtr err) +{ + int ret = virNetworkDestroy(network); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkFreeWrapper(virNetworkPtr network, + virErrorPtr err) +{ + int ret = virNetworkFree(network); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkGetAutostartWrapper(virNetworkPtr network, + int *autostart, + virErrorPtr err) +{ + int ret = virNetworkGetAutostart(network, autostart); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +char * +virNetworkGetBridgeNameWrapper(virNetworkPtr network, + virErrorPtr err) +{ + char * ret = virNetworkGetBridgeName(network); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +virConnectPtr +virNetworkGetConnectWrapper(virNetworkPtr net, + virErrorPtr err) +{ + virConnectPtr ret = virNetworkGetConnect(net); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkGetDHCPLeasesWrapper(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasePtr **leases, + unsigned int flags, + virErrorPtr err) +{ +#if LIBVIR_VERSION_NUMBER < 1002006 + assert(0); // Caller should have checked version +#else + int ret = virNetworkGetDHCPLeases(network, mac, leases, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +#endif +} + + +const char * +virNetworkGetNameWrapper(virNetworkPtr network, + virErrorPtr err) +{ + const char * ret = virNetworkGetName(network); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkGetUUIDWrapper(virNetworkPtr network, + unsigned char *uuid, + virErrorPtr err) +{ + int ret = virNetworkGetUUID(network, uuid); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkGetUUIDStringWrapper(virNetworkPtr network, + char *buf, + virErrorPtr err) +{ + int ret = virNetworkGetUUIDString(network, buf); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +char * +virNetworkGetXMLDescWrapper(virNetworkPtr network, + unsigned int flags, + virErrorPtr err) +{ + char * ret = virNetworkGetXMLDesc(network, flags); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkIsActiveWrapper(virNetworkPtr net, + virErrorPtr err) +{ + int ret = virNetworkIsActive(net); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkIsPersistentWrapper(virNetworkPtr net, + virErrorPtr err) +{ + int ret = virNetworkIsPersistent(net); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkRefWrapper(virNetworkPtr network, + virErrorPtr err) +{ + int ret = virNetworkRef(network); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkSetAutostartWrapper(virNetworkPtr network, + int autostart, + virErrorPtr err) +{ + int ret = virNetworkSetAutostart(network, autostart); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkUndefineWrapper(virNetworkPtr network, + virErrorPtr err) +{ + int ret = virNetworkUndefine(network); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virNetworkUpdateWrapper(virNetworkPtr network, + unsigned int command, + unsigned int section, + int parentIndex, + const char *xml, + unsigned int flags, + virErrorPtr err) +{ + int ret = virNetworkUpdate(network, command, section, parentIndex, xml, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + */ import "C" diff --git a/network_wrapper.h b/network_wrapper.h index 8a36d13..405bf89 100644 --- a/network_wrapper.h +++ b/network_wrapper.h @@ -31,13 +31,89 @@ #include <libvirt/virterror.h> #include "network_compat.h" +int +virNetworkCreateWrapper(virNetworkPtr network, + virErrorPtr err); + void virNetworkDHCPLeaseFreeWrapper(virNetworkDHCPLeasePtr lease); +int +virNetworkDestroyWrapper(virNetworkPtr network, + virErrorPtr err); + +int +virNetworkFreeWrapper(virNetworkPtr network, + virErrorPtr err); + +int +virNetworkGetAutostartWrapper(virNetworkPtr network, + int *autostart, + virErrorPtr err); + +char * +virNetworkGetBridgeNameWrapper(virNetworkPtr network, + virErrorPtr err); + +virConnectPtr +virNetworkGetConnectWrapper(virNetworkPtr net, + virErrorPtr err); + int virNetworkGetDHCPLeasesWrapper(virNetworkPtr network, const char *mac, virNetworkDHCPLeasePtr **leases, - unsigned int flags); + unsigned int flags, + virErrorPtr err); + +const char * +virNetworkGetNameWrapper(virNetworkPtr network, + virErrorPtr err); + +int +virNetworkGetUUIDWrapper(virNetworkPtr network, + unsigned char *uuid, + virErrorPtr err); + +int +virNetworkGetUUIDStringWrapper(virNetworkPtr network, + char *buf, + virErrorPtr err); + +char * +virNetworkGetXMLDescWrapper(virNetworkPtr network, + unsigned int flags, + virErrorPtr err); + +int +virNetworkIsActiveWrapper(virNetworkPtr net, + virErrorPtr err); + +int +virNetworkIsPersistentWrapper(virNetworkPtr net, + virErrorPtr err); + +int +virNetworkRefWrapper(virNetworkPtr network, + virErrorPtr err); + +int +virNetworkSetAutostartWrapper(virNetworkPtr network, + int autostart, + virErrorPtr err); + +int +virNetworkUndefineWrapper(virNetworkPtr network, + virErrorPtr err); + +int +virNetworkUpdateWrapper(virNetworkPtr network, + unsigned int command, + unsigned int section, + int parentIndex, + const char *xml, + unsigned int flags, + virErrorPtr err); + #endif /* LIBVIRT_GO_NETWORK_WRAPPER_H__ */ -- 2.17.1