[libvirt] [go PATCH 29/37] domain snapshot: fix error reporting thread safety

Create wrapper functions for each domain snapshot 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; --- domain_snapshot.go | 62 ++++++++----- domain_snapshot_wrapper.go | 181 +++++++++++++++++++++++++++++++++++++ domain_snapshot_wrapper.h | 70 ++++++++++++++ 3 files changed, 288 insertions(+), 25 deletions(-) diff --git a/domain_snapshot.go b/domain_snapshot.go index 706c1a9..65fbbb5 100644 --- a/domain_snapshot.go +++ b/domain_snapshot.go @@ -90,45 +90,50 @@ type DomainSnapshot struct { // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) Free() error { - ret := C.virDomainSnapshotFree(s.ptr) + var err C.virError + ret := C.virDomainSnapshotFreeWrapper(s.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (c *DomainSnapshot) Ref() error { - ret := C.virDomainSnapshotRef(c.ptr) + var err C.virError + ret := C.virDomainSnapshotRefWrapper(c.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) Delete(flags DomainSnapshotDeleteFlags) error { - result := C.virDomainSnapshotDelete(s.ptr, C.uint(flags)) + var err C.virError + result := C.virDomainSnapshotDeleteWrapper(s.ptr, C.uint(flags), &err) if result != 0 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainRe... func (s *DomainSnapshot) RevertToSnapshot(flags DomainSnapshotRevertFlags) error { - result := C.virDomainRevertToSnapshot(s.ptr, C.uint(flags)) + var err C.virError + result := C.virDomainRevertToSnapshotWrapper(s.ptr, C.uint(flags), &err) if result != 0 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) IsCurrent(flags uint32) (bool, error) { - result := C.virDomainSnapshotIsCurrent(s.ptr, C.uint(flags)) + var err C.virError + result := C.virDomainSnapshotIsCurrentWrapper(s.ptr, C.uint(flags), &err) if result == -1 { - return false, GetLastError() + return false, makeError(&err) } if result == 1 { return true, nil @@ -138,9 +143,10 @@ func (s *DomainSnapshot) IsCurrent(flags uint32) (bool, error) { // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) HasMetadata(flags uint32) (bool, error) { - result := C.virDomainSnapshotHasMetadata(s.ptr, C.uint(flags)) + var err C.virError + result := C.virDomainSnapshotHasMetadataWrapper(s.ptr, C.uint(flags), &err) if result == -1 { - return false, GetLastError() + return false, makeError(&err) } if result == 1 { return true, nil @@ -150,9 +156,10 @@ func (s *DomainSnapshot) HasMetadata(flags uint32) (bool, error) { // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) GetXMLDesc(flags DomainXMLFlags) (string, error) { - result := C.virDomainSnapshotGetXMLDesc(s.ptr, C.uint(flags)) + var err C.virError + result := C.virDomainSnapshotGetXMLDescWrapper(s.ptr, C.uint(flags), &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } xml := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -161,27 +168,30 @@ func (s *DomainSnapshot) GetXMLDesc(flags DomainXMLFlags) (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) GetName() (string, error) { - name := C.virDomainSnapshotGetName(s.ptr) + var err C.virError + name := C.virDomainSnapshotGetNameWrapper(s.ptr, &err) if name == nil { - return "", GetLastError() + return "", makeError(&err) } return C.GoString(name), nil } // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) GetParent(flags uint32) (*DomainSnapshot, error) { - ptr := C.virDomainSnapshotGetParent(s.ptr, C.uint(flags)) + var err C.virError + ptr := C.virDomainSnapshotGetParentWrapper(s.ptr, C.uint(flags), &err) if ptr == nil { - return nil, GetLastError() + return nil, makeError(&err) } return &DomainSnapshot{ptr: ptr}, nil } // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (s *DomainSnapshot) NumChildren(flags DomainSnapshotListFlags) (int, error) { - result := int(C.virDomainSnapshotNumChildren(s.ptr, C.uint(flags))) + var err C.virError + result := int(C.virDomainSnapshotNumChildrenWrapper(s.ptr, C.uint(flags), &err)) if result == -1 { - return 0, GetLastError() + return 0, makeError(&err) } return result, nil } @@ -191,12 +201,13 @@ func (s *DomainSnapshot) ListChildrenNames(flags DomainSnapshotListFlags) ([]str const maxNames = 1024 var names [maxNames](*C.char) namesPtr := unsafe.Pointer(&names) - numNames := C.virDomainSnapshotListChildrenNames( + var err C.virError + numNames := C.virDomainSnapshotListChildrenNamesWrapper( s.ptr, (**C.char)(namesPtr), - maxNames, C.uint(flags)) + maxNames, C.uint(flags), &err) if numNames == -1 { - return nil, GetLastError() + return nil, makeError(&err) } goNames := make([]string, numNames) for k := 0; k < int(numNames); k++ { @@ -209,9 +220,10 @@ func (s *DomainSnapshot) ListChildrenNames(flags DomainSnapshotListFlags) ([]str // See also https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSn... func (d *DomainSnapshot) ListAllChildren(flags DomainSnapshotListFlags) ([]DomainSnapshot, error) { var cList *C.virDomainSnapshotPtr - numVols := C.virDomainSnapshotListAllChildren(d.ptr, (**C.virDomainSnapshotPtr)(&cList), C.uint(flags)) + var err C.virError + numVols := C.virDomainSnapshotListAllChildrenWrapper(d.ptr, (**C.virDomainSnapshotPtr)(&cList), C.uint(flags), &err) if numVols == -1 { - return nil, GetLastError() + return nil, makeError(&err) } hdr := reflect.SliceHeader{ Data: uintptr(unsafe.Pointer(cList)), diff --git a/domain_snapshot_wrapper.go b/domain_snapshot_wrapper.go index 75e3ea5..a061dee 100644 --- a/domain_snapshot_wrapper.go +++ b/domain_snapshot_wrapper.go @@ -30,5 +30,186 @@ package libvirt #include <assert.h> #include "domain_snapshot_wrapper.h" + +int +virDomainRevertToSnapshotWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainRevertToSnapshot(snapshot, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotDeleteWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainSnapshotDelete(snapshot, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotFreeWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err) +{ + int ret = virDomainSnapshotFree(snapshot); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +virConnectPtr +virDomainSnapshotGetConnectWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err) +{ + virConnectPtr ret = virDomainSnapshotGetConnect(snapshot); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +virDomainPtr +virDomainSnapshotGetDomainWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err) +{ + virDomainPtr ret = virDomainSnapshotGetDomain(snapshot); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +const char * +virDomainSnapshotGetNameWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err) +{ + const char * ret = virDomainSnapshotGetName(snapshot); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +virDomainSnapshotPtr +virDomainSnapshotGetParentWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + virDomainSnapshotPtr ret = virDomainSnapshotGetParent(snapshot, flags); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +char * +virDomainSnapshotGetXMLDescWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + char * ret = virDomainSnapshotGetXMLDesc(snapshot, flags); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotHasMetadataWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainSnapshotHasMetadata(snapshot, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotIsCurrentWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainSnapshotIsCurrent(snapshot, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotListAllChildrenWrapper(virDomainSnapshotPtr snapshot, + virDomainSnapshotPtr **snaps, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainSnapshotListAllChildren(snapshot, snaps, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotListChildrenNamesWrapper(virDomainSnapshotPtr snapshot, + char **names, + int nameslen, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainSnapshotListChildrenNames(snapshot, names, nameslen, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotNumChildrenWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err) +{ + int ret = virDomainSnapshotNumChildren(snapshot, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virDomainSnapshotRefWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err) +{ + int ret = virDomainSnapshotRef(snapshot); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + */ import "C" diff --git a/domain_snapshot_wrapper.h b/domain_snapshot_wrapper.h index 45464ce..fcf8036 100644 --- a/domain_snapshot_wrapper.h +++ b/domain_snapshot_wrapper.h @@ -29,4 +29,74 @@ #include <libvirt/libvirt.h> #include <libvirt/virterror.h> + +int +virDomainRevertToSnapshotWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotDeleteWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotFreeWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err); + +virConnectPtr +virDomainSnapshotGetConnectWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err); + +virDomainPtr +virDomainSnapshotGetDomainWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err); + +const char * +virDomainSnapshotGetNameWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err); + +virDomainSnapshotPtr +virDomainSnapshotGetParentWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +char * +virDomainSnapshotGetXMLDescWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotHasMetadataWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotIsCurrentWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotListAllChildrenWrapper(virDomainSnapshotPtr snapshot, + virDomainSnapshotPtr **snaps, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotListChildrenNamesWrapper(virDomainSnapshotPtr snapshot, + char **names, + int nameslen, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotNumChildrenWrapper(virDomainSnapshotPtr snapshot, + unsigned int flags, + virErrorPtr err); + +int +virDomainSnapshotRefWrapper(virDomainSnapshotPtr snapshot, + virErrorPtr err); + + #endif /* LIBVIRT_GO_DOMAIN_SNAPSHOT_WRAPPER_H__ */ -- 2.17.1