[libvirt] [go PATCH 20/37] storage vol: fix error reporting thread safety

Create wrapper functions for each storage volume 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; --- storage_volume.go | 79 ++++++++------ storage_volume_wrapper.go | 211 +++++++++++++++++++++++++++++++++++++- storage_volume_wrapper.h | 79 +++++++++++++- 3 files changed, 332 insertions(+), 37 deletions(-) diff --git a/storage_volume.go b/storage_volume.go index d913c59..711c3c1 100644 --- a/storage_volume.go +++ b/storage_volume.go @@ -123,27 +123,30 @@ type StorageVolInfo struct { // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolDelete func (v *StorageVol) Delete(flags StorageVolDeleteFlags) error { - result := C.virStorageVolDelete(v.ptr, C.uint(flags)) + var err C.virError + result := C.virStorageVolDeleteWrapper(v.ptr, C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolFree func (v *StorageVol) Free() error { - ret := C.virStorageVolFree(v.ptr) + var err C.virError + ret := C.virStorageVolFreeWrapper(v.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolRef func (c *StorageVol) Ref() error { - ret := C.virStorageVolRef(c.ptr) + var err C.virError + ret := C.virStorageVolRefWrapper(c.ptr, &err) if ret == -1 { - return GetLastError() + return makeError(&err) } return nil } @@ -151,9 +154,10 @@ func (c *StorageVol) Ref() error { // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetInfo func (v *StorageVol) GetInfo() (*StorageVolInfo, error) { var cinfo C.virStorageVolInfo - result := C.virStorageVolGetInfo(v.ptr, &cinfo) + var err C.virError + result := C.virStorageVolGetInfoWrapper(v.ptr, &cinfo, &err) if result == -1 { - return nil, GetLastError() + return nil, makeError(&err) } return &StorageVolInfo{ Type: StorageVolType(cinfo._type), @@ -169,9 +173,10 @@ func (v *StorageVol) GetInfoFlags(flags StorageVolInfoFlags) (*StorageVolInfo, e } var cinfo C.virStorageVolInfo - result := C.virStorageVolGetInfoFlagsWrapper(v.ptr, &cinfo, C.uint(flags)) + var err C.virError + result := C.virStorageVolGetInfoFlagsWrapper(v.ptr, &cinfo, C.uint(flags), &err) if result == -1 { - return nil, GetLastError() + return nil, makeError(&err) } return &StorageVolInfo{ Type: StorageVolType(cinfo._type), @@ -182,27 +187,30 @@ func (v *StorageVol) GetInfoFlags(flags StorageVolInfoFlags) (*StorageVolInfo, e // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetKey func (v *StorageVol) GetKey() (string, error) { - key := C.virStorageVolGetKey(v.ptr) + var err C.virError + key := C.virStorageVolGetKeyWrapper(v.ptr, &err) if key == nil { - return "", GetLastError() + return "", makeError(&err) } return C.GoString(key), nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetName func (v *StorageVol) GetName() (string, error) { - name := C.virStorageVolGetName(v.ptr) + var err C.virError + name := C.virStorageVolGetNameWrapper(v.ptr, &err) if name == nil { - return "", GetLastError() + return "", makeError(&err) } return C.GoString(name), nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetPath func (v *StorageVol) GetPath() (string, error) { - result := C.virStorageVolGetPath(v.ptr) + var err C.virError + result := C.virStorageVolGetPathWrapper(v.ptr, &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } path := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -211,9 +219,10 @@ func (v *StorageVol) GetPath() (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolGetXML... func (v *StorageVol) GetXMLDesc(flags uint32) (string, error) { - result := C.virStorageVolGetXMLDesc(v.ptr, C.uint(flags)) + var err C.virError + result := C.virStorageVolGetXMLDescWrapper(v.ptr, C.uint(flags), &err) if result == nil { - return "", GetLastError() + return "", makeError(&err) } xml := C.GoString(result) C.free(unsafe.Pointer(result)) @@ -222,54 +231,60 @@ func (v *StorageVol) GetXMLDesc(flags uint32) (string, error) { // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolResize func (v *StorageVol) Resize(capacity uint64, flags StorageVolResizeFlags) error { - result := C.virStorageVolResize(v.ptr, C.ulonglong(capacity), C.uint(flags)) + var err C.virError + result := C.virStorageVolResizeWrapper(v.ptr, C.ulonglong(capacity), C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolWipe func (v *StorageVol) Wipe(flags uint32) error { - result := C.virStorageVolWipe(v.ptr, C.uint(flags)) + var err C.virError + result := C.virStorageVolWipeWrapper(v.ptr, C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolWipePa... func (v *StorageVol) WipePattern(algorithm StorageVolWipeAlgorithm, flags uint32) error { - result := C.virStorageVolWipePattern(v.ptr, C.uint(algorithm), C.uint(flags)) + var err C.virError + result := C.virStorageVolWipePatternWrapper(v.ptr, C.uint(algorithm), C.uint(flags), &err) if result == -1 { - return GetLastError() + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolUpload func (v *StorageVol) Upload(stream *Stream, offset, length uint64, flags StorageVolUploadFlags) error { - if C.virStorageVolUpload(v.ptr, stream.ptr, C.ulonglong(offset), - C.ulonglong(length), C.uint(flags)) == -1 { - return GetLastError() + var err C.virError + if C.virStorageVolUploadWrapper(v.ptr, stream.ptr, C.ulonglong(offset), + C.ulonglong(length), C.uint(flags), &err) == -1 { + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolDownload func (v *StorageVol) Download(stream *Stream, offset, length uint64, flags StorageVolDownloadFlags) error { - if C.virStorageVolDownload(v.ptr, stream.ptr, C.ulonglong(offset), - C.ulonglong(length), C.uint(flags)) == -1 { - return GetLastError() + var err C.virError + if C.virStorageVolDownloadWrapper(v.ptr, stream.ptr, C.ulonglong(offset), + C.ulonglong(length), C.uint(flags), &err) == -1 { + return makeError(&err) } return nil } // See also https://libvirt.org/html/libvirt-libvirt-storage.html#virStoragePoolLooku... func (v *StorageVol) LookupPoolByVolume() (*StoragePool, error) { - poolPtr := C.virStoragePoolLookupByVolume(v.ptr) + var err C.virError + poolPtr := C.virStoragePoolLookupByVolumeWrapper(v.ptr, &err) if poolPtr == nil { - return nil, GetLastError() + return nil, makeError(&err) } return &StoragePool{ptr: poolPtr}, nil } diff --git a/storage_volume_wrapper.go b/storage_volume_wrapper.go index 63cc2b5..d41a8a7 100644 --- a/storage_volume_wrapper.go +++ b/storage_volume_wrapper.go @@ -31,16 +31,219 @@ package libvirt #include <assert.h> #include "storage_volume_wrapper.h" -int virStorageVolGetInfoFlagsWrapper(virStorageVolPtr vol, - virStorageVolInfoPtr info, - unsigned int flags) +virStoragePoolPtr +virStoragePoolLookupByVolumeWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + virStoragePoolPtr ret = virStoragePoolLookupByVolume(vol); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolDeleteWrapper(virStorageVolPtr vol, + unsigned int flags, + virErrorPtr err) +{ + int ret = virStorageVolDelete(vol, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolDownloadWrapper(virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags, + virErrorPtr err) +{ + int ret = virStorageVolDownload(vol, stream, offset, length, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolFreeWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + int ret = virStorageVolFree(vol); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +virConnectPtr +virStorageVolGetConnectWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + virConnectPtr ret = virStorageVolGetConnect(vol); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolGetInfoWrapper(virStorageVolPtr vol, + virStorageVolInfoPtr info, + virErrorPtr err) +{ + int ret = virStorageVolGetInfo(vol, info); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolGetInfoFlagsWrapper(virStorageVolPtr vol, + virStorageVolInfoPtr info, + unsigned int flags, + virErrorPtr err) { #if LIBVIR_VERSION_NUMBER < 3000000 assert(0); // Caller should have checked version #else - return virStorageVolGetInfoFlags(vol, info, flags); + int ret = virStorageVolGetInfoFlags(vol, info, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; #endif } + +const char * +virStorageVolGetKeyWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + const char *ret = virStorageVolGetKey(vol); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +const char * +virStorageVolGetNameWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + const char *ret = virStorageVolGetName(vol); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +char * +virStorageVolGetPathWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + char *ret = virStorageVolGetPath(vol); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +char * +virStorageVolGetXMLDescWrapper(virStorageVolPtr vol, + unsigned int flags, + virErrorPtr err) +{ + char *ret = virStorageVolGetXMLDesc(vol, flags); + if (!ret) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolRefWrapper(virStorageVolPtr vol, + virErrorPtr err) +{ + int ret = virStorageVolRef(vol); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolResizeWrapper(virStorageVolPtr vol, + unsigned long long capacity, + unsigned int flags, + virErrorPtr err) +{ + int ret = virStorageVolResize(vol, capacity, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolUploadWrapper(virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags, + virErrorPtr err) +{ + int ret = virStorageVolUpload(vol, stream, offset, length, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolWipeWrapper(virStorageVolPtr vol, + unsigned int flags, + virErrorPtr err) +{ + int ret = virStorageVolWipe(vol, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + +int +virStorageVolWipePatternWrapper(virStorageVolPtr vol, + unsigned int algorithm, + unsigned int flags, + virErrorPtr err) +{ + int ret = virStorageVolWipePattern(vol, algorithm, flags); + if (ret < 0) { + virCopyLastError(err); + } + return ret; +} + + */ import "C" diff --git a/storage_volume_wrapper.h b/storage_volume_wrapper.h index 6fd8974..b3ed2a9 100644 --- a/storage_volume_wrapper.h +++ b/storage_volume_wrapper.h @@ -31,9 +31,86 @@ #include <libvirt/virterror.h> #include "storage_volume_compat.h" +virStoragePoolPtr +virStoragePoolLookupByVolumeWrapper(virStorageVolPtr vol, + virErrorPtr err); + +int +virStorageVolDeleteWrapper(virStorageVolPtr vol, + unsigned int flags, + virErrorPtr err); + +int +virStorageVolDownloadWrapper(virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags, + virErrorPtr err); + +int +virStorageVolFreeWrapper(virStorageVolPtr vol, + virErrorPtr err); + +virConnectPtr +virStorageVolGetConnectWrapper(virStorageVolPtr vol, + virErrorPtr err); + +int +virStorageVolGetInfoWrapper(virStorageVolPtr vol, + virStorageVolInfoPtr info, + virErrorPtr err); + int virStorageVolGetInfoFlagsWrapper(virStorageVolPtr vol, virStorageVolInfoPtr info, - unsigned int flags); + unsigned int flags, + virErrorPtr err); + +const char * +virStorageVolGetKeyWrapper(virStorageVolPtr vol, + virErrorPtr err); + +const char * +virStorageVolGetNameWrapper(virStorageVolPtr vol, + virErrorPtr err); + +char * +virStorageVolGetPathWrapper(virStorageVolPtr vol, + virErrorPtr err); + +char * +virStorageVolGetXMLDescWrapper(virStorageVolPtr vol, + unsigned int flags, + virErrorPtr err); + +int +virStorageVolRefWrapper(virStorageVolPtr vol, + virErrorPtr err); + +int +virStorageVolResizeWrapper(virStorageVolPtr vol, + unsigned long long capacity, + unsigned int flags, + virErrorPtr err); + +int +virStorageVolUploadWrapper(virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags, + virErrorPtr err); + +int +virStorageVolWipeWrapper(virStorageVolPtr vol, + unsigned int flags, + virErrorPtr err); + +int +virStorageVolWipePatternWrapper(virStorageVolPtr vol, + unsigned int algorithm, + unsigned int flags, + virErrorPtr err); #endif /* LIBVIRT_GO_STORAGE_VOLUME_WRAPPER_H__ */ -- 2.17.1