The driver.{c,h} files are primarily targetted at loading hypervisor drivers and some helper functions in that area. It also, however, contains a generically useful function for loading extension modules that is called by the storage driver. Split that functionality off into a new virmodule.{c,h} file to isolate it. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/driver.c | 133 +------------------------------- src/driver.h | 3 - src/libvirt_driver_modules.syms | 1 - src/libvirt_private.syms | 3 + src/storage/storage_backend.c | 3 +- src/util/Makefile.inc.am | 2 + src/util/virmodule.c | 163 ++++++++++++++++++++++++++++++++++++++++ src/util/virmodule.h | 29 +++++++ 8 files changed, 201 insertions(+), 136 deletions(-) create mode 100644 src/util/virmodule.c create mode 100644 src/util/virmodule.h diff --git a/src/driver.c b/src/driver.c index 447f61d554..8b5ade763f 100644 --- a/src/driver.c +++ b/src/driver.c @@ -28,6 +28,7 @@ #include "viralloc.h" #include "virfile.h" #include "virlog.h" +#include "virmodule.h" #include "virthread.h" #include "configmake.h" @@ -38,136 +39,6 @@ VIR_LOG_INIT("driver"); /* XXX re-implement this for other OS, or use libtools helper lib ? */ #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver" -#ifdef HAVE_DLFCN_H -# include <dlfcn.h> - - -static void * -virDriverLoadModuleFile(const char *file) -{ - void *handle = NULL; - int flags = RTLD_NOW | RTLD_GLOBAL; - -# ifdef RTLD_NODELETE - flags |= RTLD_NODELETE; -# endif - - VIR_DEBUG("Load module file '%s'", file); - - virUpdateSelfLastChanged(file); - - if (!(handle = dlopen(file, flags))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to load module '%s': %s"), file, dlerror()); - return NULL; - } - - return handle; -} - - -static void * -virDriverLoadModuleFunc(void *handle, - const char *file, - const char *funcname) -{ - void *regsym; - - VIR_DEBUG("Lookup function '%s'", funcname); - - if (!(regsym = dlsym(handle, funcname))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to find symbol '%s' in module '%s': %s"), - funcname, file, dlerror()); - return NULL; - } - - return regsym; -} - - -/** - * virDriverLoadModuleFull: - * @path: filename of module to load - * @regfunc: name of the function that registers the module - * - * Loads a loadable module named @path and calls the - * registration function @regfunc. The module will never - * be unloaded because unloading is not safe in a multi-threaded - * application. - * - * The module is automatically looked up in the appropriate place (git or - * installed directory). - * - * Returns 0 on success, 1 if the module was not found and -1 on any error. - */ -int -virDriverLoadModuleFull(const char *path, - const char *regfunc, - bool required) -{ - void *rethandle = NULL; - int (*regsym)(void); - int ret = -1; - - if (!virFileExists(path)) { - if (required) { - virReportSystemError(errno, - _("Failed to find module '%s'"), path); - return -1; - } else { - VIR_INFO("Module '%s' does not exist", path); - return 1; - } - } - - if (!(rethandle = virDriverLoadModuleFile(path))) - goto cleanup; - - if (!(regsym = virDriverLoadModuleFunc(rethandle, path, regfunc))) - goto cleanup; - - if ((*regsym)() < 0) { - /* regsym() should report an error itself, but lets - * just make sure */ - virErrorPtr err = virGetLastError(); - if (err == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to execute symbol '%s' in module '%s'"), - regfunc, path); - } - goto cleanup; - } - - rethandle = NULL; - ret = 0; - - cleanup: - if (rethandle) - dlclose(rethandle); - return ret; -} - -#else /* ! HAVE_DLFCN_H */ -int -virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED, - const char *regfunc ATTRIBUTE_UNUSED, - bool required) -{ - VIR_DEBUG("dlopen not available on this platform"); - if (required) { - virReportSystemError(ENOSYS, - _("Failed to find module '%s': %s"), path); - return -1; - } else { - /* Since we have no dlopen(), but definition we have no - * loadable modules on disk, so we can resaonably - * return '1' instead of an error. - */ - return 1; - } -} -#endif /* ! HAVE_DLFCN_H */ int @@ -188,7 +59,7 @@ virDriverLoadModule(const char *name, "LIBVIRT_DRIVER_DIR"))) return -1; - ret = virDriverLoadModuleFull(modfile, regfunc, required); + ret = virModuleLoad(modfile, regfunc, required); VIR_FREE(modfile); diff --git a/src/driver.h b/src/driver.h index b4e50ab987..0b1f7a2269 100644 --- a/src/driver.h +++ b/src/driver.h @@ -110,9 +110,6 @@ int virSetSharedStorageDriver(virStorageDriverPtr driver) ATTRIBUTE_RETURN_CHECK int virDriverLoadModule(const char *name, const char *regfunc, bool required); -int virDriverLoadModuleFull(const char *path, - const char *regfunc, - bool required); virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); diff --git a/src/libvirt_driver_modules.syms b/src/libvirt_driver_modules.syms index bd9bf1c315..f9d0ee9b97 100644 --- a/src/libvirt_driver_modules.syms +++ b/src/libvirt_driver_modules.syms @@ -4,7 +4,6 @@ # driver.h virDriverLoadModule; -virDriverLoadModuleFull; # Let emacs know we want case-insensitive sorting # Local Variables: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d2728749fb..1051a105b8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2228,6 +2228,9 @@ virMediatedDeviceTypeFree; virMediatedDeviceTypeReadAttrs; +# util/virmodule.h +virModuleLoad; + # util/virnetdev.h virNetDevAddMulti; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index cb1bcc0944..7d226f3d3a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -33,6 +33,7 @@ #include "virstoragefile.h" #include "storage_backend.h" #include "virlog.h" +#include "virmodule.h" #include "virfile.h" #include "configmake.h" @@ -97,7 +98,7 @@ virStorageDriverLoadBackendModule(const char *name, "LIBVIRT_STORAGE_BACKEND_DIR"))) return -1; - ret = virDriverLoadModuleFull(modfile, regfunc, forceload); + ret = virModuleLoad(modfile, regfunc, forceload); VIR_FREE(modfile); diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index 9624fb687c..ec8745da7e 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -100,6 +100,8 @@ UTIL_SOURCES = \ util/virmacaddr.h \ util/virmacmap.c \ util/virmacmap.h \ + util/virmodule.c \ + util/virmodule.h \ util/virnetdev.c \ util/virnetdev.h \ util/virnetdevbandwidth.c \ diff --git a/src/util/virmodule.c b/src/util/virmodule.c new file mode 100644 index 0000000000..ff8c22752e --- /dev/null +++ b/src/util/virmodule.c @@ -0,0 +1,163 @@ +/* + * virmodule.c: APIs for dlopen'ing extension modules + * + * Copyright (C) 2012-2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#include <config.h> + +#include "internal.h" +#include "virmodule.h" +#include "virerror.h" +#include "virfile.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.module"); + +#ifdef HAVE_DLFCN_H +# include <dlfcn.h> + +static void * +virModuleLoadFile(const char *file) +{ + void *handle = NULL; + int flags = RTLD_NOW | RTLD_GLOBAL; + +# ifdef RTLD_NODELETE + flags |= RTLD_NODELETE; +# endif + + VIR_DEBUG("Load module file '%s'", file); + + virUpdateSelfLastChanged(file); + + if (!(handle = dlopen(file, flags))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to load module '%s': %s"), file, dlerror()); + return NULL; + } + + return handle; +} + + +static void * +virModuleLoadFunc(void *handle, + const char *file, + const char *funcname) +{ + void *regsym; + + VIR_DEBUG("Lookup function '%s'", funcname); + + if (!(regsym = dlsym(handle, funcname))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find symbol '%s' in module '%s': %s"), + funcname, file, dlerror()); + return NULL; + } + + return regsym; +} + + +/** + * virModuleLoad: + * @path: filename of module to load + * @regfunc: name of the function that registers the module + * @required: true if module must exist on disk, false to silently skip + * + * Loads a loadable module named @path and calls the + * registration function @regfunc. The module will never + * be unloaded because unloading is not safe in a multi-threaded + * application. + * + * The module is automatically looked up in the appropriate place (git or + * installed directory). + * + * Returns 0 on success, 1 if the module was not found and -1 on any error. + */ +int +virModuleLoad(const char *path, + const char *regfunc, + bool required) +{ + void *rethandle = NULL; + int (*regsym)(void); + int ret = -1; + + if (!virFileExists(path)) { + if (required) { + virReportSystemError(errno, + _("Failed to find module '%s'"), path); + return -1; + } else { + VIR_INFO("Module '%s' does not exist", path); + return 1; + } + } + + if (!(rethandle = virModuleLoadFile(path))) + goto cleanup; + + if (!(regsym = virModuleLoadFunc(rethandle, path, regfunc))) + goto cleanup; + + if ((*regsym)() < 0) { + /* regsym() should report an error itself, but lets + * just make sure */ + virErrorPtr err = virGetLastError(); + if (err == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to execute symbol '%s' in module '%s'"), + regfunc, path); + } + goto cleanup; + } + + rethandle = NULL; + ret = 0; + + cleanup: + if (rethandle) + dlclose(rethandle); + return ret; +} + +#else /* ! HAVE_DLFCN_H */ +int +virModuleLoad(const char *path ATTRIBUTE_UNUSED, + const char *regfunc ATTRIBUTE_UNUSED, + bool required) +{ + VIR_DEBUG("dlopen not available on this platform"); + if (required) { + virReportSystemError(ENOSYS, + _("Failed to find module '%s': %s"), path); + return -1; + } else { + /* Since we have no dlopen(), but definition we have no + * loadable modules on disk, so we can resaonably + * return '1' instead of an error. + */ + return 1; + } +} +#endif /* ! HAVE_DLFCN_H */ diff --git a/src/util/virmodule.h b/src/util/virmodule.h new file mode 100644 index 0000000000..cccd836b41 --- /dev/null +++ b/src/util/virmodule.h @@ -0,0 +1,29 @@ +/* + * virmodule.h: APIs for dlopen'ing extension modules + * + * Copyright (C) 2012-2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#ifndef __VIR_MODULE_H__ +# define __VIR_MODULE_H__ + +int virModuleLoad(const char *path, + const char *regfunc, + bool required); + +#endif /* __VIR_MODULE_H__ */ -- 2.14.3

The storage file code needs to be run in the hypervisor drivers, while the storage backend code needs to be run in the storage driver. Split the source code as a preparatory step for creating separate loadable modules. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/storage/Makefile.inc.am | 2 + src/storage/storage_backend_gluster.c | 305 +--------------------------- src/storage/storage_backend_gluster.h | 2 +- src/storage/storage_file_gluster.c | 366 ++++++++++++++++++++++++++++++++++ src/storage/storage_file_gluster.h | 27 +++ 5 files changed, 399 insertions(+), 303 deletions(-) create mode 100644 src/storage/storage_file_gluster.c create mode 100644 src/storage/storage_file_gluster.h diff --git a/src/storage/Makefile.inc.am b/src/storage/Makefile.inc.am index ae9fdeb6a9..1e81249272 100644 --- a/src/storage/Makefile.inc.am +++ b/src/storage/Makefile.inc.am @@ -55,6 +55,8 @@ STORAGE_DRIVER_SHEEPDOG_SOURCES = \ STORAGE_DRIVER_GLUSTER_SOURCES = \ storage/storage_backend_gluster.h \ storage/storage_backend_gluster.c \ + storage/storage_file_gluster.h \ + storage/storage_file_gluster.c \ $(NULL) STORAGE_DRIVER_ZFS_SOURCES = \ diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index c6cc531e2f..aca772676c 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -1,7 +1,7 @@ /* * storage_backend_gluster.c: storage backend for Gluster handling * - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2018 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -24,11 +24,11 @@ #include <glusterfs/api/glfs.h> #include "storage_backend_gluster.h" +#include "storage_file_gluster.h" #include "storage_conf.h" #include "viralloc.h" #include "virerror.h" #include "virlog.h" -#include "virstoragefilebackend.h" #include "virstring.h" #include "viruri.h" #include "storage_util.h" @@ -561,312 +561,13 @@ virStorageBackend virStorageBackendGluster = { }; -typedef struct _virStorageFileBackendGlusterPriv virStorageFileBackendGlusterPriv; -typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr; - -struct _virStorageFileBackendGlusterPriv { - glfs_t *vol; - char *canonpath; -}; - - -static void -virStorageFileBackendGlusterDeinit(virStorageSourcePtr src) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - - VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%u/%s%s)", - src, src->hosts->name, src->hosts->port, src->volume, src->path); - - if (priv->vol) - glfs_fini(priv->vol); - VIR_FREE(priv->canonpath); - - VIR_FREE(priv); - src->drv->priv = NULL; -} - -static int -virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPrivPtr priv, - virStorageNetHostDefPtr host) -{ - const char *transport = virStorageNetHostTransportTypeToString(host->transport); - const char *hoststr = NULL; - int port = 0; - - switch ((virStorageNetHostTransport) host->transport) { - case VIR_STORAGE_NET_HOST_TRANS_RDMA: - case VIR_STORAGE_NET_HOST_TRANS_TCP: - hoststr = host->name; - port = host->port; - break; - - case VIR_STORAGE_NET_HOST_TRANS_UNIX: - hoststr = host->socket; - break; - - case VIR_STORAGE_NET_HOST_TRANS_LAST: - break; - } - - VIR_DEBUG("adding gluster host for %p: transport=%s host=%s port=%d", - priv, transport, hoststr, port); - - if (glfs_set_volfile_server(priv->vol, transport, hoststr, port) < 0) { - virReportSystemError(errno, - _("failed to set gluster volfile server '%s'"), - hoststr); - return -1; - } - - return 0; -} - - -static int -virStorageFileBackendGlusterInit(virStorageSourcePtr src) -{ - virStorageFileBackendGlusterPrivPtr priv = NULL; - size_t i; - - if (!src->volume) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing gluster volume name for path '%s'"), - src->path); - return -1; - } - - if (VIR_ALLOC(priv) < 0) - return -1; - - VIR_DEBUG("initializing gluster storage file %p " - "(priv='%p' volume='%s' path='%s') as [%u:%u]", - src, priv, src->volume, src->path, - (unsigned int)src->drv->uid, (unsigned int)src->drv->gid); - - if (!(priv->vol = glfs_new(src->volume))) { - virReportOOMError(); - goto error; - } - - for (i = 0; i < src->nhosts; i++) { - if (virStorageFileBackendGlusterInitServer(priv, src->hosts + i) < 0) - goto error; - } - - if (glfs_init(priv->vol) < 0) { - virReportSystemError(errno, - _("failed to initialize gluster connection " - "(src=%p priv=%p)"), src, priv); - goto error; - } - - src->drv->priv = priv; - - return 0; - - error: - if (priv->vol) - glfs_fini(priv->vol); - VIR_FREE(priv); - - return -1; -} - - -static int -virStorageFileBackendGlusterCreate(virStorageSourcePtr src) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - glfs_fd_t *fd = NULL; - mode_t mode = S_IRUSR; - - if (!src->readonly) - mode |= S_IWUSR; - - if (!(fd = glfs_creat(priv->vol, src->path, - O_CREAT | O_TRUNC | O_WRONLY, mode))) - return -1; - - ignore_value(glfs_close(fd)); - return 0; -} - - -static int -virStorageFileBackendGlusterUnlink(virStorageSourcePtr src) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - - return glfs_unlink(priv->vol, src->path); -} - - -static int -virStorageFileBackendGlusterStat(virStorageSourcePtr src, - struct stat *st) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - - return glfs_stat(priv->vol, src->path, st); -} - - -static ssize_t -virStorageFileBackendGlusterRead(virStorageSourcePtr src, - size_t offset, - size_t len, - char **buf) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - glfs_fd_t *fd = NULL; - ssize_t ret = -1; - - *buf = NULL; - - if (!(fd = glfs_open(priv->vol, src->path, O_RDONLY))) { - virReportSystemError(errno, _("Failed to open file '%s'"), - src->path); - return -1; - } - - if (offset > 0) { - if (glfs_lseek(fd, offset, SEEK_SET) == (off_t) -1) { - virReportSystemError(errno, _("cannot seek into '%s'"), src->path); - goto cleanup; - } - } - - ret = virStorageBackendGlusterRead(fd, src->path, len, buf); - - cleanup: - if (fd) - glfs_close(fd); - - return ret; -} - - -static int -virStorageFileBackendGlusterAccess(virStorageSourcePtr src, - int mode) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - - return glfs_access(priv->vol, src->path, mode); -} - -static int -virStorageFileBackendGlusterReadlinkCallback(const char *path, - char **linkpath, - void *data) -{ - virStorageFileBackendGlusterPrivPtr priv = data; - char *buf = NULL; - size_t bufsiz = 0; - ssize_t ret; - struct stat st; - - *linkpath = NULL; - - if (glfs_stat(priv->vol, path, &st) < 0) { - virReportSystemError(errno, - _("failed to stat gluster path '%s'"), - path); - return -1; - } - - if (!S_ISLNK(st.st_mode)) - return 1; - - realloc: - if (VIR_EXPAND_N(buf, bufsiz, 256) < 0) - goto error; - - if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) { - virReportSystemError(errno, - _("failed to read link of gluster file '%s'"), - path); - goto error; - } - - if (ret == bufsiz) - goto realloc; - - buf[ret] = '\0'; - - *linkpath = buf; - - return 0; - - error: - VIR_FREE(buf); - return -1; -} - - -static const char * -virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - char *filePath = NULL; - - if (priv->canonpath) - return priv->canonpath; - - if (!(filePath = virStorageFileCanonicalizePath(src->path, - virStorageFileBackendGlusterReadlinkCallback, - priv))) - return NULL; - - ignore_value(virAsprintf(&priv->canonpath, "gluster://%s:%u/%s/%s", - src->hosts->name, - src->hosts->port, - src->volume, - filePath)); - - VIR_FREE(filePath); - - return priv->canonpath; -} - - -static int -virStorageFileBackendGlusterChown(const virStorageSource *src, - uid_t uid, - gid_t gid) -{ - virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - - return glfs_chown(priv->vol, src->path, uid, gid); -} - - -virStorageFileBackend virStorageFileBackendGluster = { - .type = VIR_STORAGE_TYPE_NETWORK, - .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER, - - .backendInit = virStorageFileBackendGlusterInit, - .backendDeinit = virStorageFileBackendGlusterDeinit, - - .storageFileCreate = virStorageFileBackendGlusterCreate, - .storageFileUnlink = virStorageFileBackendGlusterUnlink, - .storageFileStat = virStorageFileBackendGlusterStat, - .storageFileRead = virStorageFileBackendGlusterRead, - .storageFileAccess = virStorageFileBackendGlusterAccess, - .storageFileChown = virStorageFileBackendGlusterChown, - - .storageFileGetUniqueIdentifier = virStorageFileBackendGlusterGetUniqueIdentifier, -}; - - int virStorageBackendGlusterRegister(void) { if (virStorageBackendRegister(&virStorageBackendGluster) < 0) return -1; - if (virStorageFileBackendRegister(&virStorageFileBackendGluster) < 0) + if (virStorageFileGlusterRegister() < 0) return -1; return 0; diff --git a/src/storage/storage_backend_gluster.h b/src/storage/storage_backend_gluster.h index 91b8d8275d..12a1c04f8d 100644 --- a/src/storage/storage_backend_gluster.h +++ b/src/storage/storage_backend_gluster.h @@ -1,7 +1,7 @@ /* * storage_backend_gluster.h: storage backend for Gluster handling * - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013-2018 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c new file mode 100644 index 0000000000..f8bbde8cfe --- /dev/null +++ b/src/storage/storage_file_gluster.c @@ -0,0 +1,366 @@ +/* + * storage_file_gluster.c: storage file backend for Gluster handling + * + * Copyright (C) 2013-2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#include <config.h> + +#include <glusterfs/api/glfs.h> + +#include "storage_file_gluster.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virstoragefilebackend.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +VIR_LOG_INIT("storage.storage_file_gluster"); + + +typedef struct _virStorageFileBackendGlusterPriv virStorageFileBackendGlusterPriv; +typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr; + +struct _virStorageFileBackendGlusterPriv { + glfs_t *vol; + char *canonpath; +}; + +static void +virStorageFileBackendGlusterDeinit(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%u/%s%s)", + src, src->hosts->name, src->hosts->port, src->volume, src->path); + + if (priv->vol) + glfs_fini(priv->vol); + VIR_FREE(priv->canonpath); + + VIR_FREE(priv); + src->drv->priv = NULL; +} + +static int +virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPrivPtr priv, + virStorageNetHostDefPtr host) +{ + const char *transport = virStorageNetHostTransportTypeToString(host->transport); + const char *hoststr = NULL; + int port = 0; + + switch ((virStorageNetHostTransport) host->transport) { + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_TCP: + hoststr = host->name; + port = host->port; + break; + + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + hoststr = host->socket; + break; + + case VIR_STORAGE_NET_HOST_TRANS_LAST: + break; + } + + VIR_DEBUG("adding gluster host for %p: transport=%s host=%s port=%d", + priv, transport, hoststr, port); + + if (glfs_set_volfile_server(priv->vol, transport, hoststr, port) < 0) { + virReportSystemError(errno, + _("failed to set gluster volfile server '%s'"), + hoststr); + return -1; + } + + return 0; +} + + +static int +virStorageFileBackendGlusterInit(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = NULL; + size_t i; + + if (!src->volume) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing gluster volume name for path '%s'"), + src->path); + return -1; + } + + if (VIR_ALLOC(priv) < 0) + return -1; + + VIR_DEBUG("initializing gluster storage file %p " + "(priv='%p' volume='%s' path='%s') as [%u:%u]", + src, priv, src->volume, src->path, + (unsigned int)src->drv->uid, (unsigned int)src->drv->gid); + + if (!(priv->vol = glfs_new(src->volume))) { + virReportOOMError(); + goto error; + } + + for (i = 0; i < src->nhosts; i++) { + if (virStorageFileBackendGlusterInitServer(priv, src->hosts + i) < 0) + goto error; + } + + if (glfs_init(priv->vol) < 0) { + virReportSystemError(errno, + _("failed to initialize gluster connection " + "(src=%p priv=%p)"), src, priv); + goto error; + } + + src->drv->priv = priv; + + return 0; + + error: + if (priv->vol) + glfs_fini(priv->vol); + VIR_FREE(priv); + + return -1; +} + + +static int +virStorageFileBackendGlusterCreate(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + glfs_fd_t *fd = NULL; + mode_t mode = S_IRUSR; + + if (!src->readonly) + mode |= S_IWUSR; + + if (!(fd = glfs_creat(priv->vol, src->path, + O_CREAT | O_TRUNC | O_WRONLY, mode))) + return -1; + + ignore_value(glfs_close(fd)); + return 0; +} + + +static int +virStorageFileBackendGlusterUnlink(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + return glfs_unlink(priv->vol, src->path); +} + + +static int +virStorageFileBackendGlusterStat(virStorageSourcePtr src, + struct stat *st) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + return glfs_stat(priv->vol, src->path, st); +} + + +static ssize_t +virStorageFileBackendGlusterRead(virStorageSourcePtr src, + size_t offset, + size_t len, + char **buf) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + glfs_fd_t *fd = NULL; + ssize_t ret = -1; + char *s; + size_t nread = 0; + + *buf = NULL; + + if (!(fd = glfs_open(priv->vol, src->path, O_RDONLY))) { + virReportSystemError(errno, _("Failed to open file '%s'"), + src->path); + return -1; + } + + if (offset > 0) { + if (glfs_lseek(fd, offset, SEEK_SET) == (off_t) -1) { + virReportSystemError(errno, _("cannot seek into '%s'"), src->path); + goto cleanup; + } + } + + + if (VIR_ALLOC_N(*buf, len) < 0) + return -1; + + s = *buf; + while (len) { + ssize_t r = glfs_read(fd, s, len, 0); + if (r < 0 && errno == EINTR) + continue; + if (r < 0) { + VIR_FREE(*buf); + virReportSystemError(errno, _("unable to read '%s'"), src->path); + return r; + } + if (r == 0) + return nread; + s += r; + len -= r; + nread += r; + } + + ret = nread; + + cleanup: + if (fd) + glfs_close(fd); + + return ret; +} + + +static int +virStorageFileBackendGlusterAccess(virStorageSourcePtr src, + int mode) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + return glfs_access(priv->vol, src->path, mode); +} + +static int +virStorageFileBackendGlusterReadlinkCallback(const char *path, + char **linkpath, + void *data) +{ + virStorageFileBackendGlusterPrivPtr priv = data; + char *buf = NULL; + size_t bufsiz = 0; + ssize_t ret; + struct stat st; + + *linkpath = NULL; + + if (glfs_stat(priv->vol, path, &st) < 0) { + virReportSystemError(errno, + _("failed to stat gluster path '%s'"), + path); + return -1; + } + + if (!S_ISLNK(st.st_mode)) + return 1; + + realloc: + if (VIR_EXPAND_N(buf, bufsiz, 256) < 0) + goto error; + + if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) { + virReportSystemError(errno, + _("failed to read link of gluster file '%s'"), + path); + goto error; + } + + if (ret == bufsiz) + goto realloc; + + buf[ret] = '\0'; + + *linkpath = buf; + + return 0; + + error: + VIR_FREE(buf); + return -1; +} + + +static const char * +virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + char *filePath = NULL; + + if (priv->canonpath) + return priv->canonpath; + + if (!(filePath = virStorageFileCanonicalizePath(src->path, + virStorageFileBackendGlusterReadlinkCallback, + priv))) + return NULL; + + ignore_value(virAsprintf(&priv->canonpath, "gluster://%s:%u/%s/%s", + src->hosts->name, + src->hosts->port, + src->volume, + filePath)); + + VIR_FREE(filePath); + + return priv->canonpath; +} + + +static int +virStorageFileBackendGlusterChown(const virStorageSource *src, + uid_t uid, + gid_t gid) +{ + virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; + + return glfs_chown(priv->vol, src->path, uid, gid); +} + + +virStorageFileBackend virStorageFileBackendGluster = { + .type = VIR_STORAGE_TYPE_NETWORK, + .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER, + + .backendInit = virStorageFileBackendGlusterInit, + .backendDeinit = virStorageFileBackendGlusterDeinit, + + .storageFileCreate = virStorageFileBackendGlusterCreate, + .storageFileUnlink = virStorageFileBackendGlusterUnlink, + .storageFileStat = virStorageFileBackendGlusterStat, + .storageFileRead = virStorageFileBackendGlusterRead, + .storageFileAccess = virStorageFileBackendGlusterAccess, + .storageFileChown = virStorageFileBackendGlusterChown, + + .storageFileGetUniqueIdentifier = virStorageFileBackendGlusterGetUniqueIdentifier, +}; + + +int +virStorageFileGlusterRegister(void) +{ + if (virStorageFileBackendRegister(&virStorageFileBackendGluster) < 0) + return -1; + + return 0; +} diff --git a/src/storage/storage_file_gluster.h b/src/storage/storage_file_gluster.h new file mode 100644 index 0000000000..572254aedb --- /dev/null +++ b/src/storage/storage_file_gluster.h @@ -0,0 +1,27 @@ +/* + * storage_file_gluster.h: storage file backend for Gluster handling + * + * Copyright (C) 2013-2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#ifndef __VIR_STORAGE_FILE_GLUSTER_H__ +# define __VIR_STORAGE_FILE_GLUSTER_H__ + +int virStorageFileGlusterRegister(void); + +#endif /* __VIR_STORAGE_FILE_GLUSTER_H__ */ -- 2.14.3

On Thu, Apr 26, 2018 at 09:39:44AM +0200, Peter Krempa wrote: Habit of touching date in all files in a commit... Opps, cut & paste from the other file. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wed, Apr 25, 2018 at 16:52:40 +0100, Daniel Berrange wrote: [...] Does just deleting a bunch of code warrant a copyright bump? These are not added in the src/storage/storage_file_fs.c, so they are a separate cleanup. [...] AFAIK there's no code moved which the last line would apply to. [...] As previously, the only added function is new in this commit. ACK

The virStorageFileGetBackingStoreStr method has overloaded the NULL return value to indicate both no backing available and a fatal error dealing with it. The caller is thus not able to correctly propagate the error messages. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 15 +++++++++------ src/util/virstoragefile.c | 49 +++++++++++++++++++++++++++++++---------------- src/util/virstoragefile.h | 5 +++-- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7484b00e23..672c5372eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14608,12 +14608,15 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, /* relative backing store paths need to be updated so that relative * block commit still works */ - if (reuse && - (backingStoreStr = virStorageFileGetBackingStoreStr(dd->src))) { - if (virStorageIsRelative(backingStoreStr)) - VIR_STEAL_PTR(dd->relPath, backingStoreStr); - else - VIR_FREE(backingStoreStr); + if (reuse) { + if (virStorageFileGetBackingStoreStr(dd->src, &backingStoreStr) < 0) + goto error; + if (backingStoreStr != NULL) { + if (virStorageIsRelative(backingStoreStr)) + VIR_STEAL_PTR(dd->relPath, backingStoreStr); + else + VIR_FREE(backingStoreStr); + } } /* Note that it's unsafe to assume that the disks in the persistent diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 531540ac91..f09035cd4a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4357,14 +4357,8 @@ virStorageFileRead(virStorageSourcePtr src, return -1; } - if (!src->drv->backend->storageFileRead) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("storage file reading is not supported for " - "storage type %s (protocol: %s)"), - virStorageTypeToString(src->type), - virStorageNetProtocolTypeToString(src->protocol)); + if (!src->drv->backend->storageFileRead) return -2; - } ret = src->drv->backend->storageFileRead(src, offset, len, buf); @@ -4551,8 +4545,15 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, - &buf)) < 0) + &buf)) < 0) { + if (headerLen == -2) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("storage file reading is not supported for " + "storage type %s (protocol: %s)"), + virStorageTypeToString(src->type), + virStorageNetProtocolTypeToString(src->protocol)); goto cleanup; + } if (virStorageFileGetMetadataInternal(src, buf, headerLen, &backingFormat) < 0) @@ -4664,24 +4665,36 @@ virStorageFileGetMetadata(virStorageSourcePtr src, * In case when the string can't be retrieved or does not exist NULL is * returned. */ -char * -virStorageFileGetBackingStoreStr(virStorageSourcePtr src) +int +virStorageFileGetBackingStoreStr(virStorageSourcePtr src, + char **backing) { virStorageSourcePtr tmp = NULL; char *buf = NULL; ssize_t headerLen; - char *ret = NULL; + int ret = -1; + int rv; + + *backing = NULL; /* exit if we can't load information about the current image */ if (!virStorageFileSupportsBackingChainTraversal(src)) - return NULL; + return 0; - if (virStorageFileAccess(src, F_OK) < 0) - return NULL; + rv = virStorageFileAccess(src, F_OK); + if (rv == -2) + return 0; + if (rv < 0) { + virStorageFileReportBrokenChain(errno, src, src); + return -1; + } if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, - &buf)) < 0) - return NULL; + &buf)) < 0) { + if (headerLen == -2) + return 0; + return -1; + } if (!(tmp = virStorageSourceCopy(src, false))) goto cleanup; @@ -4689,7 +4702,9 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src) if (virStorageFileGetMetadataInternal(tmp, buf, headerLen, NULL) < 0) goto cleanup; - VIR_STEAL_PTR(ret, tmp->backingStoreRaw); + VIR_STEAL_PTR(*backing, tmp->backingStoreRaw); + + ret = 0; cleanup: VIR_FREE(buf); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d129e81978..b92c1c47dd 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -474,8 +474,9 @@ int virStorageFileGetMetadata(virStorageSourcePtr src, bool report_broken) ATTRIBUTE_NONNULL(1); -char *virStorageFileGetBackingStoreStr(virStorageSourcePtr src) - ATTRIBUTE_NONNULL(1); +int virStorageFileGetBackingStoreStr(virStorageSourcePtr src, + char **backing) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virStorageFileReportBrokenChain(int errcode, virStorageSourcePtr src, -- 2.14.3

On Thu, Apr 26, 2018 at 09:56:16AM +0200, Peter Krempa wrote: Ok, will add the obvious error reporting fix. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wed, Apr 25, 2018 at 16:52:42 +0100, Daniel Berrange wrote: [...] [...] Hmm, isn't 'rv' better in the case when this variable actually is not returned? Alignment looks off As in above hunk. ACK

The storage file drivers are currently loaded as a side effect of loading the storage driver. This is a bogus dependancy because the storage file code has no interaction with the storage drivers, and even ultimately be running in a completely separate daemon. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- libvirt.spec.in | 4 ++ src/storage/Makefile.inc.am | 40 +++++++++++++++++++ src/storage/storage_backend_fs.c | 4 -- src/storage/storage_backend_gluster.c | 4 -- src/util/virstoragefile.c | 10 +++-- src/util/virstoragefilebackend.c | 72 +++++++++++++++++++++++++++-------- src/util/virstoragefilebackend.h | 9 ++--- 7 files changed, 111 insertions(+), 32 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 5090dfa2aa..e8705bbb48 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1384,6 +1384,8 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-file/*.la +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-file/*.a %if %{with_wireshark} rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la %endif @@ -1886,6 +1888,7 @@ exit 0 %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %{_libdir}/%{name}/connection-driver/libvirt_driver_storage.so %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_fs.so +%{_libdir}/%{name}/storage-file/libvirt_storage_file_fs.so %files daemon-driver-storage-disk %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_disk.so @@ -1905,6 +1908,7 @@ exit 0 %if %{with_storage_gluster} %files daemon-driver-storage-gluster %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_gluster.so +%{_libdir}/%{name}/storage-file/libvirt_storage_file_gluster.so %endif %if %{with_storage_rbd} diff --git a/src/storage/Makefile.inc.am b/src/storage/Makefile.inc.am index af2c97ab93..ea98c0ee52 100644 --- a/src/storage/Makefile.inc.am +++ b/src/storage/Makefile.inc.am @@ -14,6 +14,9 @@ STORAGE_DRIVER_SOURCES = \ STORAGE_DRIVER_FS_SOURCES = \ storage/storage_backend_fs.h \ storage/storage_backend_fs.c \ + $(NULL) + +STORAGE_FILE_FS_SOURCES = \ storage/storage_file_fs.h \ storage/storage_file_fs.c \ $(NULL) @@ -57,6 +60,9 @@ STORAGE_DRIVER_SHEEPDOG_SOURCES = \ STORAGE_DRIVER_GLUSTER_SOURCES = \ storage/storage_backend_gluster.h \ storage/storage_backend_gluster.c \ + $(NULL) + +STORAGE_FILE_GLUSTER_SOURCES = \ storage/storage_file_gluster.h \ storage/storage_file_gluster.c \ $(NULL) @@ -80,6 +86,7 @@ STATEFUL_DRIVER_SOURCE_FILES += $(STORAGE_DRIVER_SOURCES) EXTRA_DIST += \ $(STORAGE_DRIVER_SOURCES) \ $(STORAGE_DRIVER_FS_SOURCES) \ + $(STORAGE_FILE_FS_SOURCES) \ $(STORAGE_DRIVER_LVM_SOURCES) \ $(STORAGE_DRIVER_ISCSI_SOURCES) \ $(STORAGE_DRIVER_SCSI_SOURCES) \ @@ -88,6 +95,7 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_RBD_SOURCES) \ $(STORAGE_DRIVER_SHEEPDOG_SOURCES) \ $(STORAGE_DRIVER_GLUSTER_SOURCES) \ + $(STORAGE_FILE_GLUSTER_SOURCES) \ $(STORAGE_DRIVER_ZFS_SOURCES) \ $(STORAGE_DRIVER_VSTORAGE_SOURCES) \ $(STORAGE_HELPER_DISK_SOURCES) \ @@ -96,6 +104,9 @@ EXTRA_DIST += \ storagebackenddir = $(libdir)/libvirt/storage-backend storagebackend_LTLIBRARIES = +storagefiledir = $(libdir)/libvirt/storage-file +storagefile_LTLIBRARIES = + # Needed to keep automake quiet about conditionals libvirt_driver_storage_impl_la_SOURCES = libvirt_driver_storage_impl_la_CFLAGS = \ @@ -136,6 +147,19 @@ libvirt_storage_backend_fs_la_LIBADD = \ libvirt.la \ ../gnulib/lib/libgnu.la \ $(NULL) + +libvirt_storage_file_fs_la_SOURCES = $(STORAGE_FILE_FS_SOURCES) +libvirt_storage_file_fs_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) \ + $(NULL) + +storagefile_LTLIBRARIES += libvirt_storage_file_fs.la +libvirt_storage_file_fs_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_storage_file_fs_la_LIBADD = \ + libvirt.la \ + ../gnulib/lib/libgnu.la \ + $(NULL) endif WITH_STORAGE if WITH_STORAGE_LVM @@ -270,6 +294,22 @@ libvirt_storage_backend_gluster_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_gluster.la libvirt_storage_backend_gluster_la_LDFLAGS = $(AM_LDFLAGS_MOD) + + +libvirt_storage_file_gluster_la_SOURCES = $(STORAGE_FILE_GLUSTER_SOURCES) +libvirt_storage_file_gluster_la_LIBADD = \ + libvirt.la \ + $(GLUSTERFS_LIBS) \ + ../gnulib/lib/libgnu.la \ + $(NULL) +libvirt_storage_file_gluster_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(GLUSTERFS_CFLAGS) \ + $(AM_CFLAGS) \ + $(NULL) + +storagefile_LTLIBRARIES += libvirt_storage_file_gluster.la +libvirt_storage_file_gluster_la_LDFLAGS = $(AM_LDFLAGS_MOD) endif WITH_STORAGE_GLUSTER if WITH_STORAGE_ZFS diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bface86b43..37fea8c941 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -32,7 +32,6 @@ #include "virerror.h" #include "storage_backend_fs.h" -#include "storage_file_fs.h" #include "storage_util.h" #include "storage_conf.h" #include "vircommand.h" @@ -713,8 +712,5 @@ virStorageBackendFsRegister(void) return -1; #endif /* WITH_STORAGE_FS */ - if (virStorageFileFsRegister() < 0) - return -1; - return 0; } diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index aca772676c..5d4b920a60 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -24,7 +24,6 @@ #include <glusterfs/api/glfs.h> #include "storage_backend_gluster.h" -#include "storage_file_gluster.h" #include "storage_conf.h" #include "viralloc.h" #include "virerror.h" @@ -567,8 +566,5 @@ virStorageBackendGlusterRegister(void) if (virStorageBackendRegister(&virStorageBackendGluster) < 0) return -1; - if (virStorageFileGlusterRegister() < 0) - return -1; - return 0; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index da13d51d32..73937b527a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4117,7 +4117,9 @@ virStorageFileGetBackendForSupportCheck(const virStorageSource *src, actualType = virStorageSourceGetActualType(src); - *backend = virStorageFileBackendForTypeInternal(actualType, src->protocol, false); + if (virStorageFileBackendForType(actualType, src->protocol, false, backend) < 0) + return -1; + return 0; } @@ -4234,8 +4236,10 @@ virStorageFileInitAs(virStorageSourcePtr src, else src->drv->gid = gid; - if (!(src->drv->backend = virStorageFileBackendForType(actualType, - src->protocol))) + if (virStorageFileBackendForType(actualType, + src->protocol, + true, + &src->drv->backend) < 0) goto error; if (src->drv->backend->backendInit && diff --git a/src/util/virstoragefilebackend.c b/src/util/virstoragefilebackend.c index df86ee39c2..ac087dabac 100644 --- a/src/util/virstoragefilebackend.c +++ b/src/util/virstoragefilebackend.c @@ -32,6 +32,7 @@ #include "internal.h" #include "virstoragefilebackend.h" #include "virlog.h" +#include "virmodule.h" #include "virfile.h" #include "configmake.h" @@ -44,6 +45,46 @@ VIR_LOG_INIT("storage.storage_source_backend"); static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; static size_t virStorageFileBackendsCount; +#define STORAGE_FILE_MODULE_DIR LIBDIR "/libvirt/storage-file" + +static int +virStorageFileLoadBackendModule(const char *name, + const char *regfunc, + bool forceload) +{ + char *modfile = NULL; + int ret; + + if (!(modfile = virFileFindResourceFull(name, + "libvirt_storage_file_", + ".so", + abs_topbuilddir "/src/.libs", + STORAGE_FILE_MODULE_DIR, + "LIBVIRT_STORAGE_FILE_DIR"))) + return -1; + + ret = virModuleLoad(modfile, regfunc, forceload); + + VIR_FREE(modfile); + + return ret; +} + + +static int virStorageFileBackendOnceInit(void) +{ +#if WITH_STORAGE_DIR || WITH_STORAGE_FS + if (virStorageFileLoadBackendModule("fs", "virStorageFileFsRegister", false) < 0) + return -1; +#endif /* WITH_STORAGE_DIR || WITH_STORAGE_FS */ +#if WITH_STORAGE_GLUSTER + if (virStorageFileLoadBackendModule("gluster", "virStorageFileGlusterRegister", false) < 0) + return -1; +#endif /* WITH_STORAGE_GLUSTER */ + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virStorageFileBackend) int virStorageFileBackendRegister(virStorageFileBackendPtr backend) @@ -65,25 +106,32 @@ virStorageFileBackendRegister(virStorageFileBackendPtr backend) return 0; } -virStorageFileBackendPtr -virStorageFileBackendForTypeInternal(int type, - int protocol, - bool report) +int +virStorageFileBackendForType(int type, + int protocol, + bool required, + virStorageFileBackendPtr *backend) { size_t i; + *backend = NULL; + + if (virStorageFileBackendInitialize() < 0) + return -1; + for (i = 0; i < virStorageFileBackendsCount; i++) { if (virStorageFileBackends[i]->type == type) { if (type == VIR_STORAGE_TYPE_NETWORK && virStorageFileBackends[i]->protocol != protocol) continue; - return virStorageFileBackends[i]; + *backend = virStorageFileBackends[i]; + return 0; } } - if (!report) - return NULL; + if (!required) + return 0; if (type == VIR_STORAGE_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -96,13 +144,5 @@ virStorageFileBackendForTypeInternal(int type, virStorageTypeToString(type)); } - return NULL; -} - - -virStorageFileBackendPtr -virStorageFileBackendForType(int type, - int protocol) -{ - return virStorageFileBackendForTypeInternal(type, protocol, true); + return -1; } diff --git a/src/util/virstoragefilebackend.h b/src/util/virstoragefilebackend.h index 6cd51750ee..6686864367 100644 --- a/src/util/virstoragefilebackend.h +++ b/src/util/virstoragefilebackend.h @@ -71,11 +71,10 @@ typedef int uid_t uid, gid_t gid); -virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol); -virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type, - int protocol, - bool report); - +int virStorageFileBackendForType(int type, + int protocol, + bool required, + virStorageFileBackendPtr *backend); struct _virStorageFileBackend { int type; -- 2.14.3

On Thu, Apr 26, 2018 at 10:52:02AM +0200, Peter Krempa wrote: Yeah, it is probably the right thing todo since we try to make each loadable module independent. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|